Test the browser toolbox

RESOLVED FIXED in Firefox 47

Status

RESOLVED FIXED
3 years ago
6 months ago

People

(Reporter: ochameau, Assigned: ochameau)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 47
Dependency tree / graph

Firefox Tracking Flags

(firefox47 fixed)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

3 years ago
I'm tired of breaking the browser toolbox without knowing about it.
We desperately need a test against it.
We really often break it for very dumb exception/errors that could be detected if we were just trying to open it.
I agree we should do this if possible.  I know :jryans had some ideas about how to run an in-process BT that could debug everything except for devtools actors, which would be really interesting and easier to test, I'm sure.
(Assignee)

Comment 2

3 years ago
Created attachment 8717052 [details] [diff] [review]
patch v1

This is really dumb, but allowed me to see issues with my gDevTools patch.
We could do something more complex by instanciating a DebuggerServer
in the Browser toolbox process. There is also some opportunity with luciddream
which could potentially be able to run test script immediately within the Browser
toolbox process, which is the best. As it could run existing tests
instead of crafting naive one like this.

But given that I don't want to spend much time on this,
and instead just being covered for simple startup errors,
I came up with this.
Attachment #8717052 - Flags: review?(jryans)
(In reply to Brian Grinstead [:bgrins] from comment #1)
> I agree we should do this if possible.  I know :jryans had some ideas about
> how to run an in-process BT that could debug everything except for devtools
> actors, which would be really interesting and easier to test, I'm sure.

I did make a tiny attempt at this, and it does work for non-debugger things... but (of course) if you stop at a breakpoint, everything hangs since the debugger UI is stopped as well.

There might be something more clever we could do, like have the content process run the UI, similar in concept to the Browser Content Toolbox but backwards (in that case the parent runs the UI and connects to the child).

So, still possibly viable, but not super simple.
(In reply to Brian Grinstead [:bgrins] from comment #1)
> I agree we should do this if possible.  I know :jryans had some ideas about
> how to run an in-process BT that could debug everything except for devtools
> actors, which would be really interesting and easier to test, I'm sure.

Filed bug 1246715 for this.
(Assignee)

Comment 6

3 years ago
Try run is failing. Here is a new one.
Could it be that appinfo.isOfficial is true on try?
I'm wondering what other check we could use?
(Assignee)

Comment 7

3 years ago
Last try confirms that isOfficial is true on try...
May be AppConstants could help?

http://mxr.mozilla.org/mozilla-central/source/toolkit/modules/AppConstants.jsm#38
AppConstants.MOZZILA_OFFICIAL looks like what I'm looking for.
(Assignee)

Updated

3 years ago
Assignee: nobody → poirot.alex
Comment on attachment 8717052 [details] [diff] [review]
patch v1

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

I am on board with the idea, seems like it can help!

isOfficial is the same as MOZILLA_OFFICIAL[1], so that's still not quite right here.

I am not sure there are flags that differ between published builds and test runs...?  That seems generally bad, since you'd run your tests against something different than what you deliver to users.

What about this:

1. Main process has some kind of "we're testing" pref set by your test
2. ToolboxProcess.jsm already copies all its prefs into the special BT process profile[2]
3. Check that pref instead in toolbox-process-window

[1]: https://dxr.mozilla.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#1149
[2]: https://dxr.mozilla.org/mozilla-central/source/devtools/client/framework/ToolboxProcess.jsm#184
Attachment #8717052 - Flags: review?(jryans) → feedback+
(Assignee)

Comment 10

3 years ago
Created attachment 8717980 [details] [diff] [review]
patch v2

With a pref.
Attachment #8717980 - Flags: review?(jryans)
(Assignee)

Updated

3 years ago
Attachment #8717052 - Attachment is obsolete: true
Comment on attachment 8717980 [details] [diff] [review]
patch v2

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

This is great!  Thanks for looking into this.

::: devtools/client/framework/test/browser_browser_toolbox.js
@@ +20,5 @@
> +  let onCustomMessage = new Promise(done => {
> +    Services.obs.addObserver(function listener() {
> +      Services.obs.removeObserver(listener, "plop");
> +      done();
> +    }, "plop", false);

Choose a more descriptive name than "plop" :P
Attachment #8717980 - Flags: review?(jryans) → review+
(Assignee)

Comment 12

3 years ago
It would have been too easy, it failed on debug build:

09:32:00     INFO -  WebConsolePanel open failed. timeout: Connection timeout. Check the Error Console on both ends for potential error messages. Reopen the Web Console to try again.
09:32:00     INFO -  *************************
09:32:00     INFO -  A coding exception was thrown in a Promise resolution callback.
09:32:00     INFO -  See https://developer.mozilla.org/Mozilla/JavaScript_code_modules/Promise.jsm/Promise
09:32:00     INFO -  Full message: TypeError: panel is undefined
09:32:00     INFO -  Full stack: Toolbox.prototype.loadTool/onLoad/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/framework/toolbox.js:1272:13
(Assignee)

Comment 13

3 years ago
Created attachment 8717988 [details] [diff] [review]
patch v3

With a better event name.
Attachment #8717988 - Flags: review+
(Assignee)

Updated

3 years ago
Attachment #8717980 - Attachment is obsolete: true
(Assignee)

Comment 14

3 years ago
Could it be that the console takes more than 20s to be ready?!
Would the test slave be *that slow*?!! ://
(Assignee)

Comment 16

3 years ago
Still not green. It seems to be going further with the pref to extend the timeout.
Now the test seem to complete, we see its last log message, but for some reason it still timeouts.
(Assignee)

Comment 18

3 years ago
So It takes more than 20s to get the console to be ready.
And a bit more than 50s to finish the test.
(Assignee)

Comment 20

3 years ago
Created attachment 8718268 [details] [diff] [review]
patch v4

Just added requestLongerTimeout and bumped the remote-timeout pref.
Let's see if that pass on all platforms...
Attachment #8718268 - Flags: review+
(Assignee)

Updated

3 years ago
Attachment #8717988 - Attachment is obsolete: true
(Assignee)

Comment 21

3 years ago
https://hg.mozilla.org/integration/fx-team/rev/4f0106747ea904aeef430be56f0063c55977e282
Bug 1246692 - Test that the browser toolbox has a working console. r=jryans

Comment 22

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/4f0106747ea9
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47

Updated

6 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.