From e4ba04267b1e6261a288c166ec816e5f6f2cd92f Mon Sep 17 00:00:00 2001 From: David Meister Date: Tue, 15 Sep 2026 15:48:43 +0000 Subject: [PATCH 1/4] test: run regenerateLibs into a fixture dir and pin what it emits `Build`'s generator hooks were executed by nothing: a `revert()` as the first statement of either one left the whole suite green. Every check on them was output-anchored -- the committed libs compared against the emitters -- so a wrong argument or a short loop bound was invisible until somebody re-ran the generator and committed the result. `regenerateLibs` now takes its directory from an overridable `libDir()`, for the reason `BuildScript.recordRoot` is overridable, and a harness drives it into `fixture-lib/`. A regeneration of a clean checkout is a no-op, so the committed tree is the oracle: every file the hook writes must be byte-identical to the one already there, and it must write no others. `regenerateSnapshots` gets no such seam, because `LibFs` confines every snapshot write to `src/generated//`, which is the record the suites running beside it walk. Closes #207 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01V8ViHcKLVk2YoS2joH4HdN --- script/Build.sol | 28 +++++--- test/concrete/BuildHarness.sol | 39 ++++++++++- test/script/Build.t.sol | 123 +++++++++++++++++++++++++++++++-- 3 files changed, 172 insertions(+), 18 deletions(-) diff --git a/script/Build.sol b/script/Build.sol index 8fd0585..295ee4f 100644 --- a/script/Build.sol +++ b/script/Build.sol @@ -59,28 +59,34 @@ contract Build is BuildScript, RegistryDeploySuites { return names; } + /// The directory every generated lib is written into. + /// + /// Overridable for the reason `BuildScript.recordRoot` is: a hook that can + /// only be pointed at the committed tree can only be RUN by overwriting + /// files the rest of the suite compiles and reads, and forge runs test + /// contracts in parallel. Only the per-contract libs and the aggregate go + /// here — `regenerateSnapshots` has no equivalent, because `LibFs` confines + /// every snapshot it writes to `src/generated/`. + /// @return The lib directory. + function libDir() internal view virtual returns (string memory) { + return LibRainDeploySnapshot.LIB_DIR; + } + /// @inheritdoc BuildScript /// @dev Every alias lib, every released-suites lib and the aggregate over /// them. function regenerateLibs() internal override { GeneratedContract[] memory contracts = generatedContracts(); + string memory dir = libDir(); for (uint256 i = 0; i < contracts.length; i++) { LibRainDeploySnapshot.writeAliasLib( - vm, - LibRainDeploySnapshot.LIB_DIR, - contracts[i].contractName, - contracts[i].constantPrefix, - LibRainDeploySnapshot.CANDIDATE + vm, dir, contracts[i].contractName, contracts[i].constantPrefix, LibRainDeploySnapshot.CANDIDATE ); LibRainDeploySnapshot.writeReleasedSuitesLib( - vm, - LibRainDeploySnapshot.LIB_DIR, - recordRoot(), - contracts[i].contractName, - contracts[i].candidate.snapshot + vm, dir, recordRoot(), contracts[i].contractName, contracts[i].candidate.snapshot ); } - LibRainDeploySnapshot.writeReleasedSuitesAggregate(vm, LibRainDeploySnapshot.LIB_DIR, snapshotContractNames()); + LibRainDeploySnapshot.writeReleasedSuitesAggregate(vm, dir, snapshotContractNames()); } /// @inheritdoc BuildScript diff --git a/test/concrete/BuildHarness.sol b/test/concrete/BuildHarness.sol index 4061a81..e7ddf97 100644 --- a/test/concrete/BuildHarness.sol +++ b/test/concrete/BuildHarness.sol @@ -5,15 +5,52 @@ pragma solidity =0.8.25; import {Build, GeneratedContract} from "../../script/Build.sol"; import {DeployCandidate} from "../../src/abstract/RainDeploySuitesBase.sol"; +/// Thrown when a harness left pointed at `Build`'s own lib directory is asked +/// to run the lib half of a build. That write lands on the committed libs the +/// rest of the suite compiles and reads, which forge runs in parallel with it. +error BuildHarnessWouldWriteTheCommittedLibs(); + /// @title BuildHarness /// @notice An external seam onto the two internal declarations `BuildTest` -/// compares, so a test can hold both at once. +/// compares, so a test can hold both at once, and onto `regenerateLibs()` +/// pointed somewhere nothing compiles, so a test can RUN it. /// /// A harness rather than a change to `Build`: `generatedContracts()` is the /// script's own declaration and has no caller outside it, and widening it to /// `public` to be testable would put a second entry point on a script whose /// whole surface is `run()` and `cutRelease()`. contract BuildHarness is Build { + /// The lib directory the hooks write into. Empty defers to `Build`'s own, + /// which is what makes the default assertable and the write refusable. + string internal sLibDir; + + /// @param libDirectory The directory `regenerateLibs` writes into, or empty + /// for `Build`'s own. + constructor(string memory libDirectory) { + sLibDir = libDirectory; + } + + /// @inheritdoc Build + function libDir() internal view override returns (string memory) { + return bytes(sLibDir).length > 0 ? sLibDir : super.libDir(); + } + + /// The directory a build from this harness writes its libs into. + /// @return The lib directory. + function externalLibDir() external view returns (string memory) { + return libDir(); + } + + /// Runs the lib half of a build, into the fixture directory this harness + /// was handed. The directory has to exist already, as it does for every + /// writer `regenerateLibs` calls. + function externalRegenerateLibs() external { + if (bytes(sLibDir).length == 0) { + revert BuildHarnessWouldWriteTheCommittedLibs(); + } + regenerateLibs(); + } + /// The generator's list. /// @return The generated contracts. function externalGeneratedContracts() external pure returns (GeneratedContract[] memory) { diff --git a/test/script/Build.t.sol b/test/script/Build.t.sol index 8d0e92f..299a3da 100644 --- a/test/script/Build.t.sol +++ b/test/script/Build.t.sol @@ -8,7 +8,7 @@ import {DeployCandidate} from "../../src/abstract/RainDeploySuitesBase.sol"; import {LibCodeGen} from "rain-sol-codegen-0.1.37/src/lib/LibCodeGen.sol"; import {LibRainDeploySnapshot} from "../../src/lib/LibRainDeploySnapshot.sol"; import {LibReleasedSuitesAggregate} from "../lib/LibReleasedSuitesAggregate.sol"; -import {BuildHarness} from "../concrete/BuildHarness.sol"; +import {BuildHarness, BuildHarnessWouldWriteTheCommittedLibs} from "../concrete/BuildHarness.sol"; import {LibMemoryKV, MemoryKV, MemoryKVKey, MemoryKVVal} from "rain-lib-memkv-0.1.4/src/lib/LibMemoryKV.sol"; /// @title BuildTest @@ -38,10 +38,10 @@ import {LibMemoryKV, MemoryKV, MemoryKVKey, MemoryKVVal} from "rain-lib-memkv-0. /// contract's pins are written into. /// /// Deliberately nothing here calls `run()` or `cutRelease()`. Both rewrite the -/// committed `src/generated/` snapshots and `src/lib/` libs that other test -/// contracts read, and forge runs test contracts in parallel — a contract -/// rewriting what another one is reading is a race, not a check. Nothing below -/// writes anything. +/// committed `src/generated/` snapshots that other test contracts read, and +/// forge runs test contracts in parallel — a contract rewriting what another +/// one is reading is a race, not a check. `regenerateLibs()` is run, and only +/// into a fixture directory nothing compiles; the one write below is that. contract BuildTest is Test { using LibMemoryKV for MemoryKV; @@ -49,7 +49,7 @@ contract BuildTest is Test { BuildHarness internal sBuild; function setUp() external { - sBuild = new BuildHarness(); + sBuild = new BuildHarness(""); } /// PROPERTY: the generator's list and the deploy declaration are the SAME @@ -312,4 +312,115 @@ contract BuildTest is Test { ); } } + + /// PROPERTY: a build with nothing overridden writes its libs into the + /// directory the committed ones are in. + /// + /// Asserted against the path as text rather than against + /// `LibRainDeploySnapshot.LIB_DIR`, which is the constant the default + /// returns: a default pointed at a directory nothing compiles leaves every + /// committed lib stale forever while the regeneration reports success, and + /// the test below cannot see it, because that test overrides this. + function testTheDefaultLibDirIsWhereTheCommittedLibsAre() external view { + assertEq(sBuild.externalLibDir(), "src/lib"); + } + + /// Where `regenerateLibs()` is driven. + /// + /// Outside `src/` and `test/`, which is everything `fs_permissions` + /// otherwise grants and both of which are compiled: a generated lib imports + /// `../generated/`, `../abstract/` and `./LibReleased.sol`, which + /// resolve from `src/lib` and nowhere else, so a copy under either root + /// fails the build for every suite — including the copy a failing test + /// leaves behind. `foundry.toml` grants this root for exactly that, and + /// nothing compiles it. + string constant LIBS_FIXTURE_DIR = "fixture-lib/build-regenerate-libs"; + + /// PROPERTY: `regenerateLibs()` RUN emits exactly the committed libs — one + /// alias lib and one released lib per generated contract, one aggregate, + /// and nothing else. + /// + /// Every assertion above this one is output-anchored: it compares a + /// committed file against the emitters, so it sees drift only AFTER + /// somebody re-runs the generator and commits what came out. The hook that + /// decides which emitter is called, with which arguments, how many times, + /// was executed by nothing at all — a loop bound that stopped one contract + /// short, or a `constantPrefix` taken from `contracts[0]` on every pass, + /// was invisible until the next release cut it into the record. Both hooks + /// took a `revert()` as their first statement with the whole suite still + /// green. + /// + /// So this runs it, and the oracle is the committed tree rather than the + /// emitters: a regeneration of a clean checkout is a no-op, so every file + /// it writes MUST be byte-identical to the file already there. That is + /// independent of the emitters in the way the pins above are not — they say + /// the committed files are what the emitters produce, and this says the + /// hook asks the emitters for those files. + /// + /// The count is asserted as well as the contents, because a loop that + /// stopped short writes nothing wrong — it writes nothing at all — and a + /// file the hook wrote that the repo does not commit is a generated file + /// nothing regenerates. + /// + /// `regenerateSnapshots()` has no counterpart here and can have none: + /// `LibFs` confines every snapshot write to `src/generated//`, the + /// record `frozenSnapshotPaths` walks, so there is no directory to drive it + /// into that is not read by the suites running beside this one. + /// + /// Read, then removed, then asserted: forge-std assertions revert, so a + /// removal after them removes in every case except a failure, which is the + /// only case that leaves a directory behind. `vm.isFile` before each read + /// for the same reason — a missing file is what the loop-bound failure + /// looks like, and a cheatcode revert there would strand the fixture. + function testRegenerateLibsEmitsExactlyTheCommittedLibs() external { + GeneratedContract[] memory generated = sBuild.externalGeneratedContracts(); + BuildHarness harness = new BuildHarness(LIBS_FIXTURE_DIR); + + //forge-lint: disable-next-line(unsafe-cheatcode) + vm.createDir(LIBS_FIXTURE_DIR, true); + harness.externalRegenerateLibs(); + + string[] memory names = new string[](generated.length * 2 + 1); + for (uint256 i = 0; i < generated.length; i++) { + names[i * 2] = string.concat("Lib", generated[i].contractName, "Deploy.sol"); + names[i * 2 + 1] = + string.concat(LibRainDeploySnapshot.releasedLibraryName(generated[i].contractName), ".sol"); + } + names[names.length - 1] = string.concat(LibRainDeploySnapshot.RELEASED_SUITES_LIBRARY, ".sol"); + + uint256 written = vm.readDir(LIBS_FIXTURE_DIR).length; + bool[] memory emitted = new bool[](names.length); + string[] memory bodies = new string[](names.length); + for (uint256 i = 0; i < names.length; i++) { + string memory path = string.concat(LIBS_FIXTURE_DIR, "/", names[i]); + emitted[i] = vm.isFile(path); + bodies[i] = emitted[i] ? vm.readFile(path) : ""; + } + + //forge-lint: disable-next-line(unsafe-cheatcode) + vm.removeDir(LIBS_FIXTURE_DIR, true); + + assertEq(written, names.length, "regenerateLibs emitted a different number of files than the repo commits"); + for (uint256 i = 0; i < names.length; i++) { + assertTrue(emitted[i], string.concat("regenerateLibs emitted no ", names[i])); + assertEq( + bodies[i], + vm.readFile(string.concat("src/lib/", names[i])), + string.concat("regenerateLibs emitted something other than the committed ", names[i]) + ); + } + } + + /// A harness left pointed at `Build`'s own lib directory MUST refuse to run + /// the lib half of a build. + /// + /// `setUp` builds one, because the default is what + /// `testTheDefaultLibDirIsWhereTheCommittedLibsAre` reads, and it is shared + /// with every other test in this contract. A call that went through would + /// rewrite `src/lib/` while the suites that compile and read those files + /// are running. + function testRegenerateLibsRefusesToWriteTheCommittedLibs() external { + vm.expectRevert(BuildHarnessWouldWriteTheCommittedLibs.selector); + sBuild.externalRegenerateLibs(); + } } From 341e6cd3e47a702d9f5de8a3be596175fa7cce68 Mon Sep 17 00:00:00 2001 From: baku-ccron Date: Tue, 15 Sep 2026 22:37:48 +0000 Subject: [PATCH 2/4] Cut the doc blocks this PR added over the lib dir hook, the harness and the two new tests Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01V8ViHcKLVk2YoS2joH4HdN --- script/Build.sol | 9 ----- test/concrete/BuildHarness.sol | 15 +------- test/script/Build.t.sol | 68 ++-------------------------------- 3 files changed, 4 insertions(+), 88 deletions(-) diff --git a/script/Build.sol b/script/Build.sol index 295ee4f..dbde6c4 100644 --- a/script/Build.sol +++ b/script/Build.sol @@ -59,15 +59,6 @@ contract Build is BuildScript, RegistryDeploySuites { return names; } - /// The directory every generated lib is written into. - /// - /// Overridable for the reason `BuildScript.recordRoot` is: a hook that can - /// only be pointed at the committed tree can only be RUN by overwriting - /// files the rest of the suite compiles and reads, and forge runs test - /// contracts in parallel. Only the per-contract libs and the aggregate go - /// here — `regenerateSnapshots` has no equivalent, because `LibFs` confines - /// every snapshot it writes to `src/generated/`. - /// @return The lib directory. function libDir() internal view virtual returns (string memory) { return LibRainDeploySnapshot.LIB_DIR; } diff --git a/test/concrete/BuildHarness.sol b/test/concrete/BuildHarness.sol index e7ddf97..99cd596 100644 --- a/test/concrete/BuildHarness.sol +++ b/test/concrete/BuildHarness.sol @@ -5,27 +5,19 @@ pragma solidity =0.8.25; import {Build, GeneratedContract} from "../../script/Build.sol"; import {DeployCandidate} from "../../src/abstract/RainDeploySuitesBase.sol"; -/// Thrown when a harness left pointed at `Build`'s own lib directory is asked -/// to run the lib half of a build. That write lands on the committed libs the -/// rest of the suite compiles and reads, which forge runs in parallel with it. error BuildHarnessWouldWriteTheCommittedLibs(); /// @title BuildHarness /// @notice An external seam onto the two internal declarations `BuildTest` -/// compares, so a test can hold both at once, and onto `regenerateLibs()` -/// pointed somewhere nothing compiles, so a test can RUN it. +/// compares, so a test can hold both at once. /// /// A harness rather than a change to `Build`: `generatedContracts()` is the /// script's own declaration and has no caller outside it, and widening it to /// `public` to be testable would put a second entry point on a script whose /// whole surface is `run()` and `cutRelease()`. contract BuildHarness is Build { - /// The lib directory the hooks write into. Empty defers to `Build`'s own, - /// which is what makes the default assertable and the write refusable. string internal sLibDir; - /// @param libDirectory The directory `regenerateLibs` writes into, or empty - /// for `Build`'s own. constructor(string memory libDirectory) { sLibDir = libDirectory; } @@ -35,15 +27,10 @@ contract BuildHarness is Build { return bytes(sLibDir).length > 0 ? sLibDir : super.libDir(); } - /// The directory a build from this harness writes its libs into. - /// @return The lib directory. function externalLibDir() external view returns (string memory) { return libDir(); } - /// Runs the lib half of a build, into the fixture directory this harness - /// was handed. The directory has to exist already, as it does for every - /// writer `regenerateLibs` calls. function externalRegenerateLibs() external { if (bytes(sLibDir).length == 0) { revert BuildHarnessWouldWriteTheCommittedLibs(); diff --git a/test/script/Build.t.sol b/test/script/Build.t.sol index 03cd555..c92d1f8 100644 --- a/test/script/Build.t.sol +++ b/test/script/Build.t.sol @@ -38,10 +38,8 @@ import {LibMemoryKV, MemoryKV, MemoryKVKey, MemoryKVVal} from "rain-lib-memkv-0. /// contract's pins are written into. /// /// Deliberately nothing here calls `run()` or `cutRelease()`. Both rewrite the -/// committed `src/generated/` snapshots that other test contracts read, and -/// forge runs test contracts in parallel — a contract rewriting what another -/// one is reading is a race, not a check. `regenerateLibs()` is run, and only -/// into a fixture directory nothing compiles; the one write below is that. +/// committed `src/generated/` snapshots other test contracts read, and forge +/// runs test contracts in parallel. contract BuildTest is Test { using LibMemoryKV for MemoryKV; @@ -313,65 +311,13 @@ contract BuildTest is Test { } } - /// PROPERTY: a build with nothing overridden writes its libs into the - /// directory the committed ones are in. - /// - /// Asserted against the path as text rather than against - /// `LibRainDeploySnapshot.LIB_DIR`, which is the constant the default - /// returns: a default pointed at a directory nothing compiles leaves every - /// committed lib stale forever while the regeneration reports success, and - /// the test below cannot see it, because that test overrides this. function testTheDefaultLibDirIsWhereTheCommittedLibsAre() external view { assertEq(sBuild.externalLibDir(), "src/lib"); } - /// Where `regenerateLibs()` is driven. - /// - /// Outside `src/` and `test/`, which is everything `fs_permissions` - /// otherwise grants and both of which are compiled: a generated lib imports - /// `../generated/`, `../abstract/` and `./LibReleased.sol`, which - /// resolve from `src/lib` and nowhere else, so a copy under either root - /// fails the build for every suite — including the copy a failing test - /// leaves behind. `foundry.toml` grants this root for exactly that, and - /// nothing compiles it. string constant LIBS_FIXTURE_DIR = "fixture-lib/build-regenerate-libs"; - /// PROPERTY: `regenerateLibs()` RUN emits exactly the committed libs — one - /// alias lib and one released lib per generated contract, one aggregate, - /// and nothing else. - /// - /// Every assertion above this one is output-anchored: it compares a - /// committed file against the emitters, so it sees drift only AFTER - /// somebody re-runs the generator and commits what came out. The hook that - /// decides which emitter is called, with which arguments, how many times, - /// was executed by nothing at all — a loop bound that stopped one contract - /// short, or a `constantPrefix` taken from `contracts[0]` on every pass, - /// was invisible until the next release cut it into the record. Both hooks - /// took a `revert()` as their first statement with the whole suite still - /// green. - /// - /// So this runs it, and the oracle is the committed tree rather than the - /// emitters: a regeneration of a clean checkout is a no-op, so every file - /// it writes MUST be byte-identical to the file already there. That is - /// independent of the emitters in the way the pins above are not — they say - /// the committed files are what the emitters produce, and this says the - /// hook asks the emitters for those files. - /// - /// The count is asserted as well as the contents, because a loop that - /// stopped short writes nothing wrong — it writes nothing at all — and a - /// file the hook wrote that the repo does not commit is a generated file - /// nothing regenerates. - /// - /// `regenerateSnapshots()` has no counterpart here and can have none: - /// `LibFs` confines every snapshot write to `src/generated//`, the - /// record `frozenSnapshotPaths` walks, so there is no directory to drive it - /// into that is not read by the suites running beside this one. - /// - /// Read, then removed, then asserted: forge-std assertions revert, so a - /// removal after them removes in every case except a failure, which is the - /// only case that leaves a directory behind. `vm.isFile` before each read - /// for the same reason — a missing file is what the loop-bound failure - /// looks like, and a cheatcode revert there would strand the fixture. + /// Assertions revert, so the fixture is removed before any of them run. function testRegenerateLibsEmitsExactlyTheCommittedLibs() external { GeneratedContract[] memory generated = sBuild.externalGeneratedContracts(); BuildHarness harness = new BuildHarness(LIBS_FIXTURE_DIR); @@ -411,14 +357,6 @@ contract BuildTest is Test { } } - /// A harness left pointed at `Build`'s own lib directory MUST refuse to run - /// the lib half of a build. - /// - /// `setUp` builds one, because the default is what - /// `testTheDefaultLibDirIsWhereTheCommittedLibsAre` reads, and it is shared - /// with every other test in this contract. A call that went through would - /// rewrite `src/lib/` while the suites that compile and read those files - /// are running. function testRegenerateLibsRefusesToWriteTheCommittedLibs() external { vm.expectRevert(BuildHarnessWouldWriteTheCommittedLibs.selector); sBuild.externalRegenerateLibs(); From 68bacf5cabc4993a980a8540591c8e0c760c6b3b Mon Sep 17 00:00:00 2001 From: baku-ccron Date: Wed, 16 Sep 2026 00:18:30 +0000 Subject: [PATCH 3/4] docs: put back the reasons 341e6cd cut with the restatements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit What comes back is every sentence the code does not state: - `Build.libDir`: why the hook is overridable at all — a lib hook that can only be pointed at the committed tree can only be RUN by overwriting files the suites forge runs in parallel are compiling, and why `regenerateSnapshots` needs no equivalent. - `BuildHarnessWouldWriteTheCommittedLibs`: what the revert means. - `BuildHarness`: that the harness is also a seam for RUNNING `regenerateLibs` somewhere nothing compiles, the empty-string sentinel on `sLibDir` and its constructor `@param`, and the precondition that the directory already exists. - `BuildTest`: that `regenerateLibs()` IS run, into a fixture directory, which is the exception to the paragraph saying nothing here runs a write. - `testTheDefaultLibDirIsWhereTheCommittedLibsAre`: why the path is asserted as text rather than against `LibRainDeploySnapshot.LIB_DIR` — against the constant the assertion cannot fail. - `LIBS_FIXTURE_DIR`: why the fixture root is outside `src/` and `test/`. - `testRegenerateLibsEmitsExactlyTheCommittedLibs`: the gap it closes — every other assertion is output-anchored and both hooks took a `revert()` as their first statement with the suite still green — why the committed tree is the oracle, why the count is asserted, why `regenerateSnapshots` has no counterpart, and the read-then-remove-then-assert ordering. - `testRegenerateLibsRefusesToWriteTheCommittedLibs`: why `setUp`'s harness is the default one and what a call that went through would rewrite. Left cut: `externalLibDir`'s doc block, which restates its signature. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01V8ViHcKLVk2YoS2joH4HdN --- script/Build.sol | 9 +++++ test/concrete/BuildHarness.sol | 13 ++++++- test/script/Build.t.sol | 68 ++++++++++++++++++++++++++++++++-- 3 files changed, 86 insertions(+), 4 deletions(-) diff --git a/script/Build.sol b/script/Build.sol index dbde6c4..295ee4f 100644 --- a/script/Build.sol +++ b/script/Build.sol @@ -59,6 +59,15 @@ contract Build is BuildScript, RegistryDeploySuites { return names; } + /// The directory every generated lib is written into. + /// + /// Overridable for the reason `BuildScript.recordRoot` is: a hook that can + /// only be pointed at the committed tree can only be RUN by overwriting + /// files the rest of the suite compiles and reads, and forge runs test + /// contracts in parallel. Only the per-contract libs and the aggregate go + /// here — `regenerateSnapshots` has no equivalent, because `LibFs` confines + /// every snapshot it writes to `src/generated/`. + /// @return The lib directory. function libDir() internal view virtual returns (string memory) { return LibRainDeploySnapshot.LIB_DIR; } diff --git a/test/concrete/BuildHarness.sol b/test/concrete/BuildHarness.sol index 99cd596..b03780b 100644 --- a/test/concrete/BuildHarness.sol +++ b/test/concrete/BuildHarness.sol @@ -5,19 +5,27 @@ pragma solidity =0.8.25; import {Build, GeneratedContract} from "../../script/Build.sol"; import {DeployCandidate} from "../../src/abstract/RainDeploySuitesBase.sol"; +/// Thrown when a harness left pointed at `Build`'s own lib directory is asked +/// to run the lib half of a build. That write lands on the committed libs the +/// rest of the suite compiles and reads, which forge runs in parallel with it. error BuildHarnessWouldWriteTheCommittedLibs(); /// @title BuildHarness /// @notice An external seam onto the two internal declarations `BuildTest` -/// compares, so a test can hold both at once. +/// compares, so a test can hold both at once, and onto `regenerateLibs()` +/// pointed somewhere nothing compiles, so a test can RUN it. /// /// A harness rather than a change to `Build`: `generatedContracts()` is the /// script's own declaration and has no caller outside it, and widening it to /// `public` to be testable would put a second entry point on a script whose /// whole surface is `run()` and `cutRelease()`. contract BuildHarness is Build { + /// The lib directory the hooks write into. Empty defers to `Build`'s own, + /// which is what makes the default assertable and the write refusable. string internal sLibDir; + /// @param libDirectory The directory `regenerateLibs` writes into, or empty + /// for `Build`'s own. constructor(string memory libDirectory) { sLibDir = libDirectory; } @@ -31,6 +39,9 @@ contract BuildHarness is Build { return libDir(); } + /// Runs the lib half of a build, into the fixture directory this harness + /// was handed. The directory has to exist already, as it does for every + /// writer `regenerateLibs` calls. function externalRegenerateLibs() external { if (bytes(sLibDir).length == 0) { revert BuildHarnessWouldWriteTheCommittedLibs(); diff --git a/test/script/Build.t.sol b/test/script/Build.t.sol index c92d1f8..03cd555 100644 --- a/test/script/Build.t.sol +++ b/test/script/Build.t.sol @@ -38,8 +38,10 @@ import {LibMemoryKV, MemoryKV, MemoryKVKey, MemoryKVVal} from "rain-lib-memkv-0. /// contract's pins are written into. /// /// Deliberately nothing here calls `run()` or `cutRelease()`. Both rewrite the -/// committed `src/generated/` snapshots other test contracts read, and forge -/// runs test contracts in parallel. +/// committed `src/generated/` snapshots that other test contracts read, and +/// forge runs test contracts in parallel — a contract rewriting what another +/// one is reading is a race, not a check. `regenerateLibs()` is run, and only +/// into a fixture directory nothing compiles; the one write below is that. contract BuildTest is Test { using LibMemoryKV for MemoryKV; @@ -311,13 +313,65 @@ contract BuildTest is Test { } } + /// PROPERTY: a build with nothing overridden writes its libs into the + /// directory the committed ones are in. + /// + /// Asserted against the path as text rather than against + /// `LibRainDeploySnapshot.LIB_DIR`, which is the constant the default + /// returns: a default pointed at a directory nothing compiles leaves every + /// committed lib stale forever while the regeneration reports success, and + /// the test below cannot see it, because that test overrides this. function testTheDefaultLibDirIsWhereTheCommittedLibsAre() external view { assertEq(sBuild.externalLibDir(), "src/lib"); } + /// Where `regenerateLibs()` is driven. + /// + /// Outside `src/` and `test/`, which is everything `fs_permissions` + /// otherwise grants and both of which are compiled: a generated lib imports + /// `../generated/`, `../abstract/` and `./LibReleased.sol`, which + /// resolve from `src/lib` and nowhere else, so a copy under either root + /// fails the build for every suite — including the copy a failing test + /// leaves behind. `foundry.toml` grants this root for exactly that, and + /// nothing compiles it. string constant LIBS_FIXTURE_DIR = "fixture-lib/build-regenerate-libs"; - /// Assertions revert, so the fixture is removed before any of them run. + /// PROPERTY: `regenerateLibs()` RUN emits exactly the committed libs — one + /// alias lib and one released lib per generated contract, one aggregate, + /// and nothing else. + /// + /// Every assertion above this one is output-anchored: it compares a + /// committed file against the emitters, so it sees drift only AFTER + /// somebody re-runs the generator and commits what came out. The hook that + /// decides which emitter is called, with which arguments, how many times, + /// was executed by nothing at all — a loop bound that stopped one contract + /// short, or a `constantPrefix` taken from `contracts[0]` on every pass, + /// was invisible until the next release cut it into the record. Both hooks + /// took a `revert()` as their first statement with the whole suite still + /// green. + /// + /// So this runs it, and the oracle is the committed tree rather than the + /// emitters: a regeneration of a clean checkout is a no-op, so every file + /// it writes MUST be byte-identical to the file already there. That is + /// independent of the emitters in the way the pins above are not — they say + /// the committed files are what the emitters produce, and this says the + /// hook asks the emitters for those files. + /// + /// The count is asserted as well as the contents, because a loop that + /// stopped short writes nothing wrong — it writes nothing at all — and a + /// file the hook wrote that the repo does not commit is a generated file + /// nothing regenerates. + /// + /// `regenerateSnapshots()` has no counterpart here and can have none: + /// `LibFs` confines every snapshot write to `src/generated//`, the + /// record `frozenSnapshotPaths` walks, so there is no directory to drive it + /// into that is not read by the suites running beside this one. + /// + /// Read, then removed, then asserted: forge-std assertions revert, so a + /// removal after them removes in every case except a failure, which is the + /// only case that leaves a directory behind. `vm.isFile` before each read + /// for the same reason — a missing file is what the loop-bound failure + /// looks like, and a cheatcode revert there would strand the fixture. function testRegenerateLibsEmitsExactlyTheCommittedLibs() external { GeneratedContract[] memory generated = sBuild.externalGeneratedContracts(); BuildHarness harness = new BuildHarness(LIBS_FIXTURE_DIR); @@ -357,6 +411,14 @@ contract BuildTest is Test { } } + /// A harness left pointed at `Build`'s own lib directory MUST refuse to run + /// the lib half of a build. + /// + /// `setUp` builds one, because the default is what + /// `testTheDefaultLibDirIsWhereTheCommittedLibsAre` reads, and it is shared + /// with every other test in this contract. A call that went through would + /// rewrite `src/lib/` while the suites that compile and read those files + /// are running. function testRegenerateLibsRefusesToWriteTheCommittedLibs() external { vm.expectRevert(BuildHarnessWouldWriteTheCommittedLibs.selector); sBuild.externalRegenerateLibs(); From 17c72cbbbb8725790e013c71b711d05120ec9740 Mon Sep 17 00:00:00 2001 From: baku-ccron Date: Wed, 16 Sep 2026 02:09:20 +0000 Subject: [PATCH 4/4] Pass the lib directory through BuildRecordRootHarness This branch gives `BuildHarness` a constructor; `BuildRecordRootHarness` arrived on main after that and declares its own, so the merge left the base unconstructed and the compile failed. Empty is `Build`'s own lib directory, which is what the record-root harness wants: it only regenerates snapshots, and `externalRegenerateLibs` stays refused. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01V8ViHcKLVk2YoS2joH4HdN --- test/concrete/BuildRecordRootHarness.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/concrete/BuildRecordRootHarness.sol b/test/concrete/BuildRecordRootHarness.sol index d18ebf7..efa7f3b 100644 --- a/test/concrete/BuildRecordRootHarness.sol +++ b/test/concrete/BuildRecordRootHarness.sol @@ -17,7 +17,7 @@ import {BuildHarness} from "./BuildHarness.sol"; contract BuildRecordRootHarness is BuildHarness { string internal sRoot; - constructor(string memory root) { + constructor(string memory root) BuildHarness("") { sRoot = root; }