932144 - Add option to break into debugger on test failure (--debug-on-failure)

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[:Felipe Gomes (needinfo for replies!)]

Attachment: debugOnFailure

Add an option to break JS execution and jump to debugger when there's a testcase failure on a test run. Should be very helpful to check all the test code state at the moment that the failure happen.

This goes hand-in-hand with the --jsdebugger option from bug 895471.

And also should be extra useful to find intermittent oranges when you combine it with bug 875463 ;)

I tested it with mochitest-{plain,chrome,browser}

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 823742 [details] [diff] [review]
debugOnFailure

Review of attachment 823742 [details] [diff] [review]:
-----------------------------------------------------------------

this looks good as long as you make the requirement more firm for both options --jsdebugger and --debugOnFailure.

::: testing/mochitest/mach_commands.py
@@ +185,5 @@
>      def run_desktop_test(self, suite=None, test_file=None, debugger=None,
>          debugger_args=None, shuffle=False, keep_open=False, rerun_failures=False,
>          no_autorun=False, repeat=0, run_until_failure=False, slow=False,
> +        chunk_by_dir=0, total_chunks=None, this_chunk=None,
> +        jsdebugger=False, debug_on_failure=False):

there will be bitrot here with another patch that has startAt, endAt

::: testing/mochitest/mochitest_options.py
@@ +330,5 @@
> +        { "action": "store_true",
> +          "default": False,
> +          "dest": "debugOnFailure",
> +          "help": "breaks execution and enters the JS debugger on a test failure. Should be used together with --jsdebugger."
> +        }],

can we enforce this requirement to have a --jsdebugger option?

Comment 2

[:Gijs (he/him)]

(In reply to Joel Maher (:jmaher) from comment #1)
> can we enforce this requirement to have a --jsdebugger option?

Without having looked at the patch, IMO this option should imply the --jsdebugger option (ie set it if it's not set, and at a point where the block that guarantees --no-autorun is set still has to run, so we get that too).

Comment 3

[:Felipe Gomes (needinfo for replies!)]

The requirement is indeed already enforced in the other added block to mochitest_options.py:

> if options.debugOnFailure and not options.jsdebugger:
>   self.error("--debug-on-failure should be used together with --jsdebugger.")

I was in between adding this requirement or auto-setting of --jsdebugger, but I thought this would make the behavior more clear (i.e. easier to understand that this option is a complement to the --jsdebugger feature). But I can change that if wanted.

Comment 4

[:Felipe Gomes (needinfo for replies!)]

Attachment: patch to land

Thanks for the review and the bitrot heads up. Posting here the unbitrotted patch to land while the tree is closed..

Comment 5

[:Felipe Gomes (needinfo for replies!)]

https://hg.mozilla.org/integration/fx-team/rev/fdc73bb2dba8

Comment 6

[Jim Mathies [:jimm]]

https://hg.mozilla.org/mozilla-central/rev/fdc73bb2dba8