alexcrichton opened PR #7182 from alexcrichton:refactor-test-programs
to bytecodealliance:main
:
This commit is a large refactoring that reorganizes
test-programs
and how we tests wasms in Wasmtime. Often writing tests requires complicated interactions with the guest which can't be done via hand-written*.wat
syntax and requires a compiler to get engaged. For this purpose Wasmtime currently has thecrates/test-programs/*
test suite which builds files from source and then runs the tests. This has been somewhat cumbersome in the past though and it's not been easy to extend this over time, so this commit attempts to address this.The scheme implemented in this PR looks like:
All wasm test programs live in
crates/test-programs/src/bin/*.rs
. All of them, no exceptions.Wasm tests have shared support located at
crates/test-programs/src/lib.rs
and its submodules, such as bindings generation for WASI.Wasm tests are built by a new
crates/test-programs/artifacts
crate. This crate compiles modules and additionally creates components for all test programs. The crate itself only records the path to these outputs and a small amount of testing support, but otherwise doesn't interact withwasmtime
-the-crate itself.All tests in
crates/test-programs/tests/*.rs
have moved. For example wasi-http tests now live atcrates/wasi-http/tests/*.rs
. Legacy tests of wasi-common now live atcrates/wasi-common/tests/*.rs
. Modern tests for preview2 live atcrates/wasi/tests/*.rs
.Wasm tests are bucketed based on their filename prefix. For example
preview1_*
is tested in wasi-common and wasmtime-wasi. Thepreview2_*
prefix is only tested with wasmtime-wasi, however.A new
cli_*
prefix is used to execute tests as part oftests/all/main.rs
. This is a new submodule intests/all/cli_tests.rs
which executes these components on the command line. Many old "command" tests were migrated here.Helper macros are generated to assert that a test suite is run in its entirety. This way if a
preview1_*
test is added it's asserted to get added to both wasi-common and wasmtime-wasi in the various modes they run tests.Overall this moved a number of tests around and refactored some edges of the tests, but this should not lose any tests (except one that wasn't actually testing anything). Additionally the hope is that it's much easier to add tests in the future. The process is to add a new file in
crates/test-programs/src/bin/*.rs
named appropriately. For example a preview2 executable ispreview2_*
and a CLI tests iscli_*
. When building the test suite an error is generated in the appropriate module then of "please write a test here", and then a test is written in the same manner as the other tests in the module.<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
alexcrichton requested fitzgen for a review on PR #7182.
alexcrichton requested wasmtime-default-reviewers for a review on PR #7182.
alexcrichton requested wasmtime-core-reviewers for a review on PR #7182.
alexcrichton updated PR #7182.
alexcrichton updated PR #7182.
alexcrichton updated PR #7182.
alexcrichton updated PR #7182.
alexcrichton updated PR #7182.
alexcrichton updated PR #7182.
alexcrichton updated PR #7182.
alexcrichton updated PR #7182.
alexcrichton submitted PR review.
alexcrichton submitted PR review.
alexcrichton created PR review comment:
This is a bug somewhere and I'm not sure what, but my guess is the
clap
processing of arguments or something like that. I need to bottom this out but it's unrelated to this PR in that this PR doesn't introduce a new failure mode, it only exposes an existing one.
alexcrichton updated PR #7182.
alexcrichton updated PR #7182.
alexcrichton submitted PR review.
alexcrichton created PR review comment:
Ok I have now since fixed this. The underlying issue was present before this PR but it's only exposed in this PR so I lumped th efix in here. The problem was that
clap
's parsing ofPathBuf
requires the value to be non-empty and we were modeling all of the arguments to wasm asVec<PathBuf>
. I've switched toVec<OsString>
to enable passing through empty arguments.
alexcrichton requested elliottt for a review on PR #7182.
elliottt submitted PR review:
This is awesome, thank you!
elliottt submitted PR review:
This is awesome, thank you!
elliottt created PR review comment:
Why do we skip the first argument now?
elliottt created PR review comment:
I copied over the same comment from the reactor tests.
// Technically this should not be here for a proxy, but given the current // framework for tests it's required since this file is built as a `bin` fn main() {}
alexcrichton updated PR #7182.
alexcrichton submitted PR review.
alexcrichton created PR review comment:
Previously this test was executed through the embedder API rather than the CLI. In that situation we had precise control over the arguments and on the CLI the input executable is implicitly the first argument. This is skipped now due to the runner of the test being moved to a different location basically.
alexcrichton has enabled auto merge for PR #7182.
alexcrichton merged PR #7182.
Last updated: Dec 23 2024 at 12:05 UTC