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

ShadowRealm Web Platform integration testing and expectations #386

Closed
mgaudet opened this issue Apr 14, 2023 · 9 comments
Closed

ShadowRealm Web Platform integration testing and expectations #386

mgaudet opened this issue Apr 14, 2023 · 9 comments
Assignees

Comments

@mgaudet
Copy link

mgaudet commented Apr 14, 2023

So, in #331 a variety of interfaces were exposed within ShadowRealms. However, based on my spelunking through the web-platform tests, what has been tested thus far is simply the shadow realm IDL Harness tests, which merely tests that the interfaces are of the correct shape, not that they work sensibly (for whatever value of sensibly is desired).

I'm wondering how these things will be tested properly? For example, I discovered very quickly a crash from new ShadowReam().evaluate('new PerformanceMark("hey").startTime') [1], but there's no WPT that actually checks things like does the startTime actually get set properly; do two sequential marks (separated by a sufficient temporal distance) have distinct start times etc.

My understanding of the state of ShadowRealms is that we don't want to ship it until we have the HTML integration done; both making sure the actual spec integration is correct and the exposure of interfaces is correct; but I don't think I want to push much further on the exposure work without making sure the testing makes sense; I had actually started my work under a misunderstanding of how the SR testing works already.

[1]: Note: the performance features had their IDL exposure changes reverted due to a lack of implementation; I implemented to explore the challenges.
[2]: Side note: Exposing PerformanceMark feels weird without the ability to actually hook it up to the performance timeline, which currently AIUI is impossible in a shadow realm.

@caridy
Copy link
Collaborator

caridy commented Aug 26, 2023

cc @rwaldron

@rwaldron
Copy link
Collaborator

We've now landed 2 of several PRs that are enabling functional testing of the [Exposed=*] APIs. More are awaiting review, and still more are in progress.

@rwaldron
Copy link
Collaborator

@mgaudet Here are the ShadowRealm enabled tests in WPT as of now: https://github.com/web-platform-tests/wpt/pulls?q=is%3Apr+author%3Arwaldron+shadowrealm+is%3Aclosed

These are pending HTML spec update: https://github.com/web-platform-tests/wpt/pulls?q=is%3Aopen+is%3Apr+author%3Arwaldron+shadowrealm

The tests that are currently in WPT run with varying support across Chrome Canary, Firefox Nightly and Safari Technology Preview.


For Chrome Canary:

/Applications/Google\ Chrome\ Canary.app/Contents/MacOS/Google\ Chrome\ Canary --js-flags="--harmony-shadow-realm"

For Firefox Nightly:

Open "about:config", type:

javascript.options.experimental.shadow_realms

Toggle to true

For Safari Technology Preview:

env JSC_useShadowRealm=1 __XPC_JSC_useShadowRealm=1 /Applications/Safari\ Technology\ Preview.app/Contents/MacOS/Safari\ Technology\ Preview 

@mgaudet
Copy link
Author

mgaudet commented Nov 20, 2023

Hey @rwaldron , apologies for the long feedback delay.

So, running the tests as ./mach wpt --headless /test/path/ --setpref javascript.options.experimental.shadow_realms=true

I am noticing a lot of test failures related to undefined references. For example in the wasm tests, lots of failures of the shape assert_Table is not defined,

There's also some tests (most of the Streams tests) that for example want setTimeout, but that's currently defined as a part of the WindowOrWorkerGlobalScope mixin, which currently the ShadowRealmGlobalScope doesn't include.

I can file more bugs in the appropriate repos, but I'm not sure where would be best.

On the good news side, we're definitely getting some UNEXPECTED-OK results, which is nice.

@mgaudet
Copy link
Author

mgaudet commented Nov 20, 2023

(Oh, I see the global object definition is now in flux at whatwg/html#9893)

@mgaudet
Copy link
Author

mgaudet commented Nov 24, 2023

(In case anyone wants to see current results from Firefox, with some speculative enablement, the job results from CI are here)

@rwaldron
Copy link
Collaborator

@mgaudet I don't have any comment on whether the tests pass or not. My only goal was to ensure that the APIs that are likely to be exposed in shadowrealm are being tested in shadowrealm. It's up to implementers to run those tests and make sure their implementations pass.

@mgaudet
Copy link
Author

mgaudet commented Jan 22, 2024

Sure -- my feedback was more about failures which seemed like test integration problems (e.g. the missing assert_Table reference) or depended on undetermined elements.

@ptomato
Copy link
Collaborator

ptomato commented Nov 11, 2024

I believe I fixed the missing assert_Table errors in web-platform-tests/wpt#48687, and the setTimeout dependency in web-platform-tests/wpt#43639. I think your expectations for functional testing of each API match mine. We are tracking the progress of functional tests in #393. So I think this can be closed, feel free to reopen if I missed something!

@ptomato ptomato closed this as completed Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants