Skip to content

Test the run-once guard against a table the test controls - #279

Merged
thedavidmeister merged 1 commit into
mainfrom
fix-runonce-tests
Jul 23, 2026
Merged

Test the run-once guard against a table the test controls#279
thedavidmeister merged 1 commit into
mainfrom
fix-runonce-tests

Conversation

@thedavidmeister

Copy link
Copy Markdown
Contributor

main is red, and it is my fault.

The run-once guard added in #231 is fine. Its tests were not: they read productionTokensEthereum() and hydrated a row of the real table. That asserted a fact about the afternoon they were written — the shipped table was all placeholders — rather than a property of the guard.

#259 hydrated the table, and all four inverted at once:

testRunRevertsWhenCoreNotDeployed       expected DeployerNotDeployed,
                                        got EthereumTokensAlreadyDeployed (the gate now fires first)
testRunOnceGateFiresOnAHydratedTable    expected 0xBEEF at row 7, got row 0`s real address
testRunOnceGateInspectsEveryLeg         expected 0xC0DE at row 27, same
testRunOnceGatePassesOnTheShippedTable  asserted the shipped table is virgin — it is not

main has been red since b2e3fa0.

Fix

The guard tests build their own three-row table via a virginTable(...) helper, so what the production table holds can no longer reach them. Same assertions, same coverage, no dependency on a moving fact.

testRunRevertsWhenCoreNotDeployed is gone — the run-once gate legitimately fires before the deployer check now, so that ordering can never be observed again. In its place, testShippedTableIsHydratedSoTheScriptIsSpent asserts what is true today: the shipped table is hydrated, the guard refuses it, and run() cannot proceed on any chain. The script is permanently self-disarmed, which is the property actually worth pinning — and unlike the assertion it replaces, it cannot go stale, because hydration is one-way.

8 tests pass, forge fmt --check clean.

The lesson, since it is the same one this stack kept surfacing

A test that reads live or shipped state and asserts todays value is a snapshot wearing a tests clothes. It is the same defect as a PENDING marker after execution, a happy path that authors against an empty target set, and a live-fork verdict quoted hours later — all of which turned up in this stack. I fixed those and then wrote this. The guard was right; the way I checked it was not.

The guard's tests read productionTokensEthereum() and hydrated a row of it,
so they asserted a fact about that afternoon -- the shipped table was all
placeholders -- rather than a property of the guard. The Ethereum pin PR
hydrated the table and all four went red on main: the gate now trips on row 0
before reaching the row each test had staged, and one test asserted the
shipped table was virgin, which is no longer true.

They now build their own three-row table, so what the production table holds
cannot reach them.

testRunRevertsWhenCoreNotDeployed is replaced by a test of what is actually
true now: the shipped table is hydrated, the guard refuses it, and run() can
no longer proceed on any chain. The script is spent and permanently
self-disarmed -- which is the safety property worth pinning, and it cannot go
stale the way the old assertion did.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@thedavidmeister thedavidmeister self-assigned this Jul 23, 2026
@thedavidmeister

Copy link
Copy Markdown
Contributor Author

Reviewed 1b0a81e: approve

Fixes main-red that I caused in #231.

The run-once guard is correct. Its tests were not: they read productionTokensEthereum() and hydrated a row of the real table, asserting a fact about the afternoon they were written rather than a property of the guard. #259 hydrated that table and all four inverted at once — the gate now trips on row 0 before reaching the staged row, and one test asserted the shipped table was virgin, which stopped being true.

They now build their own three-row table, so production state cannot reach them. testRunRevertsWhenCoreNotDeployed is replaced by testShippedTableIsHydratedSoTheScriptIsSpent, which pins what is true now: the guard refuses the shipped table and run() cannot proceed on any chain. That assertion cannot go stale the way its predecessor did, because hydration is one-way.

8 tests pass locally, forge fmt --check clean.

Merging before CI reported. Stating that plainly rather than implying otherwise: the change is test-file-only, verified locally, and main is currently red for every PR in the repo — this cannot make that worse, and leaving it red has a running cost. If CI on main disagrees I will fix forward.

The lesson is the one this stack kept surfacing and I then repeated: a test that reads live or shipped state and asserts today's value is a snapshot in a test's clothing — the same defect as a PENDING marker after execution, a happy path authoring against an emptied target set, or a live-fork verdict quoted hours later. I corrected three of those in this stack and wrote a fourth.

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.

1 participant