The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in mozilla10

Status

()

Core
SVG
RESOLVED FIXED
7 years ago
4 years ago

People

(Reporter: jfkthame, Assigned: birtles)

Tracking

({intermittent-failure})

Trunk
mozilla10
x86
Mac OS X
intermittent-failure
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

7 years ago
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.

Updated

7 years ago
Duplicate of this bug: 602505
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

Updated

7 years ago
Duplicate of this bug: 602519
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.
Comment hidden (Treeherder Robot)
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 (==)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
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.
Attachment #561246 - Flags: review?(birtles)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 42

6 years ago
(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
(Assignee)

Comment 43

6 years ago
Created attachment 561375 [details] [diff] [review]
Extend the timeout

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.

Comment 44

6 years ago
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
(Assignee)

Comment 45

6 years ago
Created attachment 562627 [details] [diff] [review]
Proposed patch v1a

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)
(Assignee)

Comment 46

6 years ago
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+
(Assignee)

Comment 48

6 years ago
Created attachment 562629 [details] [diff] [review]
Proposed patch v1b

Address review feedback (comment 47).
Attachment #562627 - Attachment is obsolete: true
Attachment #562629 - Flags: review+
(Assignee)

Comment 49

6 years ago
Created attachment 562630 [details] [diff] [review]
Proposed patch v1c; r=dholbert

Forgot to add the r=dholbert to the check-in comment
Attachment #562629 - Attachment is obsolete: true
Attachment #562630 - Flags: review+
(Assignee)

Updated

6 years ago
Attachment #562630 - Attachment description: Part 2 - Proposed patch v1c; r=dholbert → Proposed patch v1c; r=dholbert
Attachment #562630 - Attachment is patch: true
(Assignee)

Comment 50

6 years ago
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.

Comment 51

6 years ago
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
(Assignee)

Comment 52

6 years ago
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.
https://hg.mozilla.org/mozilla-central/rev/3548d5c7cde5
Target Milestone: --- → mozilla10
Comment hidden (Treeherder Robot)
(Assignee)

Comment 55

6 years ago
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.
(Assignee)

Comment 56

5 years ago
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
Last Resolved: 5 years ago
Resolution: --- → FIXED
Keywords: intermittent-failure
Whiteboard: [orange]
You need to log in before you can comment on or make changes to this bug.