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

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, defect)

Description

[Johnathan Nightingale [:johnath]]

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.

Comment 1

[Johnathan Nightingale [:johnath]]

Attachment: Switch to DOMContentLoaded

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.

Comment 2

[Johnathan Nightingale [:johnath]]

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?

Comment 3

[Johnathan Nightingale [:johnath]]

http://hg.mozilla.org/mozilla-central/rev/7c4109d88624

Comment 4

[Johnathan Nightingale [:johnath]]

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.

Comment 5

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 372610 [details] [diff] [review]
Switch to DOMContentLoaded

Test only changes don't require approvals, but since you asked so nicely ... a+!

Comment 6

[Karl Tomlinson (:karlt)]

Is this still wanted for 1.9.1?

Comment 7

[Johnathan Nightingale [:johnath]]

(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.

Comment 8

[Johnathan Nightingale [:johnath]]

(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