Uh oh!
There was an error while loading. Please reload this page.
test(cli): replace fixed waits in Pi TUI coverage with observable completion - #2448
Merged
jackwener merged 1 commit intoAug 7, 2026
Conversation
…pletion Part of apache#2389. The Pi TUI runner suite guessed at scheduler timing in three ways, all sensitive to CI load: - 55 unconditional sleeps (delay(20)..delay(2500)) where the awaited fact is observable: startup paints, prompt arming, wizard frames opening/closing, drafts appearing, and retry attempts are all now waited on directly via the existing waitFor helper, a new waitForTuiPaint helper, sentinel-render barriers for must-not-render negatives, and close-anchored assertions for must-never-happen driver calls. - 81 Promise.race close watchdogs with a flat 50/500ms deadline that a loaded runner can miss; they now share CLOSE_BUDGET_MS (WAIT_BUDGET_MS-scaled), so a passing race is unchanged and only failure reporting pays the budget. - The off-screen ticker test slept 2.5s for two real 1s ticks; the shell-run elapsed ticker's clock and interval scheduling are now injectable (MakaPiTuiInput.shellRunTicker, same pattern as attentionLongTurnThresholdMs) and the test drives both ticks itself. Synchronous input-dispatch negatives (double-Escape counting, Ctrl-C/Ctrl-D gating, submit gates) settle on one macrotask turn (delay(0)) instead of a 20-30ms guess, and the never-dropped fallback-enqueue test now waits for the driver to observe the retries crossing the old give-up budget instead of sleeping 2.2s blind. Preserved as real timeouts or real process boundaries: the SIGTERM/fatal exit probes (real child processes and the 3s exit grace), the 100ms fallback retry cadence, and the 250ms hydration-retry cancellation window (now documented as schedule-derived). Focused timing (node --test dist/__tests__/pi-tui-runner.test.js, 142 tests): 15.5s -> 12.1s wall locally, with the remainder dominated by the preserved real-boundary tests, and no fixed-delay races left in the suite.
This was referenced Aug 7, 2026
Uh oh!
There was an error while loading. Please reload this page.
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 freeto 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.
Part of #2389 (Pi TUI workspace; Runtime Host, desktop e2e, and alignment audit follow in separate PRs per the issue).
What changed
Three fixed-wait patterns in
pi-tui-runner.test.ts(plus one helper intui-terminal-mock.ts), replaced with the closest deterministic mechanism:await delay(20..2500)waitForon the actual observable fact: first TUI paint (waitForTuiPaint), prompt arming ("Allow access outside the workspace?" on screen), wizard frames opening/closing, drafts appearing, driver retry countsPromise.race([run, delay(50/500).then(throw)])close watchdogsCLOSE_BUDGET_MS = max(WAIT_BUDGET_MS, 500)— a passing race still resolves the momentrunsettles; only failure reporting pays the budget (same split aswaitFor, #2221)now+scheduleare now injectable viaMakaPiTuiInput.shellRunTicker(same test-injection pattern asattentionLongTurnThresholdMs); the test fires both ticks deterministicallyFor must-NOT-happen assertions, the suite now uses three deterministic idioms instead of sleeping and hoping:
await run— every queued input has provably drained.delay(0)macrotask barrier: for gates that run synchronously off input dispatch (double-Escape counting, Ctrl-C/Ctrl-D, submit gates) — one macrotask turn runs strictly after every queued microtask, independent of machine load.The never-dropped fallback-enqueue test no longer sleeps 2.2s blind: the fake driver counts enqueue attempts, and the test waits for the retries to observably cross the old ~20-attempt give-up budget (the exact regression #1135-era bug shape), still at the real 100ms cadence so a reintroduced time-based budget would also trip it.
Retained real-boundary contracts (unchanged by design)
runSignalExitProbe/runFatalExitProbe: real child processes, real signals, and the real 3sPROCESS_EXIT_GRACE_MSdeadline (3.5s + 2×0.5s of the remaining wall time).Production surface
One additive, optional injection point:
MakaPiTuiInput.shellRunTicker?: { now?, schedule? }, threaded intocreateShellRunElapsedTicker(which already accepted both). Defaults unchanged.Timing (local,
node --test dist/__tests__/pi-tui-runner.test.js, 142 tests)The remaining wall time is dominated by the preserved real-boundary tests listed above; every other test now completes the moment its observed condition holds, and no assertion in the suite races scheduler timing anymore. Full
packages/clisuite: 492/494 pass — the 2 failures are pre-existing local noise (run-command.test.tsexpects empty stderr; Node 22.17 emits a SQLite ExperimentalWarning) on paths this PR does not touch.