Skip to content

test(tui): wait for each key's redraw instead of a fixed 25ms - #652

Merged
sirmalloc merged 1 commit into
sirmalloc:mainfrom
eric-engberg:fix/flaky-tui-tests
Oct 6, 2026
Merged

sirmalloc merged 1 commit into
sirmalloc:mainfrom
eric-engberg:fix/flaky-tui-tests

Conversation

@eric-engberg

Copy link
Copy Markdown
Contributor

Test-only. #607, #609 and #593 also touch some of these test files; happy to rebase either way.

No demo: only tests change, and nothing in the TUI or the status line looks different.

Summary

The Ink component tests slept a fixed 25ms (flushInk()) after rendering and after every key, then asserted. Ink redraws, and React re-attaches Ink's input listener, on their own schedule. On a loaded machine the assertion runs before the redraw, or the next key arrives while the listener is detached and is dropped. These tests fail intermittently under CPU load, as on a busy CI runner or with several test runs going at once.

Each test now waits for the visible effect of its key, with a timeout:

  • the selection marker moving;
  • a prompt or search result appearing;
  • a callback firing.

The waits go through a small shared helper, src/tui/__tests__/helpers/wait-for-ink.ts. After each successful wait it yields two setImmediate turns, which lets React run the passive effect that re-attaches the input listener before the next key.

No assertions change. Where a test checks that something did not happen, it does so after waiting for the positive effect. A few stdout mocks gained the clearOutput() that the others already had, so a wait only matches the new frame.

Covered (11 files): GlobalOverridesMenu, ImportPreviewDialog, InstallMenu, PowerlineSetup, PowerlineThemeSelector, RefreshIntervalMenu, StatusLinePreview and TerminalWidthMenu, plus the locale, timezone and symbol-override editors. ItemsEditor.test.ts and ColorMenu.test.tsx use the same pattern; they're left out here because I'm changing those files in other work and will convert them there.

Testing

  • Reproducing the flakiness: I ran the 11 files under Node with four CPU-busy loops and three runs at a time, 12 runs per side:
    • main: 24 failures in 912 tests, across TerminalWidthMenu (7), RefreshIntervalMenu (9), the timezone editor (7) and the locale editor (1).
    • This branch: 0 failures in 912 tests, though the load during its turn was higher (19.5 against 12).
    • The same tests also failed earlier on main when two full suites ran at once, along with the symbol-override editor and ImportPreviewDialog.
  • bun run lint and bun test (2,358 pass).
  • Under Node with Vitest: the full suite fails in the same 17 files as main (all from spying on Node built-ins), and all 11 changed files pass.

The Ink component tests slept a fixed 25ms after rendering and after each
key, then asserted. Ink redraws, and React re-attaches Ink's input listener,
on their own schedule, so on a loaded machine the assertion ran before the
redraw, or the next key arrived before the listener was back and was dropped.
Under CPU load these tests failed intermittently: TerminalWidthMenu,
RefreshIntervalMenu and the timezone editor most often.

Each test now waits for the visible effect of its key (the selection moving,
a prompt appearing, a callback firing), with a timeout, through a small
shared helper in src/tui/__tests__/helpers/wait-for-ink.ts. After each
successful wait it yields two setImmediate turns so React can re-attach the
listener before the next key. No assertions change.
@sirmalloc
sirmalloc merged commit 6e6f6de into sirmalloc:main Oct 6, 2026
3 checks passed
@eric-engberg
eric-engberg deleted the fix/flaky-tui-tests branch October 9, 2026 21:47
pcvelz added a commit to pcvelz/ccstatusline-usage that referenced this pull request Oct 11, 2026
Upstream: label prefixes editable with e (sirmalloc#601, sirmalloc#666), Session Cost Rate (sirmalloc#671) and Extra Usage Daily Budget (sirmalloc#672) widgets, own-lock-only usage lock clearing (sirmalloc#624), NO_PROXY and request deadlines, terminal control-sequence sanitizing, git hardening and linear-time parsing, shared searchable option editor (sirmalloc#649) and widget base classes (sirmalloc#646-sirmalloc#654), node: imports (sirmalloc#657), Number.* globals lint (sirmalloc#645), test waits on redraw instead of fixed delays (sirmalloc#652).

Hand edits outside conflicts:
- src/widgets/shared/searchable-option-editor.tsx: compat fix - ports the fork's ref-based input handling (Enter picks from what was just typed) into upstream's new shared editor
- src/utils/usage-fetch.ts: compat fix - fork's fetchFreshUsageData uses upstream's clearOwnUsageLock (sirmalloc#624) instead of the removed clearUsageLock
- src/widgets/ApiUsage.tsx, src/widgets/Battery.ts: compat fix - Number.isNaN/Number.parseInt for upstream's no-restricted-globals rule
- src/utils/__tests__/usage-prefetch.test.ts: compat fix - mock renamed to upstream's mockFetchUsage
- src/tui/components/__tests__/PowerlineSetup.test.ts, src/tui/components/__tests__/StatusLinePreview.test.ts, src/tui/components/__tests__/TerminalWidthMenu.test.ts: compat fix - drop the fork's flushInk import, unused after upstream's waitFor switch
- README.md: upstream notes linked docs/USAGE.md (deleted in the fork); inlined the proxy and workspace-trust text instead; upstream's own release-note entries not taken
- .fork-keep-deleted: src/widgets/FableWeeklyUsage.ts (upstream widget, unregistered in the fork, which ships WeeklyFableUsage.ts)
- src/widgets/WeeklyFableUsage.ts: compat fix - adopts upstream's shared getUsagePercentWidgetEditorDisplay, which carries the time-cursor modifier
- src/tui/__tests__/App.test.ts: fork deviation - one extra down-arrow for the fork's Off Hours main-menu row
- src/widgets/__tests__/CacheTimer.test.ts, src/widgets/CacheTimer.ts: compat fix - emoji toggle moved from e to o because upstream reserves e for edit-label
- src/widgets/AgentAddress.ts: split the session-title guard into its own if (same behaviour) so the .fork-invariants nameSource guard matches again
- docs/test-retirements.md: ledger for tests upstream renamed or replaced
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.

2 participants