-
Notifications
You must be signed in to change notification settings - Fork 3.6k
feat(runner): Integrate Test262 execution with wptrunner and add smoke tests [pt 3/5] #56842
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
base: feat/test262-serve-handler
Are you sure you want to change the base?
feat(runner): Integrate Test262 execution with wptrunner and add smoke tests [pt 3/5] #56842
Conversation
35d9c44 to
3374ad9
Compare
3374ad9 to
a2d6c63
Compare
16aa645 to
d5eb82e
Compare
a2d6c63 to
258c427
Compare
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.
This suggestion came from a Gemini code review:
Harness Adapter Robustness
Focus: resources/test262/harness-adapter.js
Critique: Bridging Test262's global-polluting API (e.g., $DONE, Test262Error) to the structured testharness.js API is error-prone.
- Exception Handling: Ensure the adapter explicitly wraps the execution of the Test262 test code in a
try/catchblock (or useswindow.onerror). Test262 tests often throw bare strings or non-Error objects. The adapter must normalize these into meaningfultestharnessfailure messages, otherwise, the runner might just report a generic "Script error." or timeout. - $DONE Semantics: Verify that calling
$DONE()(without arguments) signals a PASS, and$DONE(error)signals a FAIL. A common bug is treating any call to$DONEas a completion without checking the argument.
dcdca82 to
df146d5
Compare
…e tests This commit completes the core integration of Test262 into the WPT runner (`wptrunner`), enabling the execution of Test262 JavaScript conformance tests within the browser environment. This work builds upon the manifest tooling (PR 1 #56840) and server-side handling (PR 2 #56841) to provide a full end-to-end testing solution for Test262. Key components introduced and integrated: - **Client-Side Harness:** New JavaScript files (`harness-adapter.js`, `testharness-client.js`, `testharness.js`) under `resources/test262/` provide the necessary environment for Test262 tests to execute in an `iframe` and report results back to the main WPT testharness. - **Test262 Assertion Libraries (Vendored):** For the purpose of enabling these initial smoke tests, the standard Test262 assertion libraries (`assert.js` and `sta.js`) are directly placed into `third_party/test262/harness/`. This provides the immediate dependencies required by the smoke tests. An autoamted vendoring solution, including version tracking via `vendored.toml`, will be implemented in a subsequent PR focused on automated updates, as described in the RFC. - **Wptrunner Integration:** Modifications to `tools/wptrunner/wpttest.py` and `tools/wptrunner/browsers/*.py` enable `wptrunner` to recognize the `test262` test type and execute it using existing testharness executors. - **Smoke Tests and Metadata:** A suite of basic Test262 smoke tests (`infrastructure/test262/`) and their corresponding expected results (`infrastructure/metadata/infrastructure/test262/`) are added to verify the correct functioning of the entire integration. - **Linting Exemption:** `lint.ignore` is updated to exclude the vendored Test262 harness files from linting, respecting their upstream style. This integration allows browser vendors to run Test262 directly with `wptrunner`, streamlining conformance testing efforts as specified in the RFC: web-platform-tests/rfcs#229 This commit is the third in a series of smaller PRs split from the larger, original implementation in #55997.
258c427 to
94dd567
Compare
| global.$262 = { | ||
| createRealm: function() { | ||
| var iframe = global.document.createElement('iframe'); | ||
| iframe.style.cssText = 'display: none'; |
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.
| iframe.style.cssText = 'display: none'; | |
| iframe.style.display = 'none'; |
perhaps, although I wonder if we really want to always default to None.
| createRealm: function() { | ||
| var iframe = global.document.createElement('iframe'); | ||
| iframe.style.cssText = 'display: none'; | ||
| iframe.src = ''; // iframeSrc; |
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.
What is this line/comment for?
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.
Since I have not written this part I am not fully sure. What it does is set the src to the same document as the parent page. This does not look like what we want. @jcscottiii can we try what happens if we just remove it?
| var iframe = global.document.createElement('iframe'); | ||
| iframe.style.cssText = 'display: none'; | ||
| iframe.src = ''; // iframeSrc; | ||
| if (global.document.body === null) { |
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.
This feels potentially surprising, because it means that something like
<html>
<script>
let realm = $262.createRealm();
</script>
<meta name="example" content="test">
<body>
[…]
is going to end up with the <meta> element in the . Do we actually need the capacity to handle both the cases where there is and isn't already a ` here?
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.
We just need a place to insert the iframes. If you ensure all templates have a body that should be fine too.
| var script = global.document.createElement('script'); | ||
| script.text = src; | ||
| window.__test262_evalScript_error_ = undefined; | ||
| global.document.head.appendChild(script); |
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.
Is there a specific reason it has to go in the head? It's not totally clear what invariants we're trying to maintain about the DOM.
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.
I assume it could go anywhere in the document.
| } | ||
| }, | ||
| detachArrayBuffer: function(buffer) { | ||
| if (typeof postMessage !== 'function') { |
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.
When can this actually happen, and is it worth having a specific error message for?
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.
This happens if the environment does not support workers. I guess on the web that should never happen (?), so we can probably remove that case.
| AbstractModuleSource: function() { | ||
| throw new Error('AbstractModuleSource not available'); | ||
| }, | ||
| agent: (function () { |
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.
Would be nice to have some documentation of the high level protocol being implemented, or at least a link to something in test262 which explains it.
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.
Good point. The documentation is at: https://chromium.googlesource.com/external/github.com/tc39/test262/+/HEAD/INTERPRETING.md
| @@ -0,0 +1,77 @@ | |||
| /* | |||
| * Minimalistic testharness-client tailored to run Test262 | |||
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.
Since -client isn't an existing concept or convention, we shouldn't assume it's obvious what it actually is.
| }; | ||
|
|
||
| function log(msg) { | ||
| document.getElementById('log').innerHTML += ""+msg+"<br>"; |
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.
In normal testharness we turn this off in CI for performance reasons. I think we want to do the same here.
| } | ||
|
|
||
| function done() { | ||
| if (test_finished) { return; } |
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.
nit: split across multiple lines, please
This commit completes the core integration of Test262 into the WPT runner (
wptrunner), enabling the execution of Test262 JavaScript conformance tests within the browser environment. This work builds upon the manifest tooling (PR 1 #56840) and server-side handling (PR 2 #56841) to provide a full end-to-end testing solution for Test262.Key components introduced and integrated:
harness-adapter.js,testharness-client.js,testharness.js) underresources/test262/provide the necessary environment for Test262 tests to execute in aniframeand report results back to the main WPT testharness.assert.jsandsta.js) are directly placed intothird_party/test262/harness/. This provides the immediate dependencies required by the smoke tests. An autoamted vendoring solution, including version tracking viavendored.toml, will be implemented in a subsequent PR focused on automated updates, as described in the RFC.tools/wptrunner/wpttest.pyandtools/wptrunner/browsers/*.pyenablewptrunnerto recognize thetest262test type and execute it using existing testharness executors.infrastructure/test262/) and their corresponding expected results (infrastructure/metadata/infrastructure/test262/) are added to verify the correct functioning of the entire integration.lint.ignoreis updated to exclude the vendored Test262 harness files from linting, respecting their upstream style.This integration allows browser vendors to run Test262 directly with
wptrunner, streamlining conformance testing efforts as specified in the RFC: web-platform-tests/rfcs#229This commit is the third in a series of smaller PRs split from the larger, original implementation in #55997.