Skip to content

test: Assert that the file data poller goes quiet once a file is stable - #473

Merged
joker23 merged 2 commits into
v7from
skz/filedata-poller-test-flake
Oct 13, 2026
Merged

joker23 merged 2 commits into
v7from
skz/filedata-poller-test-flake

Conversation

@joker23

@joker23 joker23 commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

The poller test TestPollerFiresOncePerChange expected exactly one callback for one rewrite and failed on CI with "unexpected change detection" (run 38454434943). A plain rewrite is not atomic: the truncation, the write, and the modification-time update are separate steps, and a poll that lands between two of them legitimately reports an extra change. The poller promises generous detection, not one callback per logical change; the Reloader coalesces and skips the excess, and that is covered by its own tests.

Making the test write atomically would hide the behavior that real writes exercise, so the write helpers write in the ordinary way. The test, renamed TestPollerStopsFiringWhenFileIsStable, now drains the callbacks of the write and then requires silence, which still fails for a poller that does not update its baseline after a change.

Local smoke testing (the poller tests repeated many times, under the race detector, under CPU contention, and with a variant whose rewrite steps are spread over several poll intervals) is in progress; the results follow in a comment.

@joker23
joker23 requested a review from a team as a code owner October 13, 2026 19:46

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

…n ordinary write

A plain rewrite is not atomic: the truncation, the write, and the modification-time update are separate steps, and a poll that lands between two of them legitimately reports an extra change. The poller promises generous detection, not one callback per logical change; the Reloader coalesces and skips the excess. The test that expected exactly one callback was therefore flaky on CI, and making the test's write atomic would only have hidden the behavior that real writes exercise.

The write helpers write in the ordinary way again, and the test now drains the callbacks of the write and then requires silence, which still fails for a poller that does not update its baseline.
@kinyoklion kinyoklion changed the title test: make the file data poller test write files atomically test: Assert that the file data poller goes quiet once a file is stable Oct 13, 2026
@kinyoklion

Copy link
Copy Markdown
Member

Replaced the atomic-write approach in a63ddf4 at the request of @kinyoklion: a test that only passes when the write is atomic would hide what the poller does for ordinary writes, which is to report each observed step. The helpers write in the ordinary way again, and the test now checks the property that matters, that the poller goes quiet once the file is stable, by draining the callbacks of the write before requiring silence. The title and description are updated to match.

Written by Claude Code on behalf of @kinyoklion.

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