LayoutTest http/tests/performance/performance-resource-timing-cached-entries.html is flaky on all platforms Most recent failure: <https://build.webkit.org/results/Apple%20El%20Capitan%20Debug%20WK1%20(Tests)/r201023%20(5296)/results.html> Flakiness dashboard: <https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=http%2Ftests%2Fperformance%2Fperformance-resource-timing-cached-entries.html> --- /Volumes/Data/slave/elcapitan-debug-tests-wk1/build/layout-test-results/http/tests/performance/performance-resource-timing-cached-entries-expected.txt +++ /Volumes/Data/slave/elcapitan-debug-tests-wk1/build/layout-test-results/http/tests/performance/performance-resource-timing-cached-entries-actual.txt @@ -1,2 +1,2 @@ -PASS foundResource is 2 +FAIL foundResource should be 2. Was 3.
This test is super flaky, even causing EWS false positives. It needs to be dealt with very soon. Should we just roll out the patch, given that its test doesn't work reliably?
I'm looking into it now.
Created attachment 279216 [details] Patch
I wasn't able to recreate the issue locally, but since the test showed more entries than expected, clearing the entries at the beginning of the test might do the trick. I'll open a separate issue to investigate why this is required in some cases. (as it should happen automatically between navigations)
Attachment 279216 [details] did not pass style-queue: ERROR: LayoutTests/ChangeLog:8: Line contains tab character. [whitespace/tab] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 279221 [details] Patch
(In reply to comment #6) > Created attachment 279221 [details] > Patch While removing my own tabs from the ChangeLog (oops), I found several others, so removed them as well.
Comment on attachment 279221 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=279221&action=review > LayoutTests/ChangeLog:8 > + This patch attempts to unflake the test. What is your plan for investigating the root cause? With the test deflaked on the bots, and with the issue not reproducible for you locally, I don't see how it can be actionable.
I'm not sure, but I thought unflaking the test would help avoid EWS false positives. We could always later on introduce an ignored test that just checks for this flakiness on the bots. Regarding the issue itself, the entries are stored inside m_performance which is referenced by DOMWindow. Is it possible that DOMWindow survives navigations/test runs? If so, the solution is most probably to clear m_performance when a new navigation is initiated. Does that make sense?
> Is it possible that DOMWindow survives navigations/test runs? That seems unlikely. Is there any logging you can add to the test to see what happened (maybe print out all the resources if the count is wrong)? You can mark the test as flaky and add the extra logging - that way you can collect information and still unbreak EWS.
(In reply to comment #10) > > Is it possible that DOMWindow survives navigations/test runs? > > That seems unlikely. > > Is there any logging you can add to the test to see what happened (maybe > print out all the resources if the count is wrong)? You can mark the test as > flaky and add the extra logging - that way you can collect information and > still unbreak EWS. That makes sense. I'll do that shortly.
Created attachment 279286 [details] Patch
Comment on attachment 279286 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=279286&action=review > LayoutTests/http/tests/performance/performance-resource-timing-cached-entries.html:11 > + if (window.performance) > + console.log("current entries: " + performance.getEntriesByType('resource').length); I was thinking of something like (right after a shouldBe()): if (foundResource !== 2) { for (var i = 0; i < resources.length; ++i) { if (resources[i].name.indexOf("square") !== -1) debug(resources.name); } } That way, there will be more information logged, and you won't need to change the expectation.
Created attachment 279355 [details] Patch
Created attachment 279357 [details] Patch
Comment on attachment 279357 [details] Patch Added more logs. Landing
Comment on attachment 279357 [details] Patch Attachment 279357 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/1346582 New failing tests: storage/websql/database-lock-after-reload.html
Created attachment 279360 [details] Archive of layout-test-results from ews103 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 279357 [details] Patch The failing tests seem unrelated. Trying again to cq+
Comment on attachment 279357 [details] Patch Clearing flags on attachment: 279357 Committed r201130: <http://trac.webkit.org/changeset/201130>
All reviewed patches have been landed. Closing bug.