Closed Bug 419747 Opened 16 years ago Closed 16 years ago

qm-win2k3-01 goes orange intermittently on browser_autodiscovery.js tests


(Firefox :: General, defect, P1)

Windows Server 2003



Firefox 3


(Reporter: myk, Assigned: Gavin)




(1 file, 2 obsolete files)

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:

cc:ing folks who have touched that file.
(In reply to comment #1)
> The first time I've seen the failure was after

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.
OS: Windows 2000 → Windows Server 2003
Blocks: 380454
Severity: normal → blocker
Comment on attachment 306256 [details] [diff] [review]
setTimeout(..., 0) -> setTimeout(..., 20) 

20ms might be enough.
Attachment #306256 - Flags: review?(
If 20ms is not, do we get intermittent orange again? How about polling?

Attached patch simple, ugly polling (obsolete) — Splinter Review
Attachment #306256 - Attachment is obsolete: true
Attachment #306392 - Flags: review?(
Attachment #306256 - Flags: review?(
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.
Assignee: nobody →
Attachment #306392 - Attachment is obsolete: true
Attachment #306632 - Flags: review?(Olli.Pettay)
Attachment #306392 - Flags: review?(
Attachment #306632 - Flags: review?(Olli.Pettay) → review+
Keywords: checkin-needed
Priority: -- → P1
Target Milestone: --- → Firefox 3
mozilla/browser/base/content/test/browser_autodiscovery.js 	1.4 
Closed: 16 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.