LibRainDeploySnapshotTest tests race each other over committed src/lib files, and can zero one

[thedavidmeister]

Pre-existing on main at cafbd08, from PR #134. Found while reviewing #126;
#126 has the same defect in its own two new tests and fixes it there, this one
is not that PR's and is deliberately not widened into it.

The defect

Four tests in LibRainDeploySnapshotTest overwrite a COMMITTED source file,
read it back, then restore it from a before they captured at the top. They
come in two pairs, and each pair writes ONE path:

test	path it overwrites
testWriteAliasLibWritesTheLibAtItsPath (test/src/lib/LibRainDeploySnapshot.t.sol:547)	src/lib/LibAddressRegistryDeploy.sol
testWriteAliasLibDefaultsToTheOrgHeader (:1704)	src/lib/LibAddressRegistryDeploy.sol
testWriteReleasedSuitesLibWritesTheLibAtItsPath (:1135)	src/lib/LibAddressRegistryReleased.sol
testWriteReleasedSuitesLibDefaultsToTheOrgHeader (:1730)	src/lib/LibAddressRegistryReleased.sol

Forge runs the test functions in one contract concurrently. This repo states
that itself, at test/src/lib/LibRainDeploySnapshot.t.sol:563:

forge runs the tests in a contract concurrently, and two of them writing one
record root see each other's releases

which is why RELEASED_FIXTURE_ROOT, SELECTED_FIXTURE_ROOT and the two
FREEZE_* roots each have a directory of their own. These four have no
separation at all — they have no fixture directory, they use the live committed
path, and each pair uses the same one.

What goes wrong

Each test is a read-modify-write over a shared path with no ordering between
them:

1. A reads before.
2. B truncates the file and writes its own content.
3. A writes before back.

Step 1 landing between B's truncate and B's write hands A an EMPTY before,
and step 3 then commits that empty string to the committed file. The file is
zeroed in the working tree and the repo does not compile until it is restored
by hand — and the run that did it reports something else entirely, so nothing
points at the file that was destroyed.

The interleaving is also what makes the assertions themselves unsound: emitted == before in testWriteAliasLibWritesTheLibAtItsPath is comparing a read of a
file the other test may have rewritten in between.

Measurements

Measured on main at cafbd08, nix develop -c forge test --mt testWriteAliasLib, 100 runs, git checkout -- src/ between runs so each run
starts from a clean committed tree and one corruption cannot be counted twice.
The alias pair only; the released-lib pair is the same shape over a different
path and was not separately measured.

• 26 of 100 runs failed. 28 test failures across them (two runs failed both
  tests). Every run compiled and every run executed both tests — 200 test
  executions, 0 compile failures — so no failure here is a filter that matched
  nothing.
• 25 of the 28 are assertion failed: != // SPDX-License-Identifier: LicenseRef-DCL-1.0 — an EMPTY string read where the committed file should
  be, which is the interleaving above caught in the act.
• 3 of the 28 are the same comparison the other way round, one test seeing
  the other's content rather than nothing.
• 1 run in 100 left src/lib/LibAddressRegistryDeploy.sol zeroed on disk
  (run 2, size=0, restored by the harness before run 3). Left alone, the repo
  does not compile from that point on and every later run fails for a reason
  that has nothing to do with the file that was destroyed.

Both failing tests are in the same pair, so the rate is a property of the pair
and not of one test.

Severity

Value at risk is a developer's working tree and CI time, not a release: the
zeroing is loud (the repo stops compiling) and a forge test cannot reach the
release job, which runs script/Build.sol rather than the suite. What it does
cost is a suite that fails at random on work that has nothing to do with these
writers, and a committed source file that can be silently destroyed and then
committed by anyone who does not notice the dirty tree — including
src/lib/LibAddressRegistryDeploy.sol, which is where the deterministic deploy
address and code hash consumers pin are declared.

Proposed fix

Split the two things these tests assert, which is what #126 does for the
aggregate:

• "the committed file is what the generator emits" needs no write at all.
  It can read the committed path and compare against the emitter's expected
  string, exactly as testTheCommittedAggregateIsWhatTheGeneratorEmits on Generate the released-suites aggregate instead of writing it by hand #126
  already does. That removes the write, and with it the race, from the half of
  the assertion that is about the committed file.
• "the writer lands the file at its path" and "the defaulting arity
  writes what the parameterised one writes" need a fixture directory the test
  owns, one per test. writeAliasLib and writeReleasedSuitesLib currently
  write to pathForLib(libraryName), which takes no directory and is always
  LIB_DIR, so they need the libDir parameter writeReleasedSuitesAggregate
  already has.

The fixture directory must NOT be under src/ or test/. Both are compiled,
and a generated lib left behind under either — which is what a failure
deliberately leaves — fails the whole build, because its sibling and
../abstract/ imports resolve from src/lib and nowhere else. #126 adds a
fixture-lib root and the fs_permissions entry for it for exactly this
reason; these tests can use the same root with their own subdirectories.