Skip to content

test(watch): burst detector on a caller-supplied clock; storm test asserts exact counts - #221

Merged
JordanCoin merged 2 commits into
mainfrom
fix/201-burst-test-determinism
Oct 11, 2026
Merged

JordanCoin merged 2 commits into
mainfrom
fix/201-burst-test-determinism

Conversation

@JordanCoin

Copy link
Copy Markdown
Owner

Follow-up to #215. TestCheckoutStormCoalescesIntoOneBatch flaked on #219's ubuntu 1.26 run: 27 git subprocesses against the ≤17 bound, because 500 events in 200 ms against 250 ms wall-clock windows split into two bursts on a loaded runner.

Change

The detection logic leaves the event loop and becomes burstDetector (watch/burst.go): observe(now), enqueue(event, now) time.Duration, take(), pending(). None of them read the wall clock; the loop passes time.Now() and owns only the flush timer. Behaviour is unchanged; watch/events.go shrinks by the inlined closures.

Tests

test clock asserts
TestBurstDetectorOpensOnceForAStorm synthetic, 0.4 ms apart exactly burstThreshold events handled per-file, then a batch of 292 (one per remaining file, last op wins), burst closed after take, a lone event afterwards is not batched
TestBurstDetectorCeilingClipsSettle synthetic settle delay clipped at the ceiling, 0 past it
TestHandleBurstPaysEachSharedCostOnce synthetic git, topology and dependency refresh each exactly 1; 300 events recorded; 1 dirty
TestCheckoutStormIsBatchedThroughTheEventLoop wall clock, real loop per-file costs at most 4 * (burstThreshold + 1) = 68 against 500 unbatched: proves batching happened, tolerates a runner that splits the storm four ways
TestFewEventsAreNotBatched wall clock unchanged

Proof

  • Reproduction attempt first, on main before the change: go test ./watch/ -race -cpu 1 -run TestCheckoutStormCoalescesIntoOneBatch -count=30 with go test ./scanner/ -count=4 running in parallel: 30/30 passed on this Mac, so the CI split did not reproduce locally; the fix is by construction, not by observation.
  • After: macOS -race -count=50 clean; macOS -race -cpu 1 -count=50 with the scanner suite running alongside clean (54 s); Linux golang:1.24 in Docker -race -count=20 clean.
  • go build ./... && go test ./... && gofmt -l . clean.

Not changed: the constants, handleBurst, the seams from #215. Based on main at 0a324a7; #220 (daemon.go lifecycle) was still open and touches no line this PR edits.

Refs #201

🤖 Generated with Claude Code

https://claude.ai/code/session_01NRaBgKRAZMX9hjbkkefA7T

…st asserts exact counts on a synthetic clock

TestCheckoutStormCoalescesIntoOneBatch fed 500 events in 200 ms against
250 ms wall-clock windows and asserted at most 17 git calls; on a loaded
CI runner the storm split into two bursts and 27 were observed (#219's
ubuntu 1.26 run). The detection logic moves out of the event loop into
burstDetector, whose observe, enqueue and take all take `now`, so the loop
passes time.Now() and tests pass a fixed clock. The exact counts (16
per-file, then one batch of 292 files; git, topology and refresh each
exactly once) are asserted on the synthetic clock, and the one test that
still goes through the real loop asserts only that batching happened,
with the bound derived from burstThreshold.

Refs #201

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NRaBgKRAZMX9hjbkkefA7T
Copilot AI balanced review requested due to automatic review settings October 11, 2026 21:04

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@JordanCoin
JordanCoin merged commit 1f03d23 into main Oct 11, 2026
12 of 13 checks passed
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.

2 participants