Closed Bug 602504 Opened 15 years ago Closed 14 years ago

intermittent reftest failure in reftests/svg/smil/event/event-begin-timeevent-3.svg | image comparison (==)

Categories

(Core :: SVG, defect)

Product:
Core
Component:
SVG
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla10

People

(Reporter: jfkthame, Assigned: birtles)

References

Details

(Keywords: intermittent-failure)

Attachments

(3 files, 2 obsolete files)

Disable the test
1.02 KB, patch
Details | Diff | Splinter Review
Extend the timeout
1.39 KB, patch
Details | Diff | Splinter Review
Proposed patch v1c; r=dholbert
2.29 KB, patch
birtles
: review+
Details | Diff | Splinter Review
This failed with a timeout on OS X opt RGL today: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1286456060.1286456889.14338.gz However, another test run on the same changeset passed, so it is apparently an intermittent issue.
Blocks: 438871
Whiteboard: [orange]
For future reference, add "[orange]" to whiteboard when filing intermittent orange bugs, so that tbpl can find them. http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1286463047.1286463938.17395.gz Rev3 MacOSX Leopard 10.5.8 mozilla-central opt test opengl on 2010/10/07 07:50:47 s: talos-r3-leopard-025
also, looks like both instances of this so far have been reftest failures, not timeouts.
as philor says in duplicate bug 602519 about the reftest snapshot: > The log isn't terribly exciting: pure red instead of pure green.
Summary: intermittent timeout in reftests/svg/smil/event/event-begin-timeevent-3.svg → intermittent reftest failure in reftests/svg/smil/event/event-begin-timeevent-3.svg | image comparison (==)
Attached patch Disable the test — — Splinter Review
This was currently random-if(Android) but it seems like it doesn't run reliably on desktop either, so we should probably make it random everywhere.
Attachment #561246 - Flags: review?(birtles)
(In reply to Jeff Muizelaar [:jrmuizel] from comment #40) > Created attachment 561246 [details] [diff] [review] > Disable the test > > This was currently random-if(Android) but it seems like it doesn't run > reliably on desktop either, so we should probably make it random everywhere. Thanks very much for looking into this. Sorry I hadn't got to it yet. I've looked over the test case and I think what's happening is that we don't get to process the repeatEvent within the 100ms timeout used in this test. Probably what we want to do is rework the test to pass early and fail slow--i.e. add an event listener, if the animation starts end the test immediately, otherwise add a long timeout before failing. I'm pretty sure we're doing that elsewhere. I'll look into it next week. In the meantime, one "fix" is to just adjust the 100ms timeout to 250ms which should reduce the failure rate from once/twice a week to perhaps once a month or so? Not a great long-term solution though but at any rate, I'd rather do that than mark this test as random.
Assignee: nobody → birtles
Status: NEW → ASSIGNED
Extend the timeout. This is not the final fix but might be useful in the meantime for reducing the failure rate until a more permanent fix is done.
Try run for e775f0cd2fe9 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=e775f0cd2fe9 Results (out of 103 total builds): success: 87 warnings: 15 failure: 1 Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/bbirtles@mozilla.com-e775f0cd2fe9
Attached patch Proposed patch v1a (obsolete) — — Splinter Review
This patch implements the solution indicated in comment 42, i.e. detect the success case and end right away, or wait longer for the failure case.
Attachment #562627 - Flags: review?(dholbert)
Comment on attachment 561246 [details] [diff] [review] Disable the test Cancelling review for the patch to disable the test since I think we can fix this.
Attachment #561246 - Flags: review?(birtles)
Comment on attachment 562627 [details] [diff] [review] Proposed patch v1a r=me, with two nits: - I worry that the 5 second timeout might be short enough that it could sporadically fail on really crappy hardware with GC pauses & other stuff going on. Could we up that to 30 or 60 seconds? - Could you declare the timeout amount as a constant at the top of the JS block, so it's clear what it's there for (and easy to tweak if we want to increase / decrease it)? Thanks!
Attachment #562627 - Flags: review?(dholbert) → review+
Attached patch Proposed patch v1b (obsolete) — — Splinter Review
Address review feedback (comment 47).
Attachment #562627 - Attachment is obsolete: true
Attachment #562629 - Flags: review+
Forgot to add the r=dholbert to the check-in comment
Attachment #562629 - Attachment is obsolete: true
Attachment #562630 - Flags: review+
Attachment #562630 - Attachment description: Part 2 - Proposed patch v1c; r=dholbert → Proposed patch v1c; r=dholbert
Attachment #562630 - Attachment is patch: true
I've sent an older version of this test off to Try so I'll wait to check that everything is ok there before landing this.
Try run for 3e287957617f is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=3e287957617f Results (out of 33 total builds): success: 30 warnings: 3 Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/bbirtles@mozilla.com-3e287957617f
Pushed patch to m-i: https://hg.mozilla.org/integration/mozilla-inbound/rev/3548d5c7cde5 I'd like to leave this bug open for about 6 weeks though to be sure this has fixed it.
Ignoring comment 54 since it's on aurora which is moz 9 and the patch I'm hoping fixes this has target milestone moz 10.
It's six weeks now since this landed and there have been no failures (on any branch where this has landed) so I'm pretty confident this is fixed.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [orange]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: