refactor(e2e): expose Docker prerequisites through Vitest fixtures - #6365
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds a ChangesDocker Prerequisite Fixture
Estimated code review effort: 2 (Simple) | ~12 minutes Sequence Diagram(s)sequenceDiagram
participant Test as sandbox-operations.test.ts
participant Fixture as E2ETestFixtures
participant Prereq as DockerPrerequisite
participant Probe as DockerProbe
Test->>Fixture: request docker fixture
Fixture->>Probe: new DockerProbe(artifacts, redact)
Fixture->>Prereq: new DockerPrerequisite(probe, skip)
Fixture-->>Test: docker fixture
Test->>Prereq: requireDocker()
Prereq->>Probe: run(["info"])
Probe-->>Prereq: DockerCommandResult
alt exitCode === 0
Prereq-->>Test: resolved (Docker available)
else exitCode !== 0
alt isCi
Prereq-->>Test: throw error
else
Prereq->>Test: skip(reason)
end
end
Possibly related issues
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
cv
left a comment
There was a problem hiding this comment.
Maintainer review: the Docker prerequisite fixture exposes the existing availability contract without changing skip/failure semantics, with focused fixture and target coverage. Current-main synchronization cleared the stale growth failure; all exact-head checks, DCO, signatures, and review threads are clean.
…VIDIA#6365) ## Summary Expose the existing redacting Docker probe through the live Vitest fixture with explicit required, optional, and intentionally-missing policies. Closes NVIDIA#6354 Parent epic: NVIDIA#6346 ## Changes - Add `DockerPrerequisite` with `probeDocker`, `requireDocker`, and `expectMissingDocker`. - Centralize local-skip versus CI-failure behavior for required Docker. - Expose the prerequisite as the `docker` E2E fixture. - Migrate the sandbox-operations prerequisite block. - Preserve isolated Docker config, redacted output, and artifacts through `DockerProbe`. - Add 9 focused prerequisite/probe tests. ## Verification - [x] Signed/Verified commit and all hooks passed - [x] `npm run build:cli` - [x] `npm run typecheck:cli` - [x] Docker support tests: 9 passed - [x] No product behavior, docs, or secrets changed --- Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved Docker availability checks for live test runs, with clearer handling when Docker is missing. * Tests now skip more gracefully on local machines and fail more explicitly in CI when Docker is unavailable. * Added coverage for Docker-present, Docker-missing, and expected-missing scenarios to make test behavior more reliable. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
Summary
Expose the existing redacting Docker probe through the live Vitest fixture with explicit required, optional, and intentionally-missing policies.
Closes #6354
Parent epic: #6346
Changes
DockerPrerequisitewithprobeDocker,requireDocker, andexpectMissingDocker.dockerE2E fixture.DockerProbe.Verification
npm run build:clinpm run typecheck:cliSigned-off-by: Julie Yaunches jyaunches@nvidia.com
Summary by CodeRabbit