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

Refactor/rewrite this repository's test suite #1161

Open
alexcrichton opened this issue Feb 10, 2025 · 11 comments
Open

Refactor/rewrite this repository's test suite #1161

alexcrichton opened this issue Feb 10, 2025 · 11 comments

Comments

@alexcrichton
Copy link
Member

I've personally long lamented tests in this repository. Currently tests fall into roughly three categories:

  • codegen tests (Rust) - Rust codegen tests use some macro-magic to run a generate! macro invocation with various options over all the tests in tests/codegen/*.wit. This then compiles the output for the native host (not wasm) and then nothing actually happens at runtime as nothing is actually run, things are only generated.
  • codegen tests (not Rust) - other languages typically have a #[test]-per-generate! also using some macro-magic. Each test typically spawns some external process and ensures it passes, for example a C compiler is spawned for the wit-bindgen-c crate.
  • runtime tests - In tests/runtime/* there are a number of test that are organized as #[test]. The idea is that there's one monolithic host runner based on a custom Wasmtime embedding. Each runtime test can be implemented in an array of languages and each language gets run. Compilation happens in an external process and, like before, compilation of Rust binaries is "special" compared to other languages.

There are a lot of downsides to this, including:

  • Rust is "special" and doesn't look like other languages in testing, that's not great.
  • Compilation of runtime tests takes forever because you have to compile all of Wasmtime and its embedding.
  • Iteration on the runtime tests take forever because it takes awhile to compile.
  • It's not easy to incrementally bring a language online as it generally needs to support everything or it's not tested at all.
  • This repository is very close to a "reference test suite for WIT" but stops just shy of being something that's actually upstreamable anywhere.
  • There's no way to benefit from these "runtime" tests in any other component runtime right now.

Overall I would like to move in a new direction that solves all of these problems with a new foundation for running tests. Specifically what I'm imagining is something like this:

  1. All tests are organized into two components: a "runner" component and a "test" component. The runner imports the test.
  2. The runner component exports the wasi:cli/run interface and looks like a command line executable. In other words it's a "command" in terms of WASI.
  3. The test component looks like a "reactor" in terms of WASI.
  4. Both runner and test can import WASI functions, for example to print to the screen, read environment variables, etc.
  5. Tests are then created by composing the runner with the test, producing a final component that's a "command" and only imports WASI.
  6. Tests are then executed with the wasmtime CLI, critically not the embedding API.

This scheme solves many of the above concerns (except maybe the "Rust is special" part through perhaps infrastructure refactoring). Compilation of tests should be much quicker as it's just one test at a time and each test case compiles quickly. Furthermore any host can be relatively easily slotted in to reuse this test suite by swapping out the wasmtime CLI for something else. This additionally avoids the need to define wonky host APIs that are only visible in the embedding, instead it's a pair of runner and test components that work with each other. No loss of test coverage should happen because imported functions are tested by implementing a runner and exported functions are tested by being a test. Both the runner and the test could be implemented in any language which exercises various interesting cross-products of language pairs.

This refactoring should also ideally put this on a path towards making it more reasonable to be a sort of "reference test suite" because standalone wasm binaries are produced from source which can be executed anywhere on a WASI runtime. That should make it much easier to, for example, in the future publish these as a separate test suite somewhere like the wasi-testsuite repository.

@Mossaka
Copy link
Member

Mossaka commented Feb 10, 2025

Thanks for the write up and taking the initiative. Composing runner and test components totally makes sense to me and I think this would be great for https://github.com/bytecodealliance/go-modules as well since it's currently lacking the runtime tests from this repo.

alexcrichton added a commit to alexcrichton/witx-bindgen that referenced this issue Mar 5, 2025
As I've read more and more of the Rust async runtime support and other
various bits and pieces I've wanted more and more the ability to easily
write tests for guest interactions with the host. While I don't think
it's feasible to generate arbitrary hosts I do think it's possible to do
this much more easily than is done today with the testing support in
this repository. In essence this commit is an implementation of bytecodealliance#1161.

The goal of this commit is to add a `wit-bindgen test` test suite
runner. This test suite will be used to migrate all existing tests in
this repository to this new framework. In the limit this is expected to
make it easier to write tests (no Rust knowledge necessary), make it
more flexible to write tests (now you can use raw `*.wat`), and
additionally improve the quality of the test suite by making it more
reusable. The reusability isn't the highest priority at this time as
it's not clear what else would want to reuse this, but my hope is that
this refactoring is at least a large-ish leap forward towards having a
component model test suite of some kind eventually.
@alexcrichton
Copy link
Member Author

#1192 has some initial work towards this

@ydnar
Copy link

ydnar commented Mar 6, 2025

Hooray, thanks Alex!

Would the test suite be broken out into its own repo that we can submodule into https://github.com/bytecodealliance/go-modules or live here in this repo?

@alexcrichton
Copy link
Member Author

Heh I honestly hadn't thought that far forward really, so for now my plan was to leave the test suite here. One thing this makes me think of though is the possible use of "git subtrees" as a way of vendoring the test suite. That might make it easy to make local edits (e.g. annotate tests as "this is expected to fail" or something like that). Nevertheless I suspect there's still going to be integration problems for repositories outside this location (e.g. go-modules), but I'd consider those integration pain points as things to fix for sure.

@ydnar
Copy link

ydnar commented Mar 7, 2025

We can just as easily submodule this entire repo and harness tests in a subdirectory.

alexcrichton added a commit to alexcrichton/witx-bindgen that referenced this issue Mar 7, 2025
As I've read more and more of the Rust async runtime support and other
various bits and pieces I've wanted more and more the ability to easily
write tests for guest interactions with the host. While I don't think
it's feasible to generate arbitrary hosts I do think it's possible to do
this much more easily than is done today with the testing support in
this repository. In essence this commit is an implementation of bytecodealliance#1161.

The goal of this commit is to add a `wit-bindgen test` test suite
runner. This test suite will be used to migrate all existing tests in
this repository to this new framework. In the limit this is expected to
make it easier to write tests (no Rust knowledge necessary), make it
more flexible to write tests (now you can use raw `*.wat`), and
additionally improve the quality of the test suite by making it more
reusable. The reusability isn't the highest priority at this time as
it's not clear what else would want to reuse this, but my hope is that
this refactoring is at least a large-ish leap forward towards having a
component model test suite of some kind eventually.
alexcrichton added a commit to alexcrichton/witx-bindgen that referenced this issue Mar 7, 2025
As I've read more and more of the Rust async runtime support and other
various bits and pieces I've wanted more and more the ability to easily
write tests for guest interactions with the host. While I don't think
it's feasible to generate arbitrary hosts I do think it's possible to do
this much more easily than is done today with the testing support in
this repository. In essence this commit is an implementation of bytecodealliance#1161.

The goal of this commit is to add a `wit-bindgen test` test suite
runner. This test suite will be used to migrate all existing tests in
this repository to this new framework. In the limit this is expected to
make it easier to write tests (no Rust knowledge necessary), make it
more flexible to write tests (now you can use raw `*.wat`), and
additionally improve the quality of the test suite by making it more
reusable. The reusability isn't the highest priority at this time as
it's not clear what else would want to reuse this, but my hope is that
this refactoring is at least a large-ish leap forward towards having a
component model test suite of some kind eventually.
@alexcrichton
Copy link
Member Author

I thinking more about #1192 I'm starting to wonder about next steps moving forward. I'd like to sunset tests/runtime/* entirely in the near future ideally. There's a few holdouts though:

  • teavm-java - there's a few (but not many) *.java tests throughout the runtime tests. These are pretty far behind though. @dicej any objections to basically just disabling all these tests? I'd move all the sources to somewhere within crates/teavm-java/* with a README.md pointing at the last working commit though.
  • go - I assume this will be taken care of in Go: Remove Go from wit-bindgen #1195 by removing the tests
  • c-sharp - I personally can't run the tests since I can't easily develop on Windows and the tests only run on Windows. I'm not opposed to porting things over but I would need some help. @jsturtevant or @silesmo would y'all be ok if I disable all the tests and could you migrate them when you get a chance?

That would only leave the grunt work of actually porting the *.rs and *.c files over, plus the work to set aside temporarily disabled tests to get re-enabled in the future. I'm happpy to take that grunt work myself though.

@dicej
Copy link
Collaborator

dicej commented Mar 8, 2025

teavm-java - there's a few (but not many) *.java tests throughout the runtime tests. These are pretty far behind though. @dicej any objections to basically just disabling all these tests? I'd move all the sources to somewhere within crates/teavm-java/* with a README.md pointing at the last working commit though.

I'd be fine with removing the teavm-java code altogether given that nobody is maintaining it.

c-sharp - I personally can't run the tests since I can't easily develop on Windows and the tests only run on Windows.

You can run them on Mac and Linux with some effort and hacks, and I think there are CI-generated Linux builds of the NativeAOT-LLVM packages available now, so I think we could make that work out-of-the box with a bit of effort.

@jsturtevant
Copy link
Collaborator

For c#, It shouldn't be to much effort to enable running the tests for Linux. Mac would require some more work upstream in the runtimelabs repository.

I can see if I can get it running quickly on Linux in the current setup, but if you need to move forward with this I will help migrate the runtime tests as a follow up.

@jsturtevant
Copy link
Collaborator

Just took a look and the runtime tests already pass on Linux with cargo test -p wit-bindgen-cli --no-default-features -F csharp, there is a CI job that does this for example: https://github.com/bytecodealliance/wit-bindgen/actions/runs/13769906018/job/38505676495

@alexcrichton
Copy link
Member Author

I think that "passes" due to the cfg!(windows) check here. Locally what I got on x64 linux was:

------ Failure: demo --------
  component: test
  path: tests/runtime-new/demo/test.cs
  error: failed to compile component "tests/runtime-new/demo/test.cs"

  Caused by:
      command execution failed
      command: cd "/home/alex/code/wit-bindgen/target/artifacts/demo/test-c-sharp" && "dotnet" "publish" "/home/alex/code/wit-bindgen/target/artifacts/demo/test-c-sharp/TestWorld.csproj" "-r" "wasi-wasm" "-c" "Debug" "/p:PlatformTarget=AnyCPU" "/p:MSBuildEnableWorkloadResolver=false" "--self-contained" "/p:UseAppHost=false" "-o" "/home/alex/code/wit-bindgen/target/artifacts/demo/test-c-sharp/csharp-test"
      status: exit status: 1
      stdout:
          Determining projects to restore...
          All projects are up-to-date for restore.
          TestWorld -> /home/alex/code/wit-bindgen/target/artifacts/demo/test-c-sharp/bin/Debug/net9.0/wasi-wasm/csharp-test.dll
          Generating native code
          /usr/bin/sh: 2: /tmp/MSBuildTempalex/tmpb37635e26a564f76b1d9bf93d5fed132.exec.cmd: \tools\ilc: not found
        /home/alex/code/wit-bindgen/target/artifacts/demo/test-c-sharp/.packages/microsoft.dotnet.ilcompiler.llvm/10.0.0-alpha.1.25118.1/build/Microsoft.NETCore.Native.targets(363,5): error MSB3073: The command ""\tools\\ilc" @"obj/Debug/net9.0/wasi-wasm/native/csharp-test.ilc.rsp"" exited with code 127. [/home/alex/code/wit-bindgen/target/artifacts/demo/test-c-sharp/TestWorld.csproj]

(this is a fork of #1192 with some C# support)

@jsturtevant
Copy link
Collaborator

I think that "passes" due to the cfg!(windows) check here. Locally what I got on x64 linux was:

Got it, I didn't see that just the test passing. Getting these running was pretty straightforward: #1200

We can either add that or do it as part of the migration.

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

No branches or pull requests

5 participants