1075319 - devtools tests should use standard add_task instead of custom asyncTest

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[Mark Hammond [:markh] [:mhammond]]

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.

Comment 1

[Patrick Brosset <:pbro>]

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.

Comment 2

[Patrick Brosset <:pbro>]

The /browser/devtools/markupview/test/ still remain to be migrated.

Comment 3

[Patrick Brosset <:pbro>]

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.

Comment 4

[Patrick Brosset <:pbro>]

Attachment: bug1075319-use-add_task-devtools-mochitests v1.patch

Mostly search/replace patch.
Pending try build: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=03e4398cac58

Comment 5

[Patrick Brosset <:pbro>]

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

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/0011a1d087ca

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0011a1d087ca