Skip to content

Fix the four tests whose premise the first real release expires - #143

Merged
thedavidmeister merged 1 commit into
mainfrom
2026-08-19-fix-release-blocking-tests
Aug 19, 2026
Merged

Fix the four tests whose premise the first real release expires#143
thedavidmeister merged 1 commit into
mainfrom
2026-08-19-fix-release-blocking-tests

Conversation

@thedavidmeister

@thedavidmeister thedavidmeister commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Unblocks the sol-v0.1.6 release re-run. The tag's Package Release run
https://github.com/rainlanguage/rain.deploy/actions/runs/32175800185 failed at
forge test AFTER cutRelease() had already succeeded: nothing was published
or committed back, so main still says version = "0.1.5" and src/generated/
still holds only candidate/.

All four failures are one class — tests that only pass while this repo has
never cut a release
. The first real freeze expired their premise. None of
them is a defect in the release machinery: the real-record check
RegistryDeploySnapshotTest.testEveryFrozenSnapshotIsReleased PASSED
post-freeze in that same run (log 19:19:19.4325, gas 137925), so the frozen
record and the generated declaration agreed and cutRelease() did exactly what
it is supposed to.

Test-only in effect: no tag is touched, nothing is published, and neither the
foundry.toml version bump nor a frozen src/generated/0_1_6/ is committed
here. The one src/ change is the split in #1 below, which the fix requires.

1. RainDeployVerifySnapshotTest.testEveryFrozenSnapshotIsReleasedFrozenSnapshotNotReleased("src/generated/0_1_6/MigrationRegistry.sol")

Mechanism. RainDeployVerifySnapshotTest was
is ExampleDeploySuites, RainDeployVerifySnapshot. The inherited
testEveryFrozenSnapshotIsReleased walks LibRainDeploySnapshot.LIB_FS_ROOT
this repo's REAL record — and checks it against releasedSuites(), which the
harness overrides with fixtures. Pre-release the walk was empty, so the test
passed vacuously. Post-freeze it found the real 0_1_6/ files, and the exemplar
declares neither: AddressRegistry matched only BY ACCIDENT, because the
fixture imports the real candidate AddressRegistry constants whose creation
code derives the frozen file's address. MigrationRegistry is not in the
exemplar set at all, so it reverted.

The defect is a fixture harness inheriting a real-record test, not the test.

Fix — the abstract is split, and the harness inherits the narrower half.

  • src/abstract/RainDeployVerifySnapshotBase.sol (new) holds the five errors,
    all three check functions and the two tests whose subject is the inheriting
    contract's own declaration: testSnapshotInternallyConsistent and
    testSnapshotMatchesSource.
  • src/abstract/RainDeployVerifySnapshot.sol is now that base plus
    testEveryFrozenSnapshotIsReleased, and is unchanged in name, in what it
    offers and in how a consumer inherits it. The README's consumer story —
    contract MyDeploySnapshotTest is MyDeploySuites, RainDeployVerifySnapshot {}
    — is untouched, and so is RegistryDeploySnapshotTest: a non-virtual walk
    of the single LIB_FS_ROOT spelling, against this repo's real
    releasedSuites(). That binding is exactly as strong as it was, which the QA
    below proves in the state that broke CI rather than asserting.
  • test/src/abstract/RainDeployVerifySnapshot.t.sol
    RainDeployVerifySnapshotBase.t.sol, contract RainDeployVerifySnapshotTest
    RainDeployVerifySnapshotBaseTest, inheriting the base. Every one of its 24
    cases is unchanged; the record CHECK is still exercised there at every
    position and against every shape of declaration, because
    checkFrozenSnapshotsReleased takes the record as an argument. Only the
    BINDING to the real record went. The rename follows the tree convention: the
    file now tests the base, and RainDeployVerifySnapshot.sol has no fixture
    mirror for the same reason RainDeployVerifyBase.sol does not.
  • slither.config.json gains the new file by exact filename, per CLAUDE.md.

Why this shape.

The two alternatives were considered and both are worse:

  • A virtual root or a virtual test to override. The NatSpec on
    testEveryFrozenSnapshotIsReleased exists to defend the single spelling of
    the root: pointed anywhere else, the walk returns empty forever, the test
    passes with no subject, and the one check standing between a dropped release
    and a green suite is inert. A hook is that failure mode with a supported
    spelling. Inheriting a narrower contract is a choice a reader sees in the
    inheritance list; an emptied override is one they do not.
  • Give the harness a released set that covers the real record. It cannot be
    done durably. releasedSuites() is internal pure — it cannot read the
    record — so the only durable spelling is to concatenate the generated
    LibReleasedSuites.releasedSuites() into the exemplar. That contradicts
    ExampleDeploySuites's own stated design ("what the abstracts are tested
    against is written here in full, and adding or removing a contract in
    src/concrete/ does not change it"), and it is inherited by
    RainDeployVerifyChainTest, which would then fork seven real networks
    asserting this repo's real deployments from a fixture contract. A hand-listed
    0_1_6 entry rots at 0.1.7.

2 & 3. LibRainDeploySnapshotTest.testFreezeRefusesAnEmptyRelease and testFreezeLeavesNothingBehindWhenThereIsNothingToFreeze — both SnapshotAlreadyFrozen("0_1_6", "src/generated/0_1_6")

Mechanism. Both drove freeze through externalFreeze, which froze into
the REAL LIB_FS_ROOT under the real deployTag(vm). SnapshotAlreadyFrozen
is checked before either guard under test, so once src/generated/0_1_6/
exists it preempts them and the tests report a refusal that is not the one they
are about.

Fix. Both now go through externalFreezeAt against a fixture root of their
own — test/generated-freeze-empty and test/generated-freeze-nothing — which
is the precedent already set by FREEZE_FIXTURE_ROOT,
FREEZE_MULTI_FIXTURE_ROOT, RECUT_FIXTURE_ROOT and friends, and for the same
documented reason: every freeze test cuts the SAME tag, so two sharing a root
would have whichever ran second refused as a re-cut. The real deployTag(vm)
read is kept — it is the tag a freeze is about, and it is the ROOT that expired,
not the tag.

The guards they pin are unchanged and still real: EmptyRelease,
NothingToFreeze, and the write ordering that keeps a failed release retryable.
testFreezeLeavesNothingBehindWhenThereIsNothingToFreeze is slightly stronger
than it was: its fixture root holds a rolling snapshot for another contract, so
the freeze reaches its LAST guard with only the named contract's file missing,
rather than failing at a root with nothing under it at all.

externalFreeze — the real-root wrapper — had no callers left and is deleted.
No test freezes into src/generated/ any more, which is the point: a test that
can only be pointed at the real record can only be run against a record it must
not leave a release in.

4. RegistryDeployChainTest.testChainWithNothingToCheckForksNothing2 != 0

Mechanism. It asserted releasedSuites().length == 0 as its premise. Its
own NatSpec said the first release makes it fail and that failing is the check
working.

Fix — deleted, per the ruling that there is no point testing that something
has not been released.
RegistryDeployChainTest is now {}, like
RegistryDeploySnapshotTest, and its NatSpec no longer rests on "it has
released none".

The property it pinned is re-homed, not dropped. "The matrix forks nothing
when there is nothing to check" was pinned NOWHERE against a fixture — checked
before deleting. RainDeployVerifyChainTest.testChainWithASingleSubjectDoesFork
pins only the positive side (one subject → the matrix forks), and its own
NatSpec named RegistryDeployChainTest's empty case as the other half of that
argument. So it now lives in
test/src/abstract/RainDeployVerifyChainEmpty.t.sol:
RainDeployVerifyChainEmptyTest declares an empty released set and one
candidate, and runs the whole inherited entry point — so the derivation is
inside what is asserted — checking that no fork was selected. Its own contract
for the reason RainDeployVerifyChainCandidateTest is its own contract: the
suites a contract inherits are the whole of what the matrix runs over, so a
second scope is a second contract. The cross-reference in
RainDeployVerifyChain.t.sol now points at it.

Declared empty rather than observed empty is the whole difference: a deploy
repo's declaration is empty exactly once, before its first release, so a test
resting on that stops asserting anything the day the repo does the thing it
exists to do — which is precisely what happened here.

Not changed

script/Build.sol, src/abstract/BuildScript.sol and
LibRainDeploySnapshot.freeze are untouched. cutRelease() worked as designed.

QA

  • Discriminating tests: LibRainDeploySnapshotTest.testFreezeRefusesAnEmptyRelease, LibRainDeploySnapshotTest.testFreezeLeavesNothingBehindWhenThereIsNothingToFreeze, testEveryFrozenSnapshotIsReleased as bound by the fixture harness, and RainDeployVerifyChainEmptyTest.testChainWithNothingToCheckForksNothing (re-homing the deleted RegistryDeployChainTest case) - each fails on base in the 0.1.6 release state, verified by the four logged failures of tag run 32175800185; Run 2 below is that same state on this branch, green.
  • Mutations applied: M1 RainDeployVerifyChain.checkDeployedOnSupportedNetworks if (derived.length == 0) return; -> deleted -> killed by RainDeployVerifyChainEmptyTest.testChainWithNothingToCheckForksNothing; M2 freeze EmptyRelease guard -> deleted -> killed by testFreezeRefusesAnEmptyRelease; M3 freeze -> create <tag>/ before the read loop -> killed by testFreezeLeavesNothingBehindWhenThereIsNothingToFreeze; M4 (release state) LibMigrationRegistryReleased.releasedSuites() -> blanked to new DeploySuite[](0) -> killed by RegistryDeploySnapshotTest.testEveryFrozenSnapshotIsReleased with the exact CI error. Details in the mutation pass below.
  • Oracle: the tag run's own log (the four failures and the verbatim FrozenSnapshotNotReleased("src/generated/0_1_6/MigrationRegistry.sol")) plus the guards' NatSpec-stated semantics (EmptyRelease, NothingToFreeze, nothing-left-behind ordering) - expected outcomes derived from the release lifecycle, independent of the code under test.
  • Category check: the failed run asks for its four failures fixed; covered 1, 2, 3, 4 below. Their shared class - tests that only pass while the repo has never cut a release - is swept beyond the four instances: Run 2 executes the ENTIRE suite in the release state, where any remaining premise-expired test would fail, and it is green.

Both acceptance runs are below, in both tree states, plus a mutation pass on
every test this PR moves or adds. nix develop -c throughout.

Run 1 — clean tree, what main CI runs

nix develop -c forge test

Ran 21 test suites in 22.73s (90.59s CPU time): 332 tests passed, 0 failed, 0 skipped (332 total tests)

Run 2 — release state, what the tag run hits

foundry.toml [external.package].version0.1.6, then
nix develop -c bash -c 'forge script ./script/Build.sol --sig "cutRelease()" && forge fmt',
then nix develop -c forge test. This is the exact state that produced the four
failures.

First attempt: 327 passed, 5 failed — all five failures were
vm.createSelectFork transport errors from the public HYPEREVM RPC that
.env.example points at (invalid block height / block not found), all in
the two live-fork chain suites and none in a test this PR touches; CI has a
dedicated archive-RPC secret for exactly this. A rerun of those five passed,
and a full re-run of the whole suite in the same release state is green:

Ran 21 test suites in 21.94s (92.98s CPU time): 332 tests passed, 0 failed, 0 skipped (332 total tests)

RegistryDeploySnapshotTest.testEveryFrozenSnapshotIsReleased in this state has
a real subject — the two files cutRelease() froze — rather than the empty walk
it gets on a clean tree.

Reverted afterwards: the version bump, src/generated/0_1_6/ and every
regenerated lib. git status is clean except the changes in this PR.

Mutation pass

Each mutant is applied to the unmutated baseline committed on this branch, run,
and reverted. Every one is killed by the test it is aimed at.

  • M1 — delete if (derived.length == 0) return; in
    RainDeployVerifyChain.checkDeployedOnSupportedNetworks
    RainDeployVerifyChainEmptyTest.testChainWithNothingToCheckForksNothing
    FAILs ("the matrix forked a network with nothing to check"). The re-homed
    property is load-bearing.
  • M2 — delete the EmptyRelease guard in freeze
    testFreezeRefusesAnEmptyRelease FAILs ("next call did not revert as
    expected").
  • M3 — create <tag>/ before the read loop in freeze
    testFreezeLeavesNothingBehindWhenThereIsNothingToFreeze FAILs ("assertion
    failed").
  • M4 — in the RELEASE state, blank the regenerated
    LibMigrationRegistryReleased.releasedSuites() back to
    new DeploySuite[](0) and run
    --match-contract "RegistryDeploySnapshotTest|RainDeployVerifySnapshotBaseTest"
    RegistryDeploySnapshotTest.testEveryFrozenSnapshotIsReleased FAILs with
    FrozenSnapshotNotReleased("src/generated/0_1_6/MigrationRegistry.sol")
    the exact error from the CI run, now raised by the contract that should
    raise it — while RainDeployVerifySnapshotBaseTest stays green (24 passed).
    27 tests ran: 26 passed, 1 failed. The split left the real-record binding
    exactly as strong as it was.

Static gate

nix develop -c pre-commit run --all-files

Every hook passes on this branch's files. One finding, and it is not from this
PR: denofmt rewraps one paragraph of CLAUDE.md that is byte-identical to
main — pre-existing drift on a file this branch does not touch. The rewrap
was reverted, not committed; CI's actual static job (rainix-sol-static:
slither, forge fmt --check and the org convention checks) does not run
denofmt and is green on the same content on main.

Summary by CodeRabbit

  • New Features

    • Added reusable snapshot verification for deployed addresses, bytecode hashes, runtime hashes, and release status.
    • Added validation that frozen snapshots match their source records.
    • Added coverage for scenarios with no released suites, avoiding unnecessary network forks.
  • Bug Fixes

    • Improved snapshot fixture handling, including missing-contract cases and temporary-directory cleanup.
  • Documentation

    • Clarified snapshot verification responsibilities and updated test explanations for empty and multi-network scenarios.

Split RainDeployVerifySnapshot so the fixture harness inherits the checks
without the real-record binding, re-point the two freeze guards at fixture
roots, and re-home the empty-matrix property against a declared-empty fixture.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@thedavidmeister thedavidmeister self-assigned this Aug 19, 2026
@coderabbitai

coderabbitai Bot commented Aug 19, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: b9613e4d-ea77-4aeb-9add-31548bd09424

📥 Commits

Reviewing files that changed from the base of the PR and between 0d18d1b and e7409bf.

📒 Files selected for processing (8)
  • slither.config.json
  • src/abstract/RainDeployVerifySnapshot.sol
  • src/abstract/RainDeployVerifySnapshotBase.sol
  • test/src/abstract/RainDeployVerifyChain.t.sol
  • test/src/abstract/RainDeployVerifyChainEmpty.t.sol
  • test/src/abstract/RainDeployVerifySnapshotBase.t.sol
  • test/src/abstract/RegistryDeployChain.t.sol
  • test/src/lib/LibRainDeploySnapshot.t.sol

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


Walkthrough

The change adds a reusable snapshot verification base, narrows the repository consumer contract, updates chain and filesystem fixtures, and preserves the existing Slither filter value.

Changes

Snapshot verification and validation

Layer / File(s) Summary
Reusable snapshot verification base
src/abstract/RainDeployVerifySnapshotBase.sol
Adds suite consistency checks, snapshot address parsing, released-suite matching, and source anchoring tests.
Consumer contract and base tests
src/abstract/RainDeployVerifySnapshot.sol, test/src/abstract/RainDeployVerifySnapshotBase.t.sol
Moves shared assertions into the base contract. The consumer retains repository frozen-record validation.
Empty chain verification fixture
test/src/abstract/RainDeployVerifyChainEmpty.t.sol, test/src/abstract/RainDeployVerifyChain.t.sol, test/src/abstract/RegistryDeployChain.t.sol
Adds a no-release fixture that verifies no network fork occurs and updates related documentation.
Root-aware snapshot fixtures
test/src/lib/LibRainDeploySnapshot.t.sol
Uses dedicated fixture roots, root-aware paths, rolling snapshots, and cleanup checks.
Slither configuration
slither.config.json
Rewrites the filter_paths entry without changing its value or behavior.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: ⚪ Minimal · up to e7409

This test-only change separates fixture validation from real release-record validation and isolates freeze-test roots; the supplied clean-tree and post-release runs pass all 332 tests, so no actionable merge-blocking risk remains after normal checks.

Possibly related PRs

Suggested labels: ai:ready

Suggested reviewers: claude

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: fixing four tests affected by the first real release.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 2026-08-19-fix-release-blocking-tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@thedavidmeister
thedavidmeister merged commit 23518d6 into main Aug 19, 2026
1 check 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.

2 participants