Repository navigation
test(watcher): poll-only-watched-projects waits for the watched reindex, not one poll - #2408
Merged
Merged
Conversation
…ex, not one poll watcher_poll_only_watched_projects failed on the macOS LSan job of PR #2295, whose diff was Java-only, with `FAIL tests/test_watcher.c:2971: index_call_count == 0, expected 1` (8211 passed, 1 failed). The LSan report that followed points into the aborted test body (watcher and store never freed after the early FAIL): a consequence, not a separate leak. The assert expected ONE touch+poll after a dirty change to reindex exactly once. That needs every git probe of the poll to succeed. A poll whose git spawn is refused under runner load is a fail-closed no-op by design (#937: the baseline stays uncommitted and the next poll retries), and that path returns without a log line. This is the same test race that #2376 fixed in watcher_unwatch_drains_pending_free and watcher_sustained_failure_logs_once_issue2015, not a production defect. Holding one exhausted spawn with cbm_subprocess_force_spawn_eagain_for_testing at that poll reproduces the CI output exactly: watch, baseline, then `FAIL tests/test_watcher.c:2971: index_call_count == 0, expected 1 == 1`, with no watcher.changed and no watcher.git.failed. The old test was also partly inert. The Go original (TestPollAllOnlyWatched, from the #49 fix that replaced scan-all with an explicit watch list) registered BOTH projects in the store and asserted that the unwatched one gets no watcher state. The C port dropped the store registration, so nothing let the watcher learn about B, and the B half could not fail. The test now: - registers A and B in the store, and watches only A; - asserts that the baseline poll indexes nothing and does not adopt B; - makes both worktrees dirty (a dirty change, not a commit, so a late baseline still sees it); - polls until A's reindex has happened, asserting after EVERY poll that B has no watcher state (cbm_watcher_index_failure_count == -1), was never reindexed, and that the watch count is still 1; - asserts that A was reindexed exactly once. No timer or deadline decides the verdict. A watcher that never reindexes A hangs, which the runner reports as a failure, never a pass. Evidence (macOS arm64, ASan/UBSan test-runner, CBM_SANITIZED, so one exhausted spawn = 10 injected refusals), with temporary injection that has since been removed: - CI-shaped (baseline OK, first change poll refused): consumed=10 at loop poll 1, watcher.changed at loop poll 2, PASS. - Heavy (baseline poll plus loop polls 1-3 refused): loop poll 4 takes the baseline, poll 5 reindexes A; loop polls=5 watched=1 other=0, PASS. - Mutation, the pre-#49 scan-all (poll_once adopts every stored project): `FAIL tests/test_watcher.c:3001: cbm_watcher_index_failure_count(w, "projB-ow") == 0, expected -1 == -1`, both unforced and with the heavy refusals (A's and B's baseline spawns both refused). The other 82 watcher tests stay green under that mutation, so this is the only test guarding #49. Final tree: build/c/test-runner watcher, 5/5 runs, 83 passed each. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.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
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.
watcher_poll_only_watched_projects failed on the macOS LSan job of PR
#2295, whose diff was Java-only, with
FAIL tests/test_watcher.c:2971: index_call_count == 0, expected 1(8211 passed, 1 failed). The LSan report that followed points into the
aborted test body (watcher and store never freed after the early FAIL):
a consequence, not a separate leak.
The assert expected ONE touch+poll after a dirty change to reindex
exactly once. That needs every git probe of the poll to succeed. A poll
whose git spawn is refused under runner load is a fail-closed no-op by
design (#937: the baseline stays uncommitted and the next poll retries),
and that path returns without a log line. This is the same test race
that #2376 fixed in watcher_unwatch_drains_pending_free and
watcher_sustained_failure_logs_once_issue2015, not a production defect.
Holding one exhausted spawn with
cbm_subprocess_force_spawn_eagain_for_testing at that poll reproduces
the CI output exactly: watch, baseline, then
FAIL tests/test_watcher.c:2971: index_call_count == 0, expected 1 == 1,with no watcher.changed and no watcher.git.failed.
The old test was also partly inert. The Go original (TestPollAllOnlyWatched,
from the #49 fix that replaced scan-all with an explicit watch list)
registered BOTH projects in the store and asserted that the unwatched
one gets no watcher state. The C port dropped the store registration, so
nothing let the watcher learn about B, and the B half could not fail.
The test now:
baseline still sees it);
has no watcher state (cbm_watcher_index_failure_count == -1), was never
reindexed, and that the watch count is still 1;
No timer or deadline decides the verdict. A watcher that never reindexes
A hangs, which the runner reports as a failure, never a pass.
Evidence (macOS arm64, ASan/UBSan test-runner, CBM_SANITIZED, so one
exhausted spawn = 10 injected refusals), with temporary injection that
has since been removed:
loop poll 1, watcher.changed at loop poll 2, PASS.
the baseline, poll 5 reindexes A; loop polls=5 watched=1 other=0, PASS.
FAIL tests/test_watcher.c:3001: cbm_watcher_index_failure_count(w, "projB-ow") == 0, expected -1 == -1, both unforced and with the heavyrefusals (A's and B's baseline spawns both refused). The other 82
watcher tests stay green under that mutation, so this is the only
test guarding codebase-memory MCP server OOM kills entire Claude Code session #49.
Final tree: build/c/test-runner watcher, 5/5 runs, 83 passed each.
Unblocks #2295, whose only red is this flake.