Bug 189560 – Add stability to tests for web API statistics

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Woodrow Wang]

This patch is meant to increase the stability of tests for the web API statistics. The original tests had unnecessary dependencies we can remove in order to improve stability.

[Radar WebKit Bug Importer]

<rdar://problem/44396413>

[Woodrow Wang]

Created attachment 349586 [details]
Patch

[EWS Watchlist]

Comment on attachment 349586 [details]
Patch

Attachment 349586 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/9197802

New failing tests:
http/tests/webAPIStatistics/font-load-data-collection.html
http/tests/webAPIStatistics/canvas-read-and-write-data-collection.html
http/tests/webAPIStatistics/navigator-functions-accessed-data-collection.html
http/tests/webAPIStatistics/screen-functions-accessed-data-collection.html

[EWS Watchlist]

Created attachment 349615 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.4

[EWS Watchlist]

Comment on attachment 349586 [details]
Patch

Attachment 349586 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/9198691

New failing tests:
http/tests/webAPIStatistics/font-load-data-collection.html
http/tests/webAPIStatistics/screen-functions-accessed-data-collection.html
http/tests/webAPIStatistics/navigator-functions-accessed-data-collection.html
fast/animation/css-animation-resuming-when-visible-with-style-change2.html
http/tests/webAPIStatistics/canvas-read-and-write-data-collection.html

[EWS Watchlist]

Created attachment 349622 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[Chris Dumez]

Comment on attachment 349586 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=349586&action=review

> LayoutTests/http/tests/webAPIStatistics/screen-functions-accessed-data-collection.html:16
>          testRunner.dumpResourceLoadStatistics();

The testRunner.dump*() calls should likely happen much earlier, before your hostUnderTest initialization above.

[Woodrow Wang]

Created attachment 349686 [details]
Patch

[Chris Dumez]

Comment on attachment 349686 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=349686&action=review

> LayoutTests/ChangeLog:8
> +

Do you have an explanation why not using js-test would improve stability?

> LayoutTests/http/tests/webAPIStatistics/canvas-read-and-write-data-collection.html:16
>              testRunner.notifyDone();

Note that the tests should have been using finishJSTest() instead of testRunner.notifyDone() when using js-test.js.

[Woodrow Wang]

(In reply to Chris Dumez from comment #9)
> Comment on attachment 349686 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=349686&action=review
> 
> > LayoutTests/ChangeLog:8
> > +
> 
> Do you have an explanation why not using js-test would improve stability?
Using js-test added some data to the font loading collection, which made the tests dependent on js-test.js. Thus, if any changes were made to js-test.js the tests for web API statistics would fail. There seems to be some flakiness in clearing the resource load statistics map that are occurring seen here https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=http%2Ftests%2FwebAPIStatistics%2Fnavigator-functions-accessed-data-collection.html%20http%2Ftests%2FwebAPIStatistics%2Fcanvas-read-and-write-data-collection.html%20http%2Ftests%2FwebAPIStatistics%2Ffont-load-data-collection.html%20http%2Ftests%2FwebAPIStatistics%2Fscreen-functions-accessed-data-collection.html 

I am currently trying to determine why the flakiness exists, but have been unable to reproduce any similar failures locally. 
> 
> > LayoutTests/http/tests/webAPIStatistics/canvas-read-and-write-data-collection.html:16
> >              testRunner.notifyDone();
> 
> Note that the tests should have been using finishJSTest() instead of
> testRunner.notifyDone() when using js-test.js.

[Woodrow Wang]

Created attachment 349717 [details]
Patch

[Woodrow Wang]

We can significantly improve the amount of time each test takes by overriding the timer initially relied upon in the ResourceLoadObserver with an existing testRunner function.

[Per Arne Vollan]

Comment on attachment 349717 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=349717&action=review

> LayoutTests/http/tests/webAPIStatistics/canvas-read-and-write-data-collection.html:-18
> -        testRunner.statisticsResetToConsistentState(function() {
> -            testRunner.notifyDone();
> -        });

There is no need to call this function anymore?

[Woodrow Wang]

Created attachment 349781 [details]
Patch

[Chris Dumez]

Comment on attachment 349781 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=349781&action=review

> LayoutTests/ChangeLog:8
> +

Please explain *In the changelog* why this helps with stability.

> LayoutTests/http/tests/webAPIStatistics/canvas-read-and-write-data-collection.html:12
> +    testRunner.dumpAsText();

I think these testRunner calls should be with the one to testRunner.waitUntilDone(); at the end of the test.
Sorry, I know I recommended this locally but I did not see at the time that there was already a section taking care of testRunner calls.

Same comment in other tests.

> LayoutTests/http/tests/webAPIStatistics/font-load-data-collection.html:39
> +        }, 100);

Why 100? Either 0 works consistently or this will introduce flakiness.

[Woodrow Wang]

(In reply to Chris Dumez from comment #15)
> Comment on attachment 349781 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=349781&action=review
> 
> > LayoutTests/ChangeLog:8
> > +
> 
> Please explain *In the changelog* why this helps with stability.
Will do.
> 
> > LayoutTests/http/tests/webAPIStatistics/canvas-read-and-write-data-collection.html:12
> > +    testRunner.dumpAsText();
> 
> I think these testRunner calls should be with the one to
> testRunner.waitUntilDone(); at the end of the test.
> Sorry, I know I recommended this locally but I did not see at the time that
> there was already a section taking care of testRunner calls.
> 
> Same comment in other tests.
I'll move it accordingly!
> 
> > LayoutTests/http/tests/webAPIStatistics/font-load-data-collection.html:39
> > +        }, 100);
> 
> Why 100? Either 0 works consistently or this will introduce flakiness.
0 does work consistently locally, so I will change it to 0 and monitor the bots to ensure there's no unforeseen flakiness.

[Woodrow Wang]

Created attachment 349787 [details]
Patch

[Chris Dumez]

Comment on attachment 349787 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=349787&action=review

r=me

> LayoutTests/http/tests/webAPIStatistics/font-load-data-collection.html:34
> +        // Adds a timeout to allow font loads to be recorded 

Style: period needed at the end of the comment.

[Woodrow Wang]

Created attachment 349796 [details]
Patch

[Woodrow Wang]

(In reply to Woodrow Wang from comment #19)
> Created attachment 349796 [details]
> Patch
Made a mistake rebasing with style fix, so I am reuploading the patch.

[Woodrow Wang]

Created attachment 349804 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 349804 [details]
Patch

Clearing flags on attachment: 349804

Committed r236021: <https://trac.webkit.org/changeset/236021>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Truitt Savell]

This test: http/tests/webAPIStatistics/font-load-data-collection.html

is still flakey after https://trac.webkit.org/changeset/236021/webkit

I do want to note that the test is now passing faster. before it took 5 seconds now it passes or fails in under 1 second. This also seems to be the only test still failing from the tests edited in this patch. 

History:
https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=http%2Ftests%2FwebAPIStatistics%2Ffont-load-data-collection.html

Diff:
https://build.webkit.org/results/Apple%20High%20Sierra%20Release%20WK2%20(Tests)/r236073%20(6775)/http/tests/webAPIStatistics/font-load-data-collection-pretty-diff.html

[Woodrow Wang]

(In reply to Truitt Savell from comment #24)
> This test: http/tests/webAPIStatistics/font-load-data-collection.html
> 
> is still flakey after https://trac.webkit.org/changeset/236021/webkit
> 
> I do want to note that the test is now passing faster. before it took 5
> seconds now it passes or fails in under 1 second. This also seems to be the
> only test still failing from the tests edited in this patch. 

This was an expected enhancement by this patch :)
> 
> History:
> https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.
> html#showAllRuns=true&tests=http%2Ftests%2FwebAPIStatistics%2Ffont-load-data-
> collection.html
> 
> Diff:
> https://build.webkit.org/results/
> Apple%20High%20Sierra%20Release%20WK2%20(Tests)/r236073%20(6775)/http/tests/
> webAPIStatistics/font-load-data-collection-pretty-diff.html

https://bugs.webkit.org/show_bug.cgi?id=189632 is a speculative fix for the flakiness of the font-load-data-collection.html test.

[Ryan Haddad]

(In reply to Woodrow Wang from comment #25)
> (In reply to Truitt Savell from comment #24)
> https://bugs.webkit.org/show_bug.cgi?id=189632 is a speculative fix for the
> flakiness of the font-load-data-collection.html test.

http/tests/webAPIStatistics/font-load-data-collection.html continues to be extremely flaky on the bots after that fix.

[Woodrow Wang]

(In reply to Ryan Haddad from comment #26)
> (In reply to Woodrow Wang from comment #25)
> > (In reply to Truitt Savell from comment #24)
> > https://bugs.webkit.org/show_bug.cgi?id=189632 is a speculative fix for the
> > flakiness of the font-load-data-collection.html test.
> 
> http/tests/webAPIStatistics/font-load-data-collection.html continues to be
> extremely flaky on the bots after that fix.

I'm actively investigating the flakiness and am working on https://bugs.webkit.org/show_bug.cgi?id=189684 as another speculative fix. I am not able to reproduce the flakiness locally as of now, so if this speculative fix does not work, I will mark the test as flaky.