-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
service worker WPT tests: split very big registration tests into multiple files #7006
service worker WPT tests: split very big registration tests into multiple files #7006
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already reviewed downstream.
Build ERROREDStarted: 2017-08-31 09:38:03 Failing Jobs
View more information about this build on: |
e8e2734
to
ad134fb
Compare
ad134fb
to
c7f2723
Compare
cc @mattto I've retried the Travis jobs and they are still timing out. Can you please take a look? |
@wanderview Do you know how to debug this type of error? It looks like the failure is caused by Firefox Nightly but I'm not having good luck reading the bot output. |
Probably need to ask @jgraham. |
@jgraham can you take a look? If it's a hassle, I guess I can just revert this change. It's not super critical but seems nicer to split up the huge test file into parts. |
The stability checker is actually timing out on all browsers, it's just that Firefox isn't an allowed failure and the others are. First trying to rebase this on master to see if a new run is any different. |
…iple files registration.https.html and link-element-register.https.html do about 70 register calls, most of which result in a service worker starting up. These test files always came close to the test harness timeout, and with the new snapshotting change r496290 which does a lot of work in Debug, started timing out on Debug. The snapshotting change is supposed to be fixed soon, but split them up into smaller files anyway. I verified that no tests were lost by generating the new files then `cat registration-*expected* | sort | uniq` and comparing to the existing test output (and similar for link-element-register*). Bug: 758481 Change-Id: I7e522e3c4e87df11fcb5197da59005e2d9e25f92 Reviewed-on: https://chromium-review.googlesource.com/635065 Commit-Queue: Matt Falkenhagen <[email protected]> Reviewed-by: Hiroki Nakagawa <[email protected]> Cr-Commit-Position: refs/heads/master@{#497717}
c7f2723
to
82e5997
Compare
Worth noting in http://wpt.fyi/service-workers/service-worker/register-link-element.https.html is that the old test was TIMEOUT and NOTRUN across the board. So at least nothing is made worse by splitting, even if the new tests all fail as well. (But they at least pass in Chromium's imported wpt, fixing some timeouts that were seen.) |
I've filed an issue about this in #7073 and am going to force merge this. Let's see if these tests then start passing in wpt.fyi. |
Thanks for investigating this, @foolip ! |
registration.https.html and link-element-register.https.html do about 70
register calls, most of which result in a service worker starting up. These
test files always came close to the test harness timeout, and with the new
snapshotting change r496290 which does a lot of work in Debug, started timing
out on Debug. The snapshotting change is supposed to be fixed soon, but split
them up into smaller files anyway.
I verified that no tests were lost by generating the new files then
cat registration-*expected* | sort | uniq
and comparing tothe existing test output (and similar for link-element-register*).
Bug: 758481
Change-Id: I7e522e3c4e87df11fcb5197da59005e2d9e25f92
Reviewed-on: https://chromium-review.googlesource.com/635065
Commit-Queue: Matt Falkenhagen [email protected]
Reviewed-by: Hiroki Nakagawa [email protected]
Cr-Commit-Position: refs/heads/master@{#497717}