Skip to content

SCANNPM-130 Refactor integration tests to use tsx and Node test runner#383

Merged
vdiez merged 1 commit intomasterfrom
refactor/integration-tests-tsx
Jan 9, 2026
Merged

SCANNPM-130 Refactor integration tests to use tsx and Node test runner#383
vdiez merged 1 commit intomasterfrom
refactor/integration-tests-tsx

Conversation

@vdiez
Copy link
Contributor

@vdiez vdiez commented Jan 8, 2026

Summary

  • Migrate integration tests from Jest to Node's native test runner (node:test)
  • Use tsx to run TypeScript directly without compilation step
  • Move orchestrator from tools/orchestrator/ to test/integration/orchestrator/
  • Convert scanner.test.js to scanner.test.ts with ESM imports
  • Add CLI test alongside existing API test
  • Add tsconfig.json for both unit and integration tests
  • Move test/setup.ts to test/unit/setup.ts
  • Simplify CI workflow by removing orchestrator build step
  • Add CLAUDE.md with project documentation

Test plan

  • Unit tests pass (npm test)
  • Integration tests pass (npm run test-integration)
  • Build succeeds (npm run build)
  • CI workflow passes

🤖 Generated with Claude Code

- Migrate integration tests from Jest to Node's native test runner
- Use tsx to run TypeScript directly without compilation
- Move orchestrator from tools/orchestrator to test/integration/orchestrator
- Convert scanner.test.js to scanner.test.ts with ESM imports
- Add CLI test alongside existing API test
- Add tsconfig.json for both unit and integration tests
- Move test/setup.ts to test/unit/setup.ts
- Simplify CI workflow by removing orchestrator build step
- Add CLAUDE.md with project documentation

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@vdiez vdiez requested a review from zglicz January 8, 2026 16:11
@hashicorp-vault-sonar-prod hashicorp-vault-sonar-prod bot changed the title Refactor integration tests to use tsx and Node test runner SCANNPM-130 Refactor integration tests to use tsx and Node test runner Jan 8, 2026
@hashicorp-vault-sonar-prod
Copy link

hashicorp-vault-sonar-prod bot commented Jan 8, 2026

SCANNPM-130

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 8, 2026

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: Restructure integration test setup and move orchestrator tools into test directory

Review Focus: The PR migrates integration tests from Jest to Node's native test runner with ESM support, consolidates orchestrator tools within the test directory, and removes duplication in the CI workflow. Key changes include:

  • Migration to Node test runner with TypeScript via tsx
  • Orchestrator tools moved from tools/ to test/integration/
  • Integration test setup streamlined in CI workflow
  • Added CLAUDE.md documentation file

Start review at: test/integration/scanner.test.ts. This is the core integration test file that demonstrates the new test structure and validates both API and CLI invocation patterns.

Model B:

Summary: Restructures the integration test infrastructure by relocating the orchestrator from tools/ to test/integration/, converting it to ESM, and modernizing the test runner from Jest to Node's native test framework with tsx.

Review Focus:

  • The CI/CD workflow change moving npm pack earlier in the build pipeline—verify this doesn't break the integration test setup
  • ESM conversion in orchestrator files, particularly __dirname/__filename polyfills and import statement changes
  • Integration test migration from Jest to Node's native test runner (scanner.test.js → scanner.test.ts)
  • The new test-integration script now runs npm install before tests—ensure lock file is committed and accurate

Start review at: .github/workflows/build.yml. The workflow restructuring is the linchpin of this PR—moving npm pack from the "Setup integration test" step into the build step, then removing that entire setup step. This affects test execution flow and should be validated against the relocated orchestrator paths and the updated integration test imports.

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

Quality Gate Failed Quality Gate failed

Failed conditions
1 New issue

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

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.

Great overall, small qs

Comment on lines -93 to -100
- name: Setup integration test
run: |
(cd build && npm pack)
cp build/sonar-scan-SNAPSHOT.tgz test/integration
(cd test/integration && npm install --no-save sonar-scan-SNAPSHOT.tgz)
(cd tools/orchestrator && npm run build)
shell: bash

Copy link
Contributor

Choose a reason for hiding this comment

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

nice

Comment on lines +21 to +26
import * as https from 'node:https';
import * as fs from 'node:fs';
import { execSync } from 'node:child_process';
import * as path from 'node:path';
import * as mkdirp from 'mkdirp';
import * as os from 'os';
import * as os from 'node:os';
Copy link
Contributor

Choose a reason for hiding this comment

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

we don't have this as a rule already? or was it accepted?

"dependencies": {
"axios": "1.13.2",
"mkdirp": "3.0.1",
"@sonar/scan": "file:../../build/sonar-scan-SNAPSHOT.tgz"
Copy link
Contributor

Choose a reason for hiding this comment

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

sooo much better!

return path.join(__dirname.replace(/\\+/g, '/'), '/fixtures/fake_project_for_integration/src');
}

async function assertOneIssueAtLine21(projectKey: string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

thats a bit specific function name

Comment on lines +94 to +103
execSync(
`npx sonar ` +
`-Dsonar.host.url=${SONAR_HOST_URL} ` +
`-Dsonar.token=${token} ` +
`-Dsonar.projectKey=${projectKey} ` +
`-Dsonar.projectName=${projectKey} ` +
`-Dsonar.log.level=DEBUG ` +
`-Dsonar.sources=${getSourcesPath()}`,
{ stdio: 'inherit' },
);
Copy link
Contributor

Choose a reason for hiding this comment

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

hm... 1 worry I could have here that the developer could have @sonar/scan installed locally and it wouldn't be the one picked up by npx sonar . Can we add a verify so that this actually runs the locally installed package?


## Project Overview

`@sonar/scan` is an NPM module that triggers SonarQube Server and SonarCloud analyses on JavaScript codebases without requiring specific tools or Java runtime installation. The scanner detects server capabilities and either uses JRE provisioning (preferred, SonarQube 10.6+) or falls back to native sonar-scanner-cli.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would remove any mention of versions in the this md file

Comment on lines +155 to +161
- `getLatestSonarQube()` - Download/cache SonarQube
- `startAndReady(sqPath, maxWaitMs)` - Start and wait until operational
- `stop(sqPath)` - Stop SonarQube instance
- `generateToken()` - Generate GLOBAL_ANALYSIS_TOKEN
- `createProject()` - Create project with random key
- `waitForAnalysisFinished(maxWaitMs)` - Poll until analysis queue empty
- `getIssues(projectKey)` - Fetch detected issues
Copy link
Contributor

Choose a reason for hiding this comment

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

is that a bit too much specifics of the implementation?

"tools/orchestrator": {
"entry": ["scripts/*"],
"ignore": ["dist/**/*"]
"test/integration": {
Copy link
Contributor

Choose a reason for hiding this comment

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

should unit tests be also included here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i will once i remove jest

"build": "npm run license && tsc && tsx scripts/generate-package-json.ts",
"test": "node_modules/.bin/jest --coverage",
"test-integration": "cd test/integration && npm test",
"test-integration": "cd test/integration && npm install && npm test",
Copy link
Contributor

Choose a reason for hiding this comment

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

does this command change where the user is? perhaps it should be in parenthesis?

@vdiez
Copy link
Contributor Author

vdiez commented Jan 9, 2026

I will handle the comments in the next PR, @zglicz

@vdiez vdiez merged commit 0a88f6e into master Jan 9, 2026
9 of 10 checks passed
@vdiez vdiez deleted the refactor/integration-tests-tsx branch January 9, 2026 12:01
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