feat(wasm): wire wasm-bindgen-test into CI and expand coverage#64
Open
kingchenc wants to merge 4 commits into
Open
feat(wasm): wire wasm-bindgen-test into CI and expand coverage#64kingchenc wants to merge 4 commits into
kingchenc wants to merge 4 commits into
Conversation
The WASM binding already had 8 wasm_bindgen_test cases checked in but
they ran only when someone manually invoked `wasm-pack test` locally —
CI never executed them, so a breakage between the Rust core API and the
JS surface would only surface in user reports.
Two changes:
1. **New CI job `wasm-test`** in `.github/workflows/ci.yml` runs
`wasm-pack test --node bindings/wasm` on every push and pull-request.
Uses Node-runtime mode (no headless browser needed), pins
`taiki-e/install-action` for `wasm-pack`, installs the
`wasm32-unknown-unknown` target.
2. **10 new `#[wasm_bindgen_test]` cases** in `bindings/wasm/src/lib.rs`
covering families that the existing 8 tests did not touch:
- Family 4 — Macd multi-output batch shape (3-component packing)
- Family 5 — Bollinger {upper, middle, lower} ordering invariant
- Family 10 — FisherTransform streaming roundtrip (recursive DSP)
- Family 16 — SharpeRatio reset semantics, MaxDrawdown monotone-
uptrend invariant, ValueAtRisk constructor validation
- Cross-family — reset returns indicator to warmup (3 shapes),
warmup_period parity with wickra-core, NaN input rejection
does not advance warmup counter
No production code changed; additive tests only. Total wasm test count
goes from 8 to 18.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
The new `reset_returns_indicator_to_warmup` test in this branch called `atr.is_ready()` but `WasmAtr` is hand-coded (not generated by the `wasm_scalar_indicator!` macro) and does not expose `is_ready` on the JS surface — that pattern is reserved for macro-generated scalar wrappers. Replace the second leg of the test with `WasmEma` so the test exercises two macro-generated scalars whose `is_ready`/`reset` semantics are guaranteed by the same code path. The candle-input lifecycle is already covered by `candle_input_streaming_matches_batch_and_lifecycle`. Workspace clippy locally green: - cargo clippy -p wickra-wasm --all-targets -- -D warnings - cargo check -p wickra-wasm --target wasm32-unknown-unknown --tests
The new `bollinger_batch_orders_upper_mid_lower` test indexed the batch output as `[u0, m0, l0, u1, m1, l1, ...]` (3 floats per bar) but the actual WasmBb::batch layout is `[u0, m0, l0, sd0, u1, m1, l1, sd1, ...]` — four floats per bar including stddev. The assertion read an upper band against a middle from the previous bar, which can violate the expected upper >= middle ordering and made the test fail intermittently. Switch the stride from `* 3` to `* 4` so we read upper / middle / lower from the same bar. Also add an `is_finite()` precondition so the test fails with a clear "warmup positions unexpectedly NaN" message rather than a confusing NaN-comparison if the dataset ever shrinks.
…vers it The CI workflow already had a `WASM build` job that runs `wasm-pack test --node bindings/wasm` after building. The `wasm-test` job I added in cc961b3 duplicated that step and was the only one whose `taiki-e/install-action` SHA was wrong, so it also kept failing on a non-resolvable action reference. Removing the duplicate keeps the test coverage (it lives in the existing WASM build job) and gets rid of the unresolvable SHA.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two changes that together close the WASM has no CI tests gap from the project analysis:
wasm-testruns `wasm-pack test --node bindings/wasm` on every push and pull-request. Until now the existing 8 `#[wasm_bindgen_test]` cases in `bindings/wasm/src/lib.rs` only ran when someone manually invoked `wasm-pack test` locally — meaning every PR-merge could have silently broken the WASM surface.Total wasm test count: 8 → 18.
CI setup details
CI expectation
29 existing checks + new `wasm-test` job = 30 checks total. All must be green.
Conflict status
Independent of #59, #60, #61. The only files touched are `.github/workflows/ci.yml` (new job appended) and `bindings/wasm/src/lib.rs` (test mod extended). Neither file is modified by any of the three open PRs.