Consider removing the initial wait from TestUtils.waitForCondition.
Categories
(Toolkit :: General, task, P3)
Tracking
()
People
(Reporter: emilio, Assigned: florian)
References
(Depends on 5 open bugs)
Details
Attachments
(3 files)
In https://phabricator.services.mozilla.com/D52833 (bug 1595285) I naively changed waitForCondition
from doing:
setInterval(async function() { ... }, interval);
to:
async function tryOnce() {
// ...
setTimeout(tryOnce, interval);
}
tryOnce();
That happened to break a bunch of tests which weren't expecting the initial check to be synchronous, so I had to change the last line to:
setTimeout(tryOnce, interval);
We should consider removing that.
Updated•5 years ago
|
Assignee | ||
Comment 1•3 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #0)
That happened to break a bunch of tests which weren't expecting the initial check to be synchronous,
I think it's reasonable for tests to expect this function to not be synchronous, but the initial 100ms wait is nasty as it makes tests implicitly wait for "stuff" to happen during these 100ms... and sometimes that stuff can take longer, and cause intermittent failures.
I'm proposing to change the last line to: TestUtils.executeSoon(tryOnce);
. This should speed up the tests significantly, but avoid the synchronous call, and avoid the timer.
This still causes LOTS of test failures (most of them are existing intermittent failures that become perma fail), that I'll try to resolve in dependent bugs.
Assignee | ||
Comment 2•3 years ago
|
||
Depends on D108010
Assignee | ||
Comment 3•3 years ago
|
||
Depends on D108019
Pushed by fqueze@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/3ae4c40c2d0f Avoid browser_aaa_eventTelemetry_run_first.js failures by putting back a 100ms timer before the waitForCondition call, r=Gijs.
Comment 5•3 years ago
|
||
bugherder |
Assignee | ||
Comment 6•3 years ago
|
||
Reopening, I forgot to put the leave-open keyword here before landing a first batch of test fixes.
Updated•3 years ago
|
Assignee | ||
Comment 7•3 years ago
|
||
Pushed by fqueze@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7d8fb53c8fcb add back timers for tests that permafail without the waitForCondition initial timer, r=Gijs. https://hg.mozilla.org/integration/autoland/rev/4eccbb3d6549 Remove the initial timer in TestUtils.waitForCondition, r=Gijs.
Comment 9•3 years ago
|
||
Backed out 2 changesets (bug 1596165) for bc failures in browser_bookmarkProperties_folderSelection.js and browser_doorhanger_submit_telemetry.js.
https://hg.mozilla.org/integration/autoland/rev/f616889f61563e180009ee1bcd8edc4a435032d4
Push with failures:
https://treeherder.mozilla.org/jobs?repo=autoland&revision=4eccbb3d6549091c7763aef9b2d896072a50f5bd&selectedTaskRun=fBgRgidgQ1miQOdWMMCIyw.0
Failure log:
browser_doorhanger_submit_telemetry.js:
https://treeherder.mozilla.org/logviewer?job_id=334344122&repo=autoland&lineNumber=6881
browser_bookmarkProperties_folderSelection.js
https://treeherder.mozilla.org/logviewer?job_id=334348413&repo=autoland&lineNumber=2312
Comment 10•3 years ago
|
||
Pushed by fqueze@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/21067aab87ee add back timers for tests that permafail without the waitForCondition initial timer, r=Gijs. https://hg.mozilla.org/integration/autoland/rev/8581dbde5b5b Remove the initial timer in TestUtils.waitForCondition, r=Gijs.
Comment 11•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/21067aab87ee
https://hg.mozilla.org/mozilla-central/rev/8581dbde5b5b
Updated•3 years ago
|
Assignee | ||
Updated•3 years ago
|
Description
•