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

appcache: remove wpt references to appcache #31638

Merged
merged 1 commit into from
Nov 23, 2021
Merged

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Nov 16, 2021

This is mostly a removal, but required some modifications as well.
Expectations are updated to remove references to removed tests.

Bug: 582750
Change-Id: I4e705f5888d3c8cd3d2cc4859a7a3816de5e88f5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3283191
Commit-Queue: enne <[email protected]>
Reviewed-by: Marijn Kruisselbrink <[email protected]>
Cr-Commit-Position: refs/heads/main@{#943332}

Copy link
Collaborator

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@quisquous
Copy link
Contributor

@valenting FYI. How is appcache removal in Firefox going? Do you think it's ok to move forward with removing these WPT appcache tests?

@quisquous
Copy link
Contributor

I ran the wpt tests locally on Chrome without these changes and got the same errors, so I think this patch is ok. I did a double check of the code as well, and even though I removed some things in the service worker directory, there are no references to anything I removed.

/c/src/testing/xvfb.py python3 ./wpt run --channel=dev --verify --verify-no-chaos-mode --verify-repeat-loop=0 --verify-repeat-restart=10 --log-mach-level=info --log-mach=- -y --no-pause --no-restart-on-unexpected --install-fonts --no-headless --verify-log-full --enable-swiftshader --binary=/c/src/out/Default/chrome chrome service-workers/service-worker/about-blank-replacement.https.html service-workers/service-worker/unregister-immediately-before-installed.https.html

...snip...

 2:07.32 INFO ## All results ##

 2:07.32 INFO ### /service-workers/service-worker/about-blank-replacement.https.html ###
 2:07.32 INFO |                                                           Subtest                                                           | Results |                                                         Messages                                                         |
 2:07.32 INFO |-----------------------------------------------------------------------------------------------------------------------------|---------|--------------------------------------------------------------------------------------------------------------------------|
 2:07.32 INFO |                                                                                                                             | OK      |                                                                                                                          |
 2:07.32 INFO | `Initial about:blank is controlled, exposed to clients.matchAll(), and matches final Client.`                               | FAIL    | `assert_false: result: failure: could not find about:blank client expected false got true`                               |
 2:07.32 INFO | `Initial about:blank modified by parent is controlled, exposed to clients.matchAll(), and matches final Client.`            | FAIL    | `assert_false: result: failure: could not find about:blank client expected false got true`                               |
 2:07.32 INFO | `Popup initial about:blank is controlled, exposed to clients.matchAll(), and matches final Client.`                         | FAIL    | `assert_false: result: failure: could not find about:blank client expected false got true`                               |
 2:07.32 INFO | `Initial about:blank is controlled, exposed to clients.matchAll(), and final Client is not controlled by a service worker.` | PASS    |                                                                                                                          |
 2:07.32 INFO | `Simple about:blank is controlled and is exposed to clients.matchAll().`                                                    | FAIL    | `promise_test: Unhandled rejection with value: object "TypeError: Cannot read properties of null (reading 'scriptURL')"` |
 2:07.32 INFO | `Nested about:srcdoc is controlled and is exposed to clients.matchAll().`                                                   | FAIL    | `promise_test: Unhandled rejection with value: object "TypeError: Cannot read properties of null (reading 'scriptURL')"` |
 2:07.32 INFO | `Dynamic about:blank is controlled and is exposed to clients.matchAll().`                                                   | FAIL    | `promise_test: Unhandled rejection with value: object "TypeError: Cannot read properties of null (reading 'scriptURL')"` |
 2:07.32 INFO 
 2:07.32 INFO ### /service-workers/service-worker/unregister-immediately-before-installed.https.html ###
 2:07.32 INFO |                                          Subtest                                          |            Results            |     Messages     |
 2:07.32 INFO |-------------------------------------------------------------------------------------------|-------------------------------|------------------|
 2:07.32 INFO |                                                                                           | **OK: 3/10, TIMEOUT: 7/10**   |                  |
 2:07.32 INFO | `Clear-Site-Data must abort service worker registration.`                                 | **PASS: 3/10, TIMEOUT: 7/10** | `Test timed out` |
 2:07.32 INFO | `Clear-Site-Data must unregister a registration with a worker in the "installing" state.` | **NOTRUN: 7/10, PASS: 3/10**  |                  |
 2:07.32 INFO 
 2:07.32 INFO ## Unstable results ##

 2:07.32 INFO |                                         Test                                         |                                          Subtest                                          |            Results            |     Messages     |
 2:07.32 INFO |--------------------------------------------------------------------------------------|-------------------------------------------------------------------------------------------|-------------------------------|------------------|
 2:07.32 INFO | `/service-workers/service-worker/unregister-immediately-before-installed.https.html` |                                                                                           | **OK: 3/10, TIMEOUT: 7/10**   |                  |
 2:07.32 INFO | `/service-workers/service-worker/unregister-immediately-before-installed.https.html` | `Clear-Site-Data must abort service worker registration.`                                 | **PASS: 3/10, TIMEOUT: 7/10** | `Test timed out` |
 2:07.32 INFO | `/service-workers/service-worker/unregister-immediately-before-installed.https.html` | `Clear-Site-Data must unregister a registration with a worker in the "installing" state.` | **NOTRUN: 7/10, PASS: 3/10**  |                  |
 2:07.32 INFO 
 2:07.32 INFO ::: Running tests in a loop with restarts 10 times : FAIL
 2:07.32 INFO :::
 2:07.32 ERROR ::: Test verification FAIL
 2:07.32 INFO :::
Command returned exit code 1

This is mostly a removal, but required some modifications as well.
Expectations are updated to remove references to removed tests.

Bug: 582750
Change-Id: I4e705f5888d3c8cd3d2cc4859a7a3816de5e88f5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3283191
Commit-Queue: enne <[email protected]>
Reviewed-by: Marijn Kruisselbrink <[email protected]>
Cr-Commit-Position: refs/heads/main@{#943332}
@quisquous
Copy link
Contributor

@valenting FYI. How is appcache removal in Firefox going? Do you think it's ok to move forward with removing these WPT appcache tests?

Followed up by email, and Firefox has already removed appcache and disabled these tests.

@foolip
Copy link
Member

foolip commented Nov 23, 2021

This was blocked by #31714, I'll close and reopen in the hope to get the #31715 workaround, which should unblock this.

@foolip
Copy link
Member

foolip commented Nov 23, 2021

The stability jobs are still failing, but judging by the nature of the changes (removing stuff) and #31638 (comment) I'm going to guess that this is an preexisting issue not caused by the changed. I'll admin merge this PR.

@foolip foolip merged commit 561641e into master Nov 23, 2021
@foolip foolip deleted the chromium-export-cl-3283191 branch November 23, 2021 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants