Closed Bug 965644 Opened 11 years ago Closed 10 years ago

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

Categories

(Core :: DOM: Workers, defect)

Product:
Core
Component:
DOM: Workers
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla33
Tracking Flags:
Tracking Status
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

People

(Reporter: KWierso, Assigned: nsm)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file, 1 obsolete file)

Fix intermittent oranges on promise tests.
2.54 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
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)
Updated main thread tests.
Attachment #8441613 - Flags: review?(bzbarsky)
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+
Depends on: 1027221
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: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: