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 support to execute tests in workerd #3891

Open
spigaz opened this issue Mar 13, 2024 · 6 comments
Open

Add support to execute tests in workerd #3891

spigaz opened this issue Mar 13, 2024 · 6 comments

Comments

@spigaz
Copy link

spigaz commented Mar 13, 2024

Motivation

I'm already developing for the workers-rs platform (workerd)

  1. About 1000 test templates running with wasm-bindgen-test and firefox
  2. Almost 400 in a single crate
  3. In the heavier setup they take almost 60h to complete on a single core
  4. Workers-rs only has about 50 tests in JS/TS running in vitest

I need to

  1. Run tests on workerd - for specific functionality
  2. Better test output - 400 random organised tests in a crate are hard to check
  3. Distribute the load between multiple executing instances
  4. Code Coverage

I already have some support and I plan to tackle this problems as much as possible, I'm highly motived and self reliant.

But I believe that your guidance is quite important:

  1. To maintain compatibility with your API
  2. To say how you want things done so that its a smooth process for all
  3. To support the provided test runner in your test runner like you support Firefox/Chrome/Safari/Node if you want
  4. To point me to the relevant information and to relevant steps in the process

Proposed Solution

The expected roadmap should be something like:

  1. Create an initial workerd test runner
  2. Execute in compatibility mode with other platforms
  3. Integrate with your test runner
  4. Execute with workerd specific functionality
  5. Add support for running tests in parallel inside the test runner
  6. Improve test output
  7. Analyse if its possible to add code coverage
  8. Integrate wasm-bindgen-test with nextest
  9. Add support for cargo-mutants

Workerd is started and configured by Miniflare, I believe it will be a test runner in JS/TS, that I'm guessing will be opaque like the other testing platforms.

Although some kind of streaming of the output could be nice.

Alternatives

I have discussed some details of the strategy with Miniflare lead dev several times, it seems reasonable and recently I was no longer told to wait, that I could start moving forward.

The integration of the runner with your test runner, would be something nice to have, I don't mind working on a more general solution that also allows other test runners.

Additional Context

Back on #3510

@daxpedda said:

Apologies, you are right your case is different, I didn't pay enough attention.

I think the same philosophy applies to the test runner as well. The preference here it not to have to cover every use case there is in wasm-bindgen. So as you said, CF should provide their own test runner.

That said, if the wasm-bindgen test runner can be adjusted to provide a general way for other runtimes/platforms to inject what they need to get things to work, that would be great as well. That would obviously require some design work, but considering the lack of time from maintainers this has to be done in small steps that can be easily discussed and reviewed.

@hamza1311 said:

using wasm-bindgen produced test files to run the tests with your test runner using workerd is a bit of a different use case.

That is a use case I would love to support. There's also a point to be made of running wasm-bindgen output in non-node.js environments like Deno or Bun. I think this PR is out of scope for this discussion. Can you create a new issue for this so we can discuss how to handle this? I think we can take inspiration from the Jasmine testing framework in the JS ecosystem

@daxpedda
Copy link
Collaborator

daxpedda commented Mar 15, 2024

3. Distribute the load between multiple executing instances

wasm-bindgen-test should mimic Cargo test as much as possible, parallelizing tests even more is done in Cargo test externally, e.g. nextest. So the goal would be to figure out how we can offer integration with wasm-bindgen-test for external test runners like this (or figure out what the current issue is).

4. Code Coverage

Code coverage is being tracked in #2276 and there was already a working implementation in #3782, which basically just needed a rebase.

It would be good however if somebody took a look at how to fix llvm-cov to find the debug information, see #3782 (comment).


The integration of the runner with your test runner, would be something nice to have, I don't mind working on a more general solution that also allows other test runners.

This is the path we want go down. A proposal how this would be tackled and some design work would be whats needed here.

I'm happy to discuss and look at any more specific proposals.

@spigaz
Copy link
Author

spigaz commented Mar 17, 2024

@daxpedda I gave a look to nextest, it's indeed relevant and in end I would like to support it also, but the missing piece seems to be in nextest.

I tested it and got the message:

info: for the target platform, using target runner `wasm-bindgen-test-runner` defined by `target.wasm32-unknown-unknown.runner` within `/Users/alexander/Developer/rust/.cargo/config.toml`
error: creating test list failed

Caused by:
  for `core_application_handler`, line 'no tests to run!' did not end with the string ': test' or ': benchmark'
full output:
no tests to run!

I'm guessing its best to add support for workerd first.

But recognising its relevance, I'm taking it in consideration, its documentation says:

By default, cargo test uses this execution model:
each test binary is run serially, and binaries are responsible for running individual tests in parallel.

A cargo-nextest run has two separate phases:
The list phase. cargo-nextest first builds all test binaries with cargo test --no-run, then queries those binaries to produce a list of all tests within them.
The run phase. cargo-nextest then executes each individual test in a separate process, in parallel. It then collects, displays and aggregates results for each individual test.

With cargo test its up to the binary to run individual tests in parallel, so in that model it would make sense for the test-runner to run tests in parallel launching multiple vms and distributing the load.

To support the cargo-nextest model its trickier because a simpler version would imply starting a vm, running a single test and return.

Starting a vm about 400 times for a single crate can be a bit expensive I guess.

So i'm guessing, that we need to have a server that manages all the executions and gets called by multiple clients, that is able to work in both modes.

I'm going to update my roadmap to include nextest and cargo-mutants.

@daxpedda
Copy link
Collaborator

With cargo test its up to the binary to run individual tests in parallel, so in that model it would make sense for the test-runner to run tests in parallel launching multiple vms and distributing the load.

To support the cargo-nextest model its trickier because a simpler version would imply starting a vm, running a single test and return.

I'm not following where VMs come into play here. Seeing the error you posted, it seems to me the issue in nextest is that the output by wasm-bindgen-test isn't the same as cargo test, which is why it is unable to compile a list of all tests to then run them on its own.

This is a general issue in wasm-bindgen-test, but should fix compatibility with nextest as well.

Let me know if I missed anything here!

@spigaz
Copy link
Author

spigaz commented Mar 17, 2024

@daxpedda Well if the problem is with wasm-bindgen-test that isn't providing the proper test list, I can start with that, no problem.

I need to learn that part anyway.


Regarding the execution, nextest invokes the binary once for each test, that would imply starting the browser / node / workerd, run the test and exit, in a simple direct implementation.

@daxpedda
Copy link
Collaborator

@daxpedda Well if the problem is with wasm-bindgen-test that isn't providing the proper test list, I can start with that, no problem.

I think that's a good step forward: aligning the output of wasm-bindgen-test to cargo test.

Regarding the execution, nextest invokes the binary once for each test, that would imply starting the browser / node / workerd, run the test and exit, in a simple direct implementation.

This might be a bit too much overhead for parallelizing tests with wasm-bindgen-test, starting the browser consumes way more resources compared to starting single tests with cargo test. We might end up either adjusting nextest to account for that or actually consider implementing some level of parallelization in wasm-bindgen-test indeed.

In any case, this is something to ponder upon later when we actually have some data.

@spigaz
Copy link
Author

spigaz commented Apr 10, 2024

@daxpedda Regarding supporting cargo nextest, its still work in progress but I created PR #3920, so that we can discuss current status and address some issues I'm having. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants