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

[wptrunner] Decouple testdriver infrastructure from testharness #49044

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jonathan-j-lee
Copy link
Contributor

@jonathan-j-lee jonathan-j-lee commented Nov 7, 2024

Split off from #48486 in preparation for #13183. This PR itself should be a no-op, so it need not block on web-platform-tests/rfcs#211.

@jonathan-j-lee jonathan-j-lee marked this pull request as ready for review November 8, 2024 00:38
@wpt-pr-bot wpt-pr-bot added infra wptrunner The automated test runner, commonly called through ./wpt run wptserve labels Nov 8, 2024
(function() {
window.__wptrunner_is_test_context = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is to say as long as testharnessreport.js is included, it is in test context? Assume reftests won't include this file, this won't be true for them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There will be two cases where a document includes testdriver.js without testharness.js:

  1. "Main" test context for a (print-)reftest or crashtest
  2. An iframe or popup

Since we want testdriver.js to be usable in case (1) without testharnessreport.js or additional markup, testdriver.js now also defines __wptrunner_message_queue. However, this means __wptrunner_message_queue is defined everywhere and can no longer be used to tell apart the test context from case (2).

Therefore, testharnessreport.js sets an explicit __wptrunner_is_test_context flag to signal the test context to testdriver.js. For reftests/crashtests, the test context will be detected by <link rel=(mis)match> or (for crashtests, which have no other distinguishing features) class=test-wait.

message-queue.js is still served in /resources/testharnessreport.js because testharness tests that don't use testdriver still use __wptrunner_message_queue to report completion.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, but is there a better way to do this? We can wait until your next change to make a decision though.

This no-op refactor will allow other WebDriver executors, not just the
testharness executor, to perform testdriver actions.
This will allow non-testharness tests to use `testdriver.js` without
needing extra scripts. Evaluating `message-queue.js` is idempotent so
that, when using testharness with testdriver, the second inclusion is a
no-op.

Because resource scripts are cached, the size increase should not
meaningfully affect test performance.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infra wptrunner The automated test runner, commonly called through ./wpt run wptserve
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants