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
[Storage Access API] Fix anomalous behavior with tests that use iframes #30320
[Storage Access API] Fix anomalous behavior with tests that use iframes #30320
Conversation
f1fbdc5
to
ff35315
Compare
The tests for `hasStorageAccess` and `requestStorageAccess` need to test in different browsing contexts, with different relationships to the top-level browsing context, and both same-origin and cross-origin. They do this by declaring tests in the test file, and then loading that same test file in iframes using `fetch_tests_from_window`. However, a requirement of `fetch_tests_from_window` is that the window whose tests to fetch must not include `testharnessreport.js` – which isn't true for these Storage Access API tests. While this doesn't seem to make a difference when running `wpt serve` or otherwise when using the default `testharnessreport.js` file, tests run with `wpt run` might timeout, and the subtests coming from the iframes might not be reported. This change creates versions of `hasStorageAccess.sub.window.html` and `requestStorageAccess.sub.window.html` that don't import `testharnessreport.js`, specifically to be loaded as iframes.
ff35315
to
e5acd77
Compare
When running on my machine with |
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.
Thanks for the update, LGTM.
@andreubotella is this change ready to be merged? |
As I mentioned above, I was getting different results on CI than with |
The results do match this time, so I guess this can be merged now. |
Great! Let's merge, and I also have a Chromium bug tracking flakiness with these tests. Once this change hits the repo I'll confirm/investigate any remaining issues. Thanks again for the patch. |
…es (web-platform-tests#30320) The tests for `hasStorageAccess` and `requestStorageAccess` need to test in different browsing contexts, with different relationships to the top-level browsing context, and both same-origin and cross-origin. They do this by declaring tests in the test file, and then loading that same test file in iframes using `fetch_tests_from_window`. However, a requirement of `fetch_tests_from_window` is that the window whose tests to fetch must not include `testharnessreport.js` – which isn't true for these Storage Access API tests. While this doesn't seem to make a difference when running `wpt serve` or otherwise when using the default `testharnessreport.js` file, tests run with `wpt run` might timeout, and the subtests coming from the iframes might not be reported. This change creates versions of `hasStorageAccess.sub.window.html` and `requestStorageAccess.sub.window.html` that don't import `testharnessreport.js`, specifically to be loaded as iframes.
The tests for
hasStorageAccess
andrequestStorageAccess
need to test in different browsing contexts, with different relationships to the top-level browsing context, and both same-origin and cross-origin. They do this by declaring tests in the test file, and then loading that same test file in iframes usingfetch_tests_from_window
.However, a requirement of
fetch_tests_from_window
is that the window whose tests to fetch must not includetestharnessreport.js
– which isn't true for these Storage Access API tests. While this doesn't seem to make a difference when runningwpt serve
or otherwise when using the defaulttestharnessreport.js
file, tests run withwpt run
might timeout, and the subtests coming from the iframes might not be reported.This change creates versions of
hasStorageAccess.sub.window.html
andrequestStorageAccess.sub.window.html
that don't importtestharnessreport.js
, specifically to be loaded as iframes.