Skip to content

SCANNPM-131 Migrate unit tests to Node test runner#386

Merged
vdiez merged 29 commits intomasterfrom
scannpm-131-migrate-unit-tests-to-node-test-runner
Jan 19, 2026
Merged

SCANNPM-131 Migrate unit tests to Node test runner#386
vdiez merged 29 commits intomasterfrom
scannpm-131-migrate-unit-tests-to-node-test-runner

Conversation

@vdiez
Copy link
Contributor

@vdiez vdiez commented Jan 9, 2026

Summary

  • Replace Jest with Node's native test runner and tsx
  • Remove jest, ts-jest, @types/jest, and jest-sonar-reporter dependencies
  • Use dependency injection pattern for testability instead of experimental module mocking
  • Add src/deps.ts for shared dependency interfaces
  • Update all test files to use node:test and node:assert
  • Use node: prefix for all Node.js built-in imports
  • Update CLAUDE.md with current conventions

Test plan

  • All 122 unit tests pass with npm test
  • Build succeeds with npm run build
  • CI pipeline passes

🤖 Generated with Claude Code

@hashicorp-vault-sonar-prod
Copy link

SCANNPM-131

@vdiez vdiez force-pushed the scannpm-131-migrate-unit-tests-to-node-test-runner branch 3 times, most recently from 1eef4cd to 26d9c48 Compare January 9, 2026 20:12
@vdiez vdiez requested a review from zglicz January 13, 2026 14:20
Copy link
Contributor

@zglicz zglicz left a comment

Choose a reason for hiding this comment

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

Please rework the business logic, so that not all of the methods need to accept the mocks/DI code. This will make it much, much cleaner.

@vdiez
Copy link
Contributor Author

vdiez commented Jan 19, 2026

Changes in this session

Refactor: Centralized Dependency Injection

Addressed the PR feedback to clean up the dependency injection approach. Instead of passing deps parameters to every function, we now use a centralized dependency container pattern:

New files created:

  • src/deps.ts - Centralized Dependencies interface with getDeps(), setDeps(), and resetDeps() functions
  • test/unit/test-helpers.ts - Reusable mock factory functions (createMockFsDeps, createMockHttpDeps, createMockProcessDeps, createMockChildProcess, etc.)

Pattern change:

// Before: deps parameter in every function
export async function fetchJRE(properties: Props, deps: JavaDeps = {}) { ... }

// After: clean function signatures, deps accessed via getDeps()
export async function fetchJRE(properties: Props): Promise<string> {
  const { fs, http } = getDeps();
  // ...
}

Test pattern:

beforeEach(() => {
  setDeps({ http: createMockHttpDeps({ fetch: mockFetch }) });
});

afterEach(() => {
  resetDeps();
});

Removed sinon dependency

Replaced all sinon stubs with native node:test mocks:

  • scan.test.ts - Replaced sinon.stub(process, 'cwd') with setDeps({ process: createMockProcessDeps({ cwd: () => __dirname }) })
  • properties.test.ts - Refactored FakeProjectMock class to use setDeps() instead of sinon stubs
  • scanner-engine.test.ts - Removed sinon.stub(process.stdout, 'write') (was only suppressing output, test validates via mockLog)

Fixed TypeScript errors

  • Added proper Mock<T> typing from node:test when accessing mock call arguments
  • Updated test helper return types from Partial<Dependencies['X']> to full Dependencies['X']

Stats

  • 18 files changed: 949 insertions, 1296 deletions (net -347 lines)
  • All 137 tests pass
  • Coverage maintained: 90.55% statements, 83.56% branches

vdiez and others added 21 commits January 19, 2026 13:31
- Replace Jest with Node's native test runner and tsx
- Remove jest, ts-jest, @types/jest, and jest-sonar-reporter dependencies
- Use dependency injection pattern for testability instead of module mocking
- Add src/deps.ts for shared dependency interfaces
- Update all test files to use node:test and node:assert
- Use node: prefix for all Node.js built-in imports
- Update CLAUDE.md with current conventions

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

# Conflicts:
#	package-lock.json
#	package.json
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The integrity hash changes with every build, causing CI failures.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add nyc and @istanbuljs/nyc-config-typescript dependencies
- Create .nycrc configuration for TypeScript coverage
- Update test script to run with nyc coverage
- Generates lcov report for SonarCloud analysis

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove unused spawn import from scanner-cli.ts
- Remove unused spawn import from scanner-engine.ts
- Use `new Error()` instead of `Error()` in scanner-cli.ts
- Remove unused beforeEach import from file.test.ts

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add test for dumpToFile feature in scanner-engine
- Add test for npm_config_sonar_scanner_ env var conversion
- Add test for CLI debug flag setting verbose mode

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Replace fs-extra with native Node.js fs module throughout the codebase:
- fsExtra.remove() -> fs.promises.rm() with recursive/force options
- fsExtra.ensureDir() -> fs.promises.mkdir() with recursive option
- fsExtra.exists() -> custom function using fs.promises.access()

Move dependency injection interfaces from centralized deps.ts to local
interfaces in each module (FileDeps, ScannerCliFsDeps, etc.), improving
code locality and maintainability.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Update test files to import interfaces from their respective source
modules instead of the deleted deps.ts file. Remove export from
interfaces that are only used internally.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add `type` modifier to imports that are only used for type checking.
This improves tree-shaking and makes it clear which imports are
only used at compile time. Also update CLAUDE.md with this convention.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Move default values for dependency injection from inside function bodies
to parameter destructuring. This makes it clearer which defaults are used
when callers don't provide specific dependencies.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add path traversal validation when extracting tar.gz archives to prevent
malicious archives from writing files outside the target directory.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Remove redundant ignoreDependencies and entry pattern as suggested by knip.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Restored 5 tests that were accidentally omitted during the migration
from Jest to Node's native test runner:
- should download SonarScanner CLI if it does not exist on Unix without arch
- should display SonarScanner CLI output
- should only forward non-scanner env vars to Scanner CLI
- should pass proxy options to scanner
- should pass https proxy options to scanner

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Added path traversal validation for AdmZip extraction to complement
the existing tar.gz protection. All archive entries are now validated
to ensure they extract within the target directory.

Added test for tar.gz path traversal rejection.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Update zip extraction path validation to use path.normalize with
string concatenation as recommended by SonarCloud S6096 rule.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Apply same fix pattern to tar.gz extraction for consistency.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add node-reporter-sonarqube to generate test execution reports
- Update test script to output reports in SonarQube generic format
- Configure ci-analysis.js to send test execution data via
  sonar.testExecutionReportPaths

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The package is used in npm script, not imported in code.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
vdiez and others added 6 commits January 19, 2026 13:35
Remove deprecated shebang and husky.sh sourcing lines.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The test execution report needs to be shared with the analyze job.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- ChildProcessMock.ts: Not used after Jest migration
- mock-jre.tar.gz: Only referenced as string in test data, not read from disk

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Move FakeProjectMock class directly into properties.test.ts since
it's the only file using it. Removes the now-empty mocks folder.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Create src/deps.ts with centralized Dependencies interface and
  getDeps()/setDeps()/resetDeps() functions for dependency injection
- Migrate all source files to use getDeps() instead of function parameters
- Create test/unit/test-helpers.ts with reusable mock factories
- Update all unit tests to use setDeps() in beforeEach and resetDeps()
  in afterEach instead of passing deps as function arguments
- Remove sinon dependency, replacing all stubs with native node:test mocks
- Fix TypeScript errors by using proper Mock<T> typing from node:test

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@vdiez vdiez force-pushed the scannpm-131-migrate-unit-tests-to-node-test-runner branch from cefdf83 to f77eef8 Compare January 19, 2026 12:38
@sonarqubecloud
Copy link

SonarQube reviewer guide

Important

We are currently testing different models for AI Summary.
Please give us your feedback by filling this form.

Model A:

Summary: Migrate test suite from Jest to Node.js native test runner and improve dependency injection architecture.

Review Focus:

  • The new deps.ts module introduces a centralized dependency injection container - verify all dependencies are properly abstracted and mockable
  • Tests now use Node.js native node:test and node:assert APIs instead of Jest - check for correct mock lifecycle management (proper cleanup in afterEach)
  • Environment variable handling in properties tests is complex due to proxy-from-env library accessing global process.env - review the restoreProxyEnvVars() helper
  • The createMockChildProcess helper in test-helpers.ts requires manual event emission in tests - ensure tests properly trigger exit events to avoid hangs

Start review at: src/deps.ts. This is the foundation of the refactoring - it defines the dependency injection pattern that enables testing without Jest. Understanding how dependencies are centralized here (fs, process, http, spawn, etc.) is crucial to reviewing how tests mock these dependencies throughout the codebase.

Model B:

Summary: Migrates the test framework from Jest to Node.js native test runner with comprehensive refactoring.

Review Focus:
This is a major architectural change from Jest to Node's native test runner. Key areas requiring careful review:

  1. Dependency injection system (src/deps.ts) - New centralized DI container replacing scattered mocks
  2. Test helper functions (test/unit/test-helpers.ts) - How mocks are created and used across tests
  3. Test rewrites - Verify mock patterns are consistently applied, particularly around async operations and event emitters
  4. Pre-commit hook simplification (.husky/pre-commit) - Now just runs npm run precommit instead of full husky setup

Start review at: src/deps.ts. This file is the foundation of the new architecture. Understanding the dependency injection pattern is critical to reviewing all subsequent changes, as every test and module now uses this centralized container instead of direct Jest mocks.

Review in SonarQube
See all code changes, issues, and quality metrics in one place.

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
87.9% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@vdiez vdiez requested a review from zglicz January 19, 2026 14:20
@vdiez vdiez merged commit 80efa06 into master Jan 19, 2026
8 checks passed
@vdiez vdiez deleted the scannpm-131-migrate-unit-tests-to-node-test-runner branch January 19, 2026 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants