REGRESSION (r224301?): LayoutTest http/tests/workers/service/registration-task-queue-scheduling-1.html is a flaky failure

• Component: New Bugs
• Status: RESOLVED FIXED

[Ryan Haddad]

LayoutTest http/tests/workers/service/registration-task-queue-scheduling-1.html is a flaky failure

https://build.webkit.org/results/Apple%20El%20Capitan%20Release%20WK2%20(Tests)/r224502%20(5779)/results.html

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=http%2Ftests%2Fworkers%2Fservice%2Fregistration-task-queue-scheduling-1.html

--- /Volumes/Data/slave/elcapitan-release-tests-wk2/build/layout-test-results/http/tests/workers/service/registration-task-queue-scheduling-1-expected.txt
+++ /Volumes/Data/slave/elcapitan-release-tests-wk2/build/layout-test-results/http/tests/workers/service/registration-task-queue-scheduling-1-actual.txt
@@ -1,2 +1,3 @@
-CONSOLE MESSAGE: line 48: Original window resolved successfully (unexpected)
+ALERT: Popup window resolved successfully (unexpected)
+ALERT: Error in popup window

[Ryan Haddad]

This started being flaky recently. It could be related to https://trac.webkit.org/changeset/224301/webkit

[Ryan Haddad]

Marked test as flaky in https://trac.webkit.org/r224613

[Radar WebKit Bug Importer]

<rdar://problem/35478161>

[Alexey Proskuryakov]

I can reproduce this by repeating the test multiple times with an ASan build (which I just happened to have, a regular build may work too).

run-webkit-tests --no-retry --no-sample -v --repeat-each 10 http/tests/workers/service/registration-task-queue-scheduling-1.html

[Chris Dumez]

Created attachment 330852 [details]
Patch

[youenn fablet]

Comment on attachment 330852 [details]
Patch

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

> LayoutTests/http/tests/workers/service/registration-task-queue-scheduling-1-expected.txt:1
> +ALERT: Done

Usually, we like to have PASS as expected.txt.
Can we modify LayoutTests/http/tests/workers/service/resources/registration-task-queue-scheduling-1.js accordingly?

> LayoutTests/http/tests/workers/service/resources/registration-task-queue-scheduling-1-second-window.html:7
> +	navigator.serviceWorker.register("http://127.0.0.1:8000/workers/service/empty-worker.js", { })

It is not clear to me why we open http://localhost:8000/workers/service/resources/registration-task-queue-scheduling-1-second-window.html but we fetch http://127.0.0.1:8000/workers/service/empty-worker.js instead of http://localhost:8000/workers/service/empty-worker.js.

If the host has no real importance, can we use something else than empty-worker.js, something like this-worker-is-no-there.js for instance?
Or are we testing cross origin loads.

> LayoutTests/http/tests/workers/service/resources/registration-task-queue-scheduling-1.js:46
> +	navigator.serviceWorker.register("http://localhost:8000/workers/service/resources/empty-worker.js", { })

Ditto here.

> LayoutTests/http/tests/workers/service/resources/registration-task-queue-scheduling-1.js:-55
> -		

Can we check the error type?

[Chris Dumez]

Comment on attachment 330852 [details]
Patch

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

>> LayoutTests/http/tests/workers/service/registration-task-queue-scheduling-1-expected.txt:1
>> +ALERT: Done
> 
> Usually, we like to have PASS as expected.txt.
> Can we modify LayoutTests/http/tests/workers/service/resources/registration-task-queue-scheduling-1.js accordingly?

Sure, this is how Brady wrote the test but I can change that.

>> LayoutTests/http/tests/workers/service/resources/registration-task-queue-scheduling-1-second-window.html:7
>> +	navigator.serviceWorker.register("http://127.0.0.1:8000/workers/service/empty-worker.js", { })
> 
> It is not clear to me why we open http://localhost:8000/workers/service/resources/registration-task-queue-scheduling-1-second-window.html but we fetch http://127.0.0.1:8000/workers/service/empty-worker.js instead of http://localhost:8000/workers/service/empty-worker.js.
> 
> If the host has no real importance, can we use something else than empty-worker.js, something like this-worker-is-no-there.js for instance?
> Or are we testing cross origin loads.

We want the registration to fail since this is what the test expects. Registration fails if the origin of the script does not match the origin of the client, which is why I specifically chose those origins.

>> LayoutTests/http/tests/workers/service/resources/registration-task-queue-scheduling-1.js:-55
>> -		
> 
> Can we check the error type?

Yes

[Chris Dumez]

Created attachment 330872 [details]
Patch

[EWS Watchlist]

Comment on attachment 330872 [details]
Patch

Attachment 330872 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/6016636

New failing tests:
webgl/1.0.2/conformance/uniforms/uniform-default-values.html
imported/w3c/web-platform-tests/media-source/mediasource-config-change-mp4-v-framerate.html

[EWS Watchlist]

Created attachment 330883 [details]
Archive of layout-test-results from ews101 for mac-sierra

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

[youenn fablet]

Comment on attachment 330872 [details]
Patch

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

> LayoutTests/http/tests/workers/service/resources/registration-task-queue-scheduling-1-second-window.html:7
> +	navigator.serviceWorker.register("http://127.0.0.1:8000/workers/service/empty-worker.js", { })

http://127.0.0.1:8000/workers/service/empty-worker.js is probably returning a 404.
Since the test is about cross origin, we probably want to use "http://127.0.0.1:8000/workers/service/resources/empty-worker.js"

[Chris Dumez]

(In reply to youenn fablet from comment #11)
> Comment on attachment 330872 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=330872&action=review
> 
> > LayoutTests/http/tests/workers/service/resources/registration-task-queue-scheduling-1-second-window.html:7
> > +	navigator.serviceWorker.register("http://127.0.0.1:8000/workers/service/empty-worker.js", { })
> 
> http://127.0.0.1:8000/workers/service/empty-worker.js is probably returning
> a 404.
> Since the test is about cross origin, we probably want to use
> "http://127.0.0.1:8000/workers/service/resources/empty-worker.js"

Oh, that's a good point. Will fix before landing.

[Chris Dumez]

Created attachment 330919 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 330919 [details]
Patch

Clearing flags on attachment: 330919

Committed r226722: <https://trac.webkit.org/changeset/226722>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.