[issues/558] Automate 3 assisted tests with CMD_BIND_TO_CUSTOM_AI_BY_ID + dummyAi.getText()#601
Merged
Conversation
…_ID` + `dummyAi.getText()`
## Summary
Three [assisted] integration tests that required human setup (picker bind, keybinding press, PASS/FAIL verdict) are now fully automated using `CMD_BIND_TO_CUSTOM_AI_BY_ID` for programmatic binding, `CMD_COPY_LINK_RELATIVE` for programmatic send, and `dummyAi.getText()` for outcome verification. `clipboardPreservation.test.ts` now has zero `[assisted]` tests.
## Changes
- `core-send-commands-r-l-003`: replaced human-in-the-loop with programmatic bind → send → `dummyAi.getText()` assertion. Removed `waitForHumanVerdict` import.
- `clipboard-preservation-004`: same pattern. Programmatic bind to `rangelink.dummy-ai-extension`, programmatic send, `dummyAi.getText()` outcome check.
- `clipboard-preservation-010`: same pattern. Bind to `rangelink.dummy-ai-extension-focus-fail`, programmatic send, log-based assertions for focus-failure path.
- All three tests now assert exact padded values (` ${link} `) via strict equality — no `.trim()` in any assertion.
- Removed `waitForHuman` and `CLIPBOARD_SENTINEL` from clipboardPreservation imports. `CLIPBOARD_SENTINEL` was only referenced in `waitForHuman` console steps (shown to the human driver); the sentinel mechanics still work through `writeClipboardSentinel`, `assertClipboardRestored`, and `assertClipboardChanged` which use the constant internally.
- `assistedTestHelper.ts`: fixed `waitForHumanVerdict` overlap bug. When Mocha times out a verdict-blocked test and starts the next one, the new invocation now disposes the previous test's stale status bar items AND dismisses its `withProgress` notification toast before creating its own. Root cause: Mocha timeout fires on a `waitForHumanVerdict`-blocked test, Mocha fails it and starts the next test, but `settleWith` never ran so the first test's PASS/FAIL buttons and progress toast persisted. Fix: module-level `activeVerdictDisposables` + `activeVerdictReject` track the current invocation's state; a new call disposes stale items and rejects the prior promise to dismiss the toast.
- `.vscode-test.mjs`: added `RANGELINK_MOCHA_TIMEOUT` env var override for debugging timeout behavior without code changes (defaults to `ASSISTED_TIMEOUT_MS` when unset).
- QA YAML: all three TCs moved from `automated: assisted` to `automated: true`. Automated count: 156 → 159, assisted count: 112 → 108 (one other TC changed independently).
## Test Plan
- [x] All 1979 unit tests pass
- [x] `core-send-commands-r-l-003` passes fully automated (1.7s)
- [x] `clipboard-preservation-004` passes fully automated (1.6s)
- [x] `clipboard-preservation-010` passes fully automated (1.5s)
- [x] QA coverage validator passes (159 automated, 108 assisted)
- [x] Zero `.trim()` calls remain in integration test assertions
## Related
- Closes #558
Contributor
|
Warning Review limit reached
Your plan includes 1 review of capacity. Refill in 52 minutes and 57 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
WalkthroughAutomates three previously human-guided integration tests, hardens the waitForHumanVerdict helper to dispose/reject superseded prompts, adds MOCHA_TIMEOUT parsed from RANGELINK_MOCHA_TIMEOUT with fallback, and updates QA YAML entries to mark the converted tests as automated. ChangesTest Suite Automation and Infrastructure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
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 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 |
|
The PR updates existing test cases in the QA YAML, changing expected results and marking them as automated. However, it does not introduce new user-visible features or commands, indicating a gap in coverage for potential new scenarios. Suggested test cases:
Generated by QA Gap Check (GPT-4o-mini via GitHub Models) |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/rangelink-vscode-extension/src/__integration-tests__/helpers/assistedTestHelper.ts (1)
109-189: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftExtract
waitForHumanVerdictinto smaller helpers to reduce mixed concerns.Line 109+ now combines UI wiring, command registration, global supersede-state management, and promise settlement in one block. Please split into focused helpers (e.g., cleanup/supersede, status-bar setup, settlement), then keep
waitForHumanVerdictas orchestration.As per coding guidelines: "
**/*.{ts,tsx}: Extract functions exceeding 50 lines, having 3+ dependencies, or mixing concerns into separate modules..."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/rangelink-vscode-extension/src/__integration-tests__/helpers/assistedTestHelper.ts` around lines 109 - 189, The waitForHumanVerdict function mixes global-supersede logic, command registration, status-bar item setup, and promise settlement; split it into focused helpers: implement a supersedePreviousVerdict() that disposes activeVerdictDisposables and rejects activeVerdictReject (referencing activeVerdictDisposables and activeVerdictReject), a makeVerdictCommands(invocationId) that returns passCommand/failCommand strings and registers the commands (used where passCommand/failCommand are currently created), a createStatusBarItems(tcId, passCommand, failCommand) that returns the disposables for passItem and failItem (used where passItem/failItem are built), and a createVerdictPromise(disposables, resolve/reject) that contains settleWith and resolves the HumanVerdict; then refactor waitForHumanVerdict to orchestrate by calling these helpers and only perform high-level flow (increment verdictInvocationCounter, call supersedePreviousVerdict, create commands/status items, set activeVerdictDisposables, and return the promise).
🧹 Nitpick comments (1)
packages/rangelink-vscode-extension/src/__integration-tests__/suite/coreSendCommands.test.ts (1)
100-103: ⚡ Quick winRemove redundant editor open in
core-send-commands-r-l-003.Line 100 opens the file, then Lines 101-103 open/show the same file again. Keep one path to reduce noise and potential focus jitter.
Suggested simplification
- await ss.openEditor(fileUri); const doc = await vscode.workspace.openTextDocument(fileUri); const editor = await vscode.window.showTextDocument(doc);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/rangelink-vscode-extension/src/__integration-tests__/suite/coreSendCommands.test.ts` around lines 100 - 103, The test is opening the same file twice causing redundant work and potential focus jitter; remove the extra open by keeping only one opening approach (either the helper ss.openEditor(fileUri) or the explicit vscode.workspace.openTextDocument + vscode.window.showTextDocument sequence). Locate the test in coreSendCommands.test.ts and remove the duplicate calls referencing ss.openEditor, vscode.workspace.openTextDocument, and vscode.window.showTextDocument so the editor is opened exactly once and the subsequent editor selection (new vscode.Selection(0, 0, 3, 0)) still operates on that single opened editor.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/rangelink-vscode-extension/.vscode-test.mjs`:
- Around line 5-7: MOCHA_TIMEOUT is derived directly via
Number(process.env.RANGELINK_MOCHA_TIMEOUT) which can produce NaN or negative
values; update the logic that computes MOCHA_TIMEOUT so it parses the env var,
checks Number.isFinite(value) and value >= 0 (allow 0), and only then uses it;
otherwise fall back to ASSISTED_TIMEOUT_MS; ensure the validated MOCHA_TIMEOUT
is what gets passed into mocha.timeout so negatives/NaN never reach Mocha.
In `@packages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0.yaml`:
- Around line 390-392: The expected_result for scenario 'always mode: clipboard
content before AI assistant paste is restored after'
(clipboard-preservation-004) is inaccurate: replace the current "relPath#L1-L3"
description with the exact padded, character-precision payload the automated
assertion expects (the char-precise clipboard content used by
assertClipboardRestored). Edit the expected_result string so it documents the
padded, char-precision payload and mentions that the sentinel clipboard value is
restored, matching the automated contract used by the test harness.
---
Outside diff comments:
In
`@packages/rangelink-vscode-extension/src/__integration-tests__/helpers/assistedTestHelper.ts`:
- Around line 109-189: The waitForHumanVerdict function mixes global-supersede
logic, command registration, status-bar item setup, and promise settlement;
split it into focused helpers: implement a supersedePreviousVerdict() that
disposes activeVerdictDisposables and rejects activeVerdictReject (referencing
activeVerdictDisposables and activeVerdictReject), a
makeVerdictCommands(invocationId) that returns passCommand/failCommand strings
and registers the commands (used where passCommand/failCommand are currently
created), a createStatusBarItems(tcId, passCommand, failCommand) that returns
the disposables for passItem and failItem (used where passItem/failItem are
built), and a createVerdictPromise(disposables, resolve/reject) that contains
settleWith and resolves the HumanVerdict; then refactor waitForHumanVerdict to
orchestrate by calling these helpers and only perform high-level flow (increment
verdictInvocationCounter, call supersedePreviousVerdict, create commands/status
items, set activeVerdictDisposables, and return the promise).
---
Nitpick comments:
In
`@packages/rangelink-vscode-extension/src/__integration-tests__/suite/coreSendCommands.test.ts`:
- Around line 100-103: The test is opening the same file twice causing redundant
work and potential focus jitter; remove the extra open by keeping only one
opening approach (either the helper ss.openEditor(fileUri) or the explicit
vscode.workspace.openTextDocument + vscode.window.showTextDocument sequence).
Locate the test in coreSendCommands.test.ts and remove the duplicate calls
referencing ss.openEditor, vscode.workspace.openTextDocument, and
vscode.window.showTextDocument so the editor is opened exactly once and the
subsequent editor selection (new vscode.Selection(0, 0, 3, 0)) still operates on
that single opened editor.
🪄 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: e863ae06-cc46-4b5f-b8b5-bfd35e00f8d9
📒 Files selected for processing (5)
packages/rangelink-vscode-extension/.vscode-test.mjspackages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0.yamlpackages/rangelink-vscode-extension/src/__integration-tests__/helpers/assistedTestHelper.tspackages/rangelink-vscode-extension/src/__integration-tests__/suite/clipboardPreservation.test.tspackages/rangelink-vscode-extension/src/__integration-tests__/suite/coreSendCommands.test.ts
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Validate RANGELINK_MOCHA_TIMEOUT env var to prevent NaN/negative values reaching Mocha. Fix clipboard-preservation-004 expected_result to document the padded link contract. Remove redundant ss.openEditor call in core-send-commands-r-l-003. Ignored Feedback: - Extract waitForHumanVerdict into smaller helpers: Splitting tight UI-wiring code into single-caller helpers adds indirection without improving testability or readability. The function is self-contained and its concerns are inherently coupled. Ref: #601 (review)
✅ CI / Integration Tests (with extensions) — run summary
|
✅ CI / Test & Validate — run summary
|
Owner
Author
|
@coderabbitai review - I was rate-limited in #601 (comment) |
Contributor
|
✅ Actions performedReview triggered.
|
Owner
Author
|
@coderabbitai I see the PR status saying Feels like #555 created more problems than it solved, right ? Should I have bumped the |
Contributor
|
Why "Review skipped" appearedYour config has: auto_incremental_review: true
auto_pause_after_reviewed_commits: 0
The What you actually needed:
|
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
Three [assisted] integration tests that required human setup (picker bind, keybinding press, PASS/FAIL verdict) are now fully automated using
CMD_BIND_TO_CUSTOM_AI_BY_IDfor programmatic binding,CMD_COPY_LINK_RELATIVEfor programmatic send, anddummyAi.getText()for outcome verification.clipboardPreservation.test.tsnow has zero[assisted]tests.Changes
core-send-commands-r-l-003: replaced human-in-the-loop with programmatic bind → send →dummyAi.getText()assertion. RemovedwaitForHumanVerdictimport.clipboard-preservation-004: same pattern. Programmatic bind torangelink.dummy-ai-extension, programmatic send,dummyAi.getText()outcome check.clipboard-preservation-010: same pattern. Bind torangelink.dummy-ai-extension-focus-fail, programmatic send, log-based assertions for focus-failure path.${link}) via strict equality — no.trim()in any assertion.waitForHumanandCLIPBOARD_SENTINELfrom clipboardPreservation imports.CLIPBOARD_SENTINELwas only referenced inwaitForHumanconsole steps (shown to the human driver); the sentinel mechanics still work throughwriteClipboardSentinel,assertClipboardRestored, andassertClipboardChangedwhich use the constant internally.assistedTestHelper.ts: fixedwaitForHumanVerdictoverlap bug. When Mocha times out a verdict-blocked test and starts the next one, the new invocation now disposes the previous test's stale status bar items AND dismisses itswithProgressnotification toast before creating its own. Root cause: Mocha timeout fires on awaitForHumanVerdict-blocked test, Mocha fails it and starts the next test, butsettleWithnever ran so the first test's PASS/FAIL buttons and progress toast persisted. Fix: module-levelactiveVerdictDisposables+activeVerdictRejecttrack the current invocation's state; a new call disposes stale items and rejects the prior promise to dismiss the toast..vscode-test.mjs: addedRANGELINK_MOCHA_TIMEOUTenv var override for debugging timeout behavior without code changes (defaults toASSISTED_TIMEOUT_MSwhen unset).automated: assistedtoautomated: true. Automated count: 156 → 159, assisted count: 112 → 108 (one other TC changed independently).Test Plan
core-send-commands-r-l-003passes fully automated (1.7s)clipboard-preservation-004passes fully automated (1.6s)clipboard-preservation-010passes fully automated (1.5s).trim()calls remain in integration test assertionsRelated
Summary by CodeRabbit
Tests
Chores