Bug 197063 – [Mac Debug] Layout Test resize-observer/observe-element-from-other-frame.html is a flaky failure

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Shawn Roberts]

The following layout test is flaky on Mac Debug 

resize-observer/observe-element-from-other-frame.html

Probable cause:

Test was added in https://trac.webkit.org/changeset/243643/webkit and is a flaky failure since

Flakiness Dashboard:

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=resize-observer%2Fobserve-element-from-other-frame.html

Diff:

--- /Volumes/Data/slave/mojave-debug-tests-wk2/build/layout-test-results/resize-observer/observe-element-from-other-frame-expected.txt
+++ /Volumes/Data/slave/mojave-debug-tests-wk2/build/layout-test-results/resize-observer/observe-element-from-other-frame-actual.txt
@@ -1,5 +1,5 @@
 
 
 PASS ResizeObserver implemented 
-PASS test0: Observe element from other frame 
+FAIL test0: Observe element from other frame assert_equals: expected "success" but got "fail"

[Radar WebKit Bug Importer]

<rdar://problem/50020775>

[cathiechen]

Created attachment 367929 [details]
Patch

[cathiechen]

Created attachment 367933 [details]
Patch

[Frédéric Wang (:fredw)]

Comment on attachment 367933 [details]
Patch

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

> LayoutTests/ChangeLog:9
> +        In order to reduce this failure, extend ResizeTestHelper.TIMEOUT to 1000ms like 177484.

like in bug 177484

[cathiechen]

Created attachment 367934 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 367929 [details]
Patch

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

> LayoutTests/resize-observer/resources/resizeTestHelper.js:55
> +ResizeTestHelper.TIMEOUT = 1000;

Does this make a bunch of tests take 1s to run? That's really long.

[cathiechen]

Comment on attachment 367929 [details]
Patch

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

Hi Simon,
Thanks:)

>> LayoutTests/resize-observer/resources/resizeTestHelper.js:55
>> +ResizeTestHelper.TIMEOUT = 1000;
> 
> Does this make a bunch of tests take 1s to run? That's really long.

Normally, this wouldn't affect the test time if the tests running as expected. But there are 4 tests in imported/w3c/web-platform-tests/resize-observer which would take 1s to test unwanted notification. They are test4(), test5(), test9() in notify.html and test3() in observe.html.

If not running as expected, because they are promise tests, this would extend timeout time once for each resizeObserver test file at most.

[Ryosuke Niwa]

We could try 500ms first if any. It would be better if we can somehow expedite / emulate the rendering timing though. e.g. add some internal method to speed up the process only inside WebKitTestRunner.

[cathiechen]

Created attachment 368008 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 368008 [details]
Patch

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

> LayoutTests/imported/w3c/web-platform-tests/resize-observer/resources/resizeTestHelper.js:55
> -ResizeTestHelper.TIMEOUT = 1000;
> +ResizeTestHelper.TIMEOUT = 500;

Oh, please don't modify the imported tests. They're supposed to be only modified in the upstream first.
It's fine if you're uploading this patch just to see if EWS is happy with it
but you should probably manually run it like 100-500 times to make sure it's actually not flaky.
If you're on macOS, then Instruments has the ability to reduce the number of effective cores.
Reducing it to 2 cores, for example, makes it easier to induce flakiness.

[cathiechen]

(In reply to Ryosuke Niwa from comment #8)
> We could try 500ms first if any. It would be better if we can somehow
> expedite / emulate the rendering timing though. e.g. add some internal
> method to speed up the process only inside WebKitTestRunner.

Hi Ryosuke,

Thanks:)

500ms seems work well in my local env.
I also changed the one in imported/w3c/web-platform-tests/resize-observer/resources/resizeTestHelper.js to make them consistent.

[cathiechen]

Comment on attachment 368008 [details]
Patch

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

>> LayoutTests/imported/w3c/web-platform-tests/resize-observer/resources/resizeTestHelper.js:55
>> +ResizeTestHelper.TIMEOUT = 500;
> 
> Oh, please don't modify the imported tests. They're supposed to be only modified in the upstream first.
> It's fine if you're uploading this patch just to see if EWS is happy with it
> but you should probably manually run it like 100-500 times to make sure it's actually not flaky.
> If you're on macOS, then Instruments has the ability to reduce the number of effective cores.
> Reducing it to 2 cores, for example, makes it easier to induce flakiness.

Ah, OK, I'll remove the change in imported tests.

And I tried reducing the effective core number like you said, it seems to work well. Thanks:)

[cathiechen]

Created attachment 368010 [details]
Patch

[Frédéric Wang (:fredw)]

(In reply to Ryosuke Niwa from comment #10)
> Oh, please don't modify the imported tests. They're supposed to be only
> modified in the upstream first.

Note that this test was already changed in https://trac.webkit.org/changeset/244182/webkit#file10 ; probably someone should do the change upstream (if that's not already done).

[WebKit Commit Bot]

Comment on attachment 368010 [details]
Patch

Clearing flags on attachment: 368010

Committed r244593: <https://trac.webkit.org/changeset/244593>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.