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

Fix preload tests failing on wpt.fyi dashboard #14882

Merged
merged 1 commit into from
Jan 25, 2019

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Jan 16, 2019

verifyNumberOfDownloads only counts resource timing entries with
transferSize > 0, so it doesn't count if the resource came from a cache.
(See https://bugzilla.mozilla.org/show_bug.cgi?id=1222633#c38 for
background why we have it.)

link-header-preload-nonce.html and link-header-preload-srcset.tentative.html
were failing on wpt.fyi dashboard because they have used the same
subresource URLs as other tests, and preloaded resources from the disk
cache.

This patch adds unique query parameters to these URLs so that they don't
conflict with other tests.

Bug: 922343
Change-Id: I7fd88530f5fdd83ed2a22d2e2893cc446debd036
Reviewed-on: https://chromium-review.googlesource.com/c/1411963
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623157}

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.

Already reviewed downstream.

verifyNumberOfDownloads only counts resource timing entries with
transferSize > 0, so it doesn't count if the resource came from a cache.
(See https://bugzilla.mozilla.org/show_bug.cgi?id=1222633#c38 for
background why we have it.)

link-header-preload-nonce.html and link-header-preload-srcset.tentative.html
were failing on wpt.fyi dashboard because they have used the same
subresource URLs as other tests, and preloaded resources from the disk
cache.

This patch adds unique query parameters to these URLs so that they don't
conflict with other tests.

Bug: 922343
Change-Id: I7fd88530f5fdd83ed2a22d2e2893cc446debd036
Reviewed-on: https://chromium-review.googlesource.com/c/1411963
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623157}
@Hexcles
Copy link
Member

Hexcles commented Jan 24, 2019

@irori tests seem flaky on Chrome. Could you take a look?

Unstable results

Test Subtest Results Messages
/preload/link-header-preload-nonce.html Makes sure that Link headers preload resources with CSP nonce FAIL: 9/10, PASS: 1/10 assert_equals: resources/dummy.js?from-header&with-nonce expected 1 but got 0
/preload/link-header-preload-srcset.tentative.html Makes sure that Link headers preload images with (experimental) imagesrcset/imagesizes attributes. FAIL: 9/10, PASS: 1/10 assert_equals: resources/square.png?from-header&1x expected 1 but got 0

@irori
Copy link
Contributor

irori commented Jan 25, 2019

Thanks for the ping, https://crrev.com/c/1436777 should fix this.

aarongable pushed a commit to chromium/chromium that referenced this pull request Jan 25, 2019
https://crrev.com/c/1411963 fixed the url conflicts with other tests,
but the test still flakes when running repeatedly, because the resource
is loaded from disk cache in subsequent runs
(web-platform-tests/wpt#14882).

We don't care if the preloaded resources came from cache in these tests,
so let's just verify the number of Resource Timing entries.

Bug: 922343
Change-Id: I2c1d146856a125dbbb641d7efb4e6151094f6a3e
Reviewed-on: https://chromium-review.googlesource.com/c/1436777
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626019}
@irori
Copy link
Contributor

irori commented Jan 25, 2019

https://crrev.com/c/1436777 landed, but it seems that WPT PR wasn't created by the bot (because this PR wasn't merged?).
Should I merge https://crrev.com/c/1436777 into web-platform-tests/wpt manually?

@foolip
Copy link
Member

foolip commented Jan 25, 2019

@irori the reason no PR was created for https://crrev.com/c/1436777 yet is because this PR hasn't been merged yet, so the changes don't apply cleanly to master.

Since the follow up should fix this, I'll admin merge this to allow the automatic export process to continue.

@foolip foolip merged commit 1311556 into master Jan 25, 2019
@foolip foolip deleted the chromium-export-cl-1411963 branch January 25, 2019 11:59
chromium-wpt-export-bot pushed a commit that referenced this pull request Jan 25, 2019
https://crrev.com/c/1411963 fixed the url conflicts with other tests,
but the test still flakes when running repeatedly, because the resource
is loaded from disk cache in subsequent runs
(#14882).

We don't care if the preloaded resources came from cache in these tests,
so let's just verify the number of Resource Timing entries.

Bug: 922343
Change-Id: I2c1d146856a125dbbb641d7efb4e6151094f6a3e
Reviewed-on: https://chromium-review.googlesource.com/c/1436777
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626019}
@foolip
Copy link
Member

foolip commented Jan 25, 2019

That PR is #15075.

Hexcles pushed a commit that referenced this pull request Jan 28, 2019
https://crrev.com/c/1411963 fixed the url conflicts with other tests,
but the test still flakes when running repeatedly, because the resource
is loaded from disk cache in subsequent runs
(#14882).

We don't care if the preloaded resources came from cache in these tests,
so let's just verify the number of Resource Timing entries.

Bug: 922343
Change-Id: I2c1d146856a125dbbb641d7efb4e6151094f6a3e
Reviewed-on: https://chromium-review.googlesource.com/c/1436777
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626019}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Feb 5, 2019
…hboard, second attempt, a=testonly

Automatic update from web-platform-tests
Fix preload tests failing on wpt.fyi dashboard, second attempt

https://crrev.com/c/1411963 fixed the url conflicts with other tests,
but the test still flakes when running repeatedly, because the resource
is loaded from disk cache in subsequent runs
(web-platform-tests/wpt#14882).

We don't care if the preloaded resources came from cache in these tests,
so let's just verify the number of Resource Timing entries.

Bug: 922343
Change-Id: I2c1d146856a125dbbb641d7efb4e6151094f6a3e
Reviewed-on: https://chromium-review.googlesource.com/c/1436777
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626019}

--

wpt-commits: a52ae0c66f4a764f036d6b85e9d2e611acf45383
wpt-pr: 15075
mykmelez pushed a commit to mykmelez/gecko that referenced this pull request Feb 6, 2019
…hboard, second attempt, a=testonly

Automatic update from web-platform-tests
Fix preload tests failing on wpt.fyi dashboard, second attempt

https://crrev.com/c/1411963 fixed the url conflicts with other tests,
but the test still flakes when running repeatedly, because the resource
is loaded from disk cache in subsequent runs
(web-platform-tests/wpt#14882).

We don't care if the preloaded resources came from cache in these tests,
so let's just verify the number of Resource Timing entries.

Bug: 922343
Change-Id: I2c1d146856a125dbbb641d7efb4e6151094f6a3e
Reviewed-on: https://chromium-review.googlesource.com/c/1436777
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626019}

--

wpt-commits: a52ae0c66f4a764f036d6b85e9d2e611acf45383
wpt-pr: 15075
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Feb 7, 2019
…hboard, second attempt, a=testonly

Automatic update from web-platform-tests
Fix preload tests failing on wpt.fyi dashboard, second attempt

https://crrev.com/c/1411963 fixed the url conflicts with other tests,
but the test still flakes when running repeatedly, because the resource
is loaded from disk cache in subsequent runs
(web-platform-tests/wpt#14882).

We don't care if the preloaded resources came from cache in these tests,
so let's just verify the number of Resource Timing entries.

Bug: 922343
Change-Id: I2c1d146856a125dbbb641d7efb4e6151094f6a3e
Reviewed-on: https://chromium-review.googlesource.com/c/1436777
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626019}

--

wpt-commits: a52ae0c66f4a764f036d6b85e9d2e611acf45383
wpt-pr: 15075
mykmelez pushed a commit to mykmelez/gecko that referenced this pull request Feb 8, 2019
…hboard, second attempt, a=testonly

Automatic update from web-platform-tests
Fix preload tests failing on wpt.fyi dashboard, second attempt

https://crrev.com/c/1411963 fixed the url conflicts with other tests,
but the test still flakes when running repeatedly, because the resource
is loaded from disk cache in subsequent runs
(web-platform-tests/wpt#14882).

We don't care if the preloaded resources came from cache in these tests,
so let's just verify the number of Resource Timing entries.

Bug: 922343
Change-Id: I2c1d146856a125dbbb641d7efb4e6151094f6a3e
Reviewed-on: https://chromium-review.googlesource.com/c/1436777
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626019}

--

wpt-commits: a52ae0c66f4a764f036d6b85e9d2e611acf45383
wpt-pr: 15075
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 3, 2019
…hboard, second attempt, a=testonly

Automatic update from web-platform-tests
Fix preload tests failing on wpt.fyi dashboard, second attempt

https://crrev.com/c/1411963 fixed the url conflicts with other tests,
but the test still flakes when running repeatedly, because the resource
is loaded from disk cache in subsequent runs
(web-platform-tests/wpt#14882).

We don't care if the preloaded resources came from cache in these tests,
so let's just verify the number of Resource Timing entries.

Bug: 922343
Change-Id: I2c1d146856a125dbbb641d7efb4e6151094f6a3e
Reviewed-on: https://chromium-review.googlesource.com/c/1436777
Reviewed-by: Kinuko Yasuda <kinukochromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamotochromium.org>
Cr-Commit-Position: refs/heads/master{#626019}

--

wpt-commits: a52ae0c66f4a764f036d6b85e9d2e611acf45383
wpt-pr: 15075

UltraBlame original commit: 4243b5a671878451290c26710a39b04d8ea9905e
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 3, 2019
…hboard, second attempt, a=testonly

Automatic update from web-platform-tests
Fix preload tests failing on wpt.fyi dashboard, second attempt

https://crrev.com/c/1411963 fixed the url conflicts with other tests,
but the test still flakes when running repeatedly, because the resource
is loaded from disk cache in subsequent runs
(web-platform-tests/wpt#14882).

We don't care if the preloaded resources came from cache in these tests,
so let's just verify the number of Resource Timing entries.

Bug: 922343
Change-Id: I2c1d146856a125dbbb641d7efb4e6151094f6a3e
Reviewed-on: https://chromium-review.googlesource.com/c/1436777
Reviewed-by: Kinuko Yasuda <kinukochromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamotochromium.org>
Cr-Commit-Position: refs/heads/master{#626019}

--

wpt-commits: a52ae0c66f4a764f036d6b85e9d2e611acf45383
wpt-pr: 15075

UltraBlame original commit: 34865e88897289c24a6cfd1a04fc3373d7bbebcb
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 3, 2019
…hboard, second attempt, a=testonly

Automatic update from web-platform-tests
Fix preload tests failing on wpt.fyi dashboard, second attempt

https://crrev.com/c/1411963 fixed the url conflicts with other tests,
but the test still flakes when running repeatedly, because the resource
is loaded from disk cache in subsequent runs
(web-platform-tests/wpt#14882).

We don't care if the preloaded resources came from cache in these tests,
so let's just verify the number of Resource Timing entries.

Bug: 922343
Change-Id: I2c1d146856a125dbbb641d7efb4e6151094f6a3e
Reviewed-on: https://chromium-review.googlesource.com/c/1436777
Reviewed-by: Kinuko Yasuda <kinukochromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamotochromium.org>
Cr-Commit-Position: refs/heads/master{#626019}

--

wpt-commits: a52ae0c66f4a764f036d6b85e9d2e611acf45383
wpt-pr: 15075

UltraBlame original commit: 4243b5a671878451290c26710a39b04d8ea9905e
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 3, 2019
…hboard, second attempt, a=testonly

Automatic update from web-platform-tests
Fix preload tests failing on wpt.fyi dashboard, second attempt

https://crrev.com/c/1411963 fixed the url conflicts with other tests,
but the test still flakes when running repeatedly, because the resource
is loaded from disk cache in subsequent runs
(web-platform-tests/wpt#14882).

We don't care if the preloaded resources came from cache in these tests,
so let's just verify the number of Resource Timing entries.

Bug: 922343
Change-Id: I2c1d146856a125dbbb641d7efb4e6151094f6a3e
Reviewed-on: https://chromium-review.googlesource.com/c/1436777
Reviewed-by: Kinuko Yasuda <kinukochromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamotochromium.org>
Cr-Commit-Position: refs/heads/master{#626019}

--

wpt-commits: a52ae0c66f4a764f036d6b85e9d2e611acf45383
wpt-pr: 15075

UltraBlame original commit: 34865e88897289c24a6cfd1a04fc3373d7bbebcb
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 4, 2019
…hboard, second attempt, a=testonly

Automatic update from web-platform-tests
Fix preload tests failing on wpt.fyi dashboard, second attempt

https://crrev.com/c/1411963 fixed the url conflicts with other tests,
but the test still flakes when running repeatedly, because the resource
is loaded from disk cache in subsequent runs
(web-platform-tests/wpt#14882).

We don't care if the preloaded resources came from cache in these tests,
so let's just verify the number of Resource Timing entries.

Bug: 922343
Change-Id: I2c1d146856a125dbbb641d7efb4e6151094f6a3e
Reviewed-on: https://chromium-review.googlesource.com/c/1436777
Reviewed-by: Kinuko Yasuda <kinukochromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamotochromium.org>
Cr-Commit-Position: refs/heads/master{#626019}

--

wpt-commits: a52ae0c66f4a764f036d6b85e9d2e611acf45383
wpt-pr: 15075

UltraBlame original commit: 4243b5a671878451290c26710a39b04d8ea9905e
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 4, 2019
…hboard, second attempt, a=testonly

Automatic update from web-platform-tests
Fix preload tests failing on wpt.fyi dashboard, second attempt

https://crrev.com/c/1411963 fixed the url conflicts with other tests,
but the test still flakes when running repeatedly, because the resource
is loaded from disk cache in subsequent runs
(web-platform-tests/wpt#14882).

We don't care if the preloaded resources came from cache in these tests,
so let's just verify the number of Resource Timing entries.

Bug: 922343
Change-Id: I2c1d146856a125dbbb641d7efb4e6151094f6a3e
Reviewed-on: https://chromium-review.googlesource.com/c/1436777
Reviewed-by: Kinuko Yasuda <kinukochromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamotochromium.org>
Cr-Commit-Position: refs/heads/master{#626019}

--

wpt-commits: a52ae0c66f4a764f036d6b85e9d2e611acf45383
wpt-pr: 15075

UltraBlame original commit: 34865e88897289c24a6cfd1a04fc3373d7bbebcb
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