Implement js-bindgen test runner#7
Conversation
daxpedda
left a comment
There was a problem hiding this comment.
I really want to start this by saying that this is so much better then what we did in wasm-bindgen in so many areas ... its really great work!
I didn't finish reviewing everything yet but I think this is a lot already. There is one major change that has to be done: how we execute test sequences in general.
One goal that we didn't achieve in wasm-bindgen-test-runner is actually properly mimic Rustc's test while not being unsound.
The big problem is panic = "abort". Rustc's test actually does require panic = "abort" because tests are run in the same process, the same thing we are doing here. There is an unstable flag called -Zpanic-abort-tests which will support testing with panic = "abort". But this will actually run every test in its own process. We have to mimic this behavior!
This isn't just about parity, but about soundness! After panicking, we are actually not allowed to continue running any Rust code, see rust-lang/rust#117344.
So what we have to do is the following:
- For Node.js we can simply spawn many processes and execute one test each. It might be better for performance to use Node.js workers instead. This might also allow us to only compile the module once and then share it between workers.
- For browsers using workers is an option if we are testing in workers. Otherwise we can instantiate the module in sequence. I know at least with Chromedriver it is possible to open multiple "tabs" to run tests actually in parallel. For testing non-headless, we have to run them in sequence. Another option to explore might be Iframes. I think all of these things are optimizations for future work and we should just instantiate the module in sequence for each test for now.
However, there are two more things we have to account for:
- When actually running with
panic = "unwind", which we need to support as well, we can continue using the current strategy. - We also should support
-Zforce-run-in-process, in which case as soon as a test fails we have to abort everything.#[panic_abort]would not be supported in this mode.
So basically the big problem is that we have to support multiple testing strategies. I leave it up to you how you want to proceed, but at a minimum we need to support panic = "abort" tests, the only stable option at the moment.
I should also note that the panic = "abort" strategy requires that any test data handling, e.g. sending back the result, has to happen in JS. After a panic we can't re-enter the Wasm module!
I know there are a lot of details here, so feel free to ask away here or on Discord!
f73f53c to
c47a25c
Compare
0903c90 to
de33791
Compare
1442178 to
2bf1967
Compare
No description provided.