Skip to content

ci: add contract snapshot verification and drift reporting - #901

Merged
thegreatfeez merged 2 commits into
Ac0rdP:mainfrom
abimbolaalabi:ci/snapshot-verification-815
Oct 18, 2026
Merged

thegreatfeez merged 2 commits into
Ac0rdP:mainfrom
abimbolaalabi:ci/snapshot-verification-815

Conversation

@abimbolaalabi

Copy link
Copy Markdown
Contributor

Comprehensive Implementation Summary: Snapshot Verification (#815)

📌 Issue Overview

Issue Reference: #815 / CI/CD & DevOps — Automated snapshot verification
Title: Add automated snapshot verification and diff reporting in CI
Goal: Make CI detect when committed Soroban test snapshots under contracts/accord/test_snapshots/ drift from a fresh test run, report a readable diff, document the update procedure, and keep the check from blocking PRs because of an existing stale baseline.


🏗️ Architecture & Core Design Decisions

1. Advisory CI mode, strict local mode

  • CI runs the check with SNAPSHOT_VERIFY_WARN_ONLY=1: drift is reported as a warning and the full patch is uploaded as an artifact. This satisfies the issue's "fail or warn" requirement without forcing every PR to refresh the already-stale deterministic baseline.
  • Local runs are strict by default and exit 1 on drift, so contributors catch stale snapshots before opening a PR.

2. Deterministic snapshots only

  • Soroban writes a snapshot for every Env dropped during a test. Property tests generate new random inputs on every run, so their snapshots can never match across runs.
  • Added non_snapshot_env() (EnvTestConfig { capture_snapshot_at_drop: false }) and used it in all five property-test functions.
  • Removed the 228 committed property-test snapshots so they cannot churn or be mistaken for a verifiable baseline.

3. Complete, readable drift reporting

  • The script reports modified, deleted, and newly created snapshots.
  • It writes target/snapshot-verification/status.txt and snapshots.patch; untracked snapshots are included in the patch via git diff --no-index.
  • CI uploads the report as the contract-snapshot-verification artifact and appends the changed-file list to the run summary.

4. Cross-platform stability

  • Added .gitattributes with contracts/accord/test_snapshots/** text eol=lf.
  • The script compares with --ignore-cr-at-eol, so Windows core.autocrlf conversion is not mistaken for drift.

5. No fighting the merge process

  • The check never rewrites or commits snapshots.
  • Test failures are owned by the existing Contract CI job; the snapshot job only owns drift and still detects snapshots written by failing tests.

📁 File-by-File Detailed Breakdown

1. .github/workflows/contract.yml (Modified)

  • Added the Contract Snapshot Verification job.
  • Runs scripts/verify-snapshots.sh with advisory mode enabled.
  • Uploads target/snapshot-verification as a 30-day artifact.

2. scripts/verify-snapshots.sh (New)

  • Runs cargo test -p accord --lib --no-fail-fast.
  • Supports an optional test-name filter for fast local checks.
  • Detects modified/deleted tracked snapshots and untracked new snapshots.
  • Writes a readable status list and patch.
  • Exits 1 on drift in strict mode; warns and exits 0 in SNAPSHOT_VERIFY_WARN_ONLY=1 mode.

3. contracts/accord/src/test.rs (Modified)

  • Added non_snapshot_env().
  • Replaced Env::default() with non_snapshot_env() in all five proptest functions:
    • approval_count_never_exceeds_owner_count
    • total_weight_matches_all_current_owner_weights
    • approval_weight_always_equals_sum_of_approved_weights
    • active_count_never_exceeds_max
    • role_members_index_matches_per_address_roles

4. .gitattributes (New)

  • Pins snapshot files to LF.

5. .gitignore (Modified)

  • Documents and ignores the property-test snapshot patterns.

6. CONTRIBUTING.md (Modified)

  • Replaced the old UPDATE_EXPECT guidance with the actual snapshot workflow.
  • Documents local strict mode, CI advisory mode, review-before-commit policy, and the property-test exception.

7. docs/CI.md (Modified)

  • Updated the workflow map to mention advisory snapshot drift reporting.

8. contracts/accord/test_snapshots/test/ (Deleted)

  • Removed 228 nondeterministic property-test snapshots (129 + 33 + 33 + 33).

✅ Verification & CI Results

Verification Type Command Result
Shell syntax bash -n scripts/verify-snapshots.sh Passed
Strict drift detection bash scripts/verify-snapshots.sh multi_transfer_success Exit 1, reported the 6-line stale snapshot diff
CI advisory mode SNAPSHOT_VERIFY_WARN_ONLY=1 bash scripts/verify-snapshots.sh multi_transfer_success Exit 0, emitted ::warning:: and wrote the patch
New snapshot detection Added an untracked probe snapshot and ran the script Detected, ? status and diff --no-index content in patch
No-drift path bash scripts/verify-snapshots.sh bench_baseline_helpers_roundtrip Passed: Snapshot verification passed
Property snapshot isolation Ran the full suite, then counted property snapshot files 0 files recreated
Full contract suite (informational) cargo test -p accord --lib --no-fail-fast 297 passed, 20 failed; failures are pre-existing on main (resource-limit/budget tests, RBAC expectation, missing error-table entry, benchmark regression)

The regular Contract CI job continues to own test failures. This PR's new job is intentionally advisory so it can surface the existing stale deterministic snapshots without blocking merges; the uploaded artifact contains the full diff.

⚠️ Known pre-existing issues

  • The full contract suite is red on main for 20 tests unrelated to snapshot verification (resource limits, RecoveryActive missing from docs/CONTRACT_API.md, RBAC expectation mismatch, benchmark regression).
  • cargo fmt --check -p accord reports existing formatting diffs in bench.rs, lib.rs, and test.rs; the code added in this PR is formatted.

Closes #815

@vercel

vercel Bot commented Oct 18, 2026

Copy link
Copy Markdown

@abimbolaalabi is attempting to deploy a commit to the thegreatfeez's projects Team on Vercel.

A member of the Team first needs to authorize it.

@drips-wave

drips-wave Bot commented Oct 18, 2026

Copy link
Copy Markdown

@abimbolaalabi Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@thegreatfeez
thegreatfeez merged commit d731b04 into Ac0rdP:main Oct 18, 2026
4 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add automated snapshot verification and diff reporting in CI

2 participants