test(cli): wait for FinishIndex instead of racing it - #231
Merged
Conversation
Three tests in internal/watcher wait for BeginIndex and then assert on FinishIndex. Those are two sequential HTTP calls from the indexer goroutine, so the window between them is small but real, and on a loaded CI runner the assertion lands inside it. It surfaces as "expected FinishIndex to be called" on a test that took 0.02s — not a timeout, an assertion made too early. Adds waitForFinish for the tests that actually care about completion, and keeps waitForCalls for those that only assert BeginIndex. TestDebounce_MultipleEventsOnce had a different problem with the same shape: its deadline was ten times the 80ms debounce interval, which reads generous and is 800ms. A cold macOS runner loses that, and because the flush goroutine then ran after t.TempDir() cleanup, the failure printed as "cannot read a.go: no such file or directory" rather than as the assertion that failed. The property under test is that five events collapse into one flush, not how quickly, so it now waits on patience rather than on the interval. The shared deadline goes to 15s. It exits the instant the condition holds, so a long limit costs nothing when things work. Both failures appeared on macos-latest during the mac-runtime PRs (#229, #230) and passed on rerun; neither is caused by those changes, but the extra parallel load of a new test package is enough to make the races fire. 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.
Three tests in
internal/watcherwait forBeginIndexand then assert onFinishIndex. Those are two sequential HTTP calls from the indexer goroutine,so the window between them is small but real, and on a loaded CI runner the
assertion lands inside it. It surfaces as
expected FinishIndex to be calledon a test that took 0.02s — not a timeout, an assertion made too early.
TestDebounce_MultipleEventsOncehad a different problem with the same shape:a deadline of ten times the 80 ms debounce interval, which reads generous and
is 800 ms. A cold macOS runner loses it, and because the flush goroutine then
ran after
t.TempDir()cleanup, the failure printed ascannot read a.go: no such file or directoryrather than as the assertion that actually failed.Both showed up on
macos-latestduring #229 and #230 and passed on rerun.Neither is caused by those changes — but the extra parallel load of a new test
package is enough to make latent races fire, and they will keep firing.
Test-only; no production code touched.
🤖 Generated with Claude Code