[issues/481] Add log-based UI validation for integration tests#484
Merged
Conversation
added 3 commits
March 20, 2026 09:59
Integration tests can't access VS Code's OutputChannel content (write-only API). LogCapture wraps the output channel, proxying all appendLine() calls while optionally storing them in an in-memory array. Capture is gated by the RANGELINK_CAPTURE_LOGS environment variable — the integration test runner sets it to 'true'; production never sets it, so the in-memory array stays empty with zero overhead.
The LogSink interface decouples VSCodeLogger from vscode.OutputChannel — it now accepts any object with appendLine(). activate() returns { logCapture } via ext.exports so integration tests access it via vscode.extensions.getExtension().exports.logCapture. RangeLinkExtensionApi interface lives in types/ per project conventions.
Benefits:
- Enables automated toast validation for ~47 manual QA TCs (34% of manual tests) (#481)
- Zero production overhead — capture is disabled by default; appendLine only proxies to the real channel
- Tests use mark()/getLinesSince() to scope assertions to a specific command execution
Integration tests need to verify that specific toasts were shown after executing commands. These helpers parse the captured log lines (from LogCapture) and match against the VscodeAdapter function name (toast type) and exact message text from the JSON context. assertToastLogged / assertNoToastLogged cover all four toast types: info, warning, error, and statusBar. getLogCapture retrieves the LogCapture instance from ext.exports for use in test setup. Benefits: - Enables writing integration tests that assert toast behavior without UI interaction (#481) - Type-safe toast type mapping (info → VscodeAdapter.showInformationMessage, etc.) - 11 unit tests verify parsing, matching, type discrimination, and negative assertions
…te 6 manual QA TCs Resolved 4 navigation-clamping TODOs blocked on #481 by adding toast type and message assertions to existing tests (clamping-001 through -004 now verify warning vs info toast with exact clamped/unclamped message content). Wrote 7 new integration tests in toastValidation.test.ts covering unbind no-op, terminal-focus guidance, send link, send file path, dirty buffer clean path, and full-line selection. Flipped 6 QA TCs from automated: false to automated: true. The assertToastLogged/assertStatusBarMsgLogged helpers parse VscodeAdapter log JSON to match toast type (info/warning/error) and exact message text. Status bar messages are asserted separately from toasts. Benefits: - 11 integration tests now verify toast/status-bar content via log capture (4 enhanced + 7 new) - 6 QA TCs newly automated: core-send-commands-r-l-001, dirty-buffer-warning-007, full-line-selection-validation-001, send-file-path-001, send-terminal-selection-006, send-terminal-selection-007 - QA automated coverage: 39 → 45 TCs (validator 45/45 PASSED) - RANGELINK_CAPTURE_LOGS=true set in .vscode-test.mjs — all integration tests benefit from log capture
|
New LogCapture class introduced for capturing logs, which may affect logging behavior and output. Some existing test cases were updated to be automated, but coverage for the new logging functionality is lacking. Suggested test cases:
Generated by QA Gap Check (GPT-4o-mini via GitHub Models) |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughA log-capture system for VS Code integration tests was added. When RANGELINK_CAPTURE_LOGS='true', extension output is stored in-memory via a new LogCapture and exposed from activate(); tests use helpers to assert toast/status-bar events by inspecting captured logs instead of interacting with the UI. Changes
Sequence DiagramsequenceDiagram
participant TestRunner as Test Runner
participant VSCodeExt as Extension.activate()
participant OutputChan as VSCode OutputChannel
participant LogCapture as LogCapture (LogSink)
participant Logger as VSCodeLogger
participant IntegTest as Integration Test
participant Helpers as Log Assertion Helpers
TestRunner->>VSCodeExt: set RANGELINK_CAPTURE_LOGS='true' and launch
VSCodeExt->>OutputChan: create OutputChannel
VSCodeExt->>LogCapture: new LogCapture(OutputChannel)
VSCodeExt->>Logger: new VSCodeLogger(LogCapture)
VSCodeExt-->>TestRunner: return { logCapture }
IntegTest->>Helpers: getLogCapture()
Helpers->>VSCodeExt: read ext.exports.logCapture
Helpers-->>IntegTest: LogCapture instance
IntegTest->>LogCapture: mark("checkpoint")
IntegTest->>VSCodeExt: invoke command (e.g., send-range)
Logger->>OutputChan: appendLine(log message)
Logger->>LogCapture: appendLine(log message) (stores when enabled)
IntegTest->>LogCapture: getLinesSince("checkpoint")
IntegTest->>Helpers: assertToastLogged(lines, {type,message})
Helpers-->>IntegTest: assertion result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment Tip CodeRabbit can enforce grammar and style rules using `languagetool`.Configure the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/rangelink-vscode-extension/src/__integration-tests__/helpers/getLogCapture.ts (1)
13-20: Add an explicit guard for missingext.exports.logCapture.A direct guard here gives a clearer failure than propagating
undefinedinto later assertions.🛡️ Suggested defensive check
export const getLogCapture = (): LogCapture => { const ext = vscode.extensions.getExtension<RangeLinkExtensionApi>(EXTENSION_ID); if (!ext) { throw new Error(`Extension ${EXTENSION_ID} not found`); } if (!ext.isActive) { throw new Error(`Extension ${EXTENSION_ID} is not active — call ext.activate() first`); } - return ext.exports.logCapture; + const logCapture = ext.exports?.logCapture; + if (!logCapture) { + throw new Error(`Extension ${EXTENSION_ID} is active but did not export logCapture`); + } + return logCapture; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rangelink-vscode-extension/src/__integration-tests__/helpers/getLogCapture.ts` around lines 13 - 20, The function currently assumes ext.exports.logCapture exists and can return undefined later; add an explicit guard after retrieving ext.exports.logCapture in getLogCapture (checking ext.exports and ext.exports.logCapture) and throw a clear Error (including EXTENSION_ID in the message) if it's missing so callers get an immediate, descriptive failure instead of a later undefined propagation; reference the ext variable, EXTENSION_ID, RangeLinkExtensionApi and the returned logCapture export when updating the check.packages/rangelink-vscode-extension/src/__integration-tests__/suite/logBasedUiValidation.test.ts (1)
59-66: Consider extracting the sleep pattern to a helper.The
await new Promise<void>((resolve) => setTimeout(resolve, SETTLE_MS))pattern is repeated 15+ times throughout the test file. Extracting it to aconst settle = () => ...helper would reduce noise and make tests more readable.🔧 Optional refactor to reduce repetition
const SETTLE_MS = 500; +const settle = () => new Promise<void>((resolve) => setTimeout(resolve, SETTLE_MS)); // Then in tests: -await new Promise<void>((resolve) => setTimeout(resolve, SETTLE_MS)); +await settle();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rangelink-vscode-extension/src/__integration-tests__/suite/logBasedUiValidation.test.ts` around lines 59 - 66, The test file repeats the pattern await new Promise<void>((resolve) => setTimeout(resolve, SETTLE_MS)) — extract that into a small helper (e.g. const settle = () => new Promise<void>(resolve => setTimeout(resolve, SETTLE_MS))) at the top of the test file and replace all occurrences (including inside bindTerminal and other helpers) with await settle(); keep SETTLE_MS as the same constant and ensure bindTerminal still returns the terminal after awaiting settle().
🤖 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__/helpers/logBasedUiAssertions.ts`:
- Around line 29-44: The parseLogContext function currently slices JSON using
line.indexOf('}') which stops at the first closing brace and breaks when the
JSON contains nested objects/arrays; update parseLogContext to locate the
correct JSON end (e.g., use line.lastIndexOf('}') or implement a simple
brace-balancing loop) to capture the full JSON substring before calling
JSON.parse, keeping the existing validation of ctx.fn and ctx.message in the
returned object.
In
`@packages/rangelink-vscode-extension/src/errors/RangeLinkExtensionErrorCodes.ts`:
- Around line 29-32: The enum entries in RangeLinkExtensionErrorCodes must be
alphabetically ordered; move LOG_CAPTURE_DISABLED so all GENERATE_* entries are
grouped and sorted first: ensure the order is GENERATE_LINK_FORMAT_FAILED,
GENERATE_LINK_NO_SELECTION, GENERATE_LINK_SELECTION_CONVERSION_FAILED, then
LOG_CAPTURE_DISABLED to restore the documented alphabetical ordering and avoid
noisy future merges.
---
Nitpick comments:
In
`@packages/rangelink-vscode-extension/src/__integration-tests__/helpers/getLogCapture.ts`:
- Around line 13-20: The function currently assumes ext.exports.logCapture
exists and can return undefined later; add an explicit guard after retrieving
ext.exports.logCapture in getLogCapture (checking ext.exports and
ext.exports.logCapture) and throw a clear Error (including EXTENSION_ID in the
message) if it's missing so callers get an immediate, descriptive failure
instead of a later undefined propagation; reference the ext variable,
EXTENSION_ID, RangeLinkExtensionApi and the returned logCapture export when
updating the check.
In
`@packages/rangelink-vscode-extension/src/__integration-tests__/suite/logBasedUiValidation.test.ts`:
- Around line 59-66: The test file repeats the pattern await new
Promise<void>((resolve) => setTimeout(resolve, SETTLE_MS)) — extract that into a
small helper (e.g. const settle = () => new Promise<void>(resolve =>
setTimeout(resolve, SETTLE_MS))) at the top of the test file and replace all
occurrences (including inside bindTerminal and other helpers) with await
settle(); keep SETTLE_MS as the same constant and ensure bindTerminal still
returns the terminal after awaiting settle().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 88626bd1-0160-46c4-96c5-176a4fce1f2c
📒 Files selected for processing (15)
packages/rangelink-vscode-extension/.vscode-test.mjspackages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0-2026-03-18.yamlpackages/rangelink-vscode-extension/src/LogCapture.tspackages/rangelink-vscode-extension/src/VSCodeLogger.tspackages/rangelink-vscode-extension/src/__integration-tests__/helpers/getLogCapture.tspackages/rangelink-vscode-extension/src/__integration-tests__/helpers/index.tspackages/rangelink-vscode-extension/src/__integration-tests__/helpers/logBasedUiAssertions.tspackages/rangelink-vscode-extension/src/__integration-tests__/suite/logBasedUiValidation.test.tspackages/rangelink-vscode-extension/src/__integration-tests__/suite/navigationClamping.test.tspackages/rangelink-vscode-extension/src/__tests__/LogCapture.test.tspackages/rangelink-vscode-extension/src/__tests__/integration-helpers/logBasedUiAssertions.test.tspackages/rangelink-vscode-extension/src/errors/RangeLinkExtensionErrorCodes.tspackages/rangelink-vscode-extension/src/extension.tspackages/rangelink-vscode-extension/src/types/RangeLinkExtensionApi.tspackages/rangelink-vscode-extension/src/types/index.ts
added 4 commits
March 21, 2026 12:47
`parseLogContext` used `indexOf('}')` to find the end of the JSON context block, which stops at the first closing brace. This breaks when the JSON contains nested objects (e.g. `{"fn":"x","details":{"key":"val"}}`), truncating the substring before `JSON.parse`.
Switched to `lastIndexOf('}')` which correctly captures the full JSON object for single-JSON-per-line log format.
Benefits:
- Prevents silent assertion failures if log entries ever include nested JSON context
- Unit test with nested objects confirms the fix
Ref: #484 (review)
… codes enum `LOG_CAPTURE_DISABLED` was inserted between `GENERATE_LINK_FORMAT_FAILED` and `GENERATE_LINK_NO_SELECTION`, breaking the documented alphabetical ordering convention. Moved it to its correct position after all `GENERATE_LINK_*` entries. Benefits: - Restores the alphabetical invariant documented in the enum header comments - Prevents noisy diffs in future merges that add entries near the G/L boundary Ref: #484 (review)
…ture `getLogCapture` returned `ext.exports.logCapture` without checking if it was defined. When `RANGELINK_CAPTURE_LOGS` is not set, `logCapture` is undefined and callers get silent propagation instead of a clear failure. Added an explicit null guard with a descriptive error message, following the same pattern as the existing `ext` and `ext.isActive` guards. Benefits: - Fail-fast with a descriptive message instead of silent undefined propagation - Consistent guard pattern across all precondition checks in the function Ref: #484 (review)
The `await new Promise<void>((resolve) => setTimeout(resolve, SETTLE_MS))` pattern was repeated 15 times across the test file. Extracted it to a `settle()` helper to reduce noise and communicate intent more clearly. Benefits: - Reduces repetition from 15 inline promise constructions to `await settle()` calls - Named helper makes the purpose (waiting for UI to settle) immediately obvious Ref: #484 (review)
couimet
pushed a commit
that referenced
this pull request
Mar 21, 2026
PR #484 added LogCapture infrastructure for verifying toasts and status bar messages in integration tests. The unbind-001 test previously only verified unbind via clipboard side-effects (R-C after unbind). Now it also asserts the "✓ RangeLink unbound from Terminal" status bar confirmation message, making the test more precise about the user-visible feedback.
couimet
added a commit
that referenced
this pull request
Mar 21, 2026
…ns (#489) * [test] Enhance unbind-001 with log-based status bar assertion PR #484 added LogCapture infrastructure for verifying toasts and status bar messages in integration tests. The unbind-001 test previously only verified unbind via clipboard side-effects (R-C after unbind). Now it also asserts the "✓ RangeLink unbound from Terminal" status bar confirmation message, making the test more precise about the user-visible feedback. * [test] Add unbind-004 integration test for Command Palette unbind unbind-004 QA TC verifies that "RangeLink: Unbind Destination" works from the Command Palette. Since executeCommand uses the same code path as palette selection, the integration test binds a terminal, unbinds via executeCommand, and asserts the "✓ RangeLink unbound" status bar confirmation via LogCapture. Combined with the contract test verifying `enablement: rangelink.isBound`, this covers the TC. * [test] Update QA YAML unbind TCs to reflect enablement guard and new automation unbind-003 expected_result was outdated — the keybinding no longer fires when unbound (gated by rangelink.isBound), rather than being a "safe no-op" with an info message. unbind-004 flipped to automated: true now that an integration test exercises the same executeCommand code path as Command Palette invocation, and contract tests verify the enablement clause. * [docs] Add CLAUDE.md rule requiring pnpm test:release for QA/integration changes The QA validator script only checks ID matching between YAML and test files — it doesn't execute the tests. Without running `pnpm test:release`, log-based assertions and command behavior are never verified in a real VS Code host, so broken tests slip through unnoticed. * [docs] Document release test requirement and QuickPick test limitation Two documentation gaps that kept causing repeated investigation: 1. CLAUDE.md: added rule requiring `pnpm test:release` for any QA/integration test changes — the QA validator only checks ID matching, not that tests actually pass in a VS Code host. 2. TESTING.md: documented the QuickPick limitation — VS Code's extension host test runner provides no API to interact with pickers, so a QuickPick opening during a test stalls it indefinitely. Documented the command-bypass workaround and what must remain manual. Updated CLAUDE.md QA005 to cross-reference this section.
couimet
pushed a commit
that referenced
this pull request
Mar 21, 2026
…e navigation Added 5 integration tests that verify end-to-end navigation to open untitled files in a real VS Code host — creating an untitled document, generating a RangeLink referencing it, invoking handleDocumentLinkClick, and asserting cursor position + toast messages via log-based assertions (leveraging LogCapture from PR #484). Coverage: full-line (001), line-range (002), character-precision (004), line clamping with warning (005), and file-not-found for closed untitled files (003). All 5 QA TCs are automated. Benefits: - Full integration test coverage for untitled file navigation (happy + clamping + error paths) - Log-based assertions verify info/warning toast messages without UI interaction - QA validator passes with 51 automated TCs matching integration tests
couimet
added a commit
that referenced
this pull request
Mar 22, 2026
…dent fallback (#490) * [issues/101] Make untitled file navigation locale-independent The untitled file fallback was gated behind a regex that only matched English "Untitled-N" names. In non-English VS Code locales (e.g., French "Sans titre-1", German "Unbenannt-1"), the fallback never triggered and users saw a generic "Cannot find file" error instead of being navigated to the open untitled document. Removed the English-only regex gate so `findOpenUntitledFile()` is always tried when a path doesn't resolve on disk. The search uses the `untitled:` URI scheme which is locale-independent. If no matching open untitled document is found, the generic "Cannot find file" warning is shown — the same message for all unresolved paths regardless of naming pattern. Benefits: - Untitled file navigation works in all VS Code locales - Simpler control flow — one fallback path instead of regex-branching - Removed the untitled-specific warning message (dead path after this change) * [issues/101] Make findOpenUntitledFile use case-insensitive matching The display name comparison in `findOpenUntitledFile` used strict equality, which could miss matches when the link path case differs from the document's display name (e.g., searching for "untitled-1" when the document reports "Untitled-1"). Switched to `.toLowerCase()` comparison on both sides to handle cross-platform case differences. Benefits: - Handles case mismatches between link references and VS Code's internal naming - More robust on case-insensitive filesystems (Windows/macOS) * [issues/101] Remove unused WARN_NAVIGATION_UNTITLED_FILE message The untitled-specific warning message was made unreachable by S001's control flow simplification — all unresolved paths now show the generic "Cannot find file" warning regardless of naming pattern. Removed the `WARN_NAVIGATION_UNTITLED_FILE` enum entry from MessageCode and its string template from messages.en.ts. Benefits: - No dead code in the message system - One fewer message to maintain and translate * [issues/101] Add QA test cases and integration tests for untitled file navigation Added 5 integration tests that verify end-to-end navigation to open untitled files in a real VS Code host — creating an untitled document, generating a RangeLink referencing it, invoking handleDocumentLinkClick, and asserting cursor position + toast messages via log-based assertions (leveraging LogCapture from PR #484). Coverage: full-line (001), line-range (002), character-precision (004), line clamping with warning (005), and file-not-found for closed untitled files (003). All 5 QA TCs are automated. Benefits: - Full integration test coverage for untitled file navigation (happy + clamping + error paths) - Log-based assertions verify info/warning toast messages without UI interaction - QA validator passes with 51 automated TCs matching integration tests * CHANGELOG entry * [PR feedback] Add case-insensitive matching TC and use getUntitledDisplayName in tests Added `untitled-navigation-006` integration test and QA TC that verifies lowercase input (`untitled-1#L5`) matches an open `Untitled-1` document. This directly tests the case-insensitive matching behavior from the `findOpenUntitledFile` change. Replaced the inline display name derivation logic in the test with an import of `getUntitledDisplayName` from production code, eliminating duplication. Benefits: - Case-insensitive matching has dedicated regression coverage (52 automated TCs total) - Test uses the same derivation logic as production code instead of reimplementing it Ignored Feedback: - Multiple untitled match disambiguation via quick-pick: VS Code guarantees unique untitled names per window and `workspace.textDocuments` only returns current-window documents, making multiple matches impossible - Parameterize English names in QA YAML with `{{placeholders}}`: QA YAML files are human-readable test plans, not executable templates; parameterizing would reduce readability without adding value Ref: #490 (review)
12 tasks
5 tasks
5 tasks
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
VS Code's OutputChannel API is write-only — integration tests couldn't verify which toasts or status bar messages were shown after executing commands. This PR adds a LogCapture infrastructure that captures log lines in memory (gated by
RANGELINK_CAPTURE_LOGSenv var), along with assertion helpers that parse the VscodeAdapter log JSON to match message type and text. Using this, 6 previously-manual QA test cases are now automated, 4 existing navigation-clamping tests are enhanced with toast content verification, and 7 new integration tests are added.Changes
src/LogCapture.ts) — wraps OutputChannel, proxiesappendLine()to both the real channel and an in-memory array whenRANGELINK_CAPTURE_LOGS=true. Test-only methods (mark,getLinesSince,getAllLines,clear) throwLOG_CAPTURE_DISABLEDin production. Exposed viaext.exports.logCapturethroughRangeLinkExtensionApi.src/VSCodeLogger.ts) — decouples VSCodeLogger fromvscode.OutputChannel; now accepts any{ appendLine(value: string): void }.__integration-tests__/helpers/logBasedUiAssertions.ts) —assertToastLogged(info/warning/error),assertStatusBarMsgLogged, and their negative counterparts. Toasts and status bar messages are separate assertion functions. Barrel-exported withgetLogCapture().navigationClamping.test.tsthat were blocked on Can release tests use logs to validate toast operations given the current setup doesn't allow the UI interaction/validation ? #481. Tests now verify the exact toast type (warning for clamped, info for within-bounds) and message content (including clamping summary).logBasedUiValidation.test.ts) — 7 new integration tests: unbind no-op status bar, terminal-focus R-L/R-C error guidance, send link to bound terminal, send file path, dirty buffer clean path, full-line selection validation.automated: true(core-send-commands-r-l-001, dirty-buffer-warning-007, full-line-selection-validation-001, send-file-path-001, send-terminal-selection-006, send-terminal-selection-007). Validator: 45/45 PASSED..vscode-test.mjs) —env: { RANGELINK_CAPTURE_LOGS: 'true' }enables capture for all integration tests.Test Plan
Related
Summary by CodeRabbit
Tests
Chores