Refactor how wasm features are calculated for *.wast tests
#9560
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit refactors the `tests/wast.rs` test suite which runs all of the upstream spec tests as `*.wast` files as well as our own `misc_testsuite` which has its own suite of `*.wast` files. Previously the set of wasm features active for each test was a sort of random mishmash and convoluted set of conditionals which was updated and edited over time as upstream proposal test suites evolved. This was then mirrored into our own conventions for `misc_testsuite` as well. Overall though this has a number of downsides I'm trying to fix here: * The calculation of what features are enabled is quite complicated and effectively a random mishmash of `||` conditionals with hierarchies that don't make any sense beyond "this is just required to get things to pass". * There is no means of per-test configuration. For example `canonicalize-nans.wast` had hardcoded logic in `tests/wast.rs` that it needed a different setting turned on in `Config`. * There was no easy means to write tests for Wasmtime which take a union of a number of proposals together without having lots of sub-folders that may not make sense. * Tests that require a particular proposal had to have duplicate logic for Winch as it doesn't support the full suite of features of all proposals that Cranelift does. The new system implemented in this commit takes a leaf out of the `disas` tests. There is a new `TestConfig` structure in the `tests/wast.rs` harness which is decoded from each test (leading `;;!` comments) which enables specifying, in each test, what's required. This encompasses many wasm proposals but additionally captures other behavior like nan-canonicalization. This means that all test files in `misc_testsuite/**/*.wast` are now manually annotated with what wasm features they require and what's needed to run. This makes per-test configuration much easier, per-config-setting much easier, and blanket ignore-by-proposal for Winch much easier as well. For spec tests we can't modify the contents of the upstream `*.wast` files. To handle this they're handled specially where `TestConfig` is manually created and manipulated for each spec proposal and the main test suite itself. This enables per-proposal configuration that doesn't leak into any others and makes it more obvious what proposals are doing what.
alexcrichton
requested review from
pchickey and
fitzgen
and removed request for
a team
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit refactors the
tests/wast.rstest suite which runs all of the upstream spec tests as*.wastfiles as well as our ownmisc_testsuitewhich has its own suite of*.wastfiles. Previously the set of wasm features active for each test was a sort of random mishmash and convoluted set of conditionals which was updated and edited over time as upstream proposal test suites evolved. This was then mirrored into our own conventions formisc_testsuiteas well. Overall though this has a number of downsides I'm trying to fix here:The calculation of what features are enabled is quite complicated and effectively a random mishmash of
||conditionals with hierarchies that don't make any sense beyond "this is just required to get things to pass".There is no means of per-test configuration. For example
canonicalize-nans.wasthad hardcoded logic intests/wast.rsthat it needed a different setting turned on inConfig.There was no easy means to write tests for Wasmtime which take a union of a number of proposals together without having lots of sub-folders that may not make sense.
Tests that require a particular proposal had to have duplicate logic for Winch as it doesn't support the full suite of features of all proposals that Cranelift does.
The new system implemented in this commit takes a leaf out of the
disastests. There is a newTestConfigstructure in thetests/wast.rsharness which is decoded from each test (leading;;!comments) which enables specifying, in each test, what's required. This encompasses many wasm proposals but additionally captures other behavior like nan-canonicalization. This means that all test files inmisc_testsuite/**/*.wastare now manually annotated with what wasm features they require and what's needed to run. This makes per-test configuration much easier, per-config-setting much easier, and blanket ignore-by-proposal for Winch much easier as well.For spec tests we can't modify the contents of the upstream
*.wastfiles. To handle this they're handled specially whereTestConfigis manually created and manipulated for each spec proposal and the main test suite itself. This enables per-proposal configuration that doesn't leak into any others and makes it more obvious what proposals are doing what.