fix: make TestLockPortSameDirectory_NoError resilient to busy ports#83
Open
fix: make TestLockPortSameDirectory_NoError resilient to busy ports#83
Conversation
Instead of hardcoding port 3003, dynamically find a free port within the configured range (3000-4000). This prevents flakiness when the hardcoded port is in use by other services (e.g. docker-proxy). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Revert err2 back to idiomatic err (no shadowing conflict) - Fix "Step 1" comment: allocate -> allocate and lock - Add allocation state verification after re-lock (matches sibling test pattern) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Move port discovery after setup to minimize TOCTOU window - Use t.Skipf instead of t.Fatalf for first --lock call (matches project pattern for port-unavailable scenarios) - Add .gitignore entries for pr-review-fix-loop artifacts Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Narrow t.Skipf guard on first --lock call to only skip for TOCTOU-related
failures ("in use", "busy"). All other failures now use t.Fatalf so real
regressions in the --lock code path are not silently skipped.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Check for external allocation path in Step 1 output after success. If port was claimed by external process between discovery and lock, skip the test instead of continuing with misleading state. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Change t.Fatal to t.Skipf when no free port is found in range, matching project convention for port-unavailability scenarios. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Check fmt.Sscanf error return value (errcheck) - Add unreachable return after t.Fatal to satisfy staticcheck SA5011 (staticcheck doesn't recognize t.Fatal as terminating) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Use port-specific TOCTOU string matching instead of generic "in use"/"busy" - Verify external allocation state instead of blindly skipping - Add comment explaining why Step 2 is immune to TOCTOU - Log last error when no free port found in scan - Validate parsed port range (1-65535) in Sscanf test - Verify Name field preserved after re-lock Co-Authored-By: Claude Opus 4.6 <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.
Summary
TestLockPortSameDirectory_NoErrorwith dynamically discovered free portTest plan
TestLockPortSameDirectory_NoErrorpassesgo test ./...)🤖 Generated with Claude Code