test(cli): stabilize coverage dist sourcemaps#2961
Conversation
This reverts commit 4ebeae4.
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughA new source map validation feature is introduced that scans the dist directory for .js.map files, verifies that all referenced sources exist, and integrates into the pre-commit test pipeline via npm scripts and configuration updates. ChangesSource Map Validation Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Signed-off-by: Carlos Villela <[email protected]>
## Summary Remove broad coverage ignores from small runtime utility modules that already have or can have direct tests. ## Stack Navigation - Position: 45 of 60 - Previous PR: [#2943 — test(cli): cover global action facade](#2943) - Next PR: [#2961 — test(cli): stabilize coverage dist sourcemaps](#2961) ## Changes - Removed the file-level V8 ignore from the oclif runner, which is already directly tested. - Removed the file-level V8 ignore from terminal style constants. - Added a small terminal style export test to keep the presentation constants covered. ## 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) ## Verification - [x] `npx prek run --all-files` passes - [x] `npm test` passes - [x] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [ ] Docs updated for user-facing behavior changes - [ ] `make docs` builds without warnings (doc changes only) - [ ] 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 <[email protected]> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Extended CLI actions with optional runtime hook configuration for sandbox upgrades, enabling customizable upgrade handling. * **Chores** * Cleaned up obsolete code annotations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <[email protected]> Co-authored-by: Prekshi Vyas <[email protected]>
…abilize-cli-coverage-dist
Signed-off-by: Carlos Villela <[email protected]>
prekshivyas
left a comment
There was a problem hiding this comment.
LGTM. Test infrastructure / build hygiene PR — 5 files / +120 / -2, no src/ touched.
clean:cli+dist:sourcemaps:checknpm scripts added.- New
scripts/check-dist-sourcemaps.tswalks.js.mapfiles and asserts everysourcesentry resolves on disk; correctly skipsnode:/webpack:///node_modules/absolute paths. - Pre-commit
test-clihook now runsclean:cli && build:cli && dist:sourcemaps:checkbefore vitest coverage — defensive against staledist/polluting coverage. - Coverage function threshold rebaselined 32 → 31.8 to reflect the real value once stale dist files (which had been counting deleted code as "covered") are eliminated. Rationale clearly explained in the PR description.
- +35 lines of fixture-based test for the sourcemap checker.
CI: pr.yaml mostly green (lint/dco/check-hash/legacy-path-guard/changes PASS); macos-e2e/wsl-e2e/checks + pr-self-hosted builds still in flight at review time. No failures.
## Summary Extract pure Docker image parsing and orphan-selection logic from the subprocess-heavy maintenance action module. ## Stack Navigation - Position: 47 of 60 - Previous PR: [#2961 — test(cli): stabilize coverage dist sourcemaps](#2961) - Next PR: [#2963 — refactor(cli): extract upgrade sandbox helpers](#2963) ## Changes - Added `maintenance-image-helpers.ts` for Docker image row parsing and registered image-tag comparison. - Updated `garbageCollectImages` to use the extracted helpers while keeping Docker/prompt orchestration in `maintenance-actions.ts`. - Added direct helper coverage for image parsing, registered tag collection, and orphan detection. ## 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) ## Verification - [x] `npx prek run --all-files` passes - [x] `npm test` passes - [x] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [ ] Docs updated for user-facing behavior changes - [ ] `make docs` builds without warnings (doc changes only) - [ ] 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 <[email protected]> --------- Signed-off-by: Carlos Villela <[email protected]>
Summary
Stabilize CLI coverage runs by cleaning compiled CLI output before coverage builds and validating that generated JavaScript sourcemaps point at existing sources. This makes coverage use a clean
dist/baseline instead of stale generated files.Stack Navigation
Changes
npm run clean:clifor removing staledist/artifacts without changing the normalbuild:cliordering.scripts/check-dist-sourcemaps.tsandnpm run dist:sourcemaps:checkto fail fast on stale generated sourcemaps.dist/, rebuild, and validate sourcemaps before running coverage.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)Signed-off-by: Carlos Villela [email protected]
Summary by CodeRabbit
Chores
Tests