-
Notifications
You must be signed in to change notification settings - Fork 276
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 wasm tests using was-bindgen-test and wasm-pack #346
Conversation
@@ -201,6 +209,7 @@ fn rsa_jwk() { | |||
// https://jwt.io/ is often used for examples so ensure their example works with jsonwebtoken | |||
#[cfg(feature = "use_pem")] | |||
#[test] | |||
#[wasm_bindgen_test] |
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.
Do we really need to put that on every test? :(
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 think this is unavoidable. Couldn't find aything about the reusability of the standard #[test]
macro.
Added the |
I think only on integration tests. No need to run unit tests i guess. Although we should probably only have that attribute on wasm target to avoid adding compile time/deps when not needed? |
Fixed the CI, thanks! |
* Add wasm tests using was-bindgen-test and wasm-pack * Remove custom wasm feature * Add #[wasm_bindgen_test] to remaining tests * Try to fix CI --------- Co-authored-by: Vincent Prouillet <[email protected]>
A few notes:
Wasm32 related tests cannot be run that easily, see Testing strategy for wasm32-unknown-unknown rustwasm/team#173. You have to use https://rustwasm.github.io/wasm-bindgen/wasm-bindgen-test/usage.html and
wasm-pack
.The
criterion
dev-dependency, only used for benchmarks, prohibits any compilation to the wasm32 target when using its default features (rayon in particular). This is now dependant on the targeted architecture.Examples could not be compiled to wasm32, or any 32 bit target. The
Claims
struct from bothcustom_hader.rs
andvalidation.rs
had ausize
field which got initialized with a value outside the u32 (usize on wasm32) range. Switching to an explicit u64 type in these structs seemed to be a reasonable fix?Most crates provide some
wasm-...
feature, to enable wasm support. This crate uses[target.'cfg(target_arch = "wasm32")'.dependencies]
inside its Cargo.toml. This is obviously easier for users, as there is no feature which could be forgotten. Are there any downsides to this method?CI will most likely still not work, as
https://github.com/nodesource/distributions/blob/master/README.md#ubuntu-versions
cargo install wasm-pack
Example
wasm-pack
output: