Closed
Bug 425001
Opened 17 years ago
Closed 16 years ago
Tests for bug 400731, 431826 use timers, are fragile.
Categories
(Toolkit :: Safe Browsing, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: johnath, Assigned: johnath)
References
Details
(Keywords: fixed1.9.1)
Attachments
(1 file)
(deleted),
patch
|
Gavin
:
review+
beltzner
:
approval1.9.1+
|
Details | Diff | Splinter Review |
If possible, we should re-write those tests to use a load listener of some kind. Error pages don't fire onload, but DOMContentLoaded should work. If it doesn't, gavin also mentioned using setInterval instead, and polling for success, instead of a one-shot timer which may miss page load and create a false failure.
Blocks: 460474
Assignee | ||
Updated•16 years ago
|
Summary: Tests for bug 400731 use timers, might be fragile. → Tests for bug 400731, 431826 use timers, are fragile.
Assignee | ||
Comment 1•16 years ago
|
||
I am basically certain that at some point in the past I tried to use DOMContentLoaded listeners here and saw failures, but testing it just now locally, both tests passed with this change. Running the patch through try server now.
Attachment #372610 -
Flags: review?(gavin.sharp)
Assignee | ||
Comment 2•16 years ago
|
||
Try server barfed a little the first time, but after two runs, I got greens on each platform at least once. Honestly, if DOMContentLoaded listeners work "some of the time" on error pages, that's a test that should fail, because seriously, wtf?
Updated•16 years ago
|
Attachment #372610 -
Flags: review?(gavin.sharp) → review+
Assignee | ||
Comment 3•16 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/7c4109d88624
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 4•16 years ago
|
||
Comment on attachment 372610 [details] [diff] [review] Switch to DOMContentLoaded Assuming this tests green, landing it on branch should be a zero-pain way to remove an intermittent orange. This is a test-only change.
Attachment #372610 -
Flags: approval1.9.1?
Comment 5•16 years ago
|
||
Comment on attachment 372610 [details] [diff] [review] Switch to DOMContentLoaded Test only changes don't require approvals, but since you asked so nicely ... a+!
Attachment #372610 -
Flags: approval1.9.1? → approval1.9.1+
Assignee | ||
Comment 7•16 years ago
|
||
(In reply to comment #6) > Is this still wanted for 1.9.1? Opportunistically, if the tree is green and someone gets a chance, yes. It hasn't caused any oranges that I know of on trunk, so bringing it to branch might reduce the chance of the old versions causing an intermittent orange.
Assignee | ||
Comment 8•16 years ago
|
||
(In reply to comment #7) > Opportunistically, if the tree is green and someone gets a chance, yes. It > hasn't caused any oranges that I know of on trunk, so bringing it to branch > might reduce the chance of the old versions causing an intermittent orange. The tree was green, and I got a chance. :) http://hg.mozilla.org/releases/mozilla-1.9.1/rev/9964b0a18b28
Keywords: fixed1.9.1
Updated•10 years ago
|
Product: Firefox → Toolkit
You need to log in
before you can comment on or make changes to this bug.
Description
•