Closed
Bug 1246692
Opened 9 years ago
Closed 9 years ago
Test the browser toolbox
Categories
(DevTools :: Framework, defect)
DevTools
Framework
Tracking
(firefox47 fixed)
RESOLVED
FIXED
Firefox 47
Tracking | Status | |
---|---|---|
firefox47 | --- | fixed |
People
(Reporter: ochameau, Assigned: ochameau)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
ochameau
:
review+
|
Details | Diff | Splinter Review |
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•9 years ago
|
||
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•9 years ago
|
||
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.
Blocks: dbg-browser
(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 5•9 years ago
|
||
Assignee | ||
Comment 6•9 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•9 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•9 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 9•9 years ago
|
||
Assignee | ||
Updated•9 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•9 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 | ||
Updated•9 years ago
|
Attachment #8717980 -
Attachment is obsolete: true
Assignee | ||
Comment 14•9 years ago
|
||
Could it be that the console takes more than 20s to be ready?!
Would the test slave be *that slow*?!! ://
Assignee | ||
Comment 15•9 years ago
|
||
Assignee | ||
Comment 16•9 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 17•9 years ago
|
||
Assignee | ||
Comment 18•9 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 19•9 years ago
|
||
Assignee | ||
Comment 20•9 years ago
|
||
Just added requestLongerTimeout and bumped the remote-timeout pref.
Let's see if that pass on all platforms...
Attachment #8718268 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
Attachment #8717988 -
Attachment is obsolete: true
Assignee | ||
Comment 21•9 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•9 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox47:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•