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

RESOLVED FIXED

Status

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Felipe, Assigned: Felipe)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

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)
(Assignee)

Updated

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

Comment 3

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

Updated

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

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..
https://hg.mozilla.org/mozilla-central/rev/fdc73bb2dba8
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

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.