Skip to content

Fix the flaky capture-loop test at its cause, not its timeout - #76

Merged
Purple10101 merged 1 commit into
mainfrom
20260831-fix-flaky-capture-test
Aug 31, 2026
Merged

Purple10101 merged 1 commit into
mainfrom
20260831-fix-flaky-capture-test

Conversation

@Purple10101

Copy link
Copy Markdown
Collaborator

Why

test_refresh_failure_does_not_kill_capture_loop fails roughly 1 run in 20, on any machine. It took main red after an unrelated merge, and cost a re-run on #75 before that.

It is not a slow runner, and raising the timeout would not have helped.

Cause

_run() only re-renders when new_frames_since_render > 0, and the first render resets that counter:

do_render = has_viewers and (
    self._refresh_requested
    or (new_frames_since_render > 0 and now - last_render >= RENDER_INTERVAL_S)
)

The capture thread starts on service.start() and consumes frames every POLL_INTERVAL_S while the main thread is still on its way to attach(). With a fixed two-frame list there is a race: if both frames are gone before a viewer is registered, the first render fires on _refresh_requested, the counter resets, no frame ever arrives again, and the second render can never happen. calls["n"] sticks at 1 and the wait expires however long it is.

Fix

The test now takes frames from a source that never runs out, so a second render is reachable whatever order the threads happen to run in. The assertion then tests what it means to test, that the loop survives an exception, rather than incidentally testing thread scheduling.

FakeBlah2Client is deliberately left alone: other tests assert on a fixed frame count and rely on it draining.

Measured

failures
Before, on this branch's base 2 / 25
Before, on origin/main 2 / 50
After 0 / 150

Full suite: 802 passing.

test_refresh_failure_does_not_kill_capture_loop failed roughly 1 run in 20, on
any machine, and took main red after an unrelated merge. It is not a slow
runner and a longer timeout would not have helped.

_run() only re-renders when new_frames_since_render > 0, and the first render
resets that counter. The capture thread starts on service.start() and consumes
frames every POLL_INTERVAL_S while the main thread is still on its way to
attach(). With a fixed two-frame list there is a race: if both frames are gone
before a viewer is registered, the first render fires on _refresh_requested,
the counter resets, no frame ever arrives again, and the second render can
never happen. calls["n"] sticks at 1 and the wait expires however long it is.

The test now takes frames from a source that never runs out, so a second
render is reachable whatever order the threads happen to run in. The assertion
then tests what it means to test, that the loop survives an exception, rather
than incidentally testing thread scheduling.

FakeBlah2Client is left alone. Other tests assert on a fixed frame count and
rely on it draining.

Measured: 0 failures in 150 runs, against 2/25 and 2/50 before. Full suite 802
passing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Purple10101
Purple10101 merged commit 9415002 into main Aug 31, 2026
3 checks passed
@Purple10101
Purple10101 deleted the 20260831-fix-flaky-capture-test branch September 6, 2026 15:51
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.

1 participant