Bug 31997

Summary: LayoutTests/transitions/change-values-during-transition.html is flaky
Product: WebKit Reporter: Julie Parent <jparent>
Component: Tools / TestsAssignee: Julie Parent <jparent>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, cmarrin, eric
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Change check for 50% done to 500ms, not 600ms. Other assorted cleanups. darin: review+

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.