Skip to content

Commit 2994507

Browse files
committed
refactor(test): improve changed-test-mapper robustness
Implemented improvements from refactoring plan: - Add git error handling with fallback to all tests - Add debug logging via DEBUG_TEST_MAPPER env var - Improve deleted test handling (run all tests if no valid mappings) - Simplify JSDoc per CLAUDE.md standards - Add root path validation - Add missing test file warnings
1 parent 87255e7 commit 2994507

File tree

1 file changed

+53
-9
lines changed

1 file changed

+53
-9
lines changed

scripts/utils/changed-test-mapper.mjs

Lines changed: 53 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,13 @@ import {
1313
import { normalizePath } from '@socketsecurity/lib/path'
1414

1515
const rootPath = path.resolve(process.cwd())
16+
const DEBUG = process.env.DEBUG_TEST_MAPPER === '1'
17+
18+
function debug(message) {
19+
if (DEBUG) {
20+
console.log(`[test-mapper] ${message}`)
21+
}
22+
}
1623

1724
/**
1825
* Core files that require running all tests when changed.
@@ -32,8 +39,7 @@ const CORE_FILES = [
3239

3340
/**
3441
* Map source files to their corresponding test files.
35-
* @param {string} filepath - Path to source file
36-
* @returns {string[]} Array of test file paths
42+
* Returns array of test paths or ['all'] for core files.
3743
*/
3844
function mapSourceToTests(filepath) {
3945
const normalized = normalizePath(filepath)
@@ -56,9 +62,15 @@ function mapSourceToTests(filepath) {
5662

5763
// Check if corresponding test exists
5864
if (existsSync(path.join(rootPath, testFile))) {
65+
debug(`Mapped ${normalized} to ${testFile}`)
5966
return [testFile]
6067
}
6168

69+
// Warn if mapped test file is missing
70+
if (process.env.NODE_ENV !== 'test') {
71+
console.warn(`Warning: Expected test file not found: ${testFile}`)
72+
}
73+
6274
// Special mappings
6375
if (normalized.includes('src/package-url.ts')) {
6476
return ['test/package-url.test.mts', 'test/integration.test.mts']
@@ -79,14 +91,21 @@ function mapSourceToTests(filepath) {
7991

8092
/**
8193
* Get affected test files to run based on changed files.
82-
* @param {Object} options
83-
* @param {boolean} options.staged - Use staged files instead of all changes
84-
* @param {boolean} options.all - Run all tests
85-
* @returns {{tests: string[] | 'all' | null, reason?: string, mode?: string}} Object with test patterns, reason, and mode
94+
* Returns all tests in CI environment or when explicitly requested.
95+
* Returns null if no changes detected. Returns specific test files
96+
* based on source file mappings otherwise.
97+
*
98+
* @throws {Error} When root path does not exist.
99+
* @throws {Error} When git detection fails.
86100
*/
87101
export function getTestsToRun(options = {}) {
88102
const { all = false, staged = false } = options
89103

104+
// Validate root path exists
105+
if (!existsSync(rootPath)) {
106+
throw new Error(`Root path does not exist: "${rootPath}"`)
107+
}
108+
90109
// All mode runs all tests
91110
if (all || process.env.FORCE_TEST === '1') {
92111
return { tests: 'all', reason: 'explicit --all flag', mode: 'all' }
@@ -97,9 +116,22 @@ export function getTestsToRun(options = {}) {
97116
return { tests: 'all', reason: 'CI environment', mode: 'all' }
98117
}
99118

100-
// Get changed files
101-
const changedFiles = staged ? getStagedFilesSync() : getChangedFilesSync()
119+
// Get changed files with error handling
120+
let changedFiles
121+
try {
122+
changedFiles = staged ? getStagedFilesSync() : getChangedFilesSync()
123+
} catch (e) {
124+
// Fallback to all tests if git detection fails
125+
debug(`Git detection failed: ${e.message}`)
126+
return {
127+
tests: 'all',
128+
reason: 'git detection failed',
129+
mode: 'all',
130+
}
131+
}
132+
102133
const mode = staged ? 'staged' : 'changed'
134+
debug(`Found ${changedFiles.length} changed files (${mode})`)
103135

104136
if (changedFiles.length === 0) {
105137
// No changes, skip tests
@@ -165,12 +197,24 @@ export function getTestsToRun(options = {}) {
165197
}
166198

167199
if (runAllTests) {
200+
debug(`Running all tests: ${runAllReason}`)
168201
return { tests: 'all', reason: runAllReason, mode: 'all' }
169202
}
170203

171204
if (testFiles.size === 0) {
205+
// If we had source changes but no valid tests, run all tests for safety
206+
if (changedFiles.length > 0) {
207+
debug('No valid test mappings found, running all tests for safety')
208+
return {
209+
tests: 'all',
210+
reason: 'no valid test mappings found',
211+
mode: 'all',
212+
}
213+
}
172214
return { tests: null, mode }
173215
}
174216

175-
return { tests: Array.from(testFiles), mode }
217+
const tests = Array.from(testFiles)
218+
debug(`Running ${tests.length} specific test(s): ${tests.join(', ')}`)
219+
return { tests, mode }
176220
}

0 commit comments

Comments
 (0)