Title: [260577] trunk/LayoutTests
Revision
260577
Author
grao...@webkit.org
Date
2020-04-23 10:23:18 -0700 (Thu, 23 Apr 2020)

Log Message

[ Mac iOS ] animations/animation-direction-normal.html is a flaky failure
https://bugs.webkit.org/show_bug.cgi?id=210156
<rdar://problem/61411725>

Reviewed by Simon Fraser.

The tests animations/animation-direction-normal.html and animations/animation-direction-reverse.html were both written
similarly to test that an element targeted by a CSS Animation would have styles animated while the animation is running
and that those styles would no longer be animated once the CSS Animation was paused using the "animation-play-state"
CSS property.

The way those assertions were made were to use setTimeout() to check the computed style at a given time and compared it
to an expected value give or take an error margin. This design was flaky, as a system under load could easily not run the
timeout until a much larger delta than the one expected would elapse.

We use a new JS helper to write these tests in a non-flaky manner. The technique used now is to record the computed style
while the animation is running without providing expected times and values, but rather specifying delays between which we
want to record the computed style. Once all values have been recorded, a method can be used to check those recorded values
by using the Web Animations API to pause and seek the animation at recorded times and query the computed style, which allows
us to test values without an error margin.

Finally, the new JS helper also allows to check the computed style using a timeout when the animation play state is not relevant,
allowing those tests to pause the animation using the "animation-play-state" property and check after incremental timeouts
that the computed style did not change.

We also made the tests use the WPT harness for assertions and reporting.

* animations/animation-direction-normal-expected.txt:
* animations/animation-direction-normal.html:
* animations/animation-direction-reverse-expected.txt:
* animations/animation-direction-reverse.html:
* animations/resources/animation-test.js: Added.
(AnimationTest):
(AnimationTest.prototype.get animation):
(AnimationTest.prototype.get value):
(AnimationTest.prototype.async valueAfterWaitingFor):
(AnimationTest.prototype.async recordValueAfterRunningFor):
(AnimationTest.prototype.checkRecordedValues):
(AnimationTest.prototype._tickUntil):
* platform/ios-wk1/TestExpectations:
* platform/ios-wk2/TestExpectations:

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (260576 => 260577)


--- trunk/LayoutTests/ChangeLog	2020-04-23 17:15:45 UTC (rev 260576)
+++ trunk/LayoutTests/ChangeLog	2020-04-23 17:23:18 UTC (rev 260577)
@@ -1,3 +1,47 @@
+2020-04-23  Antoine Quint  <grao...@apple.com>
+
+        [ Mac iOS ] animations/animation-direction-normal.html is a flaky failure
+        https://bugs.webkit.org/show_bug.cgi?id=210156
+        <rdar://problem/61411725>
+
+        Reviewed by Simon Fraser.
+
+        The tests animations/animation-direction-normal.html and animations/animation-direction-reverse.html were both written
+        similarly to test that an element targeted by a CSS Animation would have styles animated while the animation is running
+        and that those styles would no longer be animated once the CSS Animation was paused using the "animation-play-state"
+        CSS property.
+
+        The way those assertions were made were to use setTimeout() to check the computed style at a given time and compared it
+        to an expected value give or take an error margin. This design was flaky, as a system under load could easily not run the
+        timeout until a much larger delta than the one expected would elapse.
+
+        We use a new JS helper to write these tests in a non-flaky manner. The technique used now is to record the computed style
+        while the animation is running without providing expected times and values, but rather specifying delays between which we
+        want to record the computed style. Once all values have been recorded, a method can be used to check those recorded values
+        by using the Web Animations API to pause and seek the animation at recorded times and query the computed style, which allows
+        us to test values without an error margin.
+
+        Finally, the new JS helper also allows to check the computed style using a timeout when the animation play state is not relevant,
+        allowing those tests to pause the animation using the "animation-play-state" property and check after incremental timeouts
+        that the computed style did not change.
+
+        We also made the tests use the WPT harness for assertions and reporting.
+
+        * animations/animation-direction-normal-expected.txt:
+        * animations/animation-direction-normal.html:
+        * animations/animation-direction-reverse-expected.txt:
+        * animations/animation-direction-reverse.html:
+        * animations/resources/animation-test.js: Added.
+        (AnimationTest):
+        (AnimationTest.prototype.get animation):
+        (AnimationTest.prototype.get value):
+        (AnimationTest.prototype.async valueAfterWaitingFor):
+        (AnimationTest.prototype.async recordValueAfterRunningFor):
+        (AnimationTest.prototype.checkRecordedValues):
+        (AnimationTest.prototype._tickUntil):
+        * platform/ios-wk1/TestExpectations:
+        * platform/ios-wk2/TestExpectations:
+
 2020-04-23  Chris Dumez  <cdu...@apple.com>
 
         [ Mac wk2 ] imported/w3c/web-platform-tests/notifications/event-onclose.html is flaky failing.

Modified: trunk/LayoutTests/animations/animation-direction-normal-expected.txt (260576 => 260577)


--- trunk/LayoutTests/animations/animation-direction-normal-expected.txt	2020-04-23 17:15:45 UTC (rev 260576)
+++ trunk/LayoutTests/animations/animation-direction-normal-expected.txt	2020-04-23 17:23:18 UTC (rev 260577)
@@ -1,4 +1,3 @@
-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
-PASS - "margin-left" property for "box" element at 2.5s saw something close to: 50
 
+PASS Pausing an animation using the animation-play-state property stops animating styles. 
+

Modified: trunk/LayoutTests/animations/animation-direction-normal.html (260576 => 260577)


--- trunk/LayoutTests/animations/animation-direction-normal.html	2020-04-23 17:15:45 UTC (rev 260576)
+++ trunk/LayoutTests/animations/animation-direction-normal.html	2020-04-23 17:23:18 UTC (rev 260577)
@@ -1,7 +1,7 @@
 <html lang="en">
 <head>
   <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
-  <title>Test of -webkit-animation-play-state</title>
+  <title>Test of animation-play-state</title>
   <style type="text/css" media="screen">
     body {
       margin: 0;
@@ -15,58 +15,50 @@
       width: 100px;
       background-color: red;
       margin: 0;
-      -webkit-animation-duration: 2s;
-      -webkit-animation-direction: normal;
-      -webkit-animation-iteration-count: 2;
-      -webkit-animation-timing-function: linear;
-      -webkit-animation-name: "move1";
+      animation-duration: 500ms;
+      animation-direction: normal;
+      animation-iteration-count: infinite;
+      animation-timing-function: linear;
+      animation-name: "move1";
     }
-    #safezone {
-      position: absolute;
-      top: 100px;
-      height: 100px;
-      width: 120px;
-      left: 30px;
-      background-color: green;
-    }
-    @-webkit-keyframes "move1" {
+
+    @keyframes move1 {
       from { margin-left: 0px; }
       to   { margin-left: 200px; }
     }
-    #result {
-      color: white; /* hide from pixel results */
-    }
   </style>
-  <script src="" type="text/_javascript_" charset="utf-8"></script>
-  <script type="text/_javascript_" charset="utf-8">
-    const expectedValues = [
-      // [animation-name, time, element-id, property, expected-value, tolerance]
-      ["move1", 0.5, "box", "margin-left", 50, 20],
-      ["move1", 1.0, "box", "margin-left", 100, 20],
-      ["move1", 2.5, "box", "margin-left", 50, 20],
-    ];
+  <script src=""
+  <script src=""
+  <script src=""
+</head>
+<body>
+<div id="box"></div>
+<script>
+    async_test(async t => {
+        const delay = 100;
 
-    function pauseAnimation()
-    {
-        const box = document.getElementById("box");
-        box.style.webkitAnimationPlayState = "paused";
-        box.getAnimations()[0].currentTime = 2500;
-    }
+        const test = new AnimationTest({
+            target: document.getElementById("box"),
+            styleExtractor: style => parseFloat(style.marginLeft)
+        });
 
-    function setTimers()
-    {
-        setTimeout(pauseAnimation, 2500);
-    }
+        // Record two computed values after the specified delay each.
+        await test.recordValueAfterRunningFor(delay);
+        await test.recordValueAfterRunningFor(delay);
 
-    runAnimationTest(expectedValues, setTimers, null, true);
+        // We'll now pause the animation using the CSS property "animation-play-state".
+        box.style.animationPlayState = "paused";
 
-  </script>
-</head>
-<body>
-<!-- This tests the operation of -webkit-animation-play-state. After 1 second the red boxes should be hidden by the green boxes. You should see no red boxes. -->
-<div id="box"></div>
-<div id="safezone"></div>
-<div id="result"></div>
-</div>
+        // And now we'll record values after the specified delay each and check that those are the same.
+        const initialPausedValue = await test.valueAfterWaitingFor(delay);
+        const currentPausedValue = await test.valueAfterWaitingFor(delay);
+        assert_equals(initialPausedValue, currentPausedValue, "Values recorded while paused are the same.");
+
+        // Finally, check the values recorded earlier in the test.
+        test.checkRecordedValues();
+
+        t.done();
+    }, `Pausing an animation using the animation-play-state property stops animating styles.`);
+</script>
 </body>
 </html>

Modified: trunk/LayoutTests/animations/animation-direction-reverse-expected.txt (260576 => 260577)


--- trunk/LayoutTests/animations/animation-direction-reverse-expected.txt	2020-04-23 17:15:45 UTC (rev 260576)
+++ trunk/LayoutTests/animations/animation-direction-reverse-expected.txt	2020-04-23 17:23:18 UTC (rev 260577)
@@ -1,4 +1,3 @@
-PASS - "margin-left" property for "box" element at 0.5s saw something close to: 150
-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: 0
 
+PASS Pausing an animation using the animation-play-state property stops animating styles with animation-direction: reverse. 
+

Modified: trunk/LayoutTests/animations/animation-direction-reverse.html (260576 => 260577)


--- trunk/LayoutTests/animations/animation-direction-reverse.html	2020-04-23 17:15:45 UTC (rev 260576)
+++ trunk/LayoutTests/animations/animation-direction-reverse.html	2020-04-23 17:23:18 UTC (rev 260577)
@@ -1,7 +1,7 @@
 <html lang="en">
 <head>
   <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
-  <title>Test of -webkit-animation-play-state</title>
+  <title>Test of animation-play-state</title>
   <style type="text/css" media="screen">
     body {
       margin: 0;
@@ -15,45 +15,50 @@
       width: 100px;
       background-color: red;
       margin: 0;
-      -webkit-animation-duration: 2s;
-      -webkit-animation-direction: reverse;
-      -webkit-animation-timing-function: linear;
-      -webkit-animation-name: "move1";
+      animation-duration: 500ms;
+      animation-direction: reverse;
+      animation-iteration-count: infinite;
+      animation-timing-function: linear;
+      animation-name: "move1";
     }
-    @-webkit-keyframes "move1" {
+
+    @keyframes move1 {
       from { margin-left: 0px; }
       to   { margin-left: 200px; }
     }
-    #result {
-      color: white; /* hide from pixel results */
-    }
   </style>
-  <script src="" type="text/_javascript_" charset="utf-8"></script>
-  <script type="text/_javascript_" charset="utf-8">
-    const expectedValues = [
-      // [animation-name, time, element-id, property, expected-value, tolerance]
-      ["move1", 0.5, "box", "margin-left", 150, 20],
-      ["move1", 1.0, "box", "margin-left", 100, 20],
-      ["move1", 2.5, "box", "margin-left", 0, 20],
-    ];
+  <script src=""
+  <script src=""
+  <script src=""
+</head>
+<body>
+<div id="box"></div>
+<script>
+    async_test(async t => {
+        const delay = 100;
 
-    function pauseAnimation()
-    {
-        document.getElementById("box").style.webkitAnimationPlayState = "paused";
-    }
+        const test = new AnimationTest({
+            target: document.getElementById("box"),
+            styleExtractor: style => parseFloat(style.marginLeft)
+        });
 
-    function setTimers()
-    {
-        setTimeout(pauseAnimation, 2500);
-    }
+        // Record two computed values after the specified delay each.
+        await test.recordValueAfterRunningFor(delay);
+        await test.recordValueAfterRunningFor(delay);
 
-    runAnimationTest(expectedValues, setTimers, null, true);
+        // We'll now pause the animation using the CSS property "animation-play-state".
+        box.style.animationPlayState = "paused";
 
-  </script>
-</head>
-<body>
-<div id="box"></div>
-<div id="result"></div>
-</div>
+        // And now we'll record values after 250ms each and check that those are the same.
+        const initialPausedValue = await test.valueAfterWaitingFor(delay);
+        const currentPausedValue = await test.valueAfterWaitingFor(delay);
+        assert_equals(initialPausedValue, currentPausedValue, "Values recorded while paused are the same.");
+
+        // Finally, check the values recorded earlier in the test.
+        test.checkRecordedValues();
+
+        t.done();
+    }, `Pausing an animation using the animation-play-state property stops animating styles with animation-direction: reverse.`);
+</script>
 </body>
 </html>

Added: trunk/LayoutTests/animations/resources/animation-test.js (0 => 260577)


--- trunk/LayoutTests/animations/resources/animation-test.js	                        (rev 0)
+++ trunk/LayoutTests/animations/resources/animation-test.js	2020-04-23 17:23:18 UTC (rev 260577)
@@ -0,0 +1,118 @@
+/*
+ * Copyright (C) 2020 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE INC. OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+class AnimationTest
+{
+
+    // Expects a dictionary with an Element (target) and a Function (styleExtractor) used to extract
+    // the recorded value from a computed style.
+    constructor(options)
+    {
+        this._records = [];
+        this._target = options.target;
+        this._styleExtractor = options.styleExtractor;
+    }
+
+    // Public
+
+    // Returns the Animation object. Currently, this expects a single animation is running on the
+    // provided element.
+    get animation()
+    {
+        return this._target.getAnimations()[0];
+    }
+
+    // Returns the requested value from the computed style using the provided style extractor.
+    get value()
+    {
+        return this._styleExtractor(getComputedStyle(this._target));
+    }
+
+    // Returns the requested value from the computed style after waiting a certain delay.
+    // This method runs independently of the animation play state and waits at a minimum
+    // for an animation frame to have elapsed.
+    async valueAfterWaitingFor(delay)
+    {
+        await Promise.all([
+            new Promise(requestAnimationFrame),
+            new Promise(resolve => setTimeout(resolve, delay))
+        ]);
+        return this.value;
+    }
+
+    // Records the requested value from the computed style after a given delay has elapsed while the
+    // animation is running. The record stored will be checked when checkRecordedValues() is called.
+    async recordValueAfterRunningFor(delay)
+    {
+        const animation = this.animation;
+        await animation.ready;
+
+        const targetTime = animation.currentTime + delay;
+        await this._tickUntil(elapsedTime => animation.currentTime >= targetTime);
+
+        this._records.push({
+            currentTime: animation.currentTime,
+            value: this.value
+        });
+    }
+
+    // Check that all requested values recorded using recordValueAfterRunningFor() match the same values
+    // when the animation is paused and seeked at the same animation currentTime as when the record was made.
+    checkRecordedValues()
+    {
+        const animation = this.animation;
+
+        // Reset the animation.
+        animation.cancel();
+
+        // Now seek to each of the recorded times and assert the value.
+        this._records.forEach((record, index) => {
+            animation.currentTime = record.currentTime;
+            assert_equals(record.value, this.value, `Recorded value #${index} is correct.`);
+        });
+    }
+
+    // Private
+
+    // Wait until the provided condition is true using animation frames.
+    _tickUntil(condition)
+    {
+        return new Promise(resolve => {
+            if (!this._startTime)
+                this._startTime = performance.now();
+
+            const tick = timestamp => {
+                const elapsedTime = timestamp - this._startTime;
+                if (condition(elapsedTime)) {
+                    delete this._startTime;
+                    resolve();
+                }
+                requestAnimationFrame(tick);
+            }
+            requestAnimationFrame(tick);
+        });
+    }
+
+}

Modified: trunk/LayoutTests/platform/ios-wk1/TestExpectations (260576 => 260577)


--- trunk/LayoutTests/platform/ios-wk1/TestExpectations	2020-04-23 17:15:45 UTC (rev 260576)
+++ trunk/LayoutTests/platform/ios-wk1/TestExpectations	2020-04-23 17:23:18 UTC (rev 260577)
@@ -1137,7 +1137,6 @@
 animations/animation-direction-reverse-hardware-opacity.html [ Failure Pass ]
 animations/animation-direction-reverse-hardware.html [ Failure Pass ]
 animations/animation-direction-reverse-timing-functions-hardware.html [ Failure Pass ]
-animations/animation-direction-reverse.html [ Failure Pass ]
 animations/animation-hit-test-transform.html [ Failure Pass ]
 compositing/layer-creation/mismatched-rotated-transform-transition-overlap.html [ Failure Pass ]
 compositing/layer-creation/mismatched-transform-transition-overlap.html [ Failure Pass ]

Modified: trunk/LayoutTests/platform/ios-wk2/TestExpectations (260576 => 260577)


--- trunk/LayoutTests/platform/ios-wk2/TestExpectations	2020-04-23 17:15:45 UTC (rev 260576)
+++ trunk/LayoutTests/platform/ios-wk2/TestExpectations	2020-04-23 17:23:18 UTC (rev 260577)
@@ -1327,8 +1327,6 @@
 
 webkit.org/b/207153 animations/animation-callback-timestamp.html [ Pass Failure ]
 
-webkit.org/b/209362 animations/animation-direction-reverse.html [ Pass Failure ]
-
 webkit.org/b/207156 [ Release ] http/tests/websocket/tests/hybi/workers/close.html [ Pass Failure ]
 
 webkit.org/b/207161 imported/w3c/web-platform-tests/2dcontext/imagebitmap/createImageBitmap-transfer.html [ Pass Failure ]
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to