Clean up WASM testing: remove dead crate, fix cfg proxy, add real wasm32 tests#109
Draft
cderv wants to merge 28 commits intofix/wasm-cfg-proxy-and-cleanupfrom
Draft
Clean up WASM testing: remove dead crate, fix cfg proxy, add real wasm32 tests#109cderv wants to merge 28 commits intofix/wasm-cfg-proxy-and-cleanupfrom
cderv wants to merge 28 commits intofix/wasm-cfg-proxy-and-cleanupfrom
Conversation
cderv
force-pushed
the
feature/wasm-testing-and-cleanup
branch
2 times, most recently
from
52923e7 to
60df573
Compare
Design for bd-itj9: replace cfg proxy in Lua VM setup with real WASM tests, clean up stale wasm-qmd-parser crate, and migrate away from deprecated wasm-pack tooling. Three phases: (1) remove stale artifacts, (2) fix cfg guards so native tests use real Lua stdlib, (3) add wasm-bindgen-test integration tests for pampa's Lua filter code.
- Add explicit success criteria - Require phases 2+3 land together (no validation gap) - Add --no-default-features --features lua-filter to WASM test command - Add workspace.dependencies.wasm-qmd-parser to removal checklist - Pin wasm-bindgen-cli to exact version 0.2.108 - Specify test-suite.yml as target CI workflow - Note that .cargo/config.toml runner setting doesn't affect production build
CI uses dev-setup to install version-matched wasm-bindgen-cli from Cargo.lock instead of hardcoding the version. Fix xtask.md rule to reference wasm-bindgen-cli instead of stale wasm-pack, and document the version pinning convention.
- Tighten success criteria: "no active build/test/runtime references" - Resolve hub-client-e2e.yml: remove stale wasm-pack install step - Normalize path references to crates/pampa/tests/wasm_lua.rs - Add local developer workflow section - Note WASM tests not part of xtask verify (requires nightly toolchain)
mlua/lua-src compiles Lua from C source — targeting wasm32 requires Clang + CC/CFLAGS env vars pointing to wasm-sysroot. Same setup as existing ts-test-suite.yml WASM build. Note opportunity to share toolchain setup across CI workflows. Also flag ts-test-suite.yml hardcoded wasm-bindgen-cli version for migration to xtask dev-setup.
WASM build (build-wasm.js) doesn't support Windows — no Clang wasm32 target. WASM tests have the same constraint. Both run in Linux CI only. Consistent with existing behavior.
wasm-qmd-parser is fully superseded by wasm-quarto-hub-client. The build-wasm.yml workflow only built the stale crate. wasm-pack is not used by the active WASM build pipeline (build-wasm.js uses cargo build + wasm-bindgen CLI directly). - Delete crates/wasm-qmd-parser/ entirely - Delete .github/workflows/build-wasm.yml - Remove wasm-qmd-parser from workspace exclude and deps - Remove stale wasm-pack install from hub-client-e2e.yml - Update hub-client and wasm-quarto-hub-client READMEs - Update workspace CLAUDE.md and Cargo.toml comments
Remove `test` from `#[cfg(any(target_arch = "wasm32", test))]` guards in filter.rs and shortcode.rs so native tests use Lua::new() with real C stdlib on all platforms. This fixes the 8 filter traversal tests that failed on Windows because the synthetic io.open only handles POSIX VFS paths. Add wasm-bindgen-test smoke tests in crates/pampa/tests/wasm_lua.rs that run on the real wasm32 target, validating: - Restricted Lua VM creation - Filter execution through WASM code path - Shortcode engine on WASM - Error handling (panic_unwind works) - Synthetic io/os module registration Configure wasm-bindgen-test-runner in .cargo/config.toml. WASM tests require nightly + Clang + wasm-sysroot (Linux/macOS CI only).
Rewrite dev-docs/wasm.md as single source of truth for WASM in this project: architecture, build pipeline, testing strategy, and C toolchain requirements. Add WASM testing guidance to pampa/CLAUDE.md and .claude/rules/wasm.md to prevent regression of the cfg test proxy pattern. Update testing.md to reflect the new native vs WASM testing approach.
Add wasm-tests job to test-suite.yml that runs pampa WASM smoke tests on Linux with nightly Rust + Clang + wasm-sysroot. Uses cargo xtask dev-setup for version-matched wasm-bindgen-cli installation. Migrate ts-test-suite.yml from hardcoded wasm-bindgen-cli version to cargo xtask dev-setup for consistent version management.
- Gate wasm_lua.rs on both wasm32 and lua-filter feature to prevent compilation errors in minimal-feature test runs - Fix dev-docs/wasm.md: clarify that build-wasm.js does not set -fno-builtin (only needed for WASM tests, not production builds) - Narrow shortcode coverage claim to match actual test (init only)
Debug-mode builds emit __builtin_* intrinsic calls that don't exist in the stub wasm-sysroot. Release builds inline these away.
The cc crate runs clang from OUT_DIR, not the repo root, so relative
-isystem paths don't resolve. Use $PWD for local dev instructions and
${{ github.workspace }} in CI.
The test_dofile_script_dir_stack test depends on register_wasm_dofile which pushes/pops the script-dir stack around dofile calls. On native, the C Lua dofile doesn't interact with the stack. Neither Pandoc nor Quarto CLI provide this for raw dofile() — it's a WASM-only feature. Mark the test as ignored on non-wasm32 targets and document the finding in the design spec.
cderv
force-pushed
the
feature/wasm-testing-and-cleanup
branch
from
8c6bfe2 to
89d90e0
Compare
proptest pulls in getrandom 0.3.4 which doesn't compile for wasm32-unknown-unknown. Move proptest, insta, tempfile, and tokio to the cfg(not(wasm32)) dev-dependencies section. No effect on native builds — the condition is always true on x86_64/aarch64.
Add Phase 6a documenting: sysroot absolute paths, -fno-builtin rationale, wasm-incompatible dev-deps gating, and dofile behavioral difference (linking to #112).
cargo xtask dev-setup uses --locked which may produce a different wasm-bindgen-cli binary. Revert to the hardcoded install that main uses until the root cause of the externref type mismatch is understood.
cargo xtask dev-setup with --locked causes externref mismatch in TS Test Suite. Reverted to hardcoded install. Tracked as bd-jakt.
Two independent build errors in the WASM Tests CI job: 1. Duplicate core lang item (E0152): The toolchain setup installed the prebuilt wasm32-unknown-unknown target AND used -Zbuild-std, which rebuilds core from source. Two copies of core conflict. The production WASM build (ts-test-suite.yml) correctly uses only rust-src without the prebuilt target. Removed targets: wasm32-unknown-unknown. 2. Bin targets compiled for wasm32: cargo test builds bin targets alongside integration tests (rust-lang/cargo#12980). The pampa and ast-reconcile bins use native-only types (NativeRuntime, tokio) that don't exist on wasm32. Added required-features = ["terminal-support"] so bins are skipped when --no-default-features --features lua-filter is used.
rust-toolchain.toml installs the prebuilt wasm32-unknown-unknown target (needed for the production WASM build). But -Zbuild-std rebuilds core from source, and having both causes E0152 (duplicate lang item) when building within the workspace. The production build (wasm-quarto-hub-client) avoids this because it is excluded from the workspace and gets an isolated target/ directory. The WASM test runs pampa within the workspace, where the conflict manifests. Explicitly remove the prebuilt target before running WASM tests.
The wasm-tests job failed with E0152 (duplicate lang item in core) because rust-toolchain.toml declares targets = ["wasm32-unknown-unknown"], and the rustup proxy auto-reinstalls the prebuilt target on every cargo invocation — undoing the explicit rustup target remove step. Fix: set RUSTUP_TOOLCHAIN=nightly as a job-level env var, which tells rustup to bypass rust-toolchain.toml entirely. Replace dtolnay/rust-toolchain action with plain rustup toolchain install since we only need nightly + rust-src (no prebuilt wasm32 target). The target removal step is no longer needed because the target is never installed in the first place.
rust-toolchain.toml already specifies the full toolchain configuration (nightly channel, components, targets). The dtolnay/rust-toolchain action was a redundant wrapper — rustup reads rust-toolchain.toml natively. Replace with `rustup show active-toolchain` which triggers auto-install from rust-toolchain.toml and displays the resolved toolchain. One fewer third-party action dependency across all CI workflows.
apply_lua_filters became async in e537fb8 (Apr 6) but the WASM tests written in 22f9a4f (Apr 9) called it without .await. The test binary also needs panic_abort in the -Zbuild-std list. Neither issue was caught because CI always failed earlier at E0152 (duplicate core sysroot). - Make filter/io/os test functions async, add .await on apply_lua_filters - Add panic_abort to -Zbuild-std in CI and docs
…cturing apply_lua_filters returns a FilterOutput struct (with pandoc, context, diagnostics fields), not a tuple. The tests were written with tuple destructuring that never compiled on wasm32.
The CI job no longer uses rustup target remove — it sets RUSTUP_TOOLCHAIN=nightly to bypass rust-toolchain.toml entirely, preventing the prebuilt target from being installed in the first place.
Document three rounds of CI fixes discovered after resolving E0152: - RUSTUP_TOOLCHAIN approach supersedes rustup target remove - dtolnay/rust-toolchain replaced with plain rustup - async/struct/panic_abort test compilation fixes Add wasm-c-shim extraction plan: shared crate for C stdlib stubs needed by both production WASM build and WASM integration tests.
Move the ~980-line c_shim.rs from wasm-quarto-hub-client into a new wasm-c-shim crate. This provides #[no_mangle] C stdlib stubs (malloc, fprintf, snprintf, abort, etc.) needed by tree-sitter and Lua when compiled for wasm32-unknown-unknown, which has no libc. The crate is a no-op on native targets (all exports gated on wasm32). Both wasm-quarto-hub-client (production) and pampa WASM tests now depend on wasm-c-shim for the shared symbols.
The shim code was written for edition 2021 (in wasm-quarto-hub-client).
Edition 2024 requires explicit unsafe {} blocks inside unsafe fn, which
would add wrappers to ~65 FFI call sites with no safety benefit. Use
edition 2021 for this crate and document the rationale.
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.
The Lua filter/shortcode code used
#[cfg(any(target_arch = "wasm32", test))]to forcenative tests through the restricted WASM Lua stdlib. This was a proxy — native tests
pretended to be WASM to get coverage of the restricted code path. The proxy caused
Windows test failures (the restricted stdlib doesn't work without a real wasm32 target)
and meant native tests weren't testing native behavior.
Changes
Remove the
testarm from the cfg guard so native tests useLua::new()with the fullC stdlib on all platforms. Replace the lost proxy coverage with 6 real WASM smoke tests
in
crates/pampa/tests/wasm_lua.rsthat compile and run onwasm32-unknown-unknown:panic_unwindworks)ioandosmodule registrationThese run in a new
wasm-testsCI job (Linux, nightly + Clang). They can't run locallyon Windows — the CI job is the primary execution environment.
Also:
wasm-qmd-parsercrate (superseded bywasm-quarto-hub-client)build-wasm.ymlworkflowwasm-pack installfromhub-client-e2e.yml(wasm-pack is no longer used)wasm-bindgen-cliinstall tocargo xtask dev-setup(single version pin)dev-docs/wasm.mdto document the current architecture-isystemin CI WASM test CFLAGS (thecccrate runs clangfrom
OUT_DIR, not the repo root, so relative paths don't resolve)-fno-builtin(debug-mode builds emit__builtin_*intrinsic calls not present in the stub wasm-sysroot)
proptest,insta,tempfile,tokiodev-dependencies onnot(wasm32)—proptestpulls ingetrandom 0.3.4which doesn't compile for wasm32. No effecton native builds.
dofile behavioral difference
Removing the cfg proxy exposed a behavioral difference between WASM and native for
dofile()+quarto.utils.resolve_path(). The WASMdofileoverride tracks thescript directory stack; the native C
dofiledoes not. Thetest_dofile_script_dir_stacktest is marked
#[ignore]on non-wasm32 targets. See #112 for discussion.