test(e2e): document platform parity coverage for WSL GPU Spark - #5803
test(e2e): document platform parity coverage for WSL GPU Spark#5803jyaunches wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughAdds test coverage for GPU/CDI fallback selection, Docker clone DNS injection, WSL sandbox reachability messaging, and workflow-boundary dispatch routing across canonical scenarios. ChangesOnboarding and workflow parity tests
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 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 |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most covered files.
Updated |
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
|
PR Review Advisor — No blocking findingsMerge posture: No blocking advisor findings Action checklist
Test follow-ups to resolve or justifyIf these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.
This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. 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.
Actionable comments posted: 2
🤖 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/preflight-cdi.test.ts`:
- Around line 344-351: Update the test in preflight-cdi.test.ts so the title
matches the actual inputs driving planHostRemediation: it should describe the
install_nvidia_container_toolkit branch triggered by cdiNvidiaGpuSpecMissing and
nvidiaContainerToolkitInstalled being false, not nvidia-smi availability. Remove
the redundant hasNvidiaGpu: true override if baseAssessment already sets it, and
keep the assertion focused on the remediation action selected by
planHostRemediation.
In `@test/e2e-scenario/support-tests/platform-parity-workflow-boundary.test.ts`:
- Around line 15-25: The `wsl` lookup in `canonicalScenarios().find(...)` is
still treated as possibly undefined after `expect(wsl).toBeDefined()`, so the
later property access in the same test is not type-safe. In
`platform-parity-workflow-boundary.test.ts`, add an explicit runtime guard
immediately after the `expect(wsl).toBeDefined()` assertion in the
`wsl-repo-cloud-openclaw` block so TypeScript can narrow the value before
reading `environment`, `runnerRequirements`, `suiteIds`, and `requiredSecrets`.
🪄 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: Enterprise
Run ID: 6b235d64-4548-43e0-a342-787a3906908c
📒 Files selected for processing (5)
src/lib/onboard/docker-gpu-patch-mode-selection.test.tssrc/lib/onboard/docker-gpu-patch.test.tssrc/lib/onboard/gateway-sandbox-reachability.test.tssrc/lib/onboard/preflight-cdi.test.tstest/e2e-scenario/support-tests/platform-parity-workflow-boundary.test.ts
| it("still blocks with install_nvidia_container_toolkit when nvidia-smi is unavailable but PCI GPU context exists", () => { | ||
| const actions = planHostRemediation( | ||
| baseAssessment({ | ||
| cdiNvidiaGpuSpecMissing: true, | ||
| hasNvidiaGpu: true, | ||
| nvidiaContainerToolkitInstalled: false, | ||
| }), | ||
| ); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Test title references nvidia-smi, but the assessment drives the branch via the toolkit flag.
The behavior being exercised is nvidiaContainerToolkitInstalled: false (plus cdiNvidiaGpuSpecMissing: true), which is what selects the install_nvidia_container_toolkit branch in planHostRemediation. The name "when nvidia-smi is unavailable" isn't modeled by any field here, so it may confuse the parity-coverage mapping this PR documents. Also, hasNvidiaGpu: true duplicates the baseAssessment default and can be dropped.
✏️ Suggested title/override tidy-up
- it("still blocks with install_nvidia_container_toolkit when nvidia-smi is unavailable but PCI GPU context exists", () => {
+ it("still blocks with install_nvidia_container_toolkit when the toolkit is not installed but a GPU is present", () => {
const actions = planHostRemediation(
baseAssessment({
cdiNvidiaGpuSpecMissing: true,
- hasNvidiaGpu: true,
nvidiaContainerToolkitInstalled: false,
}),
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it("still blocks with install_nvidia_container_toolkit when nvidia-smi is unavailable but PCI GPU context exists", () => { | |
| const actions = planHostRemediation( | |
| baseAssessment({ | |
| cdiNvidiaGpuSpecMissing: true, | |
| hasNvidiaGpu: true, | |
| nvidiaContainerToolkitInstalled: false, | |
| }), | |
| ); | |
| it("still blocks with install_nvidia_container_toolkit when the toolkit is not installed but a GPU is present", () => { | |
| const actions = planHostRemediation( | |
| baseAssessment({ | |
| cdiNvidiaGpuSpecMissing: true, | |
| nvidiaContainerToolkitInstalled: false, | |
| }), | |
| ); |
🤖 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/onboard/preflight-cdi.test.ts` around lines 344 - 351, Update the
test in preflight-cdi.test.ts so the title matches the actual inputs driving
planHostRemediation: it should describe the install_nvidia_container_toolkit
branch triggered by cdiNvidiaGpuSpecMissing and nvidiaContainerToolkitInstalled
being false, not nvidia-smi availability. Remove the redundant hasNvidiaGpu:
true override if baseAssessment already sets it, and keep the assertion focused
on the remediation action selected by planHostRemediation.
| const wsl = canonicalScenarios().find((entry) => entry.id === "wsl-repo-cloud-openclaw"); | ||
| expect(wsl).toBeDefined(); | ||
|
|
||
| expect(wsl.environment).toMatchObject({ | ||
| platform: "wsl-local", | ||
| runtime: "docker-running", | ||
| onboarding: "cloud-openclaw", | ||
| }); | ||
| expect(wsl.runnerRequirements).toEqual(expect.arrayContaining(["windows-latest", "wsl2"])); | ||
| expect(wsl.suiteIds).toContain("platform-wsl"); | ||
| expect(wsl.requiredSecrets).toContain("NVIDIA_INFERENCE_API_KEY"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target test file and relevant TypeScript/Vitest context.
git ls-files 'test/e2e-scenario/support-tests/platform-parity-workflow-boundary.test.ts' 'tsconfig*.json' 'package.json' 'vitest.config.*' 'test/**/support-tests/*.test.ts' | sed 's#^`#FILE` #'
echo
echo "---- target file ----"
cat -n test/e2e-scenario/support-tests/platform-parity-workflow-boundary.test.ts
echo
echo "---- search for similar patterns ----"
rg -n "toBeDefined\(\).*possible|expect\\(.*\\)\\.toBeDefined\\(\\).*\\n.*if \\(!.*\\)" test src -U || true
echo
echo "---- search for config / test runner deps ----"
rg -n "\"vitest\"|expectTypeOf|toBeDefined" package.json tsconfig*.json vitest.config.* test -g '!**/node_modules/**' || trueRepository: NVIDIA/NemoClaw
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the file and typecheck-relevant configuration in smaller slices if needed.
stat -c '%n %s bytes' test/e2e-scenario/support-tests/platform-parity-workflow-boundary.test.ts
echo
sed -n '1,220p' test/e2e-scenario/support-tests/platform-parity-workflow-boundary.test.ts
echo
echo "---- tsconfig files ----"
for f in tsconfig*.json; do
[ -f "$f" ] && echo "### $f" && cat "$f" && echo
done
echo
echo "---- package.json snippets ----"
sed -n '1,240p' package.jsonRepository: NVIDIA/NemoClaw
Length of output: 195
Add a runtime guard before dereferencing wsl.
expect(wsl).toBeDefined() doesn’t narrow the find() result for TypeScript, so the later property reads still fail type-checking. Add if (!wsl) throw new Error("Missing canonical scenario: wsl-repo-cloud-openclaw"); after the expectation.
🧰 Tools
🪛 GitHub Check: build-typecheck
[failure] 25-25:
'wsl' is possibly 'undefined'.
[failure] 24-24:
'wsl' is possibly 'undefined'.
[failure] 23-23:
'wsl' is possibly 'undefined'.
[failure] 18-18:
'wsl' is possibly 'undefined'.
🤖 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 `@test/e2e-scenario/support-tests/platform-parity-workflow-boundary.test.ts`
around lines 15 - 25, The `wsl` lookup in `canonicalScenarios().find(...)` is
still treated as possibly undefined after `expect(wsl).toBeDefined()`, so the
later property access in the same test is not type-safe. In
`platform-parity-workflow-boundary.test.ts`, add an explicit runtime guard
immediately after the `expect(wsl).toBeDefined()` assertion in the
`wsl-repo-cloud-openclaw` block so TypeScript can narrow the value before
reading `environment`, `runnerRequirements`, `suiteIds`, and `requiredSecrets`.
Source: Linters/SAST tools
|
Closing because P0-E source PRs referenced by issue #5800 are still open/unmerged, so this PR overclaims parity coverage. We'll reopen a scoped parity/waiver PR after the source scope is settled. |
|
Closing P0-E because the source PRs referenced by issue #5800 are still open/unmerged, so this PR overclaims parity coverage. We'll reopen a scoped parity/waiver PR after the P0-E source scope is settled. |
2 participants
Summary
Restore issue #5800 parity package
P0-Eby documenting and pinning platform-aware Vitest coverage for WSL GPU, Docker Desktop gateway, Linux GPU toolkit preflight, DGX Spark DNS, and platform workflow routing.Related Issues
Refs #5800
Refs #5098
Refs #5513 / #5536 / #5754
Refs #5512 / #5537 / #5538 / #5541
Refs #5489 / #5500 / #5529
Refs #5520 / #5539
Package scope
P0-E — WSL/GPU/Spark platform parity or waiver package--gpuscompatibility fallback, missing toolkit remediation whennvidia-smiis unavailable but GPU context exists, DGX Spark DNS diagnostic-only behavior, and workflow selector boundaries for WSL/GPU/Spark/Jetson platform routes.Parity map
nonesrc/lib/onboard/gateway-sandbox-reachability.test.tsasserts host-gateway WSL message is retryable and has no UFW/127.0.0.1 guidance.--gpuscompatibility fallback when CDI injection is unresolvable.nonesrc/lib/onboard/docker-gpu-patch-mode-selection.test.tsasserts CDI probe failure falls through to--gpus all; existing create-plan tests keep the GPU patch enabled and suppress direct OpenShell--gpu.*-nemoclaw-gpu-backup-*.nonesrc/lib/onboard/docker-gpu-patch-rollback.test.tsandsrc/lib/onboard/docker-gpu-sandbox-create.test.tscover deferred rollback/finalize behavior; no new assertion needed.install_nvidia_container_toolkitwhennvidia-smiis unavailable but GPU/CDI context requires toolkit repair.nonesrc/lib/onboard/preflight-cdi.test.tsasserts blocking toolkit action andnvidia-ctk cdi generateremediation.nonesrc/lib/onboard/docker-gpu-patch.test.tsasserts loopback resolver injects--dns, Spark-managed non-loopback DNS stays preserved, and no--network hostis implied.nonetest/e2e-scenario/support-tests/platform-parity-workflow-boundary.test.tsasserts WSL registry routing and GPU/Spark/Jetson free-standing selector mappings; Jetson remains explicit-only.noneInference mode support
none(support/unit/workflow-boundary only; no live inference target edited).Validation
npm run build:clinpx vitest run src/lib/onboard/gateway-sandbox-reachability.test.ts src/lib/onboard/docker-gpu-patch-mode-selection.test.ts src/lib/onboard/preflight-cdi.test.ts src/lib/onboard/docker-gpu-patch.test.ts test/e2e-scenario/support-tests/platform-parity-workflow-boundary.test.ts --reporter=defaultgit diff --checkFollow-ups / waivers
Summary by CodeRabbit