Closed Bug 419747 Opened 13 years ago Closed 13 years ago
qm-win2k3-01 goes orange intermittently on browser
_autodiscovery .js tests
According to smaug on IRC, the following browser_autodiscovery.js test failures have been happening randomly on qm-win2k3-01 lately: FAIL - rel search discovered - chrome://mochikit/content/browser/browser/base/content/test/browser_autodiscovery.js FAIL - rel is case insensitive - chrome://mochikit/content/browser/browser/base/content/test/browser_autodiscovery.js They happened again a moment ago. Here's the brief log: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1204069318.1204071563.7208.gz cc:ing folks who have touched that file.
The first time I've seen the failure was after http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1204020590&maxdate=1204022788
(In reply to comment #1) > The first time I've seen the failure was after > http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1204020590&maxdate=1204022788 > No tests for Bug 419549...
The patch for bug 380454 made DOMLinkAdded fire asynchronously, and changed the test to use a setTimeout(test, 0). Seems likely that in the failure cases, 10ms isn't long enough for the DOMLinkAdded event to have fired, especially given the unit test boxes' tendency to be resource constrained.
Comment on attachment 306256 [details] [diff] [review] setTimeout(..., 0) -> setTimeout(..., 20) 20ms might be enough.
If 20ms is not, do we get intermittent orange again? How about polling? /be
Won't this lead to infinite recursion if any of the test conditions fail?
No recursion but timer loop, which is IMHO ok for a test. Mochitest should eventually stop that test if it fails.
(In reply to comment #9) > No recursion but timer loop That's what I meant. > which is IMHO ok for a test. Mochitest should eventually stop that test if it > fails. Have you tested that? I don't really like relying on mochitest runaway-test handling, and turning any failure into a "test timed out" error hurts debugging should the test actually fail on tinderbox. Perhaps we could change modify browser.js code to allow defining arbitrary functions to be called after the browser's DOMLinkAdded listener has done it's work, and change the test to use that? Somewhat of a test-specific hack, but I imagine it may be useful to extensions as well.
(In reply to comment #10) > Have you tested that? Yes. It does give the timeout failure quite fast and whoever sees the error can look at which one was the previous succeeding test.
sayrer proposed this on IRC, it works well and avoids the need for polling.
Attachment #306632 - Flags: review?(Olli.Pettay) → review+
Priority: -- → P1
Target Milestone: --- → Firefox 3
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.