-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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 use of testharness.js/testharnessreport.js in *.window.js tests #36069
Conversation
@WeizhongX FYI this is a case of a problem that only appears with wptrunner, so we haven't noticed this when developing tests in Chromium. Might be an example to add to "bad things that can happen when we don't use the same setup across CIs to run tests." |
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.
LGTM, although it might be worth creating a lint rule for this at some point to prevent regressions.
I've filed #36071 for the lint. |
So 4 of the tests take close to 10 seconds to run. navigation-helpers.window.html in both Chrome and Firefox, and 3 tests in /permissions-policy/experimental-features/ in just Chrome. I'll add |
These scripts are already included in the template that turns *.window.js into *.window.html. Including the twice leads to the error "Identifier 'MessageQueue' has already been declared" in these tests when run using wptrunner. MessageQueue is a class declared here: https://github.com/web-platform-tests/wpt/blob/master/tools/wptrunner/wptrunner/testharnessreport.js There's also one case of testharness-helpers.js being removed. That file doesn't exist, and the assert_promise_rejects_with_message helper is defined by /bluetooth/resources/bluetooth-test.js. Add timeout=long for tests that took close to 10 seconds run and then passed. These were flagged as slow tests by the Taskcluster stability checks.
7daf2b4
to
ca181ce
Compare
Ack! |
These scripts are already included in the template that turns
*.window.js into *.window.html. Including the twice leads to the error
"Identifier 'MessageQueue' has already been declared" in these tests
when run using wptrunner. MessageQueue is a class declared here:
https://github.com/web-platform-tests/wpt/blob/master/tools/wptrunner/wptrunner/testharnessreport.js
There's also one case of testharness-helpers.js being removed. That file
doesn't exist, and the assert_promise_rejects_with_message helper is
defined by /bluetooth/resources/bluetooth-test.js.
Add timeout=long for tests that took close to 10 seconds run and then
passed. These were flagged as slow tests by the Taskcluster stability
checks.