Closed Bug 895471 Opened 11 years ago Closed 11 years ago

Allow running mochitests with the browser debugger already attached

Categories

(DevTools :: Debugger, defect, P4)

defect

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 27

People

(Reporter: fitzgen, Assigned: Gijs)

References

Details

Attachments

(2 files, 1 obsolete file)

Would be nice if we could run mochitests and have the browser debugger be attached before the test runs so we can really dive in and debug our code and use debugger statements, etc.
Priority: -- → P4
Kludgy workaround for now: 1. Open mochitest with --no-autorun 2. Open the toolbox 3. Go to the settings pane 3a. Scroll down 4. Enable browser (+ remote?) debugging 5. Close the toolbox 6. Open a new window 7. From the new window, start the browser debugger 8. Click through the prompt 9. Close the new window 10. Run the test using the button in the mochitest window! But yeah, it'd be so awesome if this was 1 step instead of 10. Nick, what needs to happen to get this moving? Does the browser debugger have a commandline switch that would fire up the browser debugger on process start? If not, that's probably the first thing that needs to happen...
Flags: needinfo?(nfitzgerald)
OS: Mac OS X → All
Hardware: x86 → All
Version: 25 Branch → Trunk
I haven't actually worked on the browser debugger much, maybe Panos can provide needed info. FWIW, I agree that a good first step would be adding a "--browser-debugger" flag to the firefox executable that opens the browser debugger as early as we can.
Flags: needinfo?(nfitzgerald) → needinfo?(past)
There is no command line flag to start the browser debugger, but there should be! See bug 860672 for a similar work-in-progress for the browser console. We should also have the test harness set devtools.debugger.prompt-connection to false in those cases, so that no prompt needs to be clicked.
Flags: needinfo?(past)
I must be missing something, but I think this is what it takes on the devtools part...
Attachment #819773 - Flags: review?(past)
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Per discussion on IRC, going for --jsdebugger instead.
Attachment #819787 - Flags: review?(past)
Attachment #819773 - Attachment is obsolete: true
Attachment #819773 - Flags: review?(past)
This is the mochitest part. I'm sure I've messed up somewhere... but it seems to work in my testing, so asking for review anyway.
Attachment #819790 - Flags: review?(ted)
Comment on attachment 819787 [details] [diff] [review] part 0: add a browser debugger flag, Review of attachment 819787 [details] [diff] [review]: ----------------------------------------------------------------- This patch brings tears to my eyes, thank you! r=me with comments addressed. ::: browser/devtools/devtools-clhandler.js @@ +38,5 @@ > window.focus(); // the Browser Console was already open > } > > if (cmdLine.state == Ci.nsICommandLine.STATE_REMOTE_AUTO) { > cmdLine.preventDefault = true; cmdLine is no longer defined here, and that breaks invoking both the browser debugger and browser console simultaneously. You need to pass it as an argument to this function. Also, I'm not 100% sure, but it looks like we would need this state check for the browser debugger case, no? @@ +45,5 @@ > > + handleDebuggerFlag: function() { > + let remoteDebuggingEnabled = false; > + try { > + remoteDebuggingEnabled = kDebuggerPrefs.every(function(pref) Services.prefs.getBoolPref(pref)); We are trying to move away from SpiderMonkey-specific syntax (bug 887895). Can you make this an arrow function please? @@ +55,5 @@ > + Cu.import("resource:///modules/devtools/DebuggerProcess.jsm"); > + BrowserDebuggerProcess.init(); > + } else { > + Cu.reportError("Could not run chrome debugger! You need the following prefs " + > + "to be set to true: " + kDebuggerPrefs.join(", ")); Since this is a flag intended to be used from a terminal, how about we added a dump() as well, to make sure people won't miss this error?
Attachment #819787 - Flags: review?(past) → review+
Comment on attachment 819790 [details] [diff] [review] part 1: allow passing the flag to mochitest-browser, Review of attachment 819790 [details] [diff] [review]: ----------------------------------------------------------------- ::: testing/mochitest/mochitest_options.py @@ +425,5 @@ > + "devtools.debugger.chrome-enabled=true", > + "devtools.chrome.enabled=true", > + "devtools.debugger.prompt-connection=false" > + ] > + options.autorun = False Is this necessary? I can imagine that it would be useful to let the test start and proceed up to a point where I've placed a debugger statement that would pause execution with the browser debugger attached.
Comment on attachment 819787 [details] [diff] [review] part 0: add a browser debugger flag, Checked in with nits fixed. Thanks for the quick review! https://hg.mozilla.org/integration/fx-team/rev/1d5adf37d61e
Attachment #819787 - Flags: checkin+
(In reply to Panos Astithas [:past] from comment #9) > Comment on attachment 819790 [details] [diff] [review] > part 1: allow passing the flag to mochitest-browser, > > Review of attachment 819790 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: testing/mochitest/mochitest_options.py > @@ +425,5 @@ > > + "devtools.debugger.chrome-enabled=true", > > + "devtools.chrome.enabled=true", > > + "devtools.debugger.prompt-connection=false" > > + ] > > + options.autorun = False > > Is this necessary? I can imagine that it would be useful to let the test > start and proceed up to a point where I've placed a debugger statement that > would pause execution with the browser debugger attached. Hrm. I hadn't thought of this - it's certainly a good point. In fact, a next thing I was thinking of adding was a flag that would auto-branch into a debugger statement in the test assertion code if an assertion in a test (is/ok) failed. OTOH, if I don't want to recompile, it's nice if I don't have to manually put in the --no-autorun. Furthermore, because the browser debugger starts asynchronously, I'm not sure how to ensure that all the scripts have loaded into the debugger if the tests start running automatically.
Depends on: 929535
(In reply to Panos Astithas [:past] from comment #8) > @@ +55,5 @@ > > + Cu.import("resource:///modules/devtools/DebuggerProcess.jsm"); > > + BrowserDebuggerProcess.init(); > > + } else { > > + Cu.reportError("Could not run chrome debugger! You need the following prefs " + > > + "to be set to true: " + kDebuggerPrefs.join(", ")); > > Since this is a flag intended to be used from a terminal, how about we added > a dump() as well, to make sure people won't miss this error? This would be a perfect place for DevToolsUtils.reportException[0] [0] http://mxr.mozilla.org/mozilla-central/source/toolkit/devtools/DevToolsUtils.js#40
(In reply to Nick Fitzgerald [:fitzgen] from comment #12) > This would be a perfect place for DevToolsUtils.reportException[0] > > [0] > http://mxr.mozilla.org/mozilla-central/source/toolkit/devtools/DevToolsUtils. > js#40 This looks like it takes an actual exception object, though... which we don't have in this case. We could throw one, but that seems unnecessary here...
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 27
There is one more patch to review and land.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 819790 [details] [diff] [review] part 1: allow passing the flag to mochitest-browser, Review of attachment 819790 [details] [diff] [review]: ----------------------------------------------------------------- ::: testing/mochitest/mach_commands.py @@ +184,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): n.b.: you and bug 926338 are going to race here.
Attachment #819790 - Flags: review?(ted) → review+
Whiteboard: [fixed-in-fx-team]
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Product: Firefox → DevTools
See Also: → 1522916
See Also: → 1522921
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: