Closed Bug 1075319 Opened 8 years ago Closed 8 years ago

devtools tests should use standard add_task instead of custom asyncTest

Categories

(DevTools :: Inspector, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 37

People

(Reporter: markh, Assigned: pbro)

References

Details

Attachments

(1 file)

This applies to many devtools test directories, but I first struck this with the style inspector.

browser_ruleview_pseudo-element_02.js is currently failing on e10s with the log showing:

21 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_ruleview_pseudo-element_02.js | {u'_capturedTaskStack': u'TaskImpl@resource://gre/modules/Task.jsm:270:20\ncreateAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:249:14\nTask_spawn@resource://gre/modules/Task.jsm:164:12\nasyncTest/<@chrome://mochitests/content/browser/browser/devtools/styleinspector/test/head.js:94:16\nTester_execTest@chrome://mochikit/content/browser-test.js:669:9\nTester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:566:7\n'} - 
Stack trace:
resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:Handler.prototype.process:868
resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:this.PromiseWalker.walkerLoop:744
null:null:0

which isn't particularly useful.  Digging in, I see this (and many others) is using a custom task implementation for tests:

let test = asyncTest(function*() {

But mochitests have recently grown native support for generators, using add_task() similarly to how it is unsed in xpcshell tests.  Changing that line to read:

add_task(function*() {

(ie, no assignment to "test" and no need to reference any additional modules) the error is reported as:

22 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_ruleview_pseudo-element_02.js | Uncaught exception - at resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/inspector.js:2809 - Error: Could not find client side for actor conn0.child2/domwalker23
Stack trace:
    exports.WalkerFront<.frontForRawNode@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/inspector.js:2809:1
    testTopLeft@chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_ruleview_pseudo-element_02.js:19:14
    Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:865:23
    this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:744:7
    Tester_execTest@chrome://mochikit/content/browser-test.js:640:9
    Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:566:7
    Tester_execTest@chrome://mochikit/content/browser-test.js:640:9
    Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:566:7

Which seems saner.
That looks indeed a lot better.
I may migrate all tests in browser/devtools/inspector/test as part of bug 985597 since I need to refactor a lot of tests for that bug anyway.
The /browser/devtools/markupview/test/ still remain to be migrated.
Oh, also: 
- browser/devtools/commandline/test/
- browser/devtools/eyedropper/test/
- browser/devtools/fontinspector/test/
- browser/devtools/framework/test/
- browser/devtools/projecteditor/test/
- browser/devtools/styleeditor/test/
- browser/devtools/webconsole/test/ (at least some)
Looks like a massive search-replace should be almost enough.
Mostly search/replace patch.
Pending try build: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=03e4398cac58
Assignee: nobody → pbrosset
Status: NEW → ASSIGNED
Attachment #8536524 - Flags: review?(mratcliffe)
Mike, to help review, here's what this patch contains:
- changes in head.js files are just removal of the custom asyncTest function
- changes in all other files are replacement of |let test = asyncTest(| by |add_task(|
Attachment #8536524 - Flags: review?(mratcliffe) → review+
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/0011a1d087ca
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/0011a1d087ca
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 37
See Also: → 1113552
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.