Bug 31997 - LayoutTests/transitions/change-values-during-transition.html is flaky
Summary: LayoutTests/transitions/change-values-during-transition.html is flaky
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Julie Parent
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-11-30 13:30 PST by Julie Parent
Modified: 2009-12-02 13:36 PST (History)
3 users (show)

See Also:


Attachments
Change check for 50% done to 500ms, not 600ms. Other assorted cleanups. (3.82 KB, patch)
2009-11-30 14:37 PST, Julie Parent
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Julie Parent 2009-11-30 13:30:36 PST
This test fails ~10% of the time for the Chromium ports.  See http://src.chromium.org/viewvc/chrome/trunk/src/webkit/tools/layout_tests/flakiness_dashboard.html#tests=LayoutTests%2Ftransitions%2Fchange-values-during-transition.html&showExpectations=true for flakiness graphs.  Running locally for Windows, I see this failing as much as 25% of the time in DRT.

The failures look legit, I believe the test is wrong.
On a setTimeout of 600ms, it checks to see that the animation is at x position 50 (+/- 10).  For a 1 sec duration animation that is moving 100px, it should be at position 50 at 500ms, not 600ms.

I'll fix this and do some other assorted cleanups to the test.  Because of the heavy usage of setTimeouts, I expect it will likely stay flaky, but hopefully much less so.
Comment 1 Julie Parent 2009-11-30 14:37:37 PST
Created attachment 44042 [details]
Change check for 50% done to 500ms, not 600ms.  Other assorted cleanups.
Comment 2 Adam Barth 2009-11-30 14:38:52 PST
style-queue ran check-webkit-style on attachment 44042 [details] without any errors.
Comment 3 Darin Adler 2009-12-01 10:23:10 PST
Comment on attachment 44042 [details]
Change check for 50% done to 500ms, not 600ms.  Other assorted cleanups.

I wish there was a way to isolate timing-dependent tests separately from the vast majority of tests that can run at any speed. I'd prefer to not have tests that pass or fail based on the speed or load of the computer, but if we do knowingly have them it would be *so* much better if they were identified somehow.

My comments have little to do with this patch, but rather with the original test.

I think it's unfortunate that our function to tell us if two values are close enough is called "isEqual" -- I would call it "isCloseEnough".

> +    function getXPosition() {
> +        var t = window.getComputedStyle(document.getElementById('box')).webkitTransform;
> +        t = t.split("(");
> +        t = t[1].split(",");
> +        return t[4];
> +    }

The brace style is to put the function opening brace on a separate line.

> +        document.getElementById('result').innerHTML = result;

Should use innerText or textContent, since this is not HTML.

r=me
Comment 4 Julie Parent 2009-12-02 13:36:43 PST
All comments addressed and landed in http://trac.webkit.org/changeset/51613.

I agree on the idea of isolating timing-dependent tests.  I have a few ideas, will send mail to webkit-dev about it.