test(watch): make the "a burst is one signal" tests independent of disk stalls - #189
Merged
Merged
Conversation
…sk stalls `a_burst_of_writes_collapses_into_one_signal` failed now and then on Linux CI with a second signal. The watcher was right; the test's premise was not. It writes ten times 10 ms apart and expects one signal, which holds only while no two writes are more than the 200 ms debounce window apart. On a CI disk shared with the other tests running in parallel, one `fs::write` (truncate, write, close) regularly stalls for 200-350 ms in writeback and dirty-page throttling; that splits the burst into two, and two signals is the correct answer. Reproduced in Docker with four `dd ... conv=fsync` loops as disk load: 21 of 40 runs failed, and timing each write showed every failing run had one write gap of 200 ms or more and every passing run had none. The watch tests in tw-watch and tw-config now use a directory on tmpfs (/dev/shm) on Linux, where a write never waits for the disk, so the premise holds. Under the same load: 0 of 40 failures, for both crates, and 0 of 10 for the whole tw-config suite. macOS keeps the system temporary directory. The debounce loop is also pulled out into `debounce_loop` and tested with no filesystem and no timing in the way: ten events queued before it starts must give exactly one signal (read from a channel large enough to see a second one, unlike the capacity-1 channel `watch` uses, which would swallow it), a later event another, and a closed source ends it. Making the loop send per event fails that test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Merged
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
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.
tw-configwatch::tests::a_burst_of_writes_collapses_into_one_signalfailed intermittently on Linux CI (a second signal within 600 ms). Seen on #183 and #185.Cause
The watcher is right; the test's premise is not guaranteed. It writes ten times, 10 ms apart, and expects exactly one signal, which holds only if no two writes are more than the 200 ms debounce window apart. On the CI disk, shared with the rest of the suite writing in parallel, a single
fs::write(truncate + write + close) regularly stalls 200–350 ms in writeback / dirty-page throttling. That splits the burst in two, and two signals is then the correct answer. It is not inotify coalescing or notify's delivery: notify's inotify loop forwards events as it reads them, and the debouncer'srecv_timeoutnever times out with an event queued.Evidence, in Docker (rust:1-bookworm) with four
dd … conv=fsyncloops as disk load:main: 21/40 runs fail. Timing each write: every failing run had a write gap ≥ 200 ms (229, 263, 247, 349 …); every passing run's largest gap was < 200 ms.Change
tw-watchandtw-configuse a directory on tmpfs (/dev/shm) on Linux, where a write never waits for the disk, so "ten writes 10 ms apart" really is one burst. Assertions unchanged. macOS keeps the system temp dir (never flaky there).debounce_loopand tested with no filesystem and no timing: ten events queued before it starts → exactly one signal, read from a channel big enough to see a second (the capacity-1 channel inwatchwould hide one); a later event → another signal; a closed source → it ends. A loop that signals per event fails this test (checked).No behavior change to the watcher.
tw-scan's watch tests have the same exposure but that crate is being moved to the desktop repo, so they are left alone.🤖 Generated with Claude Code