test: cover SelectionsLock's sentinel cleanup and exhaustion - #71
Merged
Conversation
Three gaps, all in behaviour the type's own remarks rely on. DeleteOnClose is what makes stale-lock recovery unnecessary -- "there is no stale-lock recovery code to maintain" -- but no test asserted the sentinel is actually gone after dispose. Acquire_AfterRelease_Succeeds said so in a comment and then only checked that a second acquire worked, which it would even with a leftover file that the second acquirer simply reopened. The ~5.1s backoff ladder ending in a thrown IOException was never reached, so neither its message nor its inner exception was verified. And the lock's actual job -- serialising selections read-modify-write across processes -- was only inferred from a single same-process contention test. Now asserted: the sentinel is absent after dispose; a permanently held lock throws IOException naming the path, saying another process may hold it, and preserving the inner exception; and four concurrent contenders never overlap inside the critical section. Verified to bite: replacing DeleteOnClose with FileOptions.None fails Acquire_AfterRelease_Succeeds and Acquire_SerialisesConcurrentHolders. Both passed against that same change before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #57.
The gaps
Three, all in behaviour the type's own remarks lean on:
DeleteOnClosewas never asserted. It is what makes stale-lock recovery unnecessary — "there is no stale-lock recovery code to maintain".Acquire_AfterRelease_Succeedsclaimed it in a comment and then only checked a second acquire worked, which it would even with a leftover file that the second acquirer simply reopened.IOException— its message, and whether it preserves the blocking exception — was unverified.What was added
IOExceptionnaming the path, saying another process may be holding it, and preserving the inner exception.Red-proof
Replacing
FileOptions.DeleteOnClosewithFileOptions.None:Both passed against that same change before this PR — which is precisely the regression the old comment implied was covered.
Verification
Build clean, 408 tests (354 passed, 54 skipped), up from 404.
Note the exhaustion test costs ~5 s of wall clock by design, since shortening the ladder would mean testing something other than the shipped behaviour.
🤖 Generated with Claude Code