`testFilePrefixNamesNoScript` and `testBytesToHexHasNoPrefix` assert only what a sibling in the same file already implies

[thedavidmeister]

Audit scope: whole-repo, commit 7aa85a4

Dimension 4 (code quality) · low

Locations:

• test/src/lib/LibCodeGen.filePrefix.t.sol:15-17
• test/src/lib/LibHexString.bytesToHex.t.sol:74-79

Path note: PR #56 is open and unmerged as of filing. On main today these files are test/lib/LibCodeGen.filePrefix.t.sol and test/lib/LibHexString.bytesToHex.t.sol; they become test/src/lib/... after #56 merges.

Problem

testFilePrefixNamesNoScript asserts the prefix contains no .sol;
testFilePrefixExact (19-32) pins the prefix byte for byte, so nothing can fail
the first without failing the second. testBytesToHexHasNoPrefix asserts no x
in the output; testBytesToHexCharset (89-95) asserts every byte is in
[0-9a-f], which excludes x — and both spend 2048 fuzz runs. Neither test can
discriminate anything its sibling does not.

Proposed fix

Delete both bodies and fold their intent into the surviving sibling's
docstring (why no script may be named; why a surviving prefix character is
fatal).

[thedavidmeister]

Correction: this issue's rationale holds for one of the two deletions, not both

The problem statement bundles both pairs under a single sentence — "Neither test
can discriminate anything its sibling does not" — and that is true of the
filePrefix pair and false of the bytesToHex pair. The two are different kinds
of claim, and what separates them is the fuzzer.

testFilePrefixNamesNoScript / testFilePrefixExact — the issue is right.
filePrefix() takes no arguments and is pure, so its domain is a single point.
testFilePrefixExact assertEqs the entire return value, so exactly one string
passes it. Every string the weaker test rejected is rejected there too, and so is
every other string. One input, one implication, total subsumption. That deletion
is legitimate and is what PR #99 lands.

testBytesToHexHasNoPrefix / testBytesToHexCharset — the issue is wrong.
The per-input implication the issue cites is real: x is 0x78, outside
[0-9a-f], so on any given output "every byte is a lower case hex nibble"
implies "no byte is x". But these are two fuzz tests over bytes memory, not
two assertions on one input. foundry.toml sets [fuzz] runs = 2048 and pins no
seed, so each of them runs its own independent 2048-draw campaign, and each
test's coverage of the domain is its own draws. A per-input implication is not
test subsumption when the tests do not share inputs. Deleting
testBytesToHexHasNoPrefix costs 2048 draws of the property over the same
domain — that is coverage, not redundancy.

The issue notes "both spend 2048 fuzz runs" as if that were the argument for
redundancy. It is the argument against it: the two campaigns are additive, not
duplicative.

A mutation matrix cannot rescue the bundled rationale either. Showing both tests
dying on the same mutant is exactly what a per-input implication predicts; it
cannot show that the survivor would catch a defect only some of the deleted
test's own draws reach.

Disposition. PR #99 lands the filePrefix deletion and restores
testBytesToHexHasNoPrefix unchanged. The second location in this issue
(test/lib/LibHexString.bytesToHex.t.sol) is withdrawn as a finding rather than
fixed. #99 still closes this issue, with that half explicitly not actioned — so
nobody re-files it from the issue text as written.

Cause, for the record: the brief the PR was written under required every fix to
ship a test and treated verify-by-hand as never acceptable, with no exemption for
changes that have no behaviour. That is corrected. Here it pushed a deletion
argument past what the evidence supported.