Skip to content

test: stabilize installer Docker bootstrap tests on WSL#3899

Merged
ericksoa merged 1 commit into
NVIDIA:mainfrom
jyaunches:fix-wsl-install-preflight
May 20, 2026
Merged

test: stabilize installer Docker bootstrap tests on WSL#3899
ericksoa merged 1 commit into
NVIDIA:mainfrom
jyaunches:fix-wsl-install-preflight

Conversation

@jyaunches

@jyaunches jyaunches commented May 20, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Summary

  • force the installer Docker bootstrap unit-test helper to exercise the non-WSL Linux path
  • keep the real installer behavior unchanged: WSL hosts still skip Linux Docker bootstrap

Why

PR #3824 broadened WSL detection from WSL env vars to /proc probes via is_wsl_host. That is correct for runtime behavior, but these unit tests are specifically validating Linux Docker bootstrap branches with stubbed docker, id, sudo, and systemctl. On the real WSL main-watch runner, /proc identifies WSL, so ensure_docker returns before touching the stubs and the tests fail with empty output.

This overrides is_wsl_host inside that sourced-test harness only, so the tests keep validating the intended Linux branches on every host.

Test plan

  • npm ci --ignore-scripts
  • npx vitest run test/install-preflight.test.ts --testNamePattern "installer Docker bootstrap" --testTimeout 60000

Fixes the WSL-only failures in CI / Platform Vitest Main Watch after #3824.

Summary by CodeRabbit

  • Tests
    • Enhanced testing for Linux Docker bootstrap procedures to ensure comprehensive coverage of installation scenarios across different system configurations.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 8a877db5-b5dd-47f1-8207-7413a3beb678

📥 Commits

Reviewing files that changed from the base of the PR and between 11b1937 and f0ae98e.

📒 Files selected for processing (1)
  • test/install-preflight.test.ts
📝 Walkthrough

Walkthrough

The test harness for Linux Docker bootstrap now injects an is_wsl_host() override to force non-WSL behavior during ensure_docker execution, preventing WSL-specific skip logic from being triggered.

Changes

Docker Bootstrap Test WSL Override

Layer / File(s) Summary
WSL detection override in Docker bootstrap tests
test/install-preflight.test.ts		 Added a function override that forces is_wsl_host() to return non-WSL (return code 1) inside the bash bootstrap snippet, ensuring the Linux-specific Docker bootstrap path is exercised during testing.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#3824: The main PR's test harness changes in test/install-preflight.test.ts force ensure_docker to run as a non-WSL host, directly aligning with the retrieved PR's scripts/install.sh change to use is_wsl_host inside ensure_docker for WSL-specific Docker skipping.

Suggested labels

fix, CI/CD, Platform: Windows/WSL

Suggested reviewers

  • ericksoa
  • cv

Poem

🐰 A test harness tweak with a WSL fate,
Forces non-Unix when the bash tests await,
Linux paths tested, no skippy surprise,
Docker bootstrap shines through honest eyes! 🐳

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: stabilizing Docker bootstrap tests on WSL by forcing non-WSL behavior in the test harness.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.

Comment @coderabbitai help to get the list of available commands and usage tips.

@wscurran

wscurran commented May 20, 2026

Copy link
Copy Markdown
Contributor

✨ Related open PRs:

@wscurran wscurran added CI/CD and removed CI/CD labels May 20, 2026
@jyaunches jyaunches added the v0.0.47 Release target label May 20, 2026
ericksoa
ericksoa approved these changes May 20, 2026
View reviewed changes

@ericksoa ericksoa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved after adversarial review of the test-only installer harness change. Verified the override is scoped to the sourced test subprocess, production installer behavior is unchanged, GitHub checks are green, and local focused plus full install-preflight Vitest coverage passed.

@ericksoa ericksoa merged commit fa6dd4a into NVIDIA:main May 20, 2026
29 checks passed
@wscurran wscurran added area: ci CI workflows, checks, release automation, or GitHub Actions area: packaging Packages, images, registries, installers, or distribution bug-fix PR fixes a bug or regression chore Build, CI, dependency, or tooling maintenance platform: container Affects Docker, containerd, Podman, or images platform: wsl Affects Windows Subsystem for Linux and removed Platform: Windows/WSL bug-fix PR fixes a bug or regression labels Jun 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ericksoa ericksoa ericksoa approved these changes

Assignees

No one assigned

Labels

area: ci CI workflows, checks, release automation, or GitHub Actions area: packaging Packages, images, registries, installers, or distribution chore Build, CI, dependency, or tooling maintenance platform: container Affects Docker, containerd, Podman, or images platform: wsl Affects Windows Subsystem for Linux v0.0.47 Release target

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jyaunches @wscurran @ericksoa