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.

Sign up for GitHub

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

Jump to bottom

Enable shadowrealm testing for wasm api (tests with dependencies) #41987

Merged
merged 1 commit into from
Sep 15, 2023
Merged

Enable shadowrealm testing for wasm api (tests with dependencies) #41987

merged 1 commit into from
Sep 15, 2023

Conversation

rwaldron
Copy link
Contributor

Supports tests that have helper dependencies. Exposes "expose" in testharness.js to enable exposing wasm-local helper functions

@rwaldron rwaldron requested a review from a team as a code owner September 14, 2023 20:24
rwaldron
rwaldron commented Sep 14, 2023
View reviewed changes
resources/testharness.js Outdated
@@ -4710,6 +4710,7 @@
}
target[components[components.length - 1]] = object;
}
expose(expose, 'expose');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Enables use in other helpers to "expose" testing functions in appropriate global scopes.

@Ms2ger
Copy link
Contributor

Ms2ger commented Sep 15, 2023

I'd assign to globalScope.foo explicitly rather than exposing (hah) expose. However, I don't understand why we need to do that explicitly at all. Is there a difference in how the helper files are imported?

@rwaldron
Copy link
Contributor Author

I'd assign to globalScope.foo explicitly rather than exposing (hah) expose. However, I don't understand why we need to do that explicitly at all. Is there a difference in how the helper files are imported?

Yes, globalScope.foo would work here as well. The helpers are being imported eg await import("/resources/testharness.js").

@rwaldron
Enable shadowrealm testing for wasm api (tests with dependencies)
f722ac9 
Supports tests that have helper dependencies. Exposes wasm-local helper functions
@rwaldron rwaldron force-pushed the rwaldron/shadowrealm-wasm-with-deps-tests branch from 9a6baa0 to f722ac9 Compare September 15, 2023 13:01
@rwaldron
Copy link
Contributor Author

@Ms2ger thanks for the review! Changes made as requested

Ms2ger
Ms2ger approved these changes Sep 15, 2023
View reviewed changes
Copy link
Contributor

@Ms2ger Ms2ger left a comment

Choose a reason for hiding this comment

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

Thanks! I didn't double-check the tests, but anything that works in a js shell should surely be safe

@Ms2ger Ms2ger enabled auto-merge (rebase) September 15, 2023 13:20
@Ms2ger Ms2ger merged commit e0c0429 into web-platform-tests:master Sep 15, 2023
@rwaldron rwaldron mentioned this pull request Jan 22, 2024
Open
49 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ms2ger Ms2ger Ms2ger approved these changes

@jgraham jgraham Awaiting requested review from jgraham

@titzer titzer Awaiting requested review from titzer

Assignees

@jgraham jgraham

Labels
infra testharness.js wasm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rwaldron @Ms2ger @jgraham @wpt-pr-bot