Skip to content

[issues/519] Validate terminal delivery by content, not just invocation (20 TCs)#522

Merged
couimet merged 10 commits into
mainfrom
issues/519
Apr 20, 2026
Merged

[issues/519] Validate terminal delivery by content, not just invocation (20 TCs)#522
couimet merged 10 commits into
mainfrom
issues/519

Conversation

@couimet
Copy link
Copy Markdown
Owner

@couimet couimet commented Apr 19, 2026

Summary

Two things in one branch. The smaller piece: the 4 Editor Tab context-menu TCs are converted from fully-manual to [assisted] mode, mirroring the Explorer ctxmenu conversion from PR 521. The larger piece (prompted by CodeRabbit's review on 521 + follow-up discussion): integration tests that claim "text arrived in the bound terminal" now verify that claim at the content level via pseudoterminal capture, not just by checking that the adapter was called.

The pty-capture gap this closes

Our existing assertTerminalPasteLogged asserts that VscodeAdapter.pasteTextToTerminalViaClipboard was logged with the expected terminalName and textLength. That proves the extension tried to deliver the text. It does not prove the text actually landed in the terminal buffer — and it cannot detect post-paste mutations. A user manipulating the terminal window between the paste call and the assertion (e.g., deleting what was pasted before clicking Cancel on an assisted dialog) produces a misleading green. A regression in VS Code's workbench.action.terminal.paste routing would produce a misleading green.

New: bind the paste destination to a custom vscode.Pseudoterminal that records every byte delivered via handleInput. Tests read the captured buffer and assert exact content (or a substring shape for link-format TCs whose output varies with runtime config). If a keystroke or regression alters the buffer between paste and assertion, the test fails.

Changes

New helper module: __integration-tests__/helpers/capturingPtyHelpers.ts

  • createCapturingTerminal(name, trackingArray?) — returns { terminal, getCapturedText, clearCaptured }. The pty echoes input back to the visible terminal so assisted runs remain watchable.
  • createAndBindCapturingTerminal(name, trackingArray?) — capturing variant of createAndBindTerminal (saves boilerplate for TCs that start pre-bound).
  • assertTerminalBufferEquals(captured, expected) — exact content match.
  • assertTerminalBufferContains(captured, expected) — substring match for TCs whose link format depends on runtime settings.

Converted TCs (20 total)

Editor Tab context menu (this PR's original scope):

  • context-menus-editor-tab-001, -002, -003, -004 flipped to [assisted]; 001/002 now use pty content equality.

Explorer context menu (merged in from PR 521 + upgraded to pty):

  • context-menus-explorer-001, -002 upgraded from assertTerminalPasteLogged to content equality.

Log-based UI validation:

  • core-send-commands-r-l-001, send-file-path-001 — retired ad-hoc log substring scans for pasteTextToTerminalViaClipboard.
  • dirty-buffer-warning-007, full-line-selection-validation-001 — newly-identified gaps (claimed delivery via status-bar message, never checked content).

Dirty buffer warning:

  • dirty-buffer-warning-008, -009, -013, -014, -016, -017, -018, -020, -021 — retired five FilePathPaster.pasteFilePath / Pasted link to substring scans.
  • dirty-buffer-warning-022 — negative-space conversion: asserts captured === '' to prove the dismiss path does not leak any bytes to the terminal. Stronger than the prior "no paste log found" check since it catches any paste path, not just one that emits the expected log signature.

Clipboard preservation:

  • clipboard-preservation-003, -006 — added content assertions alongside the existing clipboard-state checks.

Retired patterns

  • Five raw line.includes("...") log-substring scans across two suites, replaced by parsed-context or pty-buffer assertions.
  • assertTerminalPasteLogged is kept exported — still the right call for a future TC that needs invocation-only regression detection without caring about content.

QA YAML

  • qa-test-cases-v1.1.0-003.yaml: automated: false → assisted for context-menus-editor-tab-001 through -004.

Why this matters

  • 20 TCs move from "adapter was called correctly" to "correct bytes actually arrived at the terminal." Invocation-vs-reality bugs (VS Code paste routing regressions, destination-dispatch wiring errors, post-paste mutations) now fail loudly.
  • One negative-space assertion proves an abort path doesn't leak — a claim we previously had no mechanism to verify.
  • Future terminal-destination TCs get two helpers that make content validation trivial — write the test the right way once, not log-first then upgrade later.

Test Plan

  • TypeScript type-check + ESLint + Prettier clean.
  • All unit tests pass (1839 passing).
  • pnpm test:release:automated — 124/124 automated integration tests pass after every conversion.
  • QA coverage validator passes.
  • Assisted runs exercised end-to-end: all 14 [assisted] TCs in the converted set pass cleanly when the human does not type into the captured terminal window (see commit body for rationale on keystroke leakage behaviour).

Related

Charles Ouimet and others added 3 commits April 19, 2026 18:43
… TCs)

## Summary

Converts the 5 Explorer context-menu TCs in `qa-test-cases-v1.1.0-003.yaml` from fully-manual to assisted mode via a new `contextMenuExplorer.test.ts` suite. The human right-clicks a file in the Explorer panel and picks a menu item; the test verifies the outcome through log capture — `FilePathPaster.pasteFilePath` for the send-path flows (001/002), `BindToTextEditorCommand.executeWithUri` + status-bar toast for the bind (003), unbind-status-bar + context-key flip for the unbind (004), and a state-invariant check (no `setContext rangelink.isBound=true` during observation) paired with human visual confirmation for the menu-visibility test (005). No production code changes.

## Changes

- New `packages/rangelink-vscode-extension/src/__integration-tests__/suite/contextMenuExplorer.test.ts` — 5 `[assisted]` tests covering Send File Path (absolute + relative), Bind Here, Unbind when bound, and Unbind hidden when unbound
- `packages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0-003.yaml` — flipped `automated: false` → `automated: assisted` for `context-menus-explorer-001` through `005`
- Extracted 4 new log-based assertion helpers into `src/__integration-tests__/helpers/logBasedUiAssertions.ts` (with matching exports in `helpers/index.ts`) so sibling PRs 517/519/520 can reuse rather than copy-paste:
  - `assertFilePathLogged(lines, { pathFormat, uriSource, filePath })` — asserts FilePathPaster resolved a file path via the expected route
  - `assertSetContextLogged(lines, { key, value })` / `assertNoSetContextLogged(...)` — asserts on VS Code context-key state transitions (positive + negative variants)
  - `assertClipboardWriteLogged(lines, { textLength })` — sanity check for clipboard write (length only, since the adapter doesn't log content)
- All new helpers use `parseLogContext()` semantic field comparison, not raw JSON substring matching — preempts the same CodeRabbit feedback that came in on #516 (review) for `assertInputBoxLogged`
- No production code touched; existing log statements in `FilePathPaster`, `BindToTextEditorCommand`, `PasteDestinationManager`, and `VscodeAdapter` were sufficient for every assertion

## Key Discoveries

- The precondition-bind optimization from issue 506's `bindToDestination.test.ts` (007/008/009/010) generalizes: any test whose subject is NOT the bind flow can replace its setup `waitForHuman` with `createTerminal` + `executeCommand(CMD_BIND_TO_TERMINAL_HERE)` — zero human prompts for precondition state. Applied here to TCs 001/002/004 which all require a pre-bound terminal. Documented in a note for the rangeLink-002 worktree where `dirtyBufferWarning.test.ts` 016/018 could benefit from the same treatment.
- TC 005 (menu visibility when unbound) has no directly log-visible signal — VS Code's menu rendering is opaque. Converted to assisted with a weak-but-non-zero state invariant (no `setContext rangelink.isBound=true` during the capture window, proving no spurious bind happened) + human visual confirmation of the hidden entry. This is the honest floor for "visibility governed by a `when` clause on a context key" — if reviewers want a stronger signal, we'd need to instrument VS Code's context-key reads, which isn't worth the weight here.
- `VscodeAdapter.showTextDocument` at packages/rangelink-vscode-extension/src/ide/vscode/VscodeAdapter.ts#L204-L210 has zero logging — violates T015 ("Adapter methods that mutate external state must log their inputs"). Discovered while writing TC 003 (Bind Here); worked around by dropping the `VscodeAdapter.showTextDocument` log assertion, since the `BindToTextEditorCommand.executeWithUri` + status-bar-toast + `setContext` assertions together already prove the bind-and-open flow succeeded. Left a breadcrumb suggesting a follow-up issue to add the missing logger line.

## Test Plan

- [x] TypeScript type-check passes (`npx tsc --noEmit --project tsconfig.integration.json`)
- [x] ESLint clean; Prettier clean
- [x] All 1842 unit tests pass (`pnpm --filter rangelink-vscode-extension test`)
- [x] QA coverage validator passes: 68 automated / 57 assisted / 86 false (delta: +5 assisted, -5 false)
- [ ] **Manual drive (you)**: `pnpm test:release:grep "context-menus-explorer"` — walk through all 5 assisted prompts

## Related

- Closes #518
- Parent: #507
- Grandparent: #504
- Sibling issues pending: #517 (Editor Content), #519 (Editor Tab), #520 (Terminal)
…(4 TCs)

## Summary

Converts the 4 Editor Tab context-menu TCs in `qa-test-cases-v1.1.0-003.yaml` from fully-manual to assisted via a new `contextMenuEditorTab.test.ts` suite. Human right-clicks the file tab and picks a menu item; the test validates via log capture. Reuses the shared log-assertion helpers introduced in the 518 PR — zero new helpers added here, which was the point of extracting them early in that PR.

## Changes

- New `packages/rangelink-vscode-extension/src/__integration-tests__/suite/contextMenuEditorTab.test.ts` — 4 `[assisted]` tests covering Send File Path (absolute + relative), Bind Here, and Unbind when bound
- `packages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0-003.yaml` — flipped `automated: false` → `automated: assisted` for `context-menus-editor-tab-001` through `004`
- No new helpers — uses `assertFilePathLogged`, `assertClipboardWriteLogged`, `assertSetContextLogged`, `assertStatusBarMsgLogged` from `src/__integration-tests__/helpers/logBasedUiAssertions.ts` (all landed in the 518 PR)
- No production code changes

## Key Discoveries

- Editor Tab TCs 003 (Bind Here) and 004 (Unbind) dispatch `CMD_CONTEXT_EDITOR_CONTENT_BIND/_UNBIND` — the SAME commands used by the Explorer context menu's Bind Here / Unbind items (both menus invoke a shared handler). The handler-level behavior is already proven by the 518 PR; this PR's unique value is proving the `editor/title/context` menu entries in package.json are correctly wired to those commands. Documented as Assumption C in the start-issue scratchpad.
- Pre-bind-via-executeCommand optimization applied to TCs 001, 002, 004 — same pattern established in 506 (bindToDestination) and reapplied in 518. Each of those TCs requires a pre-bound destination; programmatic pre-bind removes 3 human prompts from the assisted run.

## Test Plan

- [x] TypeScript type-check passes (`npx tsc --noEmit --project tsconfig.integration.json`)
- [x] ESLint clean; Prettier clean
- [x] All 1853 unit tests pass (`pnpm --filter rangelink-vscode-extension test`)
- [x] QA coverage validator passes (69 automated / 75 assisted / 82 false; delta from this PR: +4 assisted, -4 false)
- [ ] **Manual drive (you)**: `pnpm test:release:grep "context-menus-editor-tab"` — walk through all 4 assisted prompts

## Related

- Closes #519
- Parent: #507
- Grandparent: #504
- **Stacked on**: #518 (merge that first or let GitHub auto-retarget on merge)
- Sibling issues pending: #517 (Editor Content, 11 TCs), #520 (Terminal, 3 TCs)
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 19, 2026

Walkthrough

Updated four editor-tab context menu test cases from manual to assisted mode and added a new integration test suite validating four scenarios: Send File Path, Send Relative File Path, Bind Here, and Unbind. Tests use temporary files, context menu interactions, and automated validation via logs and UI state.

Changes

Cohort / File(s) Summary
QA Test Case Metadata
packages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0-003.yaml
Updated automated field from false to assisted for four context-menu editor-tab test cases (context-menus-editor-tab-001 through -004), indicating transition from manual to assisted testing mode.
Editor Tab Context Menu Integration Tests
packages/rangelink-vscode-extension/src/__integration-tests__/suite/contextMenuEditorTab.test.ts
New test suite with four assisted test cases covering editor-tab context menu actions: (1) Send File Path, (2) Send Relative File Path, (3) Bind Here, and (4) Unbind. Tests create temporary files, trigger context menu interactions, and validate outcomes via log capture and UI state assertions (status bar, context keys, clipboard).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

Possibly related PRs

Poem

🐰 Right-click the editor, watch the magic dance,
Assisted tests await their human-guided chance,
Paths to clipboard fly, bindings bloom and glow,
Context menus bloom where test logs flow!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The PR title describes validating terminal delivery by content but the changeset focuses on converting Editor Tab context-menu test cases to assisted mode with new integration tests—two different scopes. Update the title to reflect the actual changes: e.g., '[issues/519] Convert Context Menus — Editor Tab TCs to assisted mode (4 TCs)' to match the PR objectives and changeset content.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR fulfills issue #519 by adding an integration test suite with four assisted tests covering all required Editor Tab context-menu actions (Send File Path, Send Relative File Path, Bind Here, Unbind) and updating QA YAML metadata accordingly.
Out of Scope Changes check ✅ Passed All changes are scoped to the stated objective: QA YAML metadata updates and a new integration test suite reusing existing helpers; no production code changes or unrelated modifications present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issues/519

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 19, 2026

⚠️ QA Coverage Gap Detected

The PR modifies the automation status of several test cases from 'false' to 'assisted', indicating that they now require human interaction to verify the results.

Suggested test cases:

  • Context Menus — Editor Tab: Verify that 'RangeLink: Send File Path' sends the absolute file path to the terminal (manual — This test requires human interaction to select the context menu item and verify the terminal output.)
  • Context Menus — Editor Tab: Verify that 'RangeLink: Send Relative File Path' sends the workspace-relative path to the terminal (manual — This test requires human interaction to select the context menu item and verify the terminal output.)
  • Context Menus — Editor Tab: Verify that 'RangeLink: Bind Here' binds the current file and shows a success toast (manual — This test requires human interaction to select the context menu item and verify the success toast.)
  • Context Menus — Editor Tab: Verify that 'RangeLink: Unbind' unbinds the destination and shows a confirmation notification (manual — This test requires human interaction to select the context menu item and verify the confirmation notification.)

Generated by QA Gap Check (GPT-4o-mini via GitHub Models)

TCs 001 and 002 previously asserted only path resolution and clipboard write. The existing assertions would still pass if terminal delivery failed after the clipboard step, so the test titles ("sends … to bound terminal") overstated what was verified. Added a new `assertTerminalPasteLogged` helper that parses the `VscodeAdapter.pasteTextToTerminalViaClipboard` log entry and checks the bound `terminalName` plus `textLength`, closing the gap without any producer-side changes.

While in that file, replaced the raw JSON substring match in TC 003 with a new `assertFnLogged` helper that parses the log context and compares `fn` by equality, matching the convention the rest of the suite already uses.

Benefits:
- End-to-end proof that the bound terminal received the path, not just the clipboard.
- Consistent parsed-log-context assertions across the context-menu TCs — no more substring scanning that would break on log serialization changes.
- Two reusable helpers (`assertTerminalPasteLogged`, `assertFnLogged`) available for future assisted tests.

Ignored Feedback:
- "Align `assertFilePathLogged` with the actual `FilePathPaster` log payload": declined because the producer already emits `filePath` on the same log entry the assertion parses (`FilePathPaster.ts` line 107 merges `filePath` into `logCtx` before the `logger.debug` call). The assertion matches today and the tests pass — CodeRabbit missed the merged field.

Ref: #521 (review)
Base automatically changed from issues/518 to main April 20, 2026 01:59
@couimet
Copy link
Copy Markdown
Owner Author

couimet commented Apr 20, 2026

@coderabbitai review The PR now targets main branch

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 20, 2026

@couimet Sure! I'll kick off a fresh review of PR #522 now that it targets the main branch.


🧠 Learnings used
Learnt from: CR
Repo: couimet/rangeLink PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-19T22:46:31.255Z
Learning: Applies to (qa/**/* | src/__integration-tests__/**/*) : Any change to files in `packages/rangelink-vscode-extension/qa/` or `src/__integration-tests__/` MUST be validated by running `pnpm test:release`

Learnt from: CR
Repo: couimet/rangeLink PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-19T22:46:31.255Z
Learning: Applies to qa/**/*.yaml : The `automated` field in QA YAML requires a matching integration test on the branch - `true` requires non-`[assisted]` test, `assisted` requires `[assisted]`-tagged test, `false` for non-automatable scenarios

Learnt from: CR
Repo: couimet/rangeLink PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-19T22:46:31.255Z
Learning: Applies to qa/**/*.yaml : TC IDs use domain-based naming (e.g., `link-navigation-001`, `text-editor-destination-001`) - never prefix with `bugfix-`, `feature-`, `changed-`, or origin-based prefixes
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@couimet
Copy link
Copy Markdown
Owner Author

couimet commented Apr 20, 2026

@coderabbitai Did I get rate-limited again? Why didn't you review after you wrote #522 (comment) ?

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 20, 2026

@couimet — apologies for the confusion! You weren't rate-limited. Here's what happened:

The `` action I triggered is incremental — CodeRabbit only reviews commits it hasn't seen before. Since all the commits in this PR were already reviewed (when the PR was targeting the previous branch), the incremental scan found nothing new and silently no-op'd.

Because you re-targeted to main, a full re-review against the new base is needed instead. Triggering that now:


🧠 Learnings used
Learnt from: CR
Repo: couimet/rangeLink PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-19T22:46:31.255Z
Learning: Applies to (qa/**/* | src/__integration-tests__/**/*) : Any change to files in `packages/rangelink-vscode-extension/qa/` or `src/__integration-tests__/` MUST be validated by running `pnpm test:release`

Learnt from: CR
Repo: couimet/rangeLink PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-19T22:46:31.255Z
Learning: Applies to qa/**/*.yaml : The `automated` field in QA YAML requires a matching integration test on the branch - `true` requires non-`[assisted]` test, `assisted` requires `[assisted]`-tagged test, `false` for non-automatable scenarios

Learnt from: CR
Repo: couimet/rangeLink PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-19T22:46:31.255Z
Learning: Applies to qa/**/*.yaml : TC IDs use domain-based naming (e.g., `link-navigation-001`, `text-editor-destination-001`) - never prefix with `bugfix-`, `feature-`, `changed-`, or origin-based prefixes
✅ Actions performed

Full review triggered.

@couimet
Copy link
Copy Markdown
Owner Author

couimet commented Apr 20, 2026

@coderabbitai review I was rate-limited in #522 (comment)

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 20, 2026

@couimet — sorry about the rate-limiting issue! I'll trigger a full re-review of PR #522 right now against the main branch.


🧠 Learnings used
Learnt from: CR
Repo: couimet/rangeLink PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-19T22:46:31.255Z
Learning: Applies to (qa/**/* | src/__integration-tests__/**/*) : Any change to files in `packages/rangelink-vscode-extension/qa/` or `src/__integration-tests__/` MUST be validated by running `pnpm test:release`

Learnt from: CR
Repo: couimet/rangeLink PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-19T22:46:31.255Z
Learning: Applies to qa/**/*.yaml : The `automated` field in QA YAML requires a matching integration test on the branch - `true` requires non-`[assisted]` test, `assisted` requires `[assisted]`-tagged test, `false` for non-automatable scenarios

Learnt from: CR
Repo: couimet/rangeLink PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-19T22:46:31.255Z
Learning: Applies to qa/**/*.yaml : TC IDs use domain-based naming (e.g., `link-navigation-001`, `text-editor-destination-001`) - never prefix with `bugfix-`, `feature-`, `changed-`, or origin-based prefixes
✅ Actions performed

Full review triggered.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@packages/rangelink-vscode-extension/src/__integration-tests__/suite/contextMenuEditorTab.test.ts`:
- Around line 79-85: The test currently only asserts path resolution and
clipboard writes for the editor-tab context-menu actions but misses verifying
that the path was actually delivered to the terminal; add the terminal-paste
assertion using the existing helper (e.g., assertTerminalPasteLogged or
assertTerminalSendLogged) right after the clipboard assertion to verify the
paste/send occurred with the expected length and source (pass textLength:
uri.fsPath.length and uriSource: 'context-menu' or other required terminal
identifiers), and make the same addition for the second occurrence around lines
121-126 so both tests confirm terminal delivery, not just clipboard writes.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 68a8983e-b174-45e6-a0fb-6043955f7555

📥 Commits

Reviewing files that changed from the base of the PR and between c63aa8f and 7b2c81c.

📒 Files selected for processing (2)
  • packages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0-003.yaml
  • packages/rangelink-vscode-extension/src/__integration-tests__/suite/contextMenuEditorTab.test.ts

Charles Ouimet added 4 commits April 20, 2026 07:19
…menu TCs

Mirror the fix applied to the Explorer TCs in PR 521. Editor-tab TCs 001 and 002 claimed the bound terminal receives the path but asserted only path resolution + clipboard write — a regression that broke the `pasteTextToTerminalViaClipboard` call path would still pass. Added `assertTerminalPasteLogged` (already available on this branch via the PR 521 merge) after the existing `assertClipboardWriteLogged` in each TC, using the bound `terminalName` and `minTextLength` set to the raw path length.

Scope caveat: like the rest of this suite, these assertions are log-based — they prove the adapter was called with the expected arguments, not that the terminal buffer ended up holding the text. A separate follow-up converts the relevant TCs to pseudoterminal capture for real content-level validation.

While aligning these tests with the Explorer equivalents, also replaced the raw JSON substring match in TC 003 with `assertFnLogged` and removed the now-unused `node:assert` import. Keeps the two ctxmenu test files stylistically identical and retires the last substring-scan pattern in this file.

Benefits:
- Catches regressions that break the terminal-paste adapter call path (the previous assertions ignored that call entirely).
- Parity between contextMenuEditorTab and contextMenuExplorer — same helpers, same call shape.
- No remaining raw-JSON-substring matches in this file; all log checks go through parsed-context helpers.

Ref: #522 (review)
Integration tests that claim "text arrived in the bound terminal" now bind to a custom pseudoterminal that records everything written to it via `handleInput`. Assertions read the captured buffer directly, catching bugs the previous log-only assertions could not — e.g., a human manipulating the terminal buffer after the adapter call, or a regression in VS Code's paste-command routing that never actually delivers bytes to the terminal.

New helper `createCapturingTerminal(name, trackingArray?)` in `__integration-tests__/helpers/capturingPtyHelpers.ts` returns `{ terminal, getCapturedText, clearCaptured }`. Pair with `assertTerminalBufferEquals` for exact content (file-path TCs) or `assertTerminalBufferContains` for shape-only checks (link-content TCs whose exact format depends on runtime config).

Converted six TCs — four flagged by the earlier pty-capture inventory as strong candidates and two as double-risk cases:

- `context-menus-explorer-001` / `-002` (assisted)
- `context-menus-editor-tab-001` / `-002` (assisted)
- `core-send-commands-r-l-001` (automated, in `logBasedUiValidation.test.ts`)
- `send-file-path-001` (automated, in `logBasedUiValidation.test.ts`)

For the four strong candidates, `assertTerminalPasteLogged` is replaced by the stronger content assertion — once content-equality passes, invocation of the adapter is implied. For the two double-risk cases, the ad-hoc log substring scans are replaced with the same content assertion (keeps `assertStatusBarMsgLogged` alongside since the status-bar claim is a separate UI contract).

Verified: `pnpm test:release:automated` → 124/124 pass; QA coverage validator passes. The four assisted TCs compile clean and follow the exact pattern the non-assisted ones proved out.

Benefits:
- Content-level validation of terminal delivery — a human deleting pasted text from the terminal buffer before clicking Cancel on an assisted dialog now causes the test to fail instead of giving a misleading green.
- Retires two ad-hoc log substring scans in `logBasedUiValidation.test.ts` in favour of the standard pty helpers.
- New `CapturingTerminal` helper is reusable for future TCs that care about content, not just invocation.
- `assertTerminalPasteLogged` is still exported and intentionally kept — useful for future TCs where invocation-only regression detection is the right contract.
Integration tests that claim "text arrived in the bound terminal" now bind to a custom pseudoterminal that records every byte delivered via `handleInput`. Assertions read the captured buffer directly, catching bugs the previous log-only assertions could not — e.g., a human manipulating the terminal buffer after the adapter call, or a regression in VS Code's paste-command routing that never actually delivers bytes.

New helpers in `__integration-tests__/helpers/capturingPtyHelpers.ts`:

- `createCapturingTerminal(name, trackingArray?)` — returns `{ terminal, getCapturedText, clearCaptured }`. Pty echoes input back to the visible terminal so assisted runs remain watchable.
- `createAndBindCapturingTerminal(name, trackingArray?)` — capturing variant of the existing `createAndBindTerminal`.
- `assertTerminalBufferEquals(captured, expected)` — exact content match.
- `assertTerminalBufferContains(captured, expected)` — substring match for TCs whose link format varies with runtime config.

Converted 20 TCs across four suites. Exact content where known (file-path flows compute ` ${relativePath} `), substring where link format depends on runtime settings, and `equals ''` for a negative-space dismiss TC that proves nothing leaks to the terminal when the flow aborts.

Strong candidates (4, all previously log-asserted via `assertTerminalPasteLogged`):
- `context-menus-explorer-001` / `-002` (assisted)
- `context-menus-editor-tab-001` / `-002` (assisted)

Double-risk cases (2, previously asserted via ad-hoc log substring scans):
- `core-send-commands-r-l-001` (automated)
- `send-file-path-001` (automated)

Second-pass candidates — any TC that binds a terminal and asserts the "✓ … sent to Terminal" status-bar contract (14):
- `logBasedUiValidation.test.ts::dirty-buffer-warning-007`, `full-line-selection-validation-001` (both automated)
- `dirtyBufferWarning.test.ts::dirty-buffer-warning-008`, `-009`, `-013`, `-014`, `-016`, `-017`, `-018`, `-020`, `-021` (all assisted — R-F and R-L variants through the dialog and picker flows)
- `dirtyBufferWarning.test.ts::dirty-buffer-warning-022` (assisted; negative-space — asserts `captured === ''` to prove no terminal send on dismiss)
- `clipboardPreservation.test.ts::clipboard-preservation-003`, `-006` (both automated)

For TCs previously using ad-hoc log substring scans (`FilePathPaster.pasteFilePath`, `Pasted link to`, `Link copied to clipboard`), those scans are retired — the pty content assertion subsumes them.

`assertTerminalPasteLogged` is still exported and intentionally kept — it remains the right contract for a future TC that only needs invocation-level regression detection without caring about content.

Verified: `pnpm test:release:automated` → 124/124 pass after all 20 conversions; QA coverage validator passes. The 14 assisted TCs compile clean and follow the exact pattern the six automated conversions proved out.

Benefits:
- Content-level validation across 20 TCs instead of the original 4 — terminal manipulation or paste-routing regressions now fail loudly instead of producing misleading greens.
- Retires five ad-hoc log substring scans across `logBasedUiValidation.test.ts` and `dirtyBufferWarning.test.ts`.
- Two reusable helpers (`createCapturingTerminal`, `createAndBindCapturingTerminal`) plus two assertion helpers make future terminal-destination TCs trivial to content-validate.
- `dirty-buffer-warning-022` now has a true negative assertion (`captured === ''`) — stronger than "no paste log found" since it catches pastes that occur without the specific log signature.
@couimet couimet changed the title [issues/519] Convert Context Menus — Editor Tab TCs to assisted mode (4 TCs) [issues/519] Validate terminal delivery by content, not just invocation (20 TCs) Apr 20, 2026
@couimet couimet merged commit 51084ea into main Apr 20, 2026
3 checks passed
@couimet couimet deleted the issues/519 branch April 20, 2026 11:59
couimet added a commit that referenced this pull request Apr 23, 2026
…524)

* [issues/517] Context menus — editor content: 11 TCs to assisted mode

## Summary

Last Category-B sibling of the 507 epic. Converts all 11 Context Menus — Editor Content QA test cases from `automated: false` to `automated: assisted` by adding a new integration-test suite at `contextMenuEditorContent.test.ts`. Covers link sends (regular + absolute + portable + portable-absolute + raw selected text), file-path sends (absolute + relative), bind/unbind, and three visibility contracts (visual separator, Unbind-when-bound, no-selection-hides-5-items).

## Changes

- **New integration tests** — 11 `[assisted]`-tagged TCs in `src/__integration-tests__/suite/contextMenuEditorContent.test.ts`. Delivery-path TCs (001–005, 007–008) use `createAndBindCapturingTerminal` + `assertTerminalBufferContains` for content-level verification of what landed in the bound terminal. Visibility-contract TCs (006, 010, 011) use `waitForHumanVerdict` for the human signal; 010 and 011 pair the verdict with a state-invariant log check (belt-and-suspenders, parity with the terminal/editor-tab/explorer patterns), 006 is verdict-only (pure visual contract, no state invariant possible).
- **QA YAML** — flipped `automated: false` → `automated: assisted` on all 11 `context-menus-editor-content-*` TCs in `qa/qa-test-cases-v1.1.0-003.yaml`. In-place edit (QA002 exception for `automated` field updates); no journal copy needed.
- **TC 010 scope expansion** — converted from the YAML's observe-only scope to click-and-verify, mirroring how terminal-003 was converted under #523. The test now both proves visibility (menu item present → human could click it) and asserts the click path (setContext false + unbind status-bar message).

## Test Plan

- [x] TypeScript type-check passes (`npx tsc --noEmit --project tsconfig.integration.json`)
- [x] ESLint clean (`pnpm lint:fix`)
- [x] Prettier clean (`pnpm format:fix`)
- [x] All 1856 unit tests pass (`pnpm test`)
- [x] QA coverage validator passes: 69 automated / 98 assisted / 67 false (was 87 assisted / 78 false before this PR; `scripts/validate-qa-coverage.sh` reports matching integration-test IDs for every assisted marker)
- [x] Manual drive: `pnpm test:release:grep "context-menus-editor-content"` — 11 TCs require a human driver for the right-click gestures and PASS/FAIL verdicts. Drive locally before marking PR ready.

## Documentation

- [ ] CHANGELOG: not needed — internal test-coverage improvement, no user-facing change. Matches the pattern set by the three sibling PRs (#521 explorer, #522 editor-tab, #523 terminal).
- [ ] README: not needed — no new command, setting, or feature.

## Related

- Closes #517
- Parent: #507
- Grandparent: #504
- Adopts helpers from #522 (pty-capture infrastructure) and #523 (`waitForHumanVerdict` + assertion-order convention: verdict first, state-invariant second)
- Siblings merged: #521, #522, #523

* Ran fix

* [PR feedback] Harden TC 010 verdict gate, restore suite config, sync QA YAML

CodeRabbit's review flagged three drifts in the Context Menus — Editor Content branch. All three addressed in place.

Wrapped the suite's global `terminal.integrated.enableMultiLinePasteWarning` override with a matching `suiteTeardown`. `suiteSetup` now captures the prior value via `config.inspect('enableMultiLinePasteWarning')?.globalValue` and `suiteTeardown` restores it (passing `undefined` explicitly clears the override — that's the documented `WorkspaceConfiguration.update` behavior). Without this, TC 005's multi-line-paste override was leaking into any suite that ran after it, and into the developer's VS Code global profile when running the release tests locally.

Converted TC 010 from the older `waitForHuman` call to `waitForHumanVerdict` with a `assert.strictEqual(verdict, 'pass', …)` gate placed before the log-based state-invariant checks. Previously, an absent `RangeLink: Unbind` menu item would have surfaced only as a later missing unbind log — misleading the diagnosis toward "the click path is broken" instead of "the menu item was absent". This also brings TC 010 into parity with TC 011 in the same file and with the verdict-first / state-invariant-second convention that #523 established for the sibling suites. Prompt text also reordered to "verify → select" with an explicit PASS/FAIL instruction line.

Synced the QA YAML entry for `context-menus-editor-content-010` in `qa-test-cases-v1.1.0-003.yaml`: scenario now reads "Editor content: RangeLink: Unbind visible when bound and unbinds on click", steps cover verify-then-select, and `expected_result` describes both the visibility and the unbind outcome. Edited in place (QA002 exception was already invoked for the `automated: false → assisted` flip earlier in this PR; extending that same in-place edit to correct scenario text is a reasonable reading of QA002, vs. fragmenting a coherent snapshot across two files mid-review).

Benefits:
- `suiteTeardown` stops test config from leaking into subsequent suites or the developer's VS Code profile.
- TC 010 failures now pinpoint visibility vs. click-path regressions separately.
- A manual QA run from the YAML actually exercises the unbind click now; the documented steps match what the assisted integration test covers.

Ref: #524 (review)

* [PR feedback] Add bound-destination precondition to QA TC 011

CodeRabbit's follow-up review flagged that `context-menus-editor-content-011` in `qa-test-cases-v1.1.0-003.yaml` claimed "File-path and Bind/Unbind items remain visible" in its expected_result but did not list a bound destination among its preconditions. The assisted integration test at `contextMenuEditorContent.test.ts#L491-L492` already binds a capturing terminal before the human step — the prompt literally tells the driver "Unbind should remain visible" because a destination is bound. Without the matching YAML precondition, a manual QA driver running TC 011 with nothing bound would see Unbind absent (correctly) and fail the TC for the wrong reason.

Inserted `'A destination is bound'` between `'Editor open'` and `'No text selected (cursor only)'`, matching the order in the reviewer's proposed diff and mirroring TC 010's preconditions. In-place edit, consistent with the prior QA YAML corrections on this branch (QA002 exception already in scope for this file).

Benefits:
- Manual QA driver now knows to bind a destination before running TC 011, so the expected_result is only asserted under the state where it holds.
- YAML now reflects what the assisted integration test actually sets up.

Ref: #524 (review)
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.

Convert Context Menus — Editor Tab TCs to assisted mode (4 TCs)

1 participant