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

Add core WebAssembly tests to WPT #49277

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

Add core WebAssembly tests to WPT #49277

wants to merge 2 commits into from

Conversation

past
Copy link
Member

@past past commented Nov 19, 2024

The Interop 2024 WebAssembly Testing Investigation intends to get WebAssembly core tests running on WPT.fyi (alongside the existing JS API and Web API tests).

This PR adds a GitHub Actions workflow that pulls the latest tests from the WebAssembly/spec repo on a regular schedule (or when run manually) and creates a PR with any new changes.

This mimics the existing process for updating WebIDL tests, which is low risk and low friction. Creating a PR that needs to be reviewed removes the risk of the workflow messing up the wpt repo at the cost of needing manual review of the changes (which is mostly rubber-stamping as in the WebIDL case). I also considered a push model from the spec repo, which could reduce CPU costs and achieve lower latency to propagate changes, but with a substantially more complex solution.

My WPT repo fork has some successful runs that you can look at:

  • The merged PRs that show up in the repo at wasm/core
  • A testing PR to verify that the action handles additions, removals and modifications.
  • A test run to ensure the scheduled action succeeds even when no changes exist in WebAssembly/spec

This should match the MVP that the investigation is pursuing and could subsequently be expanded to cover tentative tests from WebAssembly/testsuite or elsewhere. This should let the investigation get to a 67% score in the Interop dashboard.

/CC @jgraham, @gsnedders

Copy link

@dschuff dschuff left a comment

Choose a reason for hiding this comment

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

spelling nits.

.github/workflows/update-wasm-tests.yml Outdated Show resolved Hide resolved
.github/workflows/update-wasm-tests.yml Outdated Show resolved Hide resolved
.github/workflows/update-wasm-tests.yml Outdated Show resolved Hide resolved
.github/workflows/update-wasm-tests.yml Outdated Show resolved Hide resolved
.github/workflows/update-wasm-tests.yml Outdated Show resolved Hide resolved
rm -rf main/wasm/core
cp -r wasm-spec/out/ main/wasm/core/
find main/wasm/core/ -type f -name '*.html' -exec sed -i 's/\.\/js\/harness\/testharness/\/resources\/testharness/' {} \;
- name: Commit changes
Copy link

Choose a reason for hiding this comment

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

If you use the peter-evans/create-pull-request action as the WebIDL workflow does, you don't need to manually commit the changes or do anything with git itself.

Copy link

Choose a reason for hiding this comment

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

Another example is what we do in emscripten to create a PR, request a review from a github team, and set the PR to auto-merge (after approval and passing tests)

Copy link
Member Author

Choose a reason for hiding this comment

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

We do something similar in the wpt-metadata repo, but I'm always wary of using 3rd-party libraries for security reasons. However, I see the author of this action is now working for GitHub, so maybe this is less of a concern.

Copy link

Choose a reason for hiding this comment

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

Yeah, that makes sense. I'm fine either way, up to you.

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.

4 participants