Closed Bug 1365649 Opened 3 years ago Closed 3 years ago

Improve browser-chrome self-tests and the "fail-if" manifest property handling

Categories

(Testing :: Mochitest, defect)

Version 3
defect
Not set

Tracking

(firefox55 fixed)

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: Paolo, Assigned: Paolo)

References

Details

Attachments

(1 file)

Various browser-chrome self-tests are currently disabled because the "fail-if" manifest property only handles expected failures of the "ok" function. This bug corrects this in preparation for adding self-tests of unhandled rejection handling in bug 1242505.
Ted, do you have time to handle this review? It's blocking the changes and the self-tests in bug 1242505.
Flags: needinfo?(ted)
Comment on attachment 8868697 [details]
Bug 1365649 - Improve browser-chrome self-tests and the "fail-if" manifest property handling.

Hi Dave, maybe you can review this, since you just reviewed bug 1242505 that is blocked by this one?
Flags: needinfo?(ted)
Attachment #8868697 - Flags: review?(ted) → review?(dtownsend)
Comment on attachment 8868697 [details]
Bug 1365649 - Improve browser-chrome self-tests and the "fail-if" manifest property handling.

https://reviewboard.mozilla.org/r/140296/#review147142

Why the removed tests?

::: testing/mochitest/browser-test.js:1089
(Diff revision 1)
>      }
>      self.__expectedMinAsserts = min;
>      self.__expectedMaxAsserts = max;
>    };
>  
> -  this.setExpected = function test_setExpected() {
> +  this.setExpected = function test_setExpected(expectedAllowedFailureCount) {

Can we rename this function to setExpectedFailures so it is more obvious what it does? It looks like the function is only used in your patch.
Attachment #8868697 - Flags: review?(dtownsend)
Comment on attachment 8868697 [details]
Bug 1365649 - Improve browser-chrome self-tests and the "fail-if" manifest property handling.

https://reviewboard.mozilla.org/r/140296/#review147142

They were originally designed to test the sequence pass-fail and fail-pass separately, but it doesn't make sense to test that specifically, both because of the current structure of the production code, and because there are already three checks in "head.js", so they end up testing the sequence pass-pass-pass-pass-fail and pass-pass-pass-fail-pass instead.

> Can we rename this function to setExpectedFailures so it is more obvious what it does? It looks like the function is only used in your patch.

Makes sense, I kept the existing naming thinking it was a Mochitest standard but apparently it isn't.

I went for setExpectedFailuresForSelfTest (similar to disableUncaughtRejectionObserverForSelfTest) so it's more obvious it isn't meant to be used in normal tests, where "todo" is available instead.
Comment on attachment 8868697 [details]
Bug 1365649 - Improve browser-chrome self-tests and the "fail-if" manifest property handling.

https://reviewboard.mozilla.org/r/140296/#review147310
Attachment #8868697 - Flags: review?(dtownsend) → review+
Pushed by paolo.mozmail@amadzone.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6da58c7bb247
Improve browser-chrome self-tests and the "fail-if" manifest property handling. r=Mossop
Backed out bug 1365649 and bug 1242505 for frequently failing devtools/client/netmonitor/test/browser_net_simple-request-data.js:

Bug 1365649:
https://hg.mozilla.org/integration/mozilla-inbound/rev/398d167b26f90af9953523ef4cbef3e38af5eb4a

Bug 1242505:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5f54472fcf58d462a31398d15cacbdde5bcad7e1
https://hg.mozilla.org/integration/mozilla-inbound/rev/79db5648dbd2a1397954bc08293082b50a037ebe
https://hg.mozilla.org/integration/mozilla-inbound/rev/9dbe428fc209988be73d2a4bbc3c890811310d3d
https://hg.mozilla.org/integration/mozilla-inbound/rev/5703711e4ee5c2b00e8f8fc9fcaefbcf6a15fdf5
https://hg.mozilla.org/integration/mozilla-inbound/rev/824f44aad5d7bd26c59aaf301ce60188b5466cef

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=6f0394b523a66dab444b8551deb8f3c6c81d8f31&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=102651239&repo=mozilla-inbound

[task 2017-05-28T09:15:04.539439Z] 09:15:04     INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_simple-request-data.js | Item shouldn't have 'odd' class. - 
[task 2017-05-28T09:15:04.540493Z] 09:15:04     INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_simple-request-data.js | There should be a responseCookies data available. - 
[task 2017-05-28T09:15:04.541516Z] 09:15:04     INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_simple-request-data.js | There should be a requestCookies data available. - 
[task 2017-05-28T09:15:04.542558Z] 09:15:04     INFO - > Network events progress: 13/13, 0/0, got NetMonitor:NetworkEventUpdated:ResponseContent for server1.conn83.child1/netEvent30
[task 2017-05-28T09:15:04.543654Z] 09:15:04     INFO - Console message: [JavaScript Error: "TypeError: requestItem.responseCookies.cookies is undefined" {file: "chrome://mochitests/content/browser/devtools/client/netmonitor/test/browser_net_simple-request-data.js" line: 191}]
[task 2017-05-28T09:15:04.544806Z] 09:15:04     INFO - Console message: [JavaScript Error: "TypeError: requestItem.requestCookies.cookies is undefined" {file: "chrome://mochitests/content/browser/devtools/client/netmonitor/test/browser_net_simple-request-data.js" line: 141}]
[task 2017-05-28T09:15:04.545904Z] 09:15:04     INFO - Destroying the specified network monitor.
[task 2017-05-28T09:15:04.546930Z] 09:15:04     INFO - Removing tab.
[task 2017-05-28T09:15:04.547913Z] 09:15:04     INFO - Waiting for event: 'TabClose' on [object XULElement].
[task 2017-05-28T09:15:04.548954Z] 09:15:04     INFO - Got event: 'TabClose' on [object XULElement].
[task 2017-05-28T09:15:04.550095Z] 09:15:04     INFO - Tab removed and finished closing
[task 2017-05-28T09:15:04.551137Z] 09:15:04     INFO - finish() was called, cleaning up...
[task 2017-05-28T09:15:04.552123Z] 09:15:04     INFO - Buffered messages finished
[task 2017-05-28T09:15:04.553397Z] 09:15:04     INFO - TEST-UNEXPECTED-FAIL | devtools/client/netmonitor/test/browser_net_simple-request-data.js | A promise chain failed to handle a rejection: requestItem is undefined - rejection date: Sun May 28 2017 09:15:03 GMT+0000 (UTC) - stack: handler@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/event-emitter.js:138:34
[task 2017-05-28T09:15:04.554381Z] 09:15:04     INFO - emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/event-emitter.js:194:13
[task 2017-05-28T09:15:04.555464Z] 09:15:04     INFO - onResponseContent/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/netmonitor/src/connector/firefox-connector.js:719:7
[task 2017-05-28T09:15:04.556346Z] 09:15:04     INFO - promise callback*onResponseContent@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/netmonitor/src/connector/firefox-connector.js:718:5
[task 2017-05-28T09:15:04.557342Z] 09:15:04     INFO - emitOnObject@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/core.js:110:7
[task 2017-05-28T09:15:04.558254Z] 09:15:04     INFO - DevTools RDP*request@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:717:21
[task 2017-05-28T09:15:04.559155Z] 09:15:04     INFO - getResponseContent@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/webconsole/client.js:516:12
[task 2017-05-28T09:15:04.560059Z] 09:15:04     INFO - onNetworkEventUpdate@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/netmonitor/src/connector/firefox-connector.js:614:9
[task 2017-05-28T09:15:04.560935Z] 09:15:04     INFO - emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/event-emitter.js:194:13
[task 2017-05-28T09:15:04.561883Z] 09:15:04     INFO - _onNetworkEventUpdate@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/webconsole/client.js:170:5
[task 2017-05-28T09:15:04.562739Z] 09:15:04     INFO - eventSource/proto.emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:130:9
[task 2017-05-28T09:15:04.564056Z] 09:15:04     INFO - onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:1017:7
[task 2017-05-28T09:15:04.564938Z] 09:15:04     INFO - send/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:570:13
[task 2017-05-28T09:15:04.565854Z] 09:15:04     INFO - exports.makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/ThreadSafeDevToolsUtils.js:101:14
[task 2017-05-28T09:15:04.568580Z] 09:15:04     INFO - DevToolsUtils.executeSoon*exports.executeSoon@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/DevToolsUtils.js:42:19
[task 2017-05-28T09:15:04.569556Z] 09:15:04     INFO - send@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:563:9
[task 2017-05-28T09:15:04.570570Z] 09:15:04     INFO - send@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/main.js:1482:5
[task 2017-05-28T09:15:04.571538Z] 09:15:04     INFO - receiveMessage@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:761:7
[task 2017-05-28T09:15:04.572449Z] 09:15:04     INFO -  - false == true - JS frame :: resource://testing-common/PromiseTestUtils.jsm :: assertNoUncaughtRejections :: line 257
[task 2017-05-28T09:15:04.573331Z] 09:15:04     INFO - Stack trace:
[task 2017-05-28T09:15:04.574206Z] 09:15:04     INFO -     resource://testing-common/PromiseTestUtils.jsm:assertNoUncaughtRejections:257
[task 2017-05-28T09:15:04.575652Z] 09:15:04     INFO -     chrome://mochikit/content/browser-test.js:Tester.prototype.nextTest<:445
[task 2017-05-28T09:15:04.576555Z] 09:15:04     INFO -     testScope/test_finish/<@chrome://mochikit/content/browser-test.js:1063:11
[task 2017-05-28T09:15:04.577444Z] 09:15:04     INFO -     run@chrome://mochikit/content/browser-test.js:1000:9
[task 2017-05-28T09:15:04.578702Z] 09:15:04     INFO - Console message: [JavaScript Error: "TypeError: this.tabTarget is null" {file: "resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/netmonitor/src/connector/firefox-connector.js" line: 91}]
Flags: needinfo?(paolo.mozmail)
Pushed by paolo.mozmail@amadzone.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/021d6fb6bd8c
Improve browser-chrome self-tests and the "fail-if" manifest property handling. r=Mossop
This isn't related to the intermittent failures so I've relanded it separately.
Flags: needinfo?(paolo.mozmail)
https://hg.mozilla.org/mozilla-central/rev/021d6fb6bd8c
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.