Keats / jsonwebtoken

JWT lib in rust
MIT License
1.62k stars 252 forks source link

Add wasm tests using was-bindgen-test and wasm-pack #346

Closed lpotthast closed 7 months ago

lpotthast commented 8 months ago

A few notes:

1) Wasm32 related tests cannot be run that easily, see https://github.com/rustwasm/team/issues/173. You have to use https://rustwasm.github.io/wasm-bindgen/wasm-bindgen-test/usage.html and wasm-pack.

2) 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.

3) Examples could not be compiled to wasm32, or any 32 bit target. The Claims struct from both custom_hader.rs and validation.rs had a usize 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?

4) 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?

5) CI will most likely still not work, as

Example wasm-pack output:

lukas@[...]:~/dev/jsonwebtoken$ wasm-pack test --node
[INFO]: 🎯  Checking for the Wasm target...
   Compiling jsonwebtoken v9.1.0 (/home/lukas/dev/jsonwebtoken)
    Finished dev [unoptimized + debuginfo] target(s) in 0.84s
   Compiling jsonwebtoken v9.1.0 (/home/lukas/dev/jsonwebtoken)
    Finished test [unoptimized + debuginfo] target(s) in 0.28s
     Running unittests src/lib.rs (target/wasm32-unknown-unknown/debug/deps/jsonwebtoken-ae8fe4284f1b721f.wasm)
no tests to run!
     Running tests/hmac.rs (target/wasm32-unknown-unknown/debug/deps/hmac-14855738297bd30c.wasm)
no tests to run!
     Running tests/lib.rs (target/wasm32-unknown-unknown/debug/deps/lib-17a50cd144c5e104.wasm)
Set timeout to 20 seconds...
running 21 tests                                  

test lib::ecdsa::roundtrip_with_jwtio_example ... ok
test lib::ecdsa::ed_jwk ... ok
test lib::ecdsa::ec_x_y ... ok
test lib::ecdsa::round_trip_claim ... ok
test lib::ecdsa::round_trip_sign_verification_pem ... ok
test lib::ecdsa::round_trip_sign_verification_pk8 ... ok
test lib::rsa::roundtrip_with_jwtio_example_jey ... ok
test lib::rsa::rsa_jwk ... ok
test lib::rsa::rsa_modulus_exponent ... ok
test lib::rsa::round_trip_claim ... ok
test lib::rsa::round_trip_sign_verification_der ... ok
test lib::rsa::round_trip_sign_verification_pem_pkcs8 ... ok
test lib::rsa::round_trip_sign_verification_pem_pkcs1 ... ok
test lib::eddsa::ed_jwk ... ok
test lib::eddsa::ed_x ... ok
test lib::eddsa::round_trip_claim ... ok
test lib::eddsa::round_trip_sign_verification_pem ... ok
test lib::eddsa::round_trip_sign_verification_pk8 ... ok
test lib::header::x5c_der_invalid_chain ... ok
test lib::header::x5c_der_valid_chain ... ok
test lib::header::x5c_der_empty_chain ... ok

test result: ok. 21 passed; 0 failed; 0 ignored
lpotthast commented 8 months ago

Added the #[wasm_bindgen_test] macro to all remaining tests, including unit-tests. I do not know if unit-test should be kept out though, and running integration-tests only is enough for the wasm case..

Keats commented 8 months ago

I do not know if unit-test should be kept out though, and running integration-tests only is enough for the wasm case..

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?

Keats commented 7 months ago

Fixed the CI, thanks!