Skip to content

Conversation

@jjgriego
Copy link
Contributor

This commit adds support to run any.js tests in Shadow
Realm (https://github.com/tc39/proposal-shadowrealm) contexts--

(As an alternative to #32956 ;
this offers more robust support for reporting promise rejections and timeouts

If desired, I will open an RFC to this effect; the text would be substantially the same
as this commit message, though)

Shadow realms are a new sandboxing primitive, currently a stage 3 proposal in
TC39 and in the process of being integrated into relevant Web specifications;
part of this extends some pure/non-rendering/purely computational Web interfaces to shadow
realms (which otherwise do not have browser APIs and more closely resemble a
vanilla JS shell environment.) I think it makes sense to extend WPT tests for
these interfaces to also run in shadow realm contexts, so we can be assured they
still work there.


To accomplish this, I add a new ShadowRealmTestEnvironment to testharness.js
that reuses the existing machinery to run tests remotely in e.g. dedicated
workers. There are however, two main differences:

  • Shadow realms are not DOM contexts and so have no onload or similar event
    for us to use to actually begin test aggregation; instead, I add a new hook for
    the incubating realm to directly call to signal that all desired tests have been
    loaded.

  • The actual message port used to communicate test results back to a
    RemoteContext requires JSON-serialization of the mesasges (primitive values
    and callables may cross between a shadow- and incubating realm, but not
    arbitrary objects): it happens that this works correctly given the current
    encodings of Test, harness state, etc. are JSON-round-trippable, but this would
    become a hard requirement from this point forward.


There's also one loose end at present: it's unclear (to me) given the current
state of the way Shadow Realms are integrated into the HTML spec how we might be
able to deal with unhandled promise rejections. It happens, for now, that they
will be handed by the incubating Window's unhandledrejection handler, which
is installed by the parent test harness, and prints to the console. This seems
acceptable for the time being, but may need to change as the spec is clarified.

This commit adds support to run `any.js` tests in Shadow
Realm (https://github.com/tc39/proposal-shadowrealm) contexts--

Shadow realms are a new sandboxing primitive, currently a state 3 proposal in
TC39 and in the process of being integrated into relevant Web specifications;
part of this extends some computational-only Web interfaces to shadow
realms (which otherwise do not have browser APIs and more closely resemble a
vanilla JS shell environment.) I think it makes sense to extend WPT tests for
these interfaces to also run in shadow realm contexts, so we can be assured they
still work there.

---

To accomplish this, I add a new `ShadowRealmTestEnvironment` to `testharness.js`
that reuses the existing machinery to run tests remotely in e.g. dedicated
workers. There are however, two main differences:

- Shadow realms are not DOM contexts and so have no `onload` or similar event
for us to use to actually begin test aggregation; instead, I add a new hook for
the incubating realm to directly call to signal that all desired tests have been
loaded.

- The actual message port used to communicate test results back to a
`RemoteContext` requires JSON-serialization of the mesasges (primitive values
and callables may cross between a shadow- and incubating realm, but not
arbitrary objects): it *happens* that this works correctly given the current
encodings of Test, harness state, etc. are JSON-round-trippable, but this would
become a hard requirement from this point forward.

---

There's also one loose end at present: it's unclear (to me) given the current
state of the way Shadow Realms are integrated into the HTML spec how we might be
able to deal with unhandled promise rejections. It happens, for now, that they
will be handed by the incubating `Window`'s `unhandledrejection` handler, which
is installed by the parent test harness, and prints to the console. This seems
acceptable for the time being, but may need to change as the spec is clarified.
@jjgriego
Copy link
Contributor Author

I opened web-platform-tests/rfcs#107 for discussion on this, if there's concerns.

*/
function fetch_tests_from_shadow_realm(realm) {
var chan = new MessageChannel();
function recieveMessage(msg_json) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
function recieveMessage(msg_json) {
function receiveMessage(msg_json) {

chan.port1.postMessage(JSON.parse(msg_json));
}
var done = tests.fetch_tests_from_worker(chan.port2);
realm.evaluate(`begin_shadow_realm_tests`)(recieveMessage);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
realm.evaluate(`begin_shadow_realm_tests`)(recieveMessage);
realm.evaluate("begin_shadow_realm_tests")(receiveMessage);

Output.prototype.show_status = function() {
if (this.phase < this.STARTED) {
this.init();
this.init({});
Copy link
Contributor

Choose a reason for hiding this comment

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

What's this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Output.prototype.init expects a properties argument that it unconditionally checks--

.... maybe we should instead be ensuring that the output object gets manually initialized, but this line raises an unconditional error without the change

(async () => {
await import("/resources/testharness.js");
%(script)s
globalThis.self.GLOBAL = {
Copy link
Contributor

Choose a reason for hiding this comment

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

self is not formally defined here, is it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not, but I kept this after the previous discussion on the earlier version of the patch

@jjgriego
Copy link
Contributor Author

jjgriego commented Apr 1, 2022

(gentle bump) as far as i know this could be landed but I don't have a commit bit

@caitp caitp merged commit aac2d58 into web-platform-tests:master Apr 4, 2022
@lucacasonato
Copy link
Member

lucacasonato commented Apr 18, 2022

Sorry I am reporting this so late, but this broke WPT execution in Deno. It is likely the 'AbortController' in global_scope test that is bad.

Comment on lines +543 to +544
* As long as, within the shadow realm, we load the testharness before
* other libraries, this won't have any false positives, even in e.g. node
Copy link
Member

Choose a reason for hiding this comment

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

This is unfortunately not the case. This has a false positive in Deno.

@jjgriego
Copy link
Contributor Author

Oof, sorry about that. I'll work on making that check more robust (we should probably positively signal that the test environment is a shadow realm)--in the meantime, nothing needs this yet in trunk--we can revert or stub out the bad branch in create_test_environment; not sure what maintainers would prefer

@lucacasonato
Copy link
Member

@jjgriego Already have a fix up here: #33668. Let's discuss there :-)

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.

6 participants