The following layout test is flaky on Sierra and High Sierra WK1 editing/selection/navigation-clears-editor-state.html Probable cause: I do not know, though it seems to have begun around revision r233184 Flakiness Dashboard: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=editing%2Fselection%2Fnavigation-clears-editor-state.html looking into issue further
Saam attempted to fix this in https://trac.webkit.org/changeset/233555, it it failing after that revision?
Yes its still flakey after that revision. But it seems like his revision might have fixed the test for windows.
Created attachment 344423 [details] patch
Comment on attachment 344423 [details] patch Clearing flags on attachment: 344423 Committed r233581: <https://trac.webkit.org/changeset/233581>
All reviewed patches have been landed. Closing bug.
<rdar://problem/41898822>
So it looks like after that new revision the test is constantly failing on Sierra and High Sierra Debug WK1 and WK2. example of failure message: https://build.webkit.org/results/Apple%20High%20Sierra%20Debug%20WK1%20(Tests)/r233581%20(4602)/editing/selection/navigation-clears-editor-state-pretty-diff.html
Reopening per Truitt's comment.
I think we just need to skip this test on debug bots. It's too slow there.
(In reply to Ryosuke Niwa from comment #9) > I think we just need to skip this test on debug bots. It's too slow there. Agreed. I'll do this.
(In reply to Saam Barati from comment #10) > (In reply to Ryosuke Niwa from comment #9) > > I think we just need to skip this test on debug bots. It's too slow there. > > Agreed. I'll do this. I spoke with Ryosuke offline. We're going to start by just removing the 18 second timeout.
(In reply to Saam Barati from comment #11) > (In reply to Saam Barati from comment #10) > > (In reply to Ryosuke Niwa from comment #9) > > > I think we just need to skip this test on debug bots. It's too slow there. > > > > Agreed. I'll do this. > > I spoke with Ryosuke offline. We're going to start by just removing the 18 > second timeout. We're also going to mark it slow on Debug
(In reply to Saam Barati from comment #12) > (In reply to Saam Barati from comment #11) > > (In reply to Saam Barati from comment #10) > > > (In reply to Ryosuke Niwa from comment #9) > > > > I think we just need to skip this test on debug bots. It's too slow there. > > > > > > Agreed. I'll do this. > > > > I spoke with Ryosuke offline. We're going to start by just removing the 18 > > second timeout. > > We're also going to mark it slow on Debug Actually, our new approach will be: - lower iframe count from 200 to 100 - remove 18 second timer We'll mark it as slow in the future if it times out.
(In reply to Saam Barati from comment #13) > (In reply to Saam Barati from comment #12) > > (In reply to Saam Barati from comment #11) > > > (In reply to Saam Barati from comment #10) > > > > (In reply to Ryosuke Niwa from comment #9) > > > > > I think we just need to skip this test on debug bots. It's too slow there. > > > > > > > > Agreed. I'll do this. > > > > > > I spoke with Ryosuke offline. We're going to start by just removing the 18 > > > second timeout. > > > > We're also going to mark it slow on Debug > > Actually, our new approach will be: > - lower iframe count from 200 to 100 > - remove 18 second timer > > We'll mark it as slow in the future if it times out. Landed in: https://trac.webkit.org/changeset/233701/webkit
After r233701 <https://trac.webkit.org/changeset/233701/webkit> Failure is back to being Flaky on WK1 Release Platforms. It has stopped failing on Debug. Example of test output on failure: --- /Volumes/Data/slave/highsierra-release-tests-wk1/build/layout-test-results/editing/selection/navigation-clears-editor-state-expected.txt +++ /Volumes/Data/slave/highsierra-release-tests-wk1/build/layout-test-results/editing/selection/navigation-clears-editor-state-actual.txt @@ -3,7 +3,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". -PASS freed more than 15% of documents +FAIL freed less than 15% of documents PASS successfullyParsed is true TEST COMPLETE Test History: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=editing%2Fselection%2Fnavigation-clears-editor-state.html
(In reply to Truitt Savell from comment #15) > After r233701 <https://trac.webkit.org/changeset/233701/webkit> > > Failure is back to being Flaky on WK1 Release Platforms. It has stopped > failing on Debug. > > Example of test output on failure: > > --- > /Volumes/Data/slave/highsierra-release-tests-wk1/build/layout-test-results/ > editing/selection/navigation-clears-editor-state-expected.txt > +++ > /Volumes/Data/slave/highsierra-release-tests-wk1/build/layout-test-results/ > editing/selection/navigation-clears-editor-state-actual.txt > @@ -3,7 +3,7 @@ > On success, you will see a series of "PASS" messages, followed by "TEST > COMPLETE". > > > -PASS freed more than 15% of documents > +FAIL freed less than 15% of documents > PASS successfullyParsed is true > > TEST COMPLETE > > Test History: > https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard. > html#showAllRuns=true&tests=editing%2Fselection%2Fnavigation-clears-editor- > state.html How about adding <!-- webkit-test-runner [ jscOptions=--useConcurrentGC=false ] --> to the top of the test? That should make the GCing more consistent.
Ugh, we are putting more attention than needed into this test IMO. I say we just mark it as flaky and move on.
(In reply to Mark Lam from comment #16) > (In reply to Truitt Savell from comment #15) > > After r233701 <https://trac.webkit.org/changeset/233701/webkit> > > > > Failure is back to being Flaky on WK1 Release Platforms. It has stopped > > failing on Debug. > > > > Example of test output on failure: > > > > --- > > /Volumes/Data/slave/highsierra-release-tests-wk1/build/layout-test-results/ > > editing/selection/navigation-clears-editor-state-expected.txt > > +++ > > /Volumes/Data/slave/highsierra-release-tests-wk1/build/layout-test-results/ > > editing/selection/navigation-clears-editor-state-actual.txt > > @@ -3,7 +3,7 @@ > > On success, you will see a series of "PASS" messages, followed by "TEST > > COMPLETE". > > > > > > -PASS freed more than 15% of documents > > +FAIL freed less than 15% of documents > > PASS successfullyParsed is true > > > > TEST COMPLETE > > > > Test History: > > https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard. > > html#showAllRuns=true&tests=editing%2Fselection%2Fnavigation-clears-editor- > > state.html > > How about adding <!-- webkit-test-runner [ > jscOptions=--useConcurrentGC=false ] --> to the top of the test? That > should make the GCing more consistent. I believe the test is calling sync GCs manually. So I don't see how this will help
We could also just bump up the iteration count a bit again
(In reply to Saam Barati from comment #18) > (In reply to Mark Lam from comment #16) > > How about adding <!-- webkit-test-runner [ > > jscOptions=--useConcurrentGC=false ] --> to the top of the test? That > > should make the GCing more consistent. > > I believe the test is calling sync GCs manually. So I don't see how this > will help You're right: GCController.collect() ultimately ends up in GCController::garbageCollectNow() in the web process. So, this wouldn't help.
I think this test is just slow. I think 200 was working fine. So let's go back there and just mark this test as SLOW.
Created attachment 344816 [details] Another fix attempt
Committed r233754: <https://trac.webkit.org/changeset/233754>
Looks like this latest change finally made it stable!
(In reply to Ryosuke Niwa from comment #24) > Looks like this latest change finally made it stable! Great! Thanks Ryosuke