test(connect): cover probe-only outcomes#5485
Conversation
📝 WalkthroughWalkthroughThe ChangesconnectSandbox probe-only test coverage
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
E2E Advisor RecommendationRequired E2E: None Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
|
Vitest E2E Scenario RecommendationRequired Vitest E2E scenarios: None Full Vitest E2E advisor summaryVitest E2E Scenario AdvisorBase: Required Vitest E2E scenarios
Optional Vitest E2E scenarios
Relevant changed files
|
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the branch is 96%. Coverage data for the branch is not yet available. Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the branch is 46%. Coverage data for the branch is not yet available. Show a code coverage summary of the most covered files.
Updated |
PR Review AdvisorFindings: 0 needs attention, 0 worth checking, 0 nice ideas This is an automated advisory review. A human maintainer must make the final merge decision. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/actions/sandbox/connect-flow.test.ts (1)
208-223: ⚡ Quick winAssert the process-check call in the probe-only failure test.
This test verifies the failure outcome, but not that it failed via the intended inspection branch. Add an assertion for the quiet process-check call to tighten the contract.
Suggested test hardening
await expect(harness.connectSandbox("alpha", { probeOnly: true })).rejects.toThrow( "process.exit(1)", ); + expect(harness.checkAndRecoverSpy).toHaveBeenCalledWith("alpha", { quiet: true }); expect(harness.runAutoPairSpy).not.toHaveBeenCalled(); expect(harness.spawnSyncSpy).not.toHaveBeenCalledWith( "openshell", ["sandbox", "connect", "alpha"], expect.any(Object), ); expect(exitSpy).toHaveBeenCalledWith(1);🤖 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 `@src/lib/actions/sandbox/connect-flow.test.ts` around lines 208 - 223, The test for "probe-only mode exits when process inspection cannot run" verifies the failure outcome but does not assert that the process inspection was actually attempted. Add an assertion to verify that the process-check method or spy on the harness object was called during the test execution, placing this assertion after the other expectations to confirm the failure path went through the intended inspection branch rather than taking an alternative failure path.
🤖 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.
Nitpick comments:
In `@src/lib/actions/sandbox/connect-flow.test.ts`:
- Around line 208-223: The test for "probe-only mode exits when process
inspection cannot run" verifies the failure outcome but does not assert that the
process inspection was actually attempted. Add an assertion to verify that the
process-check method or spy on the harness object was called during the test
execution, placing this assertion after the other expectations to confirm the
failure path went through the intended inspection branch rather than taking an
alternative failure path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 88fb0520-c1af-49b9-962d-ef24e5bda9d6
📒 Files selected for processing (1)
src/lib/actions/sandbox/connect-flow.test.ts
1 participant
Summary
Extends connect flow coverage to the
--probe-onlyrecovery path. The tests cover recovered gateway reporting and fail-fast process-inspection failures without opening an interactive shell.Changes
src/lib/actions/sandbox/connect-flow.test.tswith configurable process-check outcomes.Type of Change
Verification
npx prek run --from-ref main --to-ref HEADpassesnpx vitest run src/lib/actions/sandbox/connect-flow.test.ts --project clinpm run typecheck:clinpm testpasses (broad runtime changes only)npm run docsbuilds without warnings (doc changes only)Note: commit and push hooks were skipped for this test-only branch because the broad local hook suite has unrelated pre-existing CLI failures in this checkout; targeted tests and CLI typecheck passed.
Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit