Address flakiness of service-workers/service-worker/update-no-cache-r… #20797
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…equest-headers.https.html The test triggers a registration update and then expects registration.installing to be non-null once the registration update promise is resolved. This is only true if the content of the service worker script is different since last update. The script included a timestamp to try and make the script different every time but it would sometimes not suffice if the update happens quickly enough (https://bugs.webkit.org/show_bug.cgi?id=205286). To address the issue, include a UUID in the script instead of a timestamp.
|
cc @youennf |
wpt-pr-bot
requested review from
asutherland,
mfalken,
mkruisselbrink and
wanderview
mfalken
approved these changes
Dec 17, 2019
There was a problem hiding this comment.
This lgtm. But the bot shows a failure on wpt-firefox-nightly-stability:
assert_equals: expected (string) "etag" but got (undefined) undefined`
This might be existing flakiness caused by another bug in this test. wpt.fyi shows the same error for Chrome and Safari today.
@asutherland are you ok with merging this?
philn
pushed a commit
to philn/old-webkit
that referenced
this pull request
Dec 17, 2019
…rs/service-worker/update-no-cache-request-headers.https.html is a flaky failure https://bugs.webkit.org/show_bug.cgi?id=205286 <rdar://problem/57976344> Reviewed by Alexey Proskuryakov. The test triggers a registration update and then expects registration.installing to be non-null once the registration update promise is resolved. This is only true if the content of the service worker script is different since last update. The script included a timestamp to try and make the script different every time but it would sometimes not suffice if the update happens quickly enough. To address the issue, include a UUID in the script instead of a timestamp. Upstream PR: web-platform-tests/wpt#20797 * web-platform-tests/service-workers/service-worker/resources/test-request-headers-worker.js: * web-platform-tests/service-workers/service-worker/resources/test-request-headers-worker.py: (main): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@253592 268f45cc-cd09-0410-ab3c-d52691b4dbfc
|
The UUID change seems fine to me. Firefox locally has a bug on the etag issue and I think my most recent analysis on that was at https://bugzilla.mozilla.org/show_bug.cgi?id=1541192#c35. |
|
How do we go about merging this? |
|
I'll admin merge per #20797 (comment). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…equest-headers.https.html
The test triggers a registration update and then expects registration.installing to be non-null
once the registration update promise is resolved. This is only true if the content of the service
worker script is different since last update. The script included a timestamp to try and make
the script different every time but it would sometimes not suffice if the update happens quickly
enough (https://bugs.webkit.org/show_bug.cgi?id=205286). To address the issue, include a UUID in
the script instead of a timestamp.