Bug 197063

Summary:

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

Product:

WebKit

Reporter:

Shawn Roberts <sroberts>

Component:

Tools / Tests

Assignee:

cathiechen <cathiechen>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ajuma, cathiechen, commit-queue, fred.wang, lforschler, rniwa, sabouhallawa, simon.fraser, webkit-bot-watchers-bugzilla, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

See Also:

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

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none

Shawn Roberts

Reported 2019-04-18 10:41:28 PDT

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"

Attachments
Patch (5.99 KB, patch) 2019-04-22 02:04 PDT, cathiechen	no flags	Details Formatted Diff Diff
Patch (1.47 KB, patch) 2019-04-22 03:22 PDT, cathiechen	no flags	Details Formatted Diff Diff
Patch (1.47 KB, patch) 2019-04-22 03:32 PDT, cathiechen	no flags	Details Formatted Diff Diff
Patch (3.04 KB, patch) 2019-04-22 20:26 PDT, cathiechen	no flags	Details Formatted Diff Diff
Patch (1.46 KB, patch) 2019-04-22 21:45 PDT, cathiechen	no flags	Details Formatted Diff Diff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2019-04-18 10:48:56 PDT

<rdar://problem/50020775>

cathiechen

Comment 2 2019-04-22 02:04:20 PDT

Created attachment 367929 [details] Patch

cathiechen

Comment 3 2019-04-22 03:22:17 PDT

Created attachment 367933 [details] Patch

Frédéric Wang (:fredw)

Comment 4 2019-04-22 03:29:05 PDT

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

Comment 5 2019-04-22 03:32:18 PDT

Created attachment 367934 [details] Patch

Simon Fraser (smfr)

Comment 6 2019-04-22 10:32:21 PDT

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 7 2019-04-22 17:00:10 PDT

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

Comment 8 2019-04-22 19:38:31 PDT

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

Comment 9 2019-04-22 20:26:44 PDT

Created attachment 368008 [details] Patch

Ryosuke Niwa

Comment 10 2019-04-22 20:31:04 PDT

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

Comment 11 2019-04-22 20:32:02 PDT

(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 12 2019-04-22 21:29:36 PDT

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

Comment 13 2019-04-22 21:45:45 PDT

Created attachment 368010 [details] Patch

Frédéric Wang (:fredw)

Comment 14 2019-04-24 08:53:57 PDT

(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 15 2019-04-24 09:21:37 PDT

Comment on attachment 368010 [details] Patch Clearing flags on attachment: 368010 Committed r244593: <https://trac.webkit.org/changeset/244593>

WebKit Commit Bot

Comment 16 2019-04-24 09:21:38 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.