Test wasm32 without js#6283
Conversation
WalkthroughThe PR replaces the broken wasm test infrastructure by relocating the wasm crate from Changes
Sequence DiagramsequenceDiagram
participant Host as Host Runtime<br>(main.rs)
participant WasmModule as WASM Module<br>(eval)
participant RustPython as RustPython VM
participant KVStore as KV Store
Host->>Host: Load WASM module from path
Host->>Host: Initialize KV store with code
Host->>WasmModule: Call eval(code_ptr, code_len)
activate WasmModule
WasmModule->>RustPython: Create Interpreter
RustPython->>RustPython: Execute Python expr "1 + 3"
RustPython-->>WasmModule: Result repr
WasmModule->>Host: Call print(msg_ptr, msg_len)
activate Host
Host->>Host: Read string from WASM memory
Host->>Host: Print to stdout
Host-->>WasmModule: return 0
deactivate Host
WasmModule-->>Host: return 0 (success)
deactivate WasmModule
Host->>Host: Report result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Sorry, something went wrong.
555c55a to
1a92d4f
Compare
November 17, 2025 11:48
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (5)
example_projects/wasm32_without_js/wasm-runtime/.gitignore (1)
1-4: Confirm whether the!wasm/rustpython.wasmexception is still neededThis override looks like a leftover from the previous layout; the new example seems to produce
rustpython-without-js/target/.../rustpython_without_js.wasminstead. Ifwasm/rustpython.wasmis no longer used, consider removing or updating this exception to avoid confusion.example_projects/wasm32_without_js/rustpython-without-js/Cargo.toml (1)
13-15: Consider dropping empty[workspace]/[patch.crates-io]sectionsIf this crate is meant to live inside the existing top-level workspace and you’re not patching crates, these sections can be removed to reduce manifest noise and avoid accidentally creating a nested workspace.
example_projects/wasm32_without_js/wasm-runtime/README.md (1)
3-11: Clarify thewat2wasmprerequisite or remove itThe README asks to “install wat2wasm and rust”, but the example usage only shows:
cargo run --release <wasm binary>If the current flow always consumes a
.wasmproduced bycargo build(as in the other README) and no.watis involved anymore, consider either:
- Explaining where
wat2wasmfits into the workflow, or- Dropping the
wat2wasmprerequisite to keep the instructions focused.example_projects/wasm32_without_js/wasm-runtime/src/main.rs (2)
56-72: Clarify the intent of commented code.The
get_codefunction is fully commented out. Should this be removed entirely, or is it planned for future use? If it's dead code, consider removing it to improve maintainability.
95-100: Clarify hard-coded Python snippet purpose.The runtime initializes the KV store with hard-coded Python code (
"a=10;b='str';f'{a}{b}'"). For an example/demo, this might be appropriate, but consider:
- Documenting why this code is hard-coded
- Making it configurable via command-line or environment variable
- Adding a comment explaining what this snippet is testing
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.github/workflows/ci.yaml(1 hunks)example_projects/wasm32_without_js/.gitignore(1 hunks)example_projects/wasm32_without_js/README.md(1 hunks)example_projects/wasm32_without_js/rustpython-without-js/Cargo.toml(1 hunks)example_projects/wasm32_without_js/rustpython-without-js/src/lib.rs(1 hunks)example_projects/wasm32_without_js/wasm-runtime/.gitignore(1 hunks)example_projects/wasm32_without_js/wasm-runtime/Cargo.toml(1 hunks)example_projects/wasm32_without_js/wasm-runtime/README.md(1 hunks)example_projects/wasm32_without_js/wasm-runtime/src/main.rs(1 hunks)wasm/wasm-unknown-test/Cargo.toml(0 hunks)wasm/wasm-unknown-test/src/lib.rs(0 hunks)
💤 Files with no reviewable changes (2)
- wasm/wasm-unknown-test/src/lib.rs
- wasm/wasm-unknown-test/Cargo.toml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-28T16:31:03.991Z
Learnt from: arihant2math
Repo: RustPython/RustPython PR: 5790
File: build.rs:2-2
Timestamp: 2025-06-28T16:31:03.991Z
Learning: In Cargo build scripts (build.rs), the environment variable CARGO_CFG_TARGET_OS is guaranteed to exist and is automatically set by Cargo during the build process, making unwrap() safe to use when accessing this variable.
Applied to files:
example_projects/wasm32_without_js/.gitignore
🧬 Code graph analysis (2)
example_projects/wasm32_without_js/wasm-runtime/src/main.rs (1)
example_projects/wasm32_without_js/rustpython-without-js/src/lib.rs (3)
kv_get(3-4)kv_put(7-7)
example_projects/wasm32_without_js/rustpython-without-js/src/lib.rs (1)
example_projects/wasm32_without_js/wasm-runtime/src/main.rs (3)
kv_get(13-41)kv_put(44-54)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: Run snippets and cpython tests (windows-latest)
- GitHub Check: Run snippets and cpython tests on wasm-wasi
- GitHub Check: Check Rust code with clippy
- GitHub Check: Check the WASM package and demo
- GitHub Check: Run snippets and cpython tests (macos-latest)
- GitHub Check: Run snippets and cpython tests (ubuntu-latest)
- GitHub Check: Run tests under miri
- GitHub Check: Run rust tests (windows-latest)
- GitHub Check: Run rust tests (ubuntu-latest)
- GitHub Check: Ensure compilation on various targets
- GitHub Check: Run rust tests (macos-latest)
- GitHub Check: auto_format
🔇 Additional comments (6)
example_projects/wasm32_without_js/.gitignore (1)
1-2: Standard Rust workspace ignore patterns look appropriate.The patterns correctly ignore build artifacts (
target/) and lock files (Cargo.lock) at the crate level within the workspace, which aligns with typical Rust project conventions for example workspaces.Optional verification: Confirm that the workspace structure matches the pattern depth (one level of subdirectories). If the workspace root itself generates
target/orCargo.lockartifacts that need ignoring, add corresponding root-level patterns (target/andCargo.lockwithout the*/prefix). If those are handled by a parent.gitignorein the RustPython repository root, no further action is needed.example_projects/wasm32_without_js/rustpython-without-js/Cargo.toml (1)
1-4: Verify that usingedition = "2024"matches your toolchain and workspaceThis crate is set to Rust 2024 edition. Please confirm CI and local toolchains actually support this edition and that it’s consistent with the rest of the workspace; otherwise you may want to stick to the edition used by the main crates (commonly 2021 today) to avoid surprise compilation issues.
example_projects/wasm32_without_js/rustpython-without-js/src/lib.rs (1)
51-60:__getrandom_v03_customhas attribute and behavioral issuesThe good news: the current
extern "Rust"ABI is correct pergetrandom0.3 documentation.The remaining issues:
#[unsafe(no_mangle)]is invalid syntax; should be#[no_mangle].- Returns
Ok(())without writing todest, leaving the buffer uninitialized (UB).Fix:
-#[unsafe(no_mangle)] -unsafe extern "Rust" fn __getrandom_v03_custom( +#[no_mangle] +unsafe extern "Rust" fn __getrandom_v03_custom( _dest: *mut u8, _len: usize, ) -> Result<(), getrandom::Error> { - // Err(getrandom::Error::UNSUPPORTED) - - // WARNING: This function **MUST** perform proper getrandom - Ok(()) + // TODO: implement proper randomness for wasm32 + Err(getrandom::Error::UNSUPPORTED) }Likely an incorrect or invalid review comment.
.github/workflows/ci.yaml (1)
412-412: Verify build profile mismatch is intentional.The wasm-runtime is built with
--releasebut loads the wasm artifact from thedebug/directory. This mismatch may be intentional for testing, but please confirm this is the desired behavior.example_projects/wasm32_without_js/wasm-runtime/src/main.rs (1)
1-9: LGTM!The structure and imports are well-organized. The
Ctxstruct appropriately usesOption<Memory>since the memory reference is established after WASM module instantiation.example_projects/wasm32_without_js/wasm-runtime/Cargo.toml (1)
7-7: wasmer 6.1.0 is current and secure.Verification confirms that version
6.1.0is the latest available version on crates.io. The only known security vulnerability (symlink sandbox bypass) affects versions<= 4.3.1and does not impact6.1.0. No action required.
Sorry, something went wrong.
Co-Authored-By: Valentyn Faychuk <valy@faychuk.com> Co-Authored-By: Lee Dogeon <dev.moreal@gmail.com>
1a92d4f to
9134cca
Compare
November 17, 2025 12:30
bab03bd
into
RustPython:main
Nov 17, 2025
Fix #6281
Thanks @valentynfaychuk and @moreal !
Summary by CodeRabbit
New Features
Documentation
Chores