fix(test): move installer integration tests to CI-only vitest project by ericksoa · Pull Request #2435 · NVIDIA/NemoClaw

ericksoa · 2026-04-24T12:33:37Z

Summary

Tests in install-preflight.test.ts and install-openshell-version-check.test.ts flake persistently under the default 5s vitest timeout. Each test spawns bash install.sh — these are integration tests, not unit tests, and belong in CI.

Changes

Set { timeout: 15_000 } on the describe block in both test files (keeps them passing when run explicitly)
Add a new installer-integration vitest project enabled only by CI=1 or NEMOCLAW_RUN_INSTALLER_TESTS=1
Exclude the two files from the cli project so pre-push hooks skip them

Run explicitly: npx vitest run --project installer-integration

Type of Change

Code change (feature, bug fix, or refactor)

Verification

npx prek run --all-files passes
Both test files pass: 65/65 tests
No secrets, API keys, or credentials committed

Signed-off-by: Aaron Erickson aerickson@nvidia.com

Summary by CodeRabbit

Tests
- Added explicit 15s timeouts to installer test suites to reduce flaky runs.
- Moved installer-specific tests into a separate, CI-gated test project so they run only in CI or when explicitly enabled, keeping normal test runs lean.

Tests that spawn `bash install.sh` or `bash install-openshell.sh` flake under the default 5s vitest timeout on loaded hosts. Set describe-level timeout to 15s for both test suites. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

coderabbitai · 2026-04-24T12:33:48Z

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 8f609408-7b0a-485b-b9d5-4a633dff9c0d

📥 Commits

Reviewing files that changed from the base of the PR and between 6cf5fd6 and 29902ec.

📒 Files selected for processing (1)

test/install-preflight.test.ts

📝 Walkthrough

Walkthrough

Two test suites set explicit 15s Vitest timeouts; Vitest config adds a gated installer-integration project that runs those tests only in CI or when NEMOCLAW_RUN_INSTALLER_TESTS=1, and excludes them from the cli project.

Changes

Cohort / File(s)	Summary
Test Timeout Configuration `test/install-openshell-version-check.test.ts`, `test/install-preflight.test.ts`	Added suite-level Vitest option `{ timeout: 15_000 }` to `describe` calls to set a 15s timeout; no test logic or assertions changed.
Vitest Projects / CI gating `vitest.config.ts`	Added a new `installer-integration` project that runs the two installer tests and is gated to run only when `CI` is truthy or `NEMOCLAW_RUN_INSTALLER_TESTS=1`; those test paths were added to `cli` project's `exclude` list.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 I nudged the timers, soft and neat,

Two sleepy suites now rest a beat,
A gated runner wakes for CI's call,
Else quietly they slumber, one and all,
Tiny hops, the tests stand tall.

🚥 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 accurately describes the main change: moving installer integration tests from the default project to a CI-only vitest project configuration.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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 docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/flaky-install-preflight-timeouts

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

These are integration tests that spawn real bash processes — they belong in CI, not in the pre-push hook. Add a new `installer-integration` vitest project enabled only when CI=1 or NEMOCLAW_RUN_INSTALLER_TESTS=1. Exclude the two files from the `cli` project so pre-push hooks skip them. Run explicitly: npx vitest run --project installer-integration Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@vitest.config.ts`:
- Line 32: The enabled flag currently uses "!!process.env.CI ||
!!process.env.NEMOCLAW_RUN_INSTALLER_TESTS" which treats any non-empty string
(e.g. "0" or "false") as true; change it to explicitly parse the env values
(e.g. check process.env.CI and process.env.NEMOCLAW_RUN_INSTALLER_TESTS against
'1' or 'true' case-insensitively, or use a small helper like isTruthyEnv(var)
that returns (val && ['1','true'].includes(val.toLowerCase())) ) and use that
helper or explicit comparisons in the enabled assignment so only explicit
true-ish values enable installer tests.

🪄 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: de1898b3-e142-45e0-8ae6-5c03cd8a7c3f

📥 Commits

Reviewing files that changed from the base of the PR and between cbb93e4 and 03d9217.

📒 Files selected for processing (3)

test/install-openshell-version-check.test.ts
test/install-preflight.test.ts
vitest.config.ts

✅ Files skipped from review due to trivial changes (2)

test/install-preflight.test.ts
test/install-openshell-version-check.test.ts

CI=0 or CI=false should not enable the installer-integration project. Check against '1' and 'true' explicitly.

…NVIDIA#2435) ## Summary Tests in `install-preflight.test.ts` and `install-openshell-version-check.test.ts` flake persistently under the default 5s vitest timeout. Each test spawns `bash install.sh` — these are integration tests, not unit tests, and belong in CI. ## Changes - Set `{ timeout: 15_000 }` on the `describe` block in both test files (keeps them passing when run explicitly) - Add a new `installer-integration` vitest project enabled only by `CI=1` or `NEMOCLAW_RUN_INSTALLER_TESTS=1` - Exclude the two files from the `cli` project so pre-push hooks skip them Run explicitly: `npx vitest run --project installer-integration` ## Type of Change - [x] Code change (feature, bug fix, or refactor) ## Verification - [x] `npx prek run --all-files` passes - [x] Both test files pass: 65/65 tests - [x] No secrets, API keys, or credentials committed --- Signed-off-by: Aaron Erickson <aerickson@nvidia.com>  ## Summary by CodeRabbit * **Tests** * Added explicit 15s timeouts to installer test suites to reduce flaky runs. * Moved installer-specific tests into a separate CI-gated project so they run only in CI or when explicitly enabled, keeping regular test runs lean.  --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

coderabbitai Bot reviewed Apr 24, 2026

View reviewed changes

Comment thread vitest.config.ts Outdated

fix(test): use explicit truthy check for CI env in vitest config

105da6c

CI=0 or CI=false should not enable the installer-integration project. Check against '1' and 'true' explicitly.

ericksoa changed the title ~~fix(test): increase timeout for flaky installer spawn tests~~ fix(test): move installer integration tests to CI-only vitest project Apr 24, 2026

Merge branch 'main' into fix/flaky-install-preflight-timeouts

6cf5fd6

cv approved these changes Apr 24, 2026

View reviewed changes

Merge branch 'main' into fix/flaky-install-preflight-timeouts

29902ec

ericksoa merged commit 79c8e2a into main Apr 25, 2026
11 of 12 checks passed

wscurran added the bug-fix PR fixes a bug or regression label Jun 8, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(test): move installer integration tests to CI-only vitest project#2435

fix(test): move installer integration tests to CI-only vitest project#2435
ericksoa merged 5 commits into
mainfrom
fix/flaky-install-preflight-timeouts

ericksoa commented Apr 24, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Apr 24, 2026 •

edited

Loading

Review failed

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

ericksoa commented Apr 24, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Type of Change

Verification

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review failed

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ericksoa commented Apr 24, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 24, 2026 •

edited

Loading