Bug 124298 – Flaky Test: media/video-fast-seek.html

• Component: Tools / Tests
• Status: RESOLVED FIXED

[WebKit Commit Bot]

This is an automatically generated bug from the commit-queue.
media/video-fast-seek.html has been flaky on the commit-queue.

media/video-fast-seek.html was authored by [e-mail].
http://trac.webkit.org/browser/trunk/LayoutTests/media/video-fast-seek.html

The commit-queue just saw media/video-fast-seek.html flake (text diff) while processing attachment 216820 [details] on bug 124292.
Bot: webkit-cq-02  Port: <class 'webkitpy.common.config.ports.MacPort'>  Platform: Mac OS X 10.8.4

The bots will update this with information from each new failure.

If you believe this bug to be fixed or invalid, feel free to close.  The bots will re-open if the flake re-occurs.

If you would like to track this test fix with another bug, please close this bug as a duplicate.  The bots will follow the duplicate chain when making future comments.

[WebKit Commit Bot]

Created attachment 216827 [details]
Archive of layout-test-results from webkit-cq-02

[Jer Noble]

Created attachment 216853 [details]
Patch

[Eric Carlson]

Comment on attachment 216853 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=216853&action=review

> LayoutTests/media/video-fast-seek.html:-30
> -            testExpected('video.currentTime', 2.5);

How about "testExpected('video.currentTime.toFixed(1)', 2.5)" instead?

> LayoutTests/media/video-fast-seek.html:-46
> -            testExpected('video.currentTime', 2.3);

Ditto.

[Martin Hock]

Comment on attachment 216853 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=216853&action=review

>> LayoutTests/media/video-fast-seek.html:-30
>> -            testExpected('video.currentTime', 2.5);
> 
> How about "testExpected('video.currentTime.toFixed(1)', 2.5)" instead?

I'm surprised we would see a failure in 2.5 as it is an exact sum of powers of two.

I like Eric's suggestion but can we instead do something like video.currentTime.toFixed(2), '2.50' just to make sure we're extra-close?

The 2.6 and 2.2 checks could also fail because we aren't guaranteed about rounding direction (typically rounding to the nearest representable number occurs which could be slightly bigger than what you expect).  Can you change them to be "within epsilon" as well?  For example:

testExpected('video.currentTime.toFixed(2)', 2.2, <=)

[Jer Noble]

(In reply to comment #4)
> (From update of attachment 216853 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=216853&action=review
> 
> >> LayoutTests/media/video-fast-seek.html:-30
> >> -            testExpected('video.currentTime', 2.5);
> > 
> > How about "testExpected('video.currentTime.toFixed(1)', 2.5)" instead?
> 
> I'm surprised we would see a failure in 2.5 as it is an exact sum of powers of two.

The problem is, as always, converting between foating point, to fixed point, and back.

> I like Eric's suggestion but can we instead do something like video.currentTime.toFixed(2), '2.50' just to make sure we're extra-close?

It's really not that important, which is why I removed that part in my first version of the patch.

> The 2.6 and 2.2 checks could also fail because we aren't guaranteed about rounding direction (typically rounding to the nearest representable number occurs which could be slightly bigger than what you expect).  Can you change them to be "within epsilon" as well?  For example:
> 
> testExpected('video.currentTime.toFixed(2)', 2.2, <=)

For Mac and Win ports, we'll always be much > than 2.6 and much < than 2.2, as the fastSeek() logic will go to the next and previous Sync frame, respectively.  For other ports who haven't adopted the fastSeek back end, they  might have rounding errors.

[Jer Noble]

Committed r159306: <http://trac.webkit.org/changeset/159306>