1246692 - Test the browser toolbox

Status: RESOLVED FIXED

(DevTools :: Framework, defect)

Description

[Alexandre Poirot [:ochameau]]

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.

Comment 1

[Brian Grinstead [:bgrins]]

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.

Comment 2

[Alexandre Poirot [:ochameau]]

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

Comment 3

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

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

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

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

Comment 5

[Alexandre Poirot [:ochameau]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b1d49a3a5579

Comment 6

[Alexandre Poirot [:ochameau]]

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?

Comment 7

[Alexandre Poirot [:ochameau]]

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.

Comment 8

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

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

Comment 9

[Alexandre Poirot [:ochameau]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b1bf4b2469f6

Comment 10

[Alexandre Poirot [:ochameau]]

Attachment: patch v2

With a pref.

Comment 11

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

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

Comment 12

[Alexandre Poirot [:ochameau]]

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

Comment 13

[Alexandre Poirot [:ochameau]]

Attachment: patch v3

With a better event name.

Comment 14

[Alexandre Poirot [:ochameau]]

Could it be that the console takes more than 20s to be ready?!
Would the test slave be *that slow*?!! ://

Comment 15

[Alexandre Poirot [:ochameau]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8c2661f6f6d0

Comment 16

[Alexandre Poirot [:ochameau]]

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.

Comment 17

[Alexandre Poirot [:ochameau]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=13adf55ad5df

Comment 18

[Alexandre Poirot [:ochameau]]

So It takes more than 20s to get the console to be ready.
And a bit more than 50s to finish the test.

Comment 19

[Alexandre Poirot [:ochameau]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=817114eec930

Comment 20

[Alexandre Poirot [:ochameau]]

Attachment: patch v4

Just added requestLongerTimeout and bumped the remote-timeout pref.
Let's see if that pass on all platforms...

Comment 21

[Alexandre Poirot [:ochameau]]

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

Comment 22

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4f0106747ea9