| Summary: | Layout Test editing/selection/navigation-clears-editor-state.html is flaky | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Truitt Savell <tsavell> | ||||||
| Component: | Tools / Tests | Assignee: | Saam Barati <saam> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | commit-queue, lforschler, mark.lam, realdawei, rniwa, ryanhaddad, saam, webkit-bug-importer, wenson_hsieh | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| See Also: |
https://bugs.webkit.org/show_bug.cgi?id=187309 https://bugs.webkit.org/show_bug.cgi?id=190890 |
||||||||
| Attachments: |
|
||||||||
|
Description
Truitt Savell
2018-07-06 09:16:36 PDT
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. 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 |