Intermittent test_promise.html, test_promise_utils.html | 50 === 20: Fastest timeout should win.

RESOLVED FIXED in Firefox 31

Status

()

defect
RESOLVED FIXED
6 years ago
11 months ago

People

(Reporter: KWierso, Assigned: nsm)

Tracking

({intermittent-failure})

unspecified
mozilla33
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox30 wontfix, firefox31 fixed, firefox32 fixed, firefox33 fixed, firefox-esr24 unaffected, b2g-v1.3 unaffected, b2g-v1.3T unaffected, b2g-v1.4 wontfix, b2g-v2.0 fixed, b2g-v2.1 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

https://tbpl.mozilla.org/php/getParsedLog.php?id=33771907&tree=Mozilla-Inbound#error0
slave: tst-linux64-ec2-018




16:27:59     INFO -  4592 INFO TEST-PASS | /tests/dom/workers/test/test_promise.html | 2 === 2: Should resolve to 2.
16:27:59     INFO -  4593 INFO TEST-PASS | /tests/dom/workers/test/test_promise.html | true: Should cast to a Promise.
16:27:59     INFO -  4594 INFO TEST-PASS | /tests/dom/workers/test/test_promise.html | [object Promise] === [object Promise]: Should return original Promise.
16:27:59     INFO -  4595 INFO TEST-PASS | /tests/dom/workers/test/test_promise.html | true: Should return a Promise.
16:27:59     INFO -  4596 INFO TEST-PASS | /tests/dom/workers/test/test_promise.html | true: Should return a Promise.
16:27:59     INFO -  4597 INFO TEST-PASS | /tests/dom/workers/test/test_promise.html | true === true: First value should win.
16:27:59     INFO -  4598 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/workers/test/test_promise.html | 50 === 20: Fastest timeout should win.
16:27:59     INFO -  4599 INFO TEST-PASS | /tests/dom/workers/test/test_promise.html | true: Should be rejected
16:27:59     INFO -  4600 INFO TEST-PASS | /tests/dom/workers/test/test_promise.html | true: Should reject with Error.
16:27:59     INFO -  4601 INFO TEST-PASS | /tests/dom/workers/test/test_promise.html | true: Message should match.
16:27:59     INFO -  4602 INFO TEST-PASS | /tests/dom/workers/test/test_promise.html | true: Should be rejected
16:27:59     INFO -  4603 INFO TEST-PASS | /tests/dom/workers/test/test_promise.html | true: Should reject with ReferenceError for function nonExistent().
https://tbpl.mozilla.org/php/getParsedLog.php?id=34471727&tree=Mozilla-Inbound
Summary: Intermittent TEST-UNEXPECTED-FAIL | /tests/dom/workers/test/test_promise.html | 50 === 20: Fastest timeout should win. → Intermittent TEST-UNEXPECTED-FAIL | /tests/dom/workers/test/test_promise.html,test_promise_utils.html | 50 === 20: Fastest timeout should win.
Summary: Intermittent TEST-UNEXPECTED-FAIL | /tests/dom/workers/test/test_promise.html,test_promise_utils.html | 50 === 20: Fastest timeout should win. → Intermittent test_promise.html, test_promise_utils.html | 50 === 20: Fastest timeout should win.
Nikhil, any idea what might be going on here? :)
Flags: needinfo?(nsm.nikhil)
Component: Async Tooling → DOM: Workers
Product: Toolkit → Core
Guess I can't rely on setTimeout() ordering. Will fix this soon.
Flags: needinfo?(nsm.nikhil)
Assignee: nobody → nsm.nikhil
Yes, using setTimeout in tests is almost always a bad idea :)
https://developer.mozilla.org/en-US/docs/Mozilla/QA/Avoiding_intermittent_oranges
Comment on attachment 8441579 [details] [diff] [review]
Fix intermittent oranges on promise worker tests.

Ah, yes.  You can't "rely on timeout ordering" because what can happen is that after the timeoutPromise(50) call you lose the timeslice for 40ms, so the timeoutPromise(20) ends up scheduled for T+60 while the timeoutPromise(50) is scheduled for T+50.

>+    new Promise(function() {}),

What resolves this promise, exactly?

Also, don't you need a similar change for test_promise_utils.html?
Flags: needinfo?(nsm.nikhil)
(In reply to Boris Zbarsky [:bz] from comment #204)
> Comment on attachment 8441579 [details] [diff] [review]
> Fix intermittent oranges on promise worker tests.
> 
> Ah, yes.  You can't "rely on timeout ordering" because what can happen is
> that after the timeoutPromise(50) call you lose the timeslice for 40ms, so
> the timeoutPromise(20) ends up scheduled for T+60 while the
> timeoutPromise(50) is scheduled for T+50.
> 
> >+    new Promise(function() {}),
> 
> What resolves this promise, exactly?

Nothing, it is unresolved.

> 
> Also, don't you need a similar change for test_promise_utils.html?

Good point.
Flags: needinfo?(nsm.nikhil)
Attachment #8441579 - Attachment is obsolete: true
Attachment #8441579 - Flags: review?(bzbarsky)
> Nothing, it is unresolved.

Ah, I guess for race() that doesn't matter, right.
Comment on attachment 8441613 [details] [diff] [review]
Fix intermittent oranges on promise tests.

r=me
Attachment #8441613 - Flags: review?(bzbarsky) → review+
Backed out in  https://hg.mozilla.org/integration/mozilla-inbound/rev/d7a66cb08140 for making test_promise.html practically permafail: https://tbpl.mozilla.org/php/getParsedLog.php?id=41912041&tree=Mozilla-Inbound
Flags: needinfo?(nsm.nikhil)
I filed bug 1027221 on fixing the JS engine threading bug the modified test exposed.  :(
Flags: needinfo?(nsm.nikhil)
Hi,
could you provide a Try link. Suggestions for what to run if you haven't
yet can be found here:
https://wiki.mozilla.org/Sheriffing/How:To:Recommended_Try_Practices
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/6cd5b454f9c2
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.