Skip to content

test(fleet): wait on conditions instead of fixed sleeps in TUI poll tests - #478

Merged
Jason Robert (jrob5756) merged 1 commit into
mainfrom
test/fleet-tui-poll-flakes
Aug 22, 2026
Merged

test(fleet): wait on conditions instead of fixed sleeps in TUI poll tests#478
Jason Robert (jrob5756) merged 1 commit into
mainfrom
test/fleet-tui-poll-flakes

Conversation

@jrob5756

Copy link
Copy Markdown
Collaborator

Problem

Main's CI has been failing intermittently. The failures are flaky tests, not a product regressionsrc/ needs no change.

Two failures on main, both in the Fleet Manager TUI pilot tests:

RunTestFailure
32296506434 (3.13/ubuntu)test_a_scan_failure_does_not_wedge_the_screen_refreshing never cleared
32272384555 (3.13/windows)test_poll_tick_removes_completed_runassert 1 == 0

The first also failed on 32044377141 (Aug 17), in its pre-#464 single-sample form — so this is a recurring flake a previous fix attempt did not close.

Root cause

These tests assert on state that only becomes true once a realset_interval poll tick lands, but wait a fixed asyncio.sleep(0.3) or sample a transient flag. CI runs pytest under coverage tracing on a small shared runner, where a tick-plus-scan is several times slower than on a developer box, so the fixed wait encodes a guess about machine speed and asserts against pre-tick state when the guess is wrong.

The guard-recovery test is the more interesting one. Instrumenting it:

  • pilot.pause() costs ~1s per call under the tests' 20 Hz timer (it waits for the app to go idle, which a 0.05s poll timer keeps deferring), so the helper's 5s deadline yielded only ~4 samples.
  • _refreshing is legitimately True19.5% of the time idle and 34.8% under coverage.

So the assertion was a coin flip that a slow machine loads — it can see True on every sample and report a wedged screen. The finally in _refresh_worker that it guards is correct and present.

Fix

  • Add wait_for(pilot, predicate, message=...) to tests/test_fleet/conftest.py: polls a condition to a deadline, samples with asyncio.sleep rather than the ~1s pilot.pause(), and fails with a message naming the regression instead of an opaque assert 1 == 0.
  • Rewrite the two guard-recovery tests to assert a monotonic, user-visible consequence — a newly written record appears in the table — rather than sampling a transient flag. That cannot be missed by any sampling rate, is unreachable if the guard latched, and proves recovery all the way to rendering.
  • Convert the affected poll sites in test_tui_runs.py and test_tui_run_detail.py.
  • Fix two latent siblings of the same bug: an assert call_count == 1 that could observe 0 before the mount refresh had entered the collector, and a selection-preservation test that passed vacuously when no tick landed inside the fixed sleep.

Verification

  • Mutation test — removing the finally in _refresh_worker makes both rewritten tests fail deterministically in ~2s with the intended message (no scan ran while the failure was injected -- the screen was already wedged...). The new assertions are not vacuous.
  • Contention test — 2 contended CPUs + coverage (~6x slowdown, worse than CI): reworked tests 8/8 pass; the previous versions reproduced the failure 1/8 under identical conditions.
  • Full CI-equivalent suite (-m "not real_api and not performance", with coverage): 7426 passed, 60 skipped.
  • make lint and make typecheck clean.
  • Bonus: the affected tests run ~2x faster, since they now return as soon as the condition holds instead of sleeping fixed durations and calling the slow settle().

…ests
The Fleet Manager TUI pilot tests assert on state that only becomes true
once a real `set_interval` poll tick lands, but waited a fixed
`asyncio.sleep(0.3)` or sampled a transient flag. CI runs pytest under
`coverage` tracing on a small runner, where a tick-plus-scan is several
times slower than on a developer box, so those waits asserted against
pre-tick state and failed intermittently on main:
* `test_a_scan_failure_does_not_wedge_the_screen` sampled the transient
`_refreshing` flag. Measured, `pilot.pause()` costs ~1s under the 20Hz
test timer, so its 5s deadline got only ~4 samples while the flag is
legitimately True 19.5% of the time idle and 34.8% under coverage --
it could see True on every sample and report a wedged screen.
* `test_poll_tick_removes_completed_run` failed on Windows CI as
`assert 1 == 0` because 0.3s was not enough for a tick.
Add `wait_for()` to the fleet conftest: poll a predicate to a deadline,
sampling with `asyncio.sleep` rather than the ~1s `pilot.pause()`, and
fail with a message naming the regression. Convert the affected sites in
`test_tui_runs.py` and `test_tui_run_detail.py`.
Rewrite the two guard-recovery tests to assert a monotonic, user-visible
consequence -- a newly written record appears in the table -- rather than
sampling a transient flag. That condition cannot be missed by any
sampling rate, is unreachable if the guard latched, and proves the screen
recovered all the way to rendering. Both still fail deterministically
when the `finally` in `_refresh_worker` is removed.
Also fix two latent siblings of the same bug: an `assert call_count == 1`
that could observe 0 before the mount refresh had entered the collector,
and a selection-preservation test that passed vacuously when no tick
landed within the fixed sleep.
Verified under 2 contended CPUs with coverage (~6x slowdown, worse than
CI): the reworked tests passed 8/8 where the previous versions
reproduced the failure 1/8. No `src/` change -- the `finally` these tests
guard was already correct.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jrob5756
Jason Robert (jrob5756) merged commit 2e5689d into mainAug 22, 2026
13 checks passed
@jrob5756
Jason Robert (jrob5756) deleted the test/fleet-tui-poll-flakes branch August 22, 2026 21:14
Sign up for freeto 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

@jrob5756