Skip to content

Initial migration to wasmtime_test #8789

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

Conversation

saulecabrera
Copy link
Member

This commit introduces the initial migration to the wasmtime_test macro.

This change starts by migrating all the applicable func.rs integration tests and at the same time it removes all the duplicated integration tests in winch.rs.

Additionally, this change introduces a slight change to how the macro works. Inspired by #8622 (review) it defaults to including all the known configuration combinations and allows the macro user to opt-out where applicable. This makes the usage of the macro less verbose.

The intention is to follow-up with subsequent PRs to migrate the all the applicable tests.

This commit introduces the initial migration to the `wasmtime_test`
macro.

This change starts by migrating all the applicable `func.rs` integration
tests and at the same time it removes all the duplicated integration
tests in `winch.rs`.

Additionally, this change introduces a slight change to how the macro
works. Inspired by bytecodealliance#8622 (review)
it defaults to including all the known configuration combinations and
allows the macro user to opt-out where applicable. This makes the usage
of the macro less verbose.

The intention is to follow-up with subsequent PRs to migrate the all the
applicable tests.
@saulecabrera saulecabrera requested a review from a team as a code owner June 12, 2024 22:22
@saulecabrera saulecabrera requested review from alexcrichton and removed request for a team June 12, 2024 22:22
This commit adds the ability to specify `wasm_features` when invoking
the macro.

If the feature is off by default, the macro will ensure that the feature
is enabled in the resulting config.

If the feature is not supported by any of the compiler strategies, no
tests will be generated for such strategy.
@saulecabrera
Copy link
Member Author

Thanks for the review @alexcrichton. Your suggestions make sense to me, I've pushed an update in 9638a94. Let me know what you think. I also removed some redundant config.wasm_<feature> for features that are now enabled by default (e.g. reference_types).

Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

Looks great, and thanks!

@alexcrichton alexcrichton enabled auto-merge June 13, 2024 20:55
@alexcrichton alexcrichton added this pull request to the merge queue Jun 13, 2024
Merged via the queue into bytecodealliance:main with commit 04416e4 Jun 13, 2024
36 checks passed
@saulecabrera saulecabrera deleted the start-migration-to-wasmtime-test branch June 13, 2024 22:16
saulecabrera added a commit to saulecabrera/wasmtime that referenced this pull request Jun 13, 2024
Follow-up to: bytecodealliance#8789;
fixes the validation error message.
github-merge-queue bot pushed a commit that referenced this pull request Jun 13, 2024
Follow-up to: #8789;
fixes the validation error message.
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

Successfully merging this pull request may close these issues.

2 participants