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

Start running the parsed file lints against support files #48320

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gsnedders
Copy link
Member

@gsnedders gsnedders commented Sep 23, 2024

This is largely motivated by ensuring that people aren't including testharnessreport.js in support files which are then included via fetch_tests_from_window (per docs), and most of the files changed here are removing redundant inclusions here.

We also find a few other errors, such as testdriver-vendor.js being omitted and invalid values for <meta name=timeout>, which are also fixed.

Finally, add a few things to the allowlist.

See also WebKit/WebKit#33969, cc @kkinnunen-apple.

For reference:

I believe the only way an HTML testharness.js support file can be included is via fetch_tests_from_window, which means we should just enforce its semantics.

The ultimate goal here is to be able to run tests within cross-origin iframes to test site isolation, and we need to be able to make sure we get the right output. web-platform-tests/rfcs#168 would really also solve this, but this still hasn't been done.

This is largely motivated by ensuring that people aren't including
testharnessreport.js in support files which are then included via
fetch_tests_from_window, and most of the files changed here are
removing redundant inclusions here.

We also find a few other errors, such as testdriver-vendor.js being
omitted and invalid values for <meta name=timeout>, which are also
fixed.

Finally, add a few things to the allowlist.
Copy link
Contributor

@Ms2ger Ms2ger left a comment

Choose a reason for hiding this comment

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

As noted opinion-haver, I didn't realize you shouldn't include threport.js in support files, so this seems like a good thing to lint for.

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.

4 participants