Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deflake WPT test registration-schedule-job.https.html #27796

Merged
merged 1 commit into from Mar 1, 2021

Conversation

@chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Feb 26, 2021

Make it so register jobs are not coalesced if they have
different updateViaCache properties, as per spec:
https://w3c.github.io/ServiceWorker/#dfn-job-equivalent

This makes the test about updateViaCache pass. The test was also
previously flaky in Chromium because it failed in two different ways
after coalescing the jobs: either registration.installing is null or
it's non-null and has the wrong updateViaCache property.

Other browsers also seemed to fail the test because they do not
make a new job and were possibly flaky. This CL also rearranges the
test to help browsers fail in a more stable way by accounting for
registration.installing being null. However it can still be flaky if
one register job is able to finish before the other one is queued up,
so it doesn't have a chance to coalesced.

Bug: 979593
Change-Id: I5395019d5733a6365b0b0ae2ae6051a077fb8176
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2721300
Reviewed-by: Asami Doi <asamidoi@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#858476}

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

The review process for this patch is being conducted in the Chromium project.

@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-2721300 branch 2 times, most recently from 15c8678 to af0a965 Feb 26, 2021
@mfalken
Copy link
Member

@mfalken mfalken commented Feb 26, 2021

This fails on the stability bots but I think this test is inherently flaky for browsers that don't pass the test. Chromium was failing it because it was coalescing the two register jobs:

register(...);
await register(...);
registration.installing;  // this would sometimes be null or not depending on how far the job got

You can see here too all browsers are currently failing and in different ways:
https://wpt.fyi/results/service-workers/service-worker/registration-schedule-job.https.html?label=master&label=experimental&aligned&q=service-workers%2Fservice-worker%2Fregistration-schedule-job.https.html

As the commit description says, this PR updates the test to be a little more stable for failing browsers but is still not 100% stable.

cc @asutherland @wanderview

Make it so register jobs are not coalesced if they have
different updateViaCache properties, as per spec:
https://w3c.github.io/ServiceWorker/#dfn-job-equivalent

This makes the test about updateViaCache pass. The test was also
previously flaky in Chromium because it failed in two different ways
after coalescing the jobs: either registration.installing is null or
it's non-null and has the wrong updateViaCache property.

Other browsers also seemed to fail the test because they do not
make a new job and were possibly flaky. This CL also rearranges the
test to help browsers fail in a more stable way by accounting for
registration.installing being null. However it can still be flaky if
one register job is able to finish before the other one is queued up,
so it doesn't have a chance to coalesced.

Bug: 979593
Change-Id: I5395019d5733a6365b0b0ae2ae6051a077fb8176
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2721300
Reviewed-by: Asami Doi <asamidoi@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#858476}
@stephenmcgruer
Copy link
Contributor

@stephenmcgruer stephenmcgruer commented Mar 1, 2021

Ack, thanks for the note. Admin-merging.

@stephenmcgruer stephenmcgruer merged commit 980c618 into master Mar 1, 2021
21 of 23 checks passed
21 of 23 checks passed
@github-actions
update-pr-preview
Details
@community-tc-integration
sink-task Community-TC (pull_request)
Details
@community-tc-integration
wpt-firefox-nightly-stability Community-TC (pull_request)
Details
@azure-pipelines
Azure Pipelines Build #20210301.9 succeeded
Details
@azure-pipelines
Azure Pipelines (./wpt test-jobs) ./wpt test-jobs succeeded
Details
@azure-pipelines
Azure Pipelines (affected tests without changes: Safari Technology Preview) affected tests without changes: Safari Technology Preview succeeded
Details
@azure-pipelines
Azure Pipelines (affected tests: Safari Technology Preview) affected tests: Safari Technology Preview succeeded
Details
@azure-pipelines
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests) wpt.fyi hook: safari-preview-affected-tests succeeded
Details
@azure-pipelines
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests-without-changes) wpt.fyi hook: safari-preview-affected-tests-without-changes succeeded
Details
@community-tc-integration
download-firefox-nightly Community-TC (pull_request)
Details
@community-tc-integration
lint Community-TC (pull_request)
Details
@staging-wpt-fyi
staging.wpt.fyi - chrome[experimental] Chrome results
Details
@staging-wpt-fyi
staging.wpt.fyi - firefox[experimental] Firefox results
Details
@staging-wpt-fyi
staging.wpt.fyi - safari[experimental] Safari results
Details
@community-tc-integration
wpt-chrome-dev-results Community-TC (pull_request)
Details
@community-tc-integration
wpt-chrome-dev-results-without-changes Community-TC (pull_request)
Details
@community-tc-integration
wpt-chrome-dev-stability Community-TC (pull_request)
Details
@community-tc-integration
wpt-decision-task Community-TC (pull_request)
Details
@community-tc-integration
wpt-firefox-nightly-results Community-TC (pull_request)
Details
@community-tc-integration
wpt-firefox-nightly-results-without-changes Community-TC (pull_request)
Details
@wpt-fyi
wpt.fyi - chrome[experimental] Chrome results
Details
@wpt-fyi
wpt.fyi - firefox[experimental] Firefox results
Details
@wpt-fyi
wpt.fyi - safari[experimental] Safari results
Details
@stephenmcgruer stephenmcgruer deleted the chromium-export-cl-2721300 branch Mar 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants