test(sandbox): exercise active create handoff - #8730
Conversation
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
📝 WalkthroughWalkthroughThe tests verify that patch application waits for create completion and that sandbox recreation waits for the create stream to close. They also update child-process termination and cleanup expectations. ChangesSandbox creation handoff
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
1 additional E2E selection from the second opinionAdvisory only. The primary lane did not select these E2E jobs or targets.
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 1 semantic terminology decisionTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite against this exact revision. Recommended E2E: 2 optional E2E recommendations
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/lib/onboard/sandbox-gpu-create-flow.test.ts`:
- Around line 515-520: Update the streamSandboxCreate mock in the sandbox
creation flow test to return a deferred promise that remains pending after
invoking onPoll; assert patch.ensureApplied has not been called while that
promise is pending, then resolve the deferred promise and await the flow before
verifying completion.
🪄 Autofix
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: Enterprise
Run ID: 66226845-c7a1-4247-93d4-c4070ed647af
📒 Files selected for processing (2)
src/lib/onboard/sandbox-gpu-create-flow.test.tstest/onboard-sandbox-recreation.test.ts
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Summary
Tests now exercise the serialized create handoff added by #8729 at the actual lifecycle boundary. This also fixes the late shard-12 failure where the integration fixture never reported that its fake create process exited after
SIGTERM.Related Issue
Follow-up to #8729 and its CodeRabbit ordering finding.
Changes
ensureApplied()order.SIGTERM, matching the ownership-release contract.Type of Change
Quality Gates
Documentation Writer Review
no-docs-neededdocs/get-started/quickstart.mdxanddocs/reference/commands.mdxalready describe the unchanged ready and recreation contract.DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailablenpx vitest run --project cli src/lib/onboard/sandbox-gpu-create-flow.test.ts src/lib/sandbox/create-stream.test.ts(56 passed);npx vitest run --project integration test/onboard-sandbox-recreation.test.ts -t "waits for the create stream to close"(1 passed); the refined deferred-boundary test passed again after review feedback.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: Not applicable; this is a two-test fixture correction protected by targeted tests and normal hooks.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Apurv Kumaria akumaria@nvidia.com