Bug 197065

Summary:

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

Product:

WebKit

Reporter:

Shawn Roberts <sroberts>

Component:

Tools / Tests

Assignee:

Ryosuke Niwa <rniwa>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

lforschler, rniwa, sabouhallawa, simon.fraser, webkit-bot-watchers-bugzilla, webkit-bug-importer, wenson_hsieh

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

See Also:

https://bugs.webkit.org/show_bug.cgi?id=177484
https://bugs.webkit.org/show_bug.cgi?id=196985

Attachments:

Description	Flags
Fixes the test	wenson_hsieh: review+

Description Shawn Roberts 2019-04-18 11:15:52 PDT

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"
 |     "

Comment 1 Radar WebKit Bug Importer 2019-04-18 11:18:29 PDT

<rdar://problem/50021964>

Comment 2 Wenson Hsieh 2019-04-19 12:36:35 PDT

This test just needs the same tweak as r244461.

Comment 3 Ryosuke Niwa 2019-04-19 12:51:42 PDT

Created attachment 367815 [details]
Fixes the test

Comment 4 Ryosuke Niwa 2019-04-19 12:56:00 PDT

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

Comment 5 Simon Fraser (smfr) 2019-04-19 13:18:16 PDT

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?

Comment 6 Wenson Hsieh 2019-04-19 13:26:31 PDT

(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.

Comment 7 Ryosuke Niwa 2019-04-22 19:39:21 PDT

Looks like the flakiness is gone!