Tests for bug 400731, 431826 use timers, are fragile.

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
12 years ago
5 years ago

People

(Reporter: johnath, Assigned: johnath)

Tracking

({fixed1.9.1})

Trunk
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

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: 400731
Summary: Tests for bug 400731 use timers, might be fragile. → Tests for bug 400731, 431826 use timers, are fragile.
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)
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?
Attachment #372610 - Flags: review?(gavin.sharp) → review+
http://hg.mozilla.org/mozilla-central/rev/7c4109d88624
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
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 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+
Is this still wanted for 1.9.1?
(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.
(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
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.