Skip to content

Test LibStringSet.holds membership directly - #175

Closed
thedavidmeister wants to merge 4 commits into
mainfrom
amt-2026-09-14-lib-string-set
Closed

thedavidmeister wants to merge 4 commits into
mainfrom
amt-2026-09-14-lib-string-set

Conversation

@thedavidmeister

@thedavidmeister thedavidmeister commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

What

Adds test/src/lib/LibStringSet.t.sol, a unit test for LibStringSet.holds.
No source change.

Why

holds had no test of its own. Its only exercise was indirect, through
GeneratedSnapshotShapeTest and BuildTest, which pass it real contract and
release names. Those names never collide in length without also colliding in
content, and the lists are never empty, so a large part of what the NatSpec
claims was asserted nowhere: that equality is over the whole content rather
than the length or a prefix of it, that an empty list holds nothing, and that
the empty string is an ordinary member, held only when an element is itself
empty.

The NatSpec is the oracle. It says membership over a string[] for a set whose
order it does not fix, compared over the whole string. Each test states one case
of that definition, with the answer derived from the definition rather than from
the loop that implements it.

QA

  • Discriminating tests: testHoldsIsFalseOnAnEmptyHaystack, testHoldsComparesTheWholeContent, testHoldsComparesBeyondTheFirstWord, testHoldsEmptyNeedleMatchesOnlyAnEmptyElement - each was verified by re-running the whole suite against the mutant it targets: the mutant survived the suite before this file and is killed by it after, with the probe naming the test.
  • Mutations applied: see the table below - each row is one line of src/lib/LibStringSet.sol replaced, the verdict of the whole suite before this file, the verdict after, and the test the probe named as the killer.
  • Oracle: the LibStringSet NatSpec - membership over a string[] for a set whose order it does not fix, compared over the whole string. Every expected value in the tests is a concrete list and needle answered from that definition, never from the keccak loop that implements it.
  • Category check: this is not filed against an issue. It is one unit of a mutation sweep of the repo; the categories are the behaviour lines of holds, and all eleven are in the table with a verdict.

Mutation matrix over src/lib/LibStringSet.sol. Suite is forge test minus the
61 fork tests that need RPC env vars absent on the machine this ran on.
LibStringSet is not compiled into AddressRegistry or MigrationRegistry, so
the pinned address and code hash tests carry real behaviour for it and were kept
in rather than excluded. Baseline before: 397 passed, 0 failed. Baseline after:
409 passed, 0 failed. Every mutant below was applied to the unmutated source one
at a time and the whole suite run against it.

Before the new file, 6 of 11 mutants were killed by pre-existing indirect tests
and 5 survived. After it, 11 of 11 are killed.

Behaviour Mutation Before After Killing test
A match returns true return true inside the loop becomes return false KILLED KILLED testSnapshotContractNamesAreTheGeneratedContracts
No match returns false the trailing return false becomes return true KILLED KILLED testFrozenSnapshotPathsExcludesTheRollingSnapshot
An empty list holds nothing the trailing return false becomes return haystack.length == 0 SURVIVED KILLED testHoldsIsFalseOnAnEmptyHaystack
Index 0 is searched the loop starts at i = 1 KILLED KILLED testSnapshotContractNamesAreTheGeneratedContracts
The last index is searched the loop bound becomes i + 1 < haystack.length KILLED KILLED testSnapshotContractNamesAreTheGeneratedContracts
Equality is content, not length the keccak comparison becomes a length comparison SURVIVED KILLED testHoldsComparesTheWholeContent
Same length, different bytes are different the comparison becomes length plus first byte only SURVIVED KILLED testHoldsComparesTheWholeContent
The empty needle is an ordinary member an early return true when the needle is empty SURVIVED KILLED testHoldsEmptyNeedleMatchesOnlyAnEmptyElement
Equality is by value, not by memory identity the comparison becomes pointer equality in assembly KILLED KILLED testSnapshotContractNamesAreTheGeneratedContracts
A match is the answer whatever follows it the loop keeps going and returns the last comparison KILLED KILLED testSnapshotContractNamesAreTheGeneratedContracts
Content past the first word is compared the comparison becomes length plus the first 32 bytes only SURVIVED KILLED testHoldsComparesBeyondTheFirstWord

No bug was found in holds. Every surviving mutant was a test gap: the current
output is the one the NatSpec calls for in each case.

🤖 Generated with Claude Code

https://claude.ai/code/session_01V8ViHcKLVk2YoS2joH4HdN

Summary by CodeRabbit

  • Tests
    • Added comprehensive coverage for string membership checks, including empty values, varied element positions, ordering, whole-content comparisons, and longer strings.
    • Added fuzz testing for membership behavior and strings exceeding the length of stored elements.
    • Added mutation-testing configuration to evaluate the robustness of the test suite.

baku-ccron and others added 3 commits September 14, 2026 15:42
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V8ViHcKLVk2YoS2joH4HdN
holds had no test of its own. Its only exercise was indirect, through the
snapshot shape and build tests, which never present two distinct strings of
the same length to it and never present an empty list or an empty needle.

Adds a unit test for membership as the NatSpec defines it: whole content
equality by value over a list whose order does not matter, an empty list
holding nothing, and the empty string held only when an element is itself
empty.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V8ViHcKLVk2YoS2joH4HdN
@coderabbitai

coderabbitai Bot commented Sep 14, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: e42e8f78-9a9a-46eb-b6c9-17125f56c4b5

📥 Commits

Reviewing files that changed from the base of the PR and between e9a922a and 3bb68fb.

📒 Files selected for processing (2)
  • mutants.toml
  • test/src/lib/LibStringSet.t.sol

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


Walkthrough

The pull request adds comprehensive tests for LibStringSet.holds and a mutation-testing configuration. The suite runs sh check.sh and defines 11 mutations that target return values, loop bounds, empty strings, equality checks, and comparison state.

Changes

String-set membership validation

Layer / File(s) Summary
Membership behavior tests
test/src/lib/LibStringSet.t.sol
Adds pure and fuzz tests for element positions, empty values, whole-content equality, value-based equality, ordering, later misses, and long strings.
Mutation suite and mutation cases
mutants.toml
Configures mutation testing with output parsing and a 900-second timeout. Defines 11 mutations for LibStringSet.holds, including altered returns, loop bounds, equality checks, empty-needle handling, pointer identity, and last-result state.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Other

Merge Risk: ⚪ Minimal · up to 04631

The added coverage does not introduce a current merge-blocking risk.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding direct membership tests for LibStringSet.holds.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
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.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch amt-2026-09-14-lib-string-set

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

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

mutants.toml is harness input for the mutation sweep, not part of the
repository, and an unlicensed file fails the REUSE check.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V8ViHcKLVk2YoS2joH4HdN
@thedavidmeister

Copy link
Copy Markdown
Contributor Author

Closing unmerged: its subject is gone. LibStringSet was deleted from this repo in #210, which replaced all sixteen call sites with LibMemoryKV. The membership behaviours these tests pinned are now covered by that library's own suite, and the five gaps they closed are moot because the loop they covered no longer exists here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant