Skip to content

test(pty): cover note focus and multiple drafts#356

Merged
benvinegar merged 1 commit into
mainfrom
test/add-note-focus-multiple
May 23, 2026
Merged

test(pty): cover note focus and multiple drafts#356
benvinegar merged 1 commit into
mainfrom
test/add-note-focus-multiple

Conversation

@benvinegar
Copy link
Copy Markdown
Member

Summary

  • add Tuistory coverage that a focused draft note consumes app shortcuts until Esc cancels it
  • add Tuistory coverage for saving multiple [+] notes on one hunk

Tests

  • bun test test/pty/ui-integration.test.ts -t "draft note focus|multiple add-note" ×3
  • bun test test/pty/ui-integration.test.ts -t "add-note|draft note focus|multiple add-note|add-note drafts"
  • bun run typecheck
  • bun run lint

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 23, 2026

Greptile Summary

This PR adds two new PTY integration tests to test/pty/ui-integration.test.ts, using the existing Tuistory harness. It exercises two previously uncovered UI behaviors: the draft note input capturing app shortcuts (blocking navigation until Esc dismisses it), and adding multiple [+] notes to different rows of a single hunk.

  • Draft note focus test — opens a note via the c keyboard shortcut, confirms ] (navigation key) is consumed by the focused draft rather than triggering hunk navigation, then cancels with \x1b and confirms navigation resumes.
  • Multiple add-note drafts test — saves two separate draft notes on the same deletion-only hunk and asserts both notes remain visible in the final terminal snapshot.

Confidence Score: 5/5

Test-only change that adds two new PTY integration tests; no production code is modified.

Both new tests follow established harness patterns (fixture creation, session.type for ESC, waitForSnapshot polling), use appropriate fixtures, and have sound assertion logic. The lineIndexOf row-0 edge case is theoretical and matches the pre-existing pattern in adjacent tests. No logic issues found.

No files require special attention.

Important Files Changed

Filename Overview
test/pty/ui-integration.test.ts Two new integration tests added following established harness patterns; minor observation around lineIndexOf row-0 edge case, but this is consistent with existing tests in the same file.

Sequence Diagram

sequenceDiagram
    participant T as Test
    participant S as PTY Session
    participant U as Hunk UI

    Note over T,U: Test 1 — draft note focus blocks app shortcuts
    T->>S: press("c")
    S->>U: open draft note
    U-->>T: waitForText("Draft note") ✓
    T->>S: type("Keep focus here")
    T->>S: press("]")
    U-->>U: ] consumed by draft input (no navigation)
    T->>S: "waitForSnapshot("Keep focus here]" present, "line60=6000" absent)"
    T->>S: type("\x1b") — cancel
    U-->>U: draft dismissed
    T->>S: waitForSnapshot("Draft note" absent)
    T->>S: press("]")
    U-->>U: ] triggers hunk navigation
    T->>S: "waitForSnapshot("line60=6000" present)"

    Note over T,U: Test 2 — multiple add-note drafts on one hunk
    T->>S: revealAddNoteOnRow(contextRow)
    T->>S: click([+])
    U-->>T: waitForText("Draft note") ✓
    T->>S: type("First note on the context row.")
    T->>S: press(["ctrl","s"])
    U-->>T: waitForText("First note on the context row.") ✓
    T->>S: revealAddNoteNear(deletionRow)
    T->>S: click([+])
    U-->>T: waitForText("Draft note") ✓
    T->>S: type("Second note on the deletion row.")
    T->>S: press(["ctrl","s"])
    U-->>T: waitForText("Second note on the deletion row.") ✓
    T->>T: assert both notes visible in snapshot
Loading

Reviews (1): Last reviewed commit: "test(pty): cover note focus and multiple..." | Re-trigger Greptile

@benvinegar benvinegar merged commit 913de24 into main May 23, 2026
6 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