Skip to content

Flaky suite: two tests write the process-global DEPLOYMENT_SUITE, so one reads the other's value #36

Description

@thedavidmeister

RainDeployBroadcastTest writes the process-global DEPLOYMENT_SUITE from two
tests, so one test can read the other's value.

vm.setEnv sets the forge process' environment. It is not scoped to a test,
a contract, or an EVM snapshot — there is no cheatcode that unsets or restores
it, and forge runs tests concurrently. Two tests in
test/src/abstract/RainDeployBroadcast.t.sol write the same variable:

  • line 34, testRunUnknownSuiteRevertsBeforeReadingTheKey: vm.setEnv("DEPLOYMENT_SUITE", "address-registry")
  • line 50, testRunUnsetSuiteReverts: vm.setEnv("DEPLOYMENT_SUITE", "")

and both then call sDeploy.run(), which reads it back through
vm.envOr("DEPLOYMENT_SUITE", string("")). Whichever write lands last is what
both reads see.

Observed once on 2026-08-14-issue-32-offline-gate, running
forge test --no-match-contract Chain:

[FAIL: Error != expected error:
 UnknownDeploymentSuite("address-registry", "address-registry-0-0-1, second-address, address-registry-candidate")
 != UnknownDeploymentSuite("", "address-registry-0-0-1, second-address, address-registry-candidate")]
 testRunUnsetSuiteReverts()

That is testRunUnsetSuiteReverts reading the other test's write, which is
the proof that the interleaving is real rather than theoretical.

Roughly 1 in 27 — 26 subsequent runs across default threading, --threads 1
and deliberate CPU contention were all green, and the trigger is unidentified.
So this cannot be chased by reproduction and a green suite says nothing: the
fix is structural. Remove the sharing.

Why it matters

Every consumer of this repo runs this suite, and main going red for reasons
unrelated to the change under test is how a red stops meaning anything. A 1-in-27
failure is frequent enough to be hit and rare enough to be re-run away, which is
the worst combination.

The constraint on the fix

Both tests assert something real, and neither may be weakened to make the flake
go away:

  • testRunUnknownSuiteRevertsBeforeReadingTheKey — the suite is resolved from
    DEPLOYMENT_SUITE and a bad one fails naming every valid suite, BEFORE
    DEPLOYMENT_KEY is read. A test that stops setting the variable stops
    asserting that the value flows from that name.
  • testRunUnsetSuiteReverts — an unset suite is an unknown suite, never a
    default. A deploy that picks something when told nothing is how the wrong
    contract reaches a chain.

Note that the second one does not currently assert what it claims: vm.setEnv( "DEPLOYMENT_SUITE", "") sets a variable that is present and empty, so
vm.envOr's default is never reached and a change of that default to a real
suite key would pass. The UnknownDeploymentSuite("") payload it asserts is
already covered by RainDeploySuitesBaseTest.testEmptySuiteIsUnknown against
suiteByName("") directly.

The fix must show, by mutation, that the surviving test(s) still flip pass→fail
on: the envOr default becoming a suite key, the variable name changing, and
the key being read before the suite.

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions