Bug 197065 – [Mac WK2] REGRESSION (r244182) editing/execCommand/change-list-type.html is a flaky failure

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Shawn Roberts]

The following layout test is flaky on Mac WK2 Release

editing/execCommand/change-list-type.html

Probable cause:

Test started to become flaky on the dashboard around r244328.

Found through local testing that changes in https://trac.webkit.org/changeset/244182/webkit caused this test to become flaky. Testing with prior revisions pass 100%

Reproduced with:

run-webkit-tests editing/execCommand/change-list-type.html --iterations 100 -f

Flakiness Dashboard:

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=editing%2FexecCommand%2Fchange-list-type.html

Diff:

--- /Volumes/Data/slave/mojave-release-tests-wk2/build/layout-test-results/editing/execCommand/change-list-type-expected.txt
+++ /Volumes/Data/slave/mojave-release-tests-wk2/build/layout-test-results/editing/execCommand/change-list-type-actual.txt
@@ -354,7 +354,64 @@
 |     "one"
 |   "
             "
-|   <ol>
+|   <ul>
+|     class="list"
+|     style="color: red"
+|     "
+                "
+|     <li>
+|       id="two"
+|       "two"
+|     "
+                "
+|     <li>
+|       id="three"
+|       "three<#selection-caret>"
+|     "
+            "
+|   "
+            "
+|   <li>
+|     id="four"
+|     "four"
+|   "
+            "
+|   <pre>
+|     "                "
+|     <ol>
+|       class="list"
+|       "
+                    "
+|       <li>
+|         id="five"
+|         "five"
+|       "
+                    "
+|       <li>
+|         id="six"
+|         "six"
+|       "
+                "
+|     "
+            "
+|   "
+        "
+| "
+    "
+
+After making the outer list unordered:
+| "
+        "
+| <ul>
+|   class="list"
+|   "
+            "
+|   <li>
+|     id="one"
+|     "one"
+|   "
+            "
+|   <ul>
 |     class="list"
 |     style="color: red"
 |     "
@@ -384,25 +441,25 @@
                     "
 |       <li>
 |         id="five"
-|         "<#selection-anchor>five"
-|       "
-                    "
-|       <li>
-|         id="six"
-|         "six<#selection-focus>"
-|       "
-                "
-|     "
-            "
-|   "
-        "
-| "
-    "
-
-After making the outer list unordered:
-| "
-        "
-| <ul>
+|         "five"
+|       "
+                    "
+|       <li>
+|         id="six"
+|         "six"
+|       "
+                "
+|     "
+            <#selection-caret>"
+|   "
+        "
+| "
+    "
+
+After making the outer list ordered again:
+| "
+        "
+| <ol>
 |   class="list"
 |   "
             "
@@ -411,64 +468,7 @@
 |     "one"
 |   "
             "
-|   <ol>
-|     class="list"
-|     style="color: red"
-|     "
-                "
-|     <li>
-|       id="two"
-|       "two"
-|     "
-                "
-|     <li>
-|       id="three"
-|       "three"
-|     "
-            "
-|   "
-            "
-|   <li>
-|     id="four"
-|     "four"
-|   "
-            "
-|   <pre>
-|     "                "
-|     <ol>
-|       class="list"
-|       "
-                    "
-|       <li>
-|         id="five"
-|         "five"
-|       "
-                    "
-|       <li>
-|         id="six"
-|         "six"
-|       "
-                "
-|     "
-            <#selection-caret>"
-|   "
-        "
-| "
-    "
-
-After making the outer list ordered again:
-| "
-        "
-| <ol>
-|   class="list"
-|   "
-            "
-|   <li>
-|     id="one"
-|     "one"
-|   "
-            "
-|   <ol>
+|   <ul>
 |     class="list"
 |     style="color: red"
 |     "

[Radar WebKit Bug Importer]

<rdar://problem/50021964>

[Wenson Hsieh]

This test just needs the same tweak as r244461.

[Ryosuke Niwa]

Created attachment 367815 [details]
Fixes the test

[Ryosuke Niwa]

Committed r244462: <https://trac.webkit.org/changeset/244462>

[Simon Fraser (smfr)]

Comment on attachment 367815 [details]
Fixes the test

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

> LayoutTests/editing/execCommand/change-list-type.html:74
> +                testRunner.runUIScript(`(function() { uiController.uiScriptComplete(); })()`, resolve);

Wouldn't doAfterPresentationUpdate() have been better?

[Wenson Hsieh]

(In reply to Simon Fraser (smfr) from comment #5)
> Comment on attachment 367815 [details]
> Fixes the test
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=367815&action=review
> 
> > LayoutTests/editing/execCommand/change-list-type.html:74
> > +                testRunner.runUIScript(`(function() { uiController.uiScriptComplete(); })()`, resolve);
> 
> Wouldn't doAfterPresentationUpdate() have been better?

I wouldn't think this needs to wait for the next layer tree flush; it simply needs to ensure that the current runloop in the UI process has finished.

[Ryosuke Niwa]

Looks like the flakiness is gone!