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"
<rdar://problem/50020775>
Created attachment 367929 [details] Patch
Created attachment 367933 [details] Patch
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
Created attachment 367934 [details] Patch
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.
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.
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.
Created attachment 368008 [details] Patch
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.
(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.
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:)
Created attachment 368010 [details] Patch
(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).
Comment on attachment 368010 [details] Patch Clearing flags on attachment: 368010 Committed r244593: <https://trac.webkit.org/changeset/244593>
All reviewed patches have been landed. Closing bug.