Add --stale to skip the contract wasm prebuild in sandbox recipes#525
Merged
Conversation
`just test-sandbox --stale` (also accepted by `just test` and `just sandbox-up`) reuses the wasms already in `target/near` instead of rebuilding all fourteen of them on every run. The flag sets `TEST_CONTRACTS_PREBUILT` on the way in, so no second variable is needed: `sandbox-up.sh` now either establishes that invariant by building, or verifies it with a new `prebuild-test-contracts --check` mode that reports missing artifacts and fails before booting any nodes. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary
Human review focus
WalkthroughChangesPrebuilt contract workflow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Justfile
participant sandbox_up
participant prebuild_script
participant prebuild_cli
Justfile->>sandbox_up: Set TEST_CONTRACTS_PREBUILT for --stale
sandbox_up->>prebuild_script: Invoke with --check
prebuild_script->>prebuild_cli: Forward --check
prebuild_cli->>prebuild_cli: Verify expected target/near Wasms
prebuild_cli-->>sandbox_up: Return success or failure
Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@contract/artifacts/src/prebuild.rs`:
- Around line 69-92: Add unit tests targeting check_all that cover every
artifact existing, one artifact missing, and an artifact path that exists but is
not a file. Use temporary workspace/target fixtures and the existing artifact
metadata/path helpers, assert the corresponding Result, and run the narrowest
templar-contract-artifacts test command.
- Around line 69-92: Update check_all to validate each computed artifact path
with is_file() instead of exists(), so directories and other non-regular paths
are rejected. Revise the missing prebuilt artifact diagnostic to state that the
expected path is absent or not a regular file, while preserving the existing
missing flag and preflight failure behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 801f3513-94cb-427a-90ce-57b487094fd4
📒 Files selected for processing (8)
AGENTS.mdcontract/artifacts/README.mdcontract/artifacts/src/prebuild.rscontract/artifacts/src/prebuild/tests/args.rsdocs/src/testing.mdjustfilescript/prebuild-test-contracts.shscript/sandbox-up.sh
`Path::exists()` is also true for a directory, which would pass the stale-mode preflight and only fail later when the harness tried to load it. `is_file()` fails fast instead. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
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.
just test-sandboxrebuilds all fourteen contract wasms on every run, which is pure overhead when you are iterating on non-contract code.--stalereuses whatever is already intarget/near:just test-sandbox --stale -p templar-market-contract just test --stale just sandbox-up --staleHow it works
No new environment variable.
TEST_CONTRACTS_PREBUILTalready means "the prebuilt wasms are intarget/near" — it is whatsandbox-up.shexports sogateway/testing/src/wasm.rsreads artifacts instead of building them in-test.--stalejust sets it on the way in, andsandbox-up.sh's job becomes: establish that invariant (build), or verify it when the caller asserts it already holds.Verification is a new
prebuild-test-contracts --checkmode: it stats everyArtifactId::ALLentry'starget/nearpath and reports the missing ones, so a--stalerun with incomplete artifacts fails immediately instead of panicking mid-test after the ~30s node boot. The script echoes when it takes the verify path, so an ambientTEST_CONTRACTS_PREBUILT=1in someone's shell is visible rather than a silent behavior change.Also drops dead code in
script/prebuild-test-contracts.sh: a sourced-vs-executed guard and a trailingexportthat nothing could observe (no caller sources that script).Verification
just test-sandbox --stale -p templar-registry-contract— no builds, 3/3 passtarget/near/mock_ftmoved aside: exits 1 before node boot, naming the missing pathjust test-sandbox -p templar-registry-contract --test-threads 2— unchanged, still prebuilds, 2 nodes, 3/3 passjust sandbox-up --staleboots the pool with no rebuild;just sandbox-up --bogusexits 2cargo fmt,cargo clippy -p templar-contract-artifacts --features workspace-loader,clap --tests -- -D warningsclean, 38/38 crate tests pass🤖 Generated with Claude Code
This change is