Repository navigation
test(vitest): isolate mock and environment state - #6700
Conversation
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage remains at 96%, unchanged from the TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most impacted files.
Updated |
E2E Advisor RecommendationRequired E2E: None Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
|
PR Review Advisor — No blocking findingsMerge posture: No blocking advisor findings This is an automated review. Required findings need action before merge. Warnings and optional suggestions do not require a response or follow-up. A human maintainer makes the final merge decision. |
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary CI now suppresses passed-test logs while preserving failure logs and GitHub annotations, and ordinary local runs let Vitest select its environment-aware reporter. Focused changed/watch commands and opt-in seeded-shuffle and async-leak diagnostics shorten developer feedback without changing deterministic coverage lanes. This is PR 6 in the #6692 stack and is based on #6700. ## Related Issue Part of #6692 ## Changes - Remove the explicit root reporter override so Vitest selects the local reporter and adds its GitHub Actions reporter, while `resolveVitestFeedback` keeps CI on `silent: "passed-only"` and preserves the existing skipped-test suppression. - Add focused `test:changed` and `test:watch` commands for the `cli`, `plugin`, and `e2e-support` projects. - Add opt-in `test:shuffle` and `test:diagnose:leaks` commands with coverage disabled; shuffle accepts Vitest's printed seed for replay, and leak diagnostics retain both the default and hanging-process reporters. - Register exact root watch triggers for current YAML, Python, shell, generated, manifest, and workflow inputs that are read outside Vite's import graph. `test/helpers/vitest-watch-triggers.ts` is required by the focused watch command, and `test/vitest-watch-triggers.test.ts` protects every concrete mapping, all 31 current shared-workflow consumers, path existence, Windows normalization, and unrelated-file exclusions. - Update `AGENTS.md` and `CONTRIBUTING.md` with the focused commands, seed replay, leak exit-code caveat, and local/CI reporter behavior. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: user-facing NemoClaw behavior is unchanged; the contributor-facing Vitest commands and feedback contract are documented in `AGENTS.md` and `CONTRIBUTING.md`. - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — 9 focused contract tests passed; real changed, watch, seeded-shuffle, and async-leak command smokes passed; CLI type-checking and the source-shape/title/test-size gates passed. - [x] Applicable broad gate passed — `npm test`: 1,442 files passed and 3 expected files skipped; 16,473 tests passed and 40 expected opt-in/platform tests skipped. `npm run test:fast` also passed 927 files and 9,876 tests with 1 expected skip. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) — passed with zero errors; Fern reported two unrelated baseline warnings. - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added focused test commands for changed tests, watch mode, shuffled execution, and async leak diagnosis. * Added watch-mode mappings to automatically run related tests when supported files change. * Added reproducible shuffle diagnostics and improved CI test output behavior. * **Documentation** * Expanded contributor guidance with examples and recommendations for using the new testing workflows. * **Tests** * Added coverage validating command options, watch triggers, path handling, and CI feedback behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.com>
## Summary Vitest project validation now compares every filesystem test candidate with Vitest's complete eight-project roster, rejecting omissions, overlaps, wrong routing, unexpected files, and unexpected project names. The shared credential-free E2E workflow now applies Vitest's native e2e/credential-free tag filter in addition to the existing safe matrix scanner. The dashboard-port regression fixture also uses a coherent fake OpenShell 0.0.72 component set and an OS-assigned gateway port so the repo-wide gate stays hermetic. This is the final entry in the ordered #6692 stack after #6693, #6696, #6697, #6699, #6700, #6701, #6702, and #6705 landed. ## Related Issue Closes #6692 ## Changes - Discover broad test/spec JavaScript and TypeScript candidates under src, test, and nemoclaw/src, including nested dist, coverage, and .venv paths that Vitest would see. - Compare candidate routing with Vitest file listings and the complete roster from list-tags JSON, including empty and opt-in projects. - Produce sorted diagnostics for zero membership, overlap, wrong project, unsupported candidates, unexpected listed files, and missing or extra projects. - Invoke Vitest portably through process.execPath and node_modules/vitest/vitest.mjs. - Require the exact native credential-free tag filter in the shared E2E workflow contract while retaining the TypeScript scanner's path, project, ID, and declaration checks. - Keep the dashboard-port exhaustion regression independent of port 8080, developer OpenShell overrides, channel selection, and network installation. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: these are contributor/test-harness safeguards; AGENTS.md now states the exhaustive project-membership contract, the E2E README already documents the native module tag, and the required documentation-writer review found no user-facing change. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: independent review covered the shared credential-free workflow and onboarding regression fixture; it confirmed the filter preserves the existing scanner and the fixture's child-only environment cannot reach a real OpenShell install. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification - [x] PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub - [x] Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — 46 integration tests passed across the boundary and dashboard-port files; 4 E2E-support workflow tests passed; project membership is exact for 1,537 candidates across 8 projects; CLI type-check, title style, Biome, and diff checks passed. The fixture also passed with hostile inherited OpenShell paths and dev-channel selection. - [x] Applicable broad gate passed — npm test passed 1,456 files / 16,515 tests before the final focused hardening; npm run check passed the repo-wide structural, CLI coverage, and plugin coverage gates, followed by focused reruns and normal hooks after the final child-environment pin. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] npm run docs builds without warnings (doc changes only) - [ ] Doc pages follow the style guide (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Tests** - Credential-free end-to-end test runs now execute only tests matching the credential-free tag. - Vitest project validation now checks for missing, overlapping, unsupported, wrong, zero-membership, and unexpected test-to-project assignments, plus roster mismatches. - Expanded boundary tests cover test candidate discovery, expected project mapping, Vitest invocation resolution, and listing/roster parsing. - Improved onboarding “dashboard port exhaustion” assertions for more hermetic, reliable verification. - **Documentation** - Updated testing guidance to require Vitest project globs be disjoint and exhaustive, enforced by the projects check. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary Deterministic Vitest projects now restore spies, clear mock call state, and undo stubbed environment variables and globals before every test while leaving live E2E projects unchanged. This is PR 5 in the NVIDIA#6692 stack and is based on NVIDIA#6699. ## Related Issue Part of NVIDIA#6692 ## Changes - Enable `clearMocks`, `restoreMocks`, `unstubEnvs`, and `unstubGlobals` in the `cli`, `integration`, `installer-integration`, `package-contract`, `plugin`, and `e2e-support` projects without enabling `mockReset`. - Keep the two live E2E projects outside automatic cleanup and add a contract that guards both project-local settings and inherited root settings. - Move the exposed `Date.now` and `console.log` spies into per-test setup, and make the non-SSH dashboard test independent of the developer's ambient SSH environment. - Share the four cleanup settings across the five root-owned deterministic projects. The plugin keeps explicit settings to preserve its package root; `test/vitest-state-isolation.test.ts` protects parity and live exclusions. - Document the test-state ownership contract for contributors, including the narrow import-time stub exception. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: user-facing NemoClaw behavior is unchanged; the contributor test contract is documented in `AGENTS.md` and `CONTRIBUTING.md`. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: independent final review found no findings; only onboarding test files changed, not production onboarding behavior. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `68` targeted tests passed across CLI, plugin, and integration projects; plugin and CLI typechecks passed; the dashboard test also passed with an ambient `SSH_CONNECTION`. - [x] Applicable broad gate passed — `npm test`: 1,440 files and 16,461 tests passed; 40 expected opt-in/platform tests skipped. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) — passed with zero errors; Fern reported two unrelated baseline warnings for unauthenticated redirect checking and existing light-mode contrast. - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Carlos Villela <cvillela@nvidia.com> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary CI now suppresses passed-test logs while preserving failure logs and GitHub annotations, and ordinary local runs let Vitest select its environment-aware reporter. Focused changed/watch commands and opt-in seeded-shuffle and async-leak diagnostics shorten developer feedback without changing deterministic coverage lanes. This is PR 6 in the NVIDIA#6692 stack and is based on NVIDIA#6700. ## Related Issue Part of NVIDIA#6692 ## Changes - Remove the explicit root reporter override so Vitest selects the local reporter and adds its GitHub Actions reporter, while `resolveVitestFeedback` keeps CI on `silent: "passed-only"` and preserves the existing skipped-test suppression. - Add focused `test:changed` and `test:watch` commands for the `cli`, `plugin`, and `e2e-support` projects. - Add opt-in `test:shuffle` and `test:diagnose:leaks` commands with coverage disabled; shuffle accepts Vitest's printed seed for replay, and leak diagnostics retain both the default and hanging-process reporters. - Register exact root watch triggers for current YAML, Python, shell, generated, manifest, and workflow inputs that are read outside Vite's import graph. `test/helpers/vitest-watch-triggers.ts` is required by the focused watch command, and `test/vitest-watch-triggers.test.ts` protects every concrete mapping, all 31 current shared-workflow consumers, path existence, Windows normalization, and unrelated-file exclusions. - Update `AGENTS.md` and `CONTRIBUTING.md` with the focused commands, seed replay, leak exit-code caveat, and local/CI reporter behavior. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: user-facing NemoClaw behavior is unchanged; the contributor-facing Vitest commands and feedback contract are documented in `AGENTS.md` and `CONTRIBUTING.md`. - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — 9 focused contract tests passed; real changed, watch, seeded-shuffle, and async-leak command smokes passed; CLI type-checking and the source-shape/title/test-size gates passed. - [x] Applicable broad gate passed — `npm test`: 1,442 files passed and 3 expected files skipped; 16,473 tests passed and 40 expected opt-in/platform tests skipped. `npm run test:fast` also passed 927 files and 9,876 tests with 1 expected skip. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) — passed with zero errors; Fern reported two unrelated baseline warnings. - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added focused test commands for changed tests, watch mode, shuffled execution, and async leak diagnosis. * Added watch-mode mappings to automatically run related tests when supported files change. * Added reproducible shuffle diagnostics and improved CI test output behavior. * **Documentation** * Expanded contributor guidance with examples and recommendations for using the new testing workflows. * **Tests** * Added coverage validating command options, watch triggers, path handling, and CI feedback behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.com>
…A#6707) ## Summary Vitest project validation now compares every filesystem test candidate with Vitest's complete eight-project roster, rejecting omissions, overlaps, wrong routing, unexpected files, and unexpected project names. The shared credential-free E2E workflow now applies Vitest's native e2e/credential-free tag filter in addition to the existing safe matrix scanner. The dashboard-port regression fixture also uses a coherent fake OpenShell 0.0.72 component set and an OS-assigned gateway port so the repo-wide gate stays hermetic. This is the final entry in the ordered NVIDIA#6692 stack after NVIDIA#6693, NVIDIA#6696, NVIDIA#6697, NVIDIA#6699, NVIDIA#6700, NVIDIA#6701, NVIDIA#6702, and NVIDIA#6705 landed. ## Related Issue Closes NVIDIA#6692 ## Changes - Discover broad test/spec JavaScript and TypeScript candidates under src, test, and nemoclaw/src, including nested dist, coverage, and .venv paths that Vitest would see. - Compare candidate routing with Vitest file listings and the complete roster from list-tags JSON, including empty and opt-in projects. - Produce sorted diagnostics for zero membership, overlap, wrong project, unsupported candidates, unexpected listed files, and missing or extra projects. - Invoke Vitest portably through process.execPath and node_modules/vitest/vitest.mjs. - Require the exact native credential-free tag filter in the shared E2E workflow contract while retaining the TypeScript scanner's path, project, ID, and declaration checks. - Keep the dashboard-port exhaustion regression independent of port 8080, developer OpenShell overrides, channel selection, and network installation. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: these are contributor/test-harness safeguards; AGENTS.md now states the exhaustive project-membership contract, the E2E README already documents the native module tag, and the required documentation-writer review found no user-facing change. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: independent review covered the shared credential-free workflow and onboarding regression fixture; it confirmed the filter preserves the existing scanner and the fixture's child-only environment cannot reach a real OpenShell install. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification - [x] PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub - [x] Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — 46 integration tests passed across the boundary and dashboard-port files; 4 E2E-support workflow tests passed; project membership is exact for 1,537 candidates across 8 projects; CLI type-check, title style, Biome, and diff checks passed. The fixture also passed with hostile inherited OpenShell paths and dev-channel selection. - [x] Applicable broad gate passed — npm test passed 1,456 files / 16,515 tests before the final focused hardening; npm run check passed the repo-wide structural, CLI coverage, and plugin coverage gates, followed by focused reruns and normal hooks after the final child-environment pin. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] npm run docs builds without warnings (doc changes only) - [ ] Doc pages follow the style guide (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Tests** - Credential-free end-to-end test runs now execute only tests matching the credential-free tag. - Vitest project validation now checks for missing, overlapping, unsupported, wrong, zero-membership, and unexpected test-to-project assignments, plus roster mismatches. - Expanded boundary tests cover test candidate discovery, expected project mapping, Vitest invocation resolution, and listing/roster parsing. - Improved onboarding “dashboard port exhaustion” assertions for more hermetic, reliable verification. - **Documentation** - Updated testing guidance to require Vitest project globs be disjoint and exhaustive, enforced by the projects check. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Summary
Deterministic Vitest projects now restore spies, clear mock call state, and undo stubbed environment variables and globals before every test while leaving live E2E projects unchanged. This is PR 5 in the #6692 stack and is based on #6699.
Related Issue
Part of #6692
Changes
clearMocks,restoreMocks,unstubEnvs, andunstubGlobalsin thecli,integration,installer-integration,package-contract,plugin, ande2e-supportprojects without enablingmockReset.Date.nowandconsole.logspies into per-test setup, and make the non-SSH dashboard test independent of the developer's ambient SSH environment.test/vitest-state-isolation.test.tsprotects parity and live exclusions.Type of Change
Quality Gates
AGENTS.mdandCONTRIBUTING.md.Verification
Verifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailable68targeted tests passed across CLI, plugin, and integration projects; plugin and CLI typechecks passed; the dashboard test also passed with an ambientSSH_CONNECTION.npm test: 1,440 files and 16,461 tests passed; 40 expected opt-in/platform tests skipped.npm run docsbuilds without warnings (doc changes only) — passed with zero errors; Fern reported two unrelated baseline warnings for unauthenticated redirect checking and existing light-mode contrast.Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit
Bug Fixes
Documentation