Bug 203734 – Flaky API Test TestWebKitAPI.ServiceWorkers.ThrottleCrash

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Aakash Jain]

TestWebKitAPI.ServiceWorkers.ThrottleCrash seems flaky on iOS. In https://ews-build.webkit.org/#/builders/9/builds/11291, the test TimedOut in run-api-tests step. However, in the immediately next retry step (re-run-api-tests), it failed. Similar thing happened in https://ews-build.webkit.org/#/builders/9/builds/11264

Also, in following builds, the test was so flaky that EWS incorrectly blamed the patch being tested:
https://ews-build.webkit.org/#/builders/9/builds/11276
https://ews-build.webkit.org/#/builders/9/builds/11269

[Radar WebKit Bug Importer]

<rdar://problem/56814638>

[Aakash Jain]

Flaked again in https://ews-build.webkit.org/#/builders/9/builds/11296 and https://ews-build.webkit.org/#/builders/9/builds/11297

[Jonathan Bedard]

Not convinced this one is flakey, I think someone introduced a regression to the test relatively recently: <https://results.webkit.org/?suite=api-tests&suite=api-tests&test=TestWebKitAPI.EditorStateTests.TypingAttributesTextAlignmentStartEnd&test=TestWebKitAPI.ServiceWorkers.ThrottleCrash&before_id=251111>

My bet is that this is the timeout on our Debug Mac queues. The only question I have left is if this regression is the same crash that we see on iOS, and was recently fixed.

[Aakash Jain]

It was added 6 months ago in https://trac.webkit.org/changeset/245327/webkit

[Aakash Jain]

It has been too flaky recently on EWS (on Mojave, but works fine on Catalina). Sometimes so flaky that EWS incorrectly blames the patch being tested for this test failure.

e.g.: 
https://ews-build.webkit.org/#/builders/9/builds/11910
https://ews-build.webkit.org/#/builders/9/builds/11900
https://ews-build.webkit.org/#/builders/9/builds/11865
https://ews-build.webkit.org/#/builders/9/builds/11825
https://ews-build.webkit.org/#/builders/9/builds/11821
https://ews-build.webkit.org/#/builders/9/builds/11814
https://ews-build.webkit.org/#/builders/9/builds/11813
https://ews-build.webkit.org/#/builders/9/builds/11808
https://ews-build.webkit.org/#/builders/9/builds/11807
https://ews-build.webkit.org/#/builders/9/builds/11805
https://ews-build.webkit.org/#/builders/9/builds/11798
https://ews-build.webkit.org/#/builders/9/builds/11789
https://ews-build.webkit.org/#/builders/9/builds/11768
https://ews-build.webkit.org/#/builders/9/builds/11761
https://ews-build.webkit.org/#/builders/9/builds/11760
https://ews-build.webkit.org/#/builders/9/builds/11757
https://ews-build.webkit.org/#/builders/9/builds/11755
https://ews-build.webkit.org/#/builders/9/builds/11745
https://ews-build.webkit.org/#/builders/9/builds/11734
https://ews-build.webkit.org/#/builders/9/builds/11731
https://ews-build.webkit.org/#/builders/9/builds/11726
https://ews-build.webkit.org/#/builders/9/builds/11725
https://ews-build.webkit.org/#/builders/9/builds/11711
https://ews-build.webkit.org/#/builders/9/builds/11710
https://ews-build.webkit.org/#/builders/9/builds/11704
https://ews-build.webkit.org/#/builders/9/builds/11702
https://ews-build.webkit.org/#/builders/9/builds/11700
https://ews-build.webkit.org/#/builders/9/builds/11699
https://ews-build.webkit.org/#/builders/9/builds/11698
https://ews-build.webkit.org/#/builders/9/builds/11682

[Aakash Jain]

Committed r252405: <https://trac.webkit.org/changeset/252405>

[Aakash Jain]

Disabled the test for now to keep infrastructure happy.

[Jonathan Bedard]

This test has flaked maybe once in post-commit testing https://results.webkit.org/?suite=api-tests&test=TestWebKitAPI.ServiceWorkers.ThrottleCrash.

This seems like something worth fixing in the test.

[Alex Christensen]

Created attachment 383503 [details]
Patch

[Alex Christensen]

*** Bug 203732 has been marked as a duplicate of this bug. ***

[Alex Christensen]

Created attachment 383505 [details]
Patch

[youenn fablet]

Comment on attachment 383505 [details]
Patch

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

> Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm:581
> +        { "/test.mp4", { "test!", {{ "Content-Type", "video/test" }}}},

Looks nice and easy to understand!

> Tools/TestWebKitAPI/cocoa/HTTPServer.h:41
> +    HTTPServer(std::initializer_list<std::pair<String, HTTPResponse>>);

explicit and &&

> Tools/TestWebKitAPI/cocoa/HTTPServer.h:53
> +    HTTPResponse(String&& body)

explicit.

> Tools/TestWebKitAPI/cocoa/HTTPServer.h:63
> +    HTTPResponse& operator=(HTTPResponse&&) = default;

Why do we need all of these?

[Alex Christensen]

(In reply to youenn fablet from comment #12)
> Comment on attachment 383505 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=383505&action=review
> 
> > Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm:581
> > +        { "/test.mp4", { "test!", {{ "Content-Type", "video/test" }}}},
> 
> Looks nice and easy to understand!

Thanks!  I think this will be a great improvement to our testing.

> > Tools/TestWebKitAPI/cocoa/HTTPServer.h:41
> > +    HTTPServer(std::initializer_list<std::pair<String, HTTPResponse>>);
> 
> explicit and &&
Unfortunately C++ std::initializer_list cannot be moved from, and there is an indication that this will not change because of ABI compatibility concerns.

> > Tools/TestWebKitAPI/cocoa/HTTPServer.h:53
> > +    HTTPResponse(String&& body)
> 
> explicit.
> 
> > Tools/TestWebKitAPI/cocoa/HTTPServer.h:63
> > +    HTTPResponse& operator=(HTTPResponse&&) = default;
> 
> Why do we need all of these?

The lack of explicit and these constructors are necessary to make it so we can just use { "string" } syntax to construct a response without requiring additional names or unneeded {} for the empty HashSet.

[WebKit Commit Bot]

Comment on attachment 383505 [details]
Patch

Clearing flags on attachment: 383505

Committed r252476: <https://trac.webkit.org/changeset/252476>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.