test: add wasm runtime API assertions for bisection-key#9
Conversation
- upgrade calcit-version to 0.12.25, @calcit/procs ^0.12.25 - add packageManager [email protected], .yarnrc.yml - upgrade CI to node 24, setup-node@v6, corepack+yarn workflow - add schema type annotations to bisection-key.core functions - add bisection-key.wasm-probe namespace with probe-bisect-basic/strings/all-count - TODO.md: document global def init issue and next steps
- add wasm-test.mjs to assert probe exports and expected API counts - split wasm scripts into compile smoke and runtime assertion - switch dictionary to literal string via cr edit for deterministic init path - remove obsolete .calcit-snippets probe files duplicated in compact.cirru - align README/TODO with current js/wasm test workflow
There was a problem hiding this comment.
Pull request overview
This PR strengthens the WASM test workflow for bisection-key by adding a Node-based runtime harness that executes exported probe functions and asserts expected results, and by separating WASM compilation from runtime verification.
Changes:
- Added
wasm-test.mjsto instantiatejs-out/program.wasm, call probe exports, and fail on mismatches. - Split
test:wasminto a compile step (test:wasm:compile) and a runtime assertion step (test:wasm). - Updated
dictionarydefinition and adjusted README/TODO to describe the JS/WASM testing workflow.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| wasm-test.mjs | New WASM runtime assertion harness that calls exported probe APIs and checks expected values. |
| package.json | Adds test:wasm:compile and updates test:wasm to compile then run runtime assertions. |
| compact.cirru | Updates dictionary definition to a direct string literal (likely to improve WASM initialization behavior). |
| TODO.md | Updates WASM testing notes and command cheat-sheet (currently contains some now-outdated statements). |
| README.md | Updates dev/test commands and documents WASM test scripts (currently contains contradictory wording). |
Comments suppressed due to low confidence (1)
TODO.md:122
- The command cheat-sheet still says
node wasm-test.mjsis only usable after P3, but this PR introduceswasm-test.mjsand wires it intoyarn test:wasm. Please update this note so the doc matches the current state.
# 编译 WASM（需要本地 cr-wasm binary）
cr-wasm compact.cirru --init-fn bisection-key.wasm-probe/probe-all-count
node wasm-test.mjs # P3 完成后可用
</details>
---
💡 <a href="/calcit-lang/bisection-key/new/main?filename=.github/instructions/*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.
| const mod = new WebAssembly.Module(readFileSync(wasmPath)); | ||
| const inst = new WebAssembly.Instance(mod, { | ||
| math: { | ||
| pow: Math.pow, | ||
| sin: Math.sin, | ||
| cos: Math.cos, | ||
| }, | ||
| io: { | ||
| log_value: () => 0, | ||
| }, | ||
| }); | ||
|
|
||
| const e = inst.exports; | ||
|
|
There was a problem hiding this comment.
If js-out/program.wasm is missing or instantiation fails due to an import mismatch, this script will currently throw and exit with a Node stack trace before reporting a clear [wasm-test] error. Consider wrapping the read/compile/instantiate steps in a try/catch and emitting a targeted failure message (and setting a non-zero exit code) so CI/local output is easier to interpret.
| const mod = new WebAssembly.Module(readFileSync(wasmPath)); | |
| const inst = new WebAssembly.Instance(mod, { | |
| math: { | |
| pow: Math.pow, | |
| sin: Math.sin, | |
| cos: Math.cos, | |
| }, | |
| io: { | |
| log_value: () => 0, | |
| }, | |
| }); | |
| const e = inst.exports; | |
| let e; | |
| try { | |
| const mod = new WebAssembly.Module(readFileSync(wasmPath)); | |
| const inst = new WebAssembly.Instance(mod, { | |
| math: { | |
| pow: Math.pow, | |
| sin: Math.sin, | |
| cos: Math.cos, | |
| }, | |
| io: { | |
| log_value: () => 0, | |
| }, | |
| }); | |
| e = inst.exports; | |
| } catch (err) { | |
| const message = err instanceof Error ? err.message : String(err); | |
| console.error(`[wasm-test] failed to load or instantiate ${wasmPath}: ${message}`); | |
| process.exit(1); | |
| } |
| process.exit(1); | ||
| } | ||
|
|
||
| console.log('[wasm-test] all probe checks passed'); |
There was a problem hiding this comment.
Using process.exit(1) can truncate pending stdout/stderr writes in some environments. Prefer setting process.exitCode = 1 and letting the process exit naturally after logging, especially since this script already tracks failed.
| process.exit(1); | |
| } | |
| console.log('[wasm-test] all probe checks passed'); | |
| process.exitCode = 1; | |
| } else { | |
| console.log('[wasm-test] all probe checks passed'); | |
| } |
| For WASM(temporary compile smoke only): | ||
|
|
||
| ```bash | ||
| yarn test:wasm:compile | ||
| yarn test:wasm | ||
| ``` | ||
|
|
||
| `test:wasm` runs runtime assertions for probe APIs and will fail when WASM runtime behavior diverges from expected API semantics. |
There was a problem hiding this comment.
README section heading says WASM is "temporary compile smoke only", but the text immediately below says test:wasm runs runtime assertions and fails on semantic divergence. Please update this wording so the WASM section consistently describes the current behavior (compile + runtime assertions).
…s to use native procs
Summary
Verification
loading: /calcit-test/compact.cirru
Test: 'test-append
Test: 'test-assoc
Test: 'test-bisect
Test: 'test-frequent-append
Test: 'test-frequent-prepend
Test: 'test-get-key
get min key: has 1 tests
get max key: has 1 tests
get nil: has 2 tests
Test: 'test-key-after
Test: 'test-key-before
Test: 'test-prepend
Test: 'test-shorten
Test: 'test-nth-ops
get key at nth: has 4 tests
get val at nth: has 4 tests
[Warn] exceeded map size
set value at nth: has 1 tests
set value before nth: has 1 tests
set value after nth: has 1 tests
find key index a: has 1 tests
find key index c: has 1 tests
find key index missing: has 1 tests
took 46.7ms: nil
loading: /calcit-test/compact.cirru
emitted: js-out/bisection-key.wasm-probe.mjs
emitted: js-out/calcit.core.mjs
... and 10 files not changed.
took 63.944ms
Test: 'test-append
Test: 'test-assoc
Test: 'test-bisect
Test: 'test-frequent-append
Test: 'test-frequent-prepend
Test: 'test-get-key
get min key: has 1 tests
get max key: has 1 tests
get nil: has 2 tests
Test: 'test-key-after
Test: 'test-key-before
Test: 'test-prepend
Test: 'test-shorten
Test: 'test-nth-ops
get key at nth: has 4 tests
get val at nth: has 4 tests
[Warn] exceeded map size
set value at nth: has 1 tests
set value before nth: has 1 tests
set value after nth: has 1 tests
find key index a: has 1 tests
find key index c: has 1 tests
find key index missing: has 1 tests
wrote WASM to: js-out/program.wasm
took 82.696ms (currently expected to fail for runtime correctness gaps, and now reports concrete assertion errors)
Notes
This PR intentionally turns wasm test from compile-only smoke into runtime assertion, so API logic regressions are visible in CI output.