Cover LibFs: path structure and what buildFileForContract actually writes

[thedavidmeister]

Tests only. src/lib/LibFs.sol had 17 lines of coverage — one assertion on
pathForContract("Foo") — and nothing at all on buildFileForContract, which
is the only thing in this repo that touches disk and the function every consumer
actually calls.

This adds 13 tests across the two functions and reports the mutation matrix that
justifies each of them.

What is covered

pathForContract (in test/lib/LibFs.t.sol, alongside the existing
testPathForContract, which is untouched):

• testPathForContractStructure — fuzzed over arbitrary names, asserts the path
  is exactly three regions positionally: src/generated/, the name byte for
  byte, .sol. The length equality is what makes it exhaustive — it forbids any
  extra byte anywhere, so a name can never be quoted, escaped, trimmed, case
  folded or truncated on the way into the path.
• testPathForContractDistinctNamesDistinctPaths — distinct names never share a
  file, so generation cannot silently overwrite one contract with another.
• testPathForContractIsRelative — the path is never absolute.

buildFileForContract (new test/lib/LibFs.buildFileForContract.t.sol), all
asserting the bytes on disk after the call, for the whole file:

• testBuildFileForContractExactContent — prefix, then the bytecode hash
  constant, then the body, nothing between and nothing after.
• testBuildFileForContractWritesToPathForContract — the file lands at the
  literal src/generated/<Name>.sol, asserted against the literal as well as
  against pathForContract, so the two functions agreeing with each other is not
  what makes it pass.
• testBuildFileForContractFreshPath — the first generation for a contract,
  where nothing is at the path yet.
• testBuildFileForContractReplacesExistingContent — a pre-existing file more
  than 1000 bytes long is fully replaced, so any surviving tail fails.
• testBuildFileForContractIdempotent — a second run over the same inputs
  produces the same bytes. This is the property CI relies on when it regenerates
  and diffs.
• testBuildFileForContractEmptyBody — the prefix and the hash constant are
  emitted even with no body, which is what the docstring promises.
• testBuildFileForContractBodyVerbatim — no separator is inserted before the
  body and nothing in it is escaped.
• testBuildFileForContractLeavesSiblingsAlone — generating one contract does
  not disturb another's file in the same directory.
• testBuildFileForContractUsesTheGivenInstance — the hash comes from the
  address passed in, not from the caller or a fixed address.
• testBuildFileForContractCommittedArtifactIsCurrent — the committed
  src/generated/CodeGennable.sol still opens with what this function writes
  today.

Each of the other tests owns a distinct name under src/generated/, none of
them is CodeGennable (the committed artifact), and each removes its file
again.

That last one closes a gap outside LibFs itself. src/generated/CodeGennable.sol
is committed and script/Build.sol produces it through buildFileForContract,
but nothing in forge test noticed if it went stale — the only guard was the
separate rainix-copy-artifacts job regenerating and diffing, which does not
run in the test job. Verified discriminating by hand: flipping one hex digit of
BYTECODE_HASH in the committed file fails it with
committed artifact is stale, regenerate with script/Build.sol.

The expected content is rebuilt in the test from the literal header text and
from address.codehash — deliberately not by calling LibCodeGen.filePrefix
and LibCodeGen.bytecodeHashConstantString. Calling those would assert the
library agrees with itself and would follow any drift in them in silence.
Consumers have the literal committed in their repos, so the literal is the
oracle.

QA

• Discriminating tests: 22 tests, 5 suites, all pass; forge fmt --check clean.
  Every new test asserts exact bytes rather than absence of a revert, and each
  was checked to fail under at least one mutation of the line it covers (matrix
  below). Baseline before this PR was 9 tests, 4 suites, 0 failures — confirmed
  green on a fresh clone of c72eb89 before any probing.

• Mutations applied: 18 mutants over the whole of src/lib/LibFs.sol, one per
  behaviour, applied with mutation-probe classifying from forge's own
  N passed; N failed tally. 17 killed, 1 survived.
  line -> mutation -> killing test:

  • L17 "src/generated/" -> "src/generated" -> testBuildFileForContractExactContent (+4)
  • L17 "src/generated/" -> "generated/" -> testBuildFileForContractExactContent (+4)
  • L17 ".sol" -> ".txt" -> testPathForContract, testPathForContractStructure, testBuildFileForContractWritesToPathForContract
  • L17 name dropped from the concat -> testBuildFileForContractFreshPath (+4)
  • L17 concat parts reordered -> testBuildFileForContractExactContent (+4)
  • L17 "src/generated/" -> "/src/generated/" -> testPathForContract, testPathForContractIsRelative, testPathForContractStructure
  • L32 pathForContract(contractName) -> pathForContract(contractName + "Mutant") -> testBuildFileForContractExactContent (+4)
  • L33 if (vm.exists(path)) -> if (!vm.exists(path)) -> testBuildFileForContractFreshPath (+4)
  • L33 if (vm.exists(path)) -> if (true) -> testBuildFileForContractFreshPath (+4)
  • L35 vm.removeFile(path); -> removed -> SURVIVED, see below
  • L39 path -> a fixed path -> testBuildFileForContractWritesToPathForContract (+4)
  • L39 filePrefix() dropped -> 8 of the 9 buildFileForContract tests
  • L39 bytecodeHashConstantString(...) dropped -> 8 of the 9
  • L39 body dropped -> 6 of the 9 (the two empty-body tests correctly cannot see it)
  • L39 prefix moved after the hash -> 8 of the 9
  • L39 body moved before the hash -> 6 of the 9
  • L39 instance -> address(0) -> 8 of the 9
  • L39 "\n" inserted before the body -> 8 of the 9

  The "8 of the 9" and "6 of the 9" rows were resolved by re-running each mutant
  and reading which tests still passed: forge prints a multi-line assertEq
  failure for these, which the probe's single-line fail-pattern cannot name a
  killer from. The KILLED verdict itself comes from the failing tally, not from
  that pattern.

• Oracle: the emitted file, not the library. Expected content is the literal
  header a consumer has committed, plus address.codehash read through
  Solidity's own member rather than through the library's extcodehash
  assembly; expected path is the literal src/generated/<Name>.sol a consumer
  imports. Both are checkable without reading LibFs or LibCodeGen at all,
  which is the point — a test built out of the functions under test can only
  prove they are self-consistent. Idempotency is asserted as a property of the
  bytes on disk across two calls, which is the same property
  rainix-copy-artifacts asserts when it regenerates and diffs.

• Category check: the ask is coverage for LibFs, and the category is every
  behaviour in the file, not the two the seed candidate named. Enumerated from
  the source: 3 concatenated parts in pathForContract, and in
  buildFileForContract the path derivation, the existence guard, the removal,
  the write target, and the 3 concatenated parts of the content plus their
  order. All 18 probed; the surviving one is reported rather than papered over.
  Nothing outside test/ changes.

The surviving mutant

Deleting vm.removeFile(path) entirely changes no observable outcome. vm.writeFile
truncates, so remove-then-write and write-alone leave identical bytes at the
path in every state reachable through cheatcodes, and no test can tell them
apart without ffi (which this repo does not enable). It is left surviving and
reported to the campaign rather than killed with a test that asserts something
the code does not actually guarantee. The docstring attributing idempotency to
the removal is a separate matter from this PR.

The existence guard around it is a different story and is genuinely load
bearing: vm.removeFile reverts on a path that is not there, so inverting or
removing the guard breaks first generation. testBuildFileForContractFreshPath
is what proves it.

[coderabbitai]

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: 7566ce7b-6cbd-4dca-806d-4faa298ea198

📥 Commits

Reviewing files that changed from the base of the PR and between c72eb89 and 9ccdb2e.

📒 Files selected for processing (2)

• test/lib/LibFs.buildFileForContract.t.sol
• test/lib/LibFs.t.sol

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

---

Walkthrough

The PR adds property-based tests for LibFs.pathForContract and comprehensive tests for LibFs.buildFileForContract. The tests cover path structure, file contents, overwrites, idempotency, body handling, file isolation, and instance-specific bytecode hashes.

Changes

LibFs behavior tests

Layer / File(s)	Summary
Generated path contract test/lib/LibFs.t.sol	Adds property-based tests for path structure, distinct contract names, and relative paths.
Contract file generation test/lib/LibFs.buildFileForContract.t.sol	Adds tests for exact contents, writes, replacement, idempotency, empty and verbatim bodies, sibling isolation, and supplied-instance bytecode hashes.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk:⚪ Minimal · up to 9ccdb

This change only adds focused coverage for filesystem path and generated-file behavior; no actionable merge-blocking risk remains beyond normal checks and review.

Possibly related PRs

• rainlanguage/rain.sol.codegen#31: Changes the generated .sol path behavior tested by this PR.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the added tests for LibFs path construction and buildFileForContract output.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 2026-08-16-libfs-coverage

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[thedavidmeister]

Reviewed bd16655: ready. Tests only — test/lib/LibFs.t.sol and a new test/lib/LibFs.buildFileForContract.t.sol. No source touched.

18 mutants, 17 killed. Every behaviour of both functions is probed: each segment of the path concat, the concat order, the relative-vs-absolute form, the vm.exists guard in both directions, the write target, and each of the three body parts buildFileForContract assembles plus their order.

The surviving mutant is deleting vm.removeFile(path); and it is left alive on purpose. vm.writeFile truncates, so remove-then-write and write-alone leave identical bytes at the path in every state reachable through cheatcodes — no discriminating test exists without ffi, which this repo does not enable. Filed as #41 rather than covered by a test asserting a guarantee the code does not make. The surrounding if (vm.exists(path)) guard IS doing work and is now pinned: vm.removeFile reverts on a missing path, so first generation breaks without it.

A harness caveat worth recording, since it applies to any repo asserting multi-line strings: the probe's fail-pattern cannot name killers for the seven mutants whose assertEq message spans newlines, because . does not cross a newline. Those KILLED verdicts come from the pass/fail tally, which is sound; the killer names were recovered by re-running each and reading the surviving [PASS] set rather than reported as killer-less kills.

One test reaches beyond LibFs and is worth knowing about: testBuildFileForContractCommittedArtifactIsCurrent asserts the committed src/generated/CodeGennable.sol still matches what buildFileForContract writes today. Nothing in forge test checked artifact staleness before — only the separate copy-artifacts job, which does not run in the test job. Verified discriminating by flipping one hex digit of BYTECODE_HASH. After this lands, a change to filePrefix or bytecodeHashConstantString without regenerating goes red in the suite rather than passing quietly.

Four findings came out of this group and are filed separately, none of them blessed by a test here: #38, #40, #41, #42.

All five checks green. CodeRabbit reports Review rate limited — green check with no review behind it, so the 0 unresolved threads is vacuous rather than clean; this rests on my read and the matrix.