Open spigaz opened 5 months ago
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 https://github.com/rustwasm/wasm-bindgen/issues/2276 and there was already a working implementation in https://github.com/rustwasm/wasm-bindgen/pull/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 https://github.com/rustwasm/wasm-bindgen/pull/3782#issuecomment-1913110704.
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.
@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.
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!
@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 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.
@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!
Motivation
I'm already developing for the workers-rs platform (workerd)
I need to
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:
Proposed Solution
The expected roadmap should be something like:
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:
@hamza1311 said: