animations/animation-direction-normal.html This test has been a flaky failure on Mac and iOS for a while. History: https://results.webkit.org/?suite=layout-tests&test=animations%2Fanimation-direction-normal.html Diff MacOS: --- /Volumes/Data/slave/catalina-debug-tests-wk1/build/layout-test-results/animations/animation-direction-normal-expected.txt +++ /Volumes/Data/slave/catalina-debug-tests-wk1/build/layout-test-results/animations/animation-direction-normal-actual.txt @@ -1,4 +1,4 @@ -PASS - "margin-left" property for "box" element at 0.5s saw something close to: 50 -PASS - "margin-left" property for "box" element at 1s saw something close to: 100 +FAIL - "margin-left" property for "box" element at 0.5s expected: 50 but saw: 79.19999694824219 +FAIL - "margin-left" property for "box" element at 1s expected: 100 but saw: 129.3000030517578 PASS - "margin-left" property for "box" element at 2.5s saw something close to: 50 Diff iOS: --- /Volumes/Data/slave/ipados-simulator-13-debug-tests-wk2/build/layout-test-results/animations/animation-direction-normal-expected.txt +++ /Volumes/Data/slave/ipados-simulator-13-debug-tests-wk2/build/layout-test-results/animations/animation-direction-normal-actual.txt @@ -1,4 +1,4 @@ -PASS - "margin-left" property for "box" element at 0.5s saw something close to: 50 +FAIL - "margin-left" property for "box" element at 0.5s expected: 50 but saw: 18.200000762939453 PASS - "margin-left" property for "box" element at 1s saw something close to: 100 PASS - "margin-left" property for "box" element at 2.5s saw something close to: 50
<rdar://problem/61411725>
This test disables the use of a pause API and thus relies exclusively on timers. Presumably this is to check the runtime behavior of the test, but then this makes the test flaky by design since it relies on timers. We should probably nuke such tests, I don't think they serve much value.
I'll still try something to improve the situation and use a requestAnimationFrame() loop rather than set timers.
That won't help either. This test needs to be rewritten completely.
*** Bug 209362 has been marked as a duplicate of this bug. ***
Created attachment 397350 [details] Patch
Comment on attachment 397350 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=397350&action=review > LayoutTests/animations/resources/animation-test.js:34 > + this._target = options.target; "target" is a bit generic. Maybe targetProperty? > LayoutTests/animations/resources/animation-test.js:91 > + // Reset the animation. > + animation.cancel(); > + > + // Make sure it's in the paused state for seeking. > + animation.pause(); Is the cancel then pause really necessary?
(In reply to Simon Fraser (smfr) from comment #7) > Comment on attachment 397350 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=397350&action=review > > > LayoutTests/animations/resources/animation-test.js:34 > > + this._target = options.target; > > "target" is a bit generic. Maybe targetProperty? This is an element and it matches the terminology for KeyframeEffect::target. > > LayoutTests/animations/resources/animation-test.js:91 > > + // Reset the animation. > > + animation.cancel(); > > + > > + // Make sure it's in the paused state for seeking. > > + animation.pause(); > > Is the cancel then pause really necessary? Yeah, I think we can get by with the cancel() call only.
Committed r260577: <https://trac.webkit.org/changeset/260577>