Skip to content

test(pty): synchronize inputs on committed screen state - #1083

Merged
elucid merged 2 commits into
mainfrom
test/pty-ci-flakes
Sep 10, 2026
Merged

test(pty): synchronize inputs on committed screen state#1083
elucid merged 2 commits into
mainfrom
test/pty-ci-flakes

Conversation

@elucid

@elucid elucid commented Sep 10, 2026

Copy link
Copy Markdown
Member

Test-only. Fixes the PTY integration flakes that have been failing unrelated PRs on ubuntu-latest (e.g. #1078 ×3, #1079's neighbours, several main runs).

Cause

tuistory's press() resolves after output-idle, which can be 20 ms and does not prove OpenTUI parsed the key or React committed the resulting state. Several tests then waited on text that is present on both the outgoing and the destination screen (a commit title in the review header and in history; a note body in the draft and the saved note), so the wait returned early and the next key went to the wrong surface. The filter focus test typed immediately after Tab, before the filter input mounted, so the t in beta fired the theme shortcut.

Fix

  • harness.pressAndWaitForSnapshot(session, key, predicate) — send one key and wait for a destination-specific screen state. Never resends on timeout; a dropped key stays a failure. Unit-tested (harness.test.ts).
  • log-integration.test.ts: return-to-history waits on Git history + Enter open chrome instead of the commit title; visual-selection cancel waits for the selection indicator to disappear.
  • chrome.test.ts: wait for the filter input before typing (as the / sibling already did).
  • notes.test.ts: saved-note navigation waits on saved-state markers rather than shared body text.

No timeout, retry, sharding, or application changes. Coalesced-input cases ("1q", "\rq", text+save in one write) are preserved.

Pass rates under contention (Bun 1.4.2; Linux pinned to 2 CPUs + hog; macOS 18 hogs)

Test Linux before → after macOS before → after
hunk log › resolves canonical history command remaps 1/20 → 30/30 20/20 → 30/30
hunk log › retains changed review layout across repeated reviews 4/20 → 30/30 15/20 → 30/30
chrome › filter focus narrows the visible review stream 8/20 → 30/30 20/20 → 30/30
notes › saved notes are visibly active and keyboard-navigable 18/20 → 30/30 20/20 → 30/30
notes › late textarea events after a fast save 18/20 → 30/30 — → 30/30
chrome › keyboard menu navigation, key-routing › menu Enter, notes › marker off 20/20 → 30/30 20/20 → 30/30

Full bun run test:integration ×3 per platform under contention: all fixed tests pass every run.

Not fixed here (pre-existing, reproduce on main)

  • highlighting › keeps key input responsive while a large added file highlights: 0/20 on 2 pinned Linux CPUs even without a hog (PageDown lands in 1.4–1.6 s vs. a 1 s budget). Left untouched — the assertion's contract needs a decision, not a bigger number.
  • macOS trust/denial persisted-state expectations and SIGHUP/SIGQUIT/SIGPIPE clean-exit tests: fail on main.
  • Linux watch-start child-exit cleanup and multi-row copy drag: occasional, reproduce on main.
  • Product bug surfaced by the filter case: Typing immediately after Tab can trigger global shortcuts instead of filling the filter #1082 (typing right after Tab dispatches global shortcuts).

Checks

bun run typecheck, bun run deps:check, bun test test/pty/harness.test.ts, oxfmt/oxlint on changed files — pass on macOS and Linux. Empty changeset (maintenance). Not run on Windows (PTY group is Unix-only).

Observe an explicit destination after sending a key instead of treating output idleness as acknowledgement. Cover delayed paint and timeout without input retries in the shared harness.
Distinguish history chrome from commit titles shared with review, observe Escape clearing visual selection, and wait for filter focus and saved-note cards before subsequent input. Preserve intentionally coalesced input and existing timeout budgets.
@vercel

vercel Bot commented Sep 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
hunk-web Ignored Ignored Sep 10, 2026 3:42am UTC

Request Review

@greptile-apps

greptile-apps Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR makes PTY integration tests synchronize subsequent input with destination-specific committed screen state instead of relying on output idleness or text shared between screens.

  • Adds a single-key synchronization helper with regression coverage for delayed transitions and timeout behavior.
  • Updates history, visual-selection, filter-focus, and note-save tests to wait for state-specific UI markers.
  • Preserves intentionally coalesced raw-input scenarios without adding retries or application changes.

Confidence Score: 5/5

The PR appears safe to merge; the test-only synchronization changes use state markers that distinguish the relevant destination screens.

No actionable failure remains: keys are sent once, polling waits for committed destination-specific snapshots, and each updated predicate is valid for its fixture and transition.

Important Files Changed

Filename Overview
test/pty/harness.ts Adds a one-shot key synchronization helper that polls for a caller-defined committed destination state.
test/pty/harness.test.ts Covers delayed destination visibility and verifies that timeout does not resend input.
test/pty/log-integration.test.ts Synchronizes review-to-history and visual-selection transitions using destination-specific history chrome.
test/pty/chrome.test.ts Waits for the focused filter input to render before typing filter text.
test/pty/notes.test.ts Synchronizes note-save tests on draft disappearance and saved-note markers rather than shared body text.

Sequence Diagram

sequenceDiagram
    participant Test
    participant Session
    participant TUI
    Test->>Session: press(key)
    Session->>TUI: send key once
    loop Until timeout
        Test->>Session: text(immediate)
        Session-->>Test: current screen snapshot
        alt Destination predicate matches
            Test-->>Test: continue with next input
        else Destination not committed
            Test->>Session: waitIdle
        end
    end
Loading

Reviews (1): Last reviewed commit: "test(pty): wait for committed history, f..." | Re-trigger Greptile

@elucid
elucid merged commit 4b93aa8 into main Sep 10, 2026
15 checks passed
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