-
Notifications
You must be signed in to change notification settings - Fork 20
Build WebAssembly tests for the wasm32-wasip1 target and run them using wasmtime
#122
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
Changes from 5 commits
896bf03
9e82cc4
7d2689f
6974cc3
6ec30b8
f7d5a12
c027aa8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,15 +41,14 @@ pub fn simd_test(_: TokenStream, item: TokenStream) -> TokenStream { | |
| #[cfg(not(any(target_arch = "x86", target_arch = "x86_64")))] | ||
| let include_avx2 = false; | ||
| // Note that we cannot feature-gate this with `target_arch`. If we run | ||
| // `wasm-pack test --headless --chrome`, then the `target_arch` will still be set to | ||
| // `cargo test --target wasm32-wasip1`, then the `target_arch` will still be set to | ||
| // the operating system you are running on. Because of this, we instead add the `target_arch` | ||
| // feature gate to the actual test. | ||
| let include_wasm = !exclude_wasm(&input_fn_name.to_string()); | ||
|
Comment on lines
43
to
47
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's worth checking whether we can use This should be a follow-up/an issue though.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can't; it's not exposed at the time proc macros are run. |
||
|
|
||
| let fallback_snippet = if include_fallback { | ||
| quote! { | ||
| #[test] | ||
| #[cfg_attr(all(target_arch = "wasm32", target_feature = "simd128"), wasm_bindgen_test::wasm_bindgen_test)] | ||
| fn #fallback_name() { | ||
| let fallback = fearless_simd::Fallback::new(); | ||
| #input_fn_name(fallback); | ||
|
|
@@ -101,7 +100,7 @@ pub fn simd_test(_: TokenStream, item: TokenStream) -> TokenStream { | |
| let wasm_snippet = if include_wasm { | ||
| quote! { | ||
| #[cfg(all(target_arch = "wasm32", target_feature = "simd128"))] | ||
| #[wasm_bindgen_test::wasm_bindgen_test] | ||
| #[test] | ||
| fn #wasm_name() { | ||
| let wasm = unsafe { fearless_simd::wasm32::WasmSimd128::new_unchecked() }; | ||
| #input_fn_name(wasm); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First, thank you for the awesome PR! This comment can be treated as non-blocking – but I am just curious and honestly lacking knowledge of other
wasm32targets.We (Canva) currently use the
vello_hybrid/vello_cpurenderers viawasm32-unknown-unknownusingwasm-bindgendirectly. From that perspective I'd be concerned that we lose that test coverage.Is there a solution for migrating from
wasm32-unknown-unknowntowasm32-wasip1for Rust on the browser?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so at the moment; there's wasm-bindgen/wasm-bindgen#3421 but it's kinda dead (as is wasm-bindgen in general, unfortunately).
The Vello tests will still run on
wasm32-unknown-unknown, so test coverage should be maintained through those. I'm also not removingwasm32-unknown-unknownsupport from this library; this is just a matter of making the tests easier to run and extend.It might be worth adding a CI check that
wasm32-unknown-unknownstill builds, but I seriously doubt there'll be any functional differences betweenwasm32-wasip1andwasm32-unknown-unknown. For example, there are a lot of JavaScript packages on NPM that run all their unit tests on Node, but that doesn't mean those packages don't work in the browser.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds good to me! Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added an additional step to the "cargo test (wasm32)" workflow that runs
cargo build --target wasm32-unknown-unknown. This should ensure that target still builds.