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



6 years ago
6 years ago


(Reporter: Felipe, Assigned: Felipe)



Firefox Tracking Flags

(Not tracked)



(2 attachments)



6 years ago
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}
Attachment #823742 - Flags: review?(ted)


6 years ago
Attachment #823742 - Flags: review?(jmaher)
Comment on attachment 823742 [details] [diff] [review]

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/
@@ +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/
@@ +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?
Attachment #823742 - Flags: review?(jmaher) → review+

Comment 2

6 years ago
(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

6 years ago
The requirement is indeed already enforced in the other added block to

> 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.


6 years ago
Attachment #823742 - Flags: review?(ted)

Comment 4

6 years ago
Posted patch patch to landSplinter Review
Thanks for the review and the bitrot heads up. Posting here the unbitrotted patch to land while the tree is closed..
Last Resolved: 6 years ago
Resolution: --- → FIXED


6 years ago
Summary: Add option to break into debugger on test failure → Add option to break into debugger on test failure (--debug-on-failure)
You need to log in before you can comment on or make changes to this bug.