Skip to content

The TUI goldens sync on a quiet period — the flake that failed reconverge's main this morning #26

Description

@vyncint

The same bug, in the sibling project, this morning

reconverge's flakiness gate failed on main on macOS at both 2 and 16 threads:

test shell_is_deterministic_at_120x40 ... FAILED
assertion `left == right` failed: 120x40: frame differs from golden shell-120x40.txt

The golden was correct. The frame was torn. Cause: the tests synced with wait_idle(150ms) — wait for the output to go quiet, then read the screen. That is a guess at how long a repaint takes, and on a loaded runner it is the wrong guess: the app pauses mid-repaint, the quiet period elapses, and the screen read is half-painted. No larger number fixes it, because there is no number that is right on every runner.

crates/launchbound-tui/tests/tui.rs has the same pattern, in seven places:

t.wait_idle(QUIET).expect("idle");
...
t.wait_idle(QUIET).expect("view settled");
...
t.wait_idle(QUIET).expect("scrolled idle");
assert_golden("ranking-scrolled-80x24.txt", &t.screen().to_string(), "ranking");

It has already bitten here once

tests/tui.rs:106-108, in this repository:

// The original golden was blessed from a too-early capture and never
// verified scrolling at all — caught by ubuntu delivering all five keys.

A golden blessed from a too-early capture is this exact failure, caught by luck rather than by the harness — and the consequence was worse than a red build: the test passed while verifying nothing. The wait_until added there fixes that one assertion; the seven wait_idle calls around it are the same hazard still standing.

The fix costs nothing here, because the app is already correct

crates/launchbound-tui/src/main.rs:47-49 already brackets every repaint:

queue!(std::io::stdout(), BeginSynchronizedUpdate)?;
terminal.draw(...)?;
queue!(std::io::stdout(), EndSynchronizedUpdate)?;

So termlens's wait_frame — which evaluates predicates on complete frames only and returns the one it matched — works against this binary today. reconverge needed a source change to get here; launchbound does not. Only the tests change.

let frame = t.wait_frame(|s| s.to_string().contains("ranking (")).expect("ranking view");
assert_golden("ranking-80x24.txt", &frame.to_string(), "ranking");

Assert on the frame wait_frame returns, not on whatever screen() holds a moment later.

One trap, recorded so it is not rediscovered

Do not mechanically swap wait_idle for wait_frame on a generic marker while leaving the preceding wait_until in place. wait_frame returns the earliest frame no earlier call has observed, so a second call with a different predicate can hand back a stale retained frame immediately. Trying that on reconverge failed 31 of 40 local runs. The two calls must merge into one wait_frame carrying the content predicate.

What it also buys

reconverge's 50-iteration gate went from 11s to 0.25s — the wait was most of the runtime. This suite runs a 100-iteration stress (stress_100_runs_at_80x24), so the saving here is larger.

Done when

tests/tui.rs contains no wait_idle, every golden assertion is made against a frame wait_frame returned, and the module's sync-policy note says so.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions