1399158 - Add a --jsdebugger flag to geckodriver

Status: RESOLVED FIXED

(Testing :: geckodriver, enhancement, P5)

Description

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

When running wdspec tests now, it's not easy or convenient to debug js code. 

Similar to --port, a --jsdebugger option in geckodriver could override relevant prefs to enable the debugger. 

The prefs for debugging can be found here: https://searchfox.org/mozilla-central/rev/51eae084534f522a502e4b808484cde8591cb502/testing/marionette/client/marionette_driver/geckoinstance.py#173


This in turn could be used when calling wptrunner if you pass something like `--webdriver-arg=--jsdebugger` (and disable timeouts).

Comment 1

[James Graham [:jgraham]]

FWIW

"""safaridriver supports two special WebDriver capabilities just for debugging. The automaticInspection capability will preload the Web Inspector and JavaScript debugger in the background; to pause test execution and bring up Web Inspector’s Debugger tab, you can simply evaluate a debugger; statement in the test page. The automaticProfiling capability will preload the Web Inspector and start a timeline recording in the background; if you later want to see details of what happened during the test, you can open the Timelines tab in Web Inspector to see the captured timeline recording in its entirety."""

Comment 2

[Øyvind Strømmen (:insulaventus)]

Is it ok if I start working on this?

Comment 3

[Øyvind Strømmen (:insulaventus)]

(In reply to Øyvind Strømmen (:insulaventus) from comment #2)
> Is it ok if I start working on this?

Sorry, nevermind. I'm gonna work on another bug and won't be able to pick this up right now.

Comment 4

[paul.eliot.warner]

If this still needs to be worked on, I can take care of it.

Comment 5

[Andreas Tolfsen ❲:ato❳]

Please go ahead.

Comment 6

[paul.eliot.warner]

Attachment: Added --jsdebugger to geckodriver

Comment 7

[paul.eliot.warner]

I've attached a patch. Let me know what you think.

Comment 8

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8944968 [details] [diff] [review]
Added --jsdebugger to geckodriver

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

Thanks for the patch!

The commit message should contain a reference to the bug and be
written in imperative form.  It would also be useful if you expanded
it to explain what the patch adds and what it is for.  The first
line should contain the bug reference followed by a short message,
then optionally followed by a blank line and a longer message, like
this:

> Bug 1399158 - Add --jsdebugger flag to geckodriver. r?ato
> 
> This patch adds a new flag, --jsdebugger, to the geckodriver
> WebDriver implementation.  The flag will cause some preferences to
> be written to the profile before Firefox is started.  These will
> start the Browser Toolbox debugger.

::: testing/geckodriver/src/main.rs
@@ +124,5 @@
>              .requires("marionette_port")
>              .help("Connect to an existing Firefox instance"))
> +        .arg(Arg::with_name("jsdebugger")
> +            .long("jsdebugger")
> +            .value_name("JSDEBUGGER")

No need for a flag value name since this is a boolean flag.

@@ +126,5 @@
> +        .arg(Arg::with_name("jsdebugger")
> +            .long("jsdebugger")
> +            .value_name("JSDEBUGGER")
> +            .takes_value(false)
> +            .help("Enable js debugging"))

This help message could be better.  I would suggest something like
“Enable browser toolbox debugger for Firefox”.

@@ +197,5 @@
>          port: marionette_port,
>          binary: binary,
>          connect_existing: matches.is_present("connect_existing"),
>          log_level: log_level,
> +        enable_debugger: matches.is_present("jsdebugger")

s/enable_debugger/jsdebugger/g as Gecko has multiple different types
of debuggers.

Missing trailing comma.

::: testing/geckodriver/src/marionette.rs
@@ +506,1 @@
>          prefs.insert_slice(&extra_prefs[..]);

This can cause the preferences to be overridden by extra_prefs
on :506.  I would consider it a bug if a user passes --jsdebugger
and it does not start.

Comment 9

[paul.eliot.warner]

Attachment: Add --jsdebugger flag to geckodriver

This patch adds a new flag, --jsdebugger, to the geckodriver WebDriver
implementation. This flag will override several preferences before Firefox is
started. These flags will start the Browser Toolbox debugger.

Comment 10

[paul.eliot.warner]

Alright, I fixed all the things you mentioned. Sorry about the little stuff - I'll make sure it doesn't happen again. The changes to the preferences have been moved to after prefs.insert_slice(&extra_prefs[..]), so the preferences should be set correctly regardless of extra preferences. Let me know what you think.

Comment 11

[Andreas Tolfsen ❲:ato❳]

This looks mostly fine now (with the exception of the variable name
enable_debugger) and it compiles fine.

However, when I launch geckodriver with --jsdebugger and start a
new session I get the global modal dialogue asking me if I’m ready
to run the Marionette tests, but the actual browser toolbox does
not open.  I can’t see that geckoinstance.py is doing this any
differently.

I will have a closer look in the morning.

Comment 12

[paul.eliot.warner]

What method are you using to launch/test geckodriver?

Comment 13

[Andreas Tolfsen ❲:ato❳]

Sorry for leaving this dangling.  The problem here is that -jsdebugger
isn’t passed on to Firefox.

Comment 14

[Andreas Tolfsen ❲:ato❳]

Attachment: Add --jsdebugger flag to geckodriver. r=ato

When --jsdebugger is passed to geckodriver, it will override
preferences related to starting the Browser Toolbox and pass
-jsdebugger on to the Firefox process.

It is functionally equivalent to "./mach marionette test --jsdebugger".

MozReview-Commit-ID: ADfrLPXtQoy

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b8a6c14ad280
Add --jsdebugger flag to geckodriver. r=ato

Comment 16

[Dorel Luca [:dluca]]

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

Comment 17

[paul.eliot.warner]

Awesome.

I'm going to be looking for some new bugs to take on. I'd like to stick around geckodriver, but if you have some other place you think I could be more useful, let me know and I'll try and help out there.