feat(rust-shell): include foundry-bin for anvil-based tests#172
Conversation
Rust crates using alloy's Anvil::new().try_spawn() (for local-EVM integration tests) need the anvil binary on PATH at test time. anvil ships with foundry-bin; adding it to rust-build-inputs makes Anvil::new() work in cargo test without dragging in the rest of the Solidity static-analysis toolchain. Closure test relaxed to assert only slither/solc absence (foundry now expected). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis PR adds foundry-bin to the Rust development environment, tightens closure test assertions to allow the new dependency while preventing Solidity static-analysis tool leaks, and adds a Bats test to verify the anvil command is available on PATH. ChangesFoundry-bin integration into Rust environment
Possibly related PRs
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/bats/devshell/rust-shell/anvil.test.bats (1)
1-4: 💤 Low valueSimple smoke test is adequate.
The test successfully verifies that
anvilis available on PATH and executes without error. For enhanced robustness, you could optionally verify the output contains "anvil" or a version pattern, but the current implementation is sufficient for the stated goal.Optional: More robust output verification
`@test` "anvil should be available on PATH" { run anvil --version [ "$status" -eq 0 ] + [[ "$output" =~ anvil ]] }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/bats/devshell/rust-shell/anvil.test.bats` around lines 1 - 4, The test "anvil should be available on PATH" currently runs `run anvil --version` and checks exit status only; to make it more robust, after `run anvil --version` add an assertion on `$output` (for example using `[[ "$output" =~ anvil|[0-9]+\.[0-9]+\.[0-9]+ ]]` or `[[ "$output" == *anvil* ]]`) so the test verifies the output contains "anvil" or a version pattern in addition to checking `$status` equals 0.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test/bats/devshell/rust-shell/anvil.test.bats`:
- Around line 1-4: The test "anvil should be available on PATH" currently runs
`run anvil --version` and checks exit status only; to make it more robust, after
`run anvil --version` add an assertion on `$output` (for example using `[[
"$output" =~ anvil|[0-9]+\.[0-9]+\.[0-9]+ ]]` or `[[ "$output" == *anvil* ]]`)
so the test verifies the output contains "anvil" or a version pattern in
addition to checking `$status` equals 0.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2e3ddaf0-4eb3-4e13-a3ae-6b87e3272afb
📒 Files selected for processing (3)
flake.nixtest/bats/devshell/rust-shell/anvil.test.batstest/bats/devshell/rust-shell/closure.test.bats
|
@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment: S/M/L PR Classification Guidelines:This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed. Small (S)Characteristics:
Review Effort: Would have taken 5-10 minutes Examples:
Medium (M)Characteristics:
Review Effort: Would have taken 15-30 minutes Examples:
Large (L)Characteristics:
Review Effort: Would have taken 45+ minutes Examples:
Additional Factors to ConsiderWhen deciding between sizes, also consider:
Notes:
|
|
Tip For best results, initiate chat on the files or code changes. SIZE=S |
1 participant
Summary
pkgs.foundry-bintorust-build-inputs. This makesanvilavailable on PATH insiderust-shelland the default shell.Anvil::new().try_spawn()for local-EVM integration tests incargo testneed anvil. Without it the spawn errorsOs { code: 2, kind: NotFound }. The slimrust-shellwas excluding the wholesol-build-inputsset, which dragged anvil out with it.test/bats/devshell/rust-shell/closure.test.batsupdated to allow foundry-bin while still rejecting slither/solc.anvil.test.batsasserting anvil is on PATH.Why option 2
Two ways to fix this were considered:
node-binding-inputsset carrying anvil and include it in rust-shell. Architectural — anvil is conceptually a node-binding test tool, distinct from sol-build (forge/slither/solc/reuse for actual Solidity work).pkgs.foundry-bindirectly torust-build-inputs. One-line change; brings forge/cast/chisel along but those are part of the same foundry-bin output so the closure cost is the same as just anvil.Going with 2 — same net cost, lower diff.
Test plan
rust-shell-test(closure + cargo-expand + anvil) greensol-shell-testdefault-shell-test🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests