Bug 210051 – [ Mojave wk1 Release ] animations/transition-and-animation-1.html is a flaky failure

• Status: RESOLVED
• Resolution: FIXED
• Component: Animations

[Truitt Savell]

animations/transition-and-animation-1.html

This test became recently flaky on Mojave wk1 Release

history
https://results.webkit.org/?suite=layout-tests&test=animations%2Ftransition-and-animation-1.html

Diff:
+++ /Volumes/Data/slave/mojave-release-tests-wk1/build/layout-test-results/animations/transition-and-animation-1-actual.txt
@@ -1,3 +1,3 @@
 This test has a transition and animation on the same property (-webkit-transform). But the transition is never triggered, so nothing should be moving when the animation finishes.
-PASS - "webkitTransform" property for "box" element at 0.55s saw something close to: none
+FAIL - "webkitTransform" property for "box" element at 0.55s expected: none but saw: matrix(1, 0, 0, 1, 200, 0)

[Radar WebKit Bug Importer]

<rdar://problem/61345177>

[Truitt Savell]

I am able to reproduce this failure using this command on a debug build:

run-webkit-tests --iterations 2000 --exit-after-n-failures 1 --exit-after-n-crashes-or-timeouts 1 --debug-rwt-logging --no-retry --force --no-build -f -g -1 animations/transition-and-animation-1.html

[Truitt Savell]

Looking deeper in the history, this test appears to fail every couple of days for nearly the whole visible history.

[Antoine Quint]

Created attachment 397194 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 397194 [details]
Patch

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

> LayoutTests/animations/transition-and-animation-1.html:50
> +document.getElementById("box").addEventListener("animationend", event => {
> +    requestAnimationFrame(timestamp => {
> +        checkExpectedValue(expectedValues, 0);
> +        endTest();
> +    });
> +});

This would be really nice if you made a re-usable promise-based "animationend" thing and used async/await here. See UIHelper.animationFrame().

[Antoine Quint]

(In reply to Simon Fraser (smfr) from comment #5)
> Comment on attachment 397194 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=397194&action=review
> 
> > LayoutTests/animations/transition-and-animation-1.html:50
> > +document.getElementById("box").addEventListener("animationend", event => {
> > +    requestAnimationFrame(timestamp => {
> > +        checkExpectedValue(expectedValues, 0);
> > +        endTest();
> > +    });
> > +});
> 
> This would be really nice if you made a re-usable promise-based
> "animationend" thing and used async/await here. See
> UIHelper.animationFrame().

We could also use the Web Animations API which already has the finished promise. I just assumed these tests were supposed to be "pure CSS" as much as possible and not use the Web Animations API.

[Antoine Quint]

Created attachment 397196 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 397196 [details]
Patch

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

> LayoutTests/animations/transition-and-animation-1.html:23
>          to   { -webkit-transform: translateX(300px); }

Might as well remove prefixes too.

> LayoutTests/animations/transition-and-animation-1.html:26
> +    <script src="../resources/ui-helper.js" type="text/javascript" charset="utf-8"></script>

You can drop the type="text/javascript" charset="utf-8" in this and the next script tag.

[Antoine Quint]

(In reply to Simon Fraser (smfr) from comment #8)
> Comment on attachment 397196 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=397196&action=review
> 
> > LayoutTests/animations/transition-and-animation-1.html:23
> >          to   { -webkit-transform: translateX(300px); }
> 
> Might as well remove prefixes too.
> 
> > LayoutTests/animations/transition-and-animation-1.html:26
> > +    <script src="../resources/ui-helper.js" type="text/javascript" charset="utf-8"></script>
> 
> You can drop the type="text/javascript" charset="utf-8" in this and the next
> script tag.

Will do in the commit.

[Antoine Quint]

Committed r260513: <https://trac.webkit.org/changeset/260513>

[Antoine Quint]

Committed r260521: <https://trac.webkit.org/changeset/260521>