chore: stop tracking test snapshots; regenerate locally instead - #84
Merged
Conversation
Committed Soroban test snapshots are derived artifacts (cargo test regenerates them from any commit) and are not byte-reproducible across runs, so they churned on every test run and inflated PR diffs past the limits of the security scanners: on PR #82 both Almanax (diff too large) and CodeRabbit (198 files) skipped review of a HIGH-severity contract change because of snapshot noise. Gitignore test_snapshots/, untrack the 238 committed files (kept on disk locally), and update the CLAUDE.md hard rule so contributors inspect regenerated snapshot diffs during review instead of committing them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (244)
Disabled knowledge base sources:
📝 WalkthroughWalkthroughThe change marks ChangesSnapshot policy
Estimated code review effort: 1 (Trivial) | ~5 minutes Poem
✨ 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 |
Shadow-MMN
added a commit
to Shadow-MMN/boundless-contract
that referenced
this pull request
Jul 18, 2026
Resolved snapshot file conflicts by accepting upstream deletion (snapshots are gitignored per repo convention). Upstream changes: - chore: stop tracking test snapshots; regenerate locally instead (boundlessfi#84) - test(admin): assert admin auth is actually required, not just mocked (boundlessfi#78) - fix(events): harden cancellation liveness (boundlessfi#82)
0xdevcollins
added a commit
to Michaelkingsdev/boundless-contract
that referenced
this pull request
Jul 20, 2026
…rror-enum cap Brings the two-step manager delegation up to current testnet (boundlessfi#90 pull-model claims, boundlessfi#84 snapshots) and makes it build and pass there. Error enum: testnet is at the 50-case contracterror cap, so a new PendingManagerMismatch variant would not compile. Instead generalize the two admin-rotation variants — PendingAdminMismatch/PendingAdminExpired -> PendingRotationMismatch/PendingRotationExpired (discriminants 12/13 unchanged, so no on-chain code change) — and share them across both two-step rotations. accept_manager now distinguishes no-pending (Mismatch) from expired (Expired), which is more precise than the original single-variant usage. Conflict resolution: - types.rs / storage.rs / event_ops.rs: PendingManager keys and helpers ordered after the 1.3.0 prize keys; append-only, no reorder. - cross_contract.rs: kept the new manager suite; the accepted-manager test now claims the prize before asserting Completed, since under the pull model select_winners records rather than pays. Verified: cargo test 216 events + 66 profile green; make build OK (events wasm 56,202 bytes, < 64 KB); fmt + clippy clean.
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.
Summary
Committed Soroban test snapshots are derived artifacts —
cargo testregenerates them from any commit — and they are not byte-reproducible across runs (BytesN::randomvalues differ per run), so they churned on every local test run and inflated PR diffs past the limits of the review tooling. On #82 both Almanax ("Diff Too Large for Analysis") and CodeRabbit (198-file limit) skipped scanning a HIGH-severity contract change because ~17k of its ~17.5k diff lines were snapshot noise.Changes
test_snapshots/to.gitignorecargo test)Verification
cargo test --allon the branch: 197 events + 66 profile tests pass🤖 Generated with Claude Code
Summary by CodeRabbit