mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-11 22:50:06 +00:00
fix: Scope CLI mutation diffs to changed tests (no-changelog) (#39285)
This commit is contained in:
@@ -129,13 +129,15 @@ pnpm mutate src/utils.ts --package-dir packages/@n8n/crdt --test-files 'src/__te
|
||||
```
|
||||
|
||||
`--test-files` needs a single target, so it does not combine with `--diff`.
|
||||
For `packages/cli`, diff mode automatically uses the test files changed in the
|
||||
patch. This keeps unrelated CLI tests out of Stryker's instrumented dry run.
|
||||
|
||||
#### `packages/cli` requires it
|
||||
#### `packages/cli` test scope
|
||||
|
||||
`packages/cli` runs vitest with `pool: 'forks'` and a global setup, so each test file costs a
|
||||
process. Related discovery finds hundreds of them for a typical source file, and the dry run alone
|
||||
outlives any usable timeout. A `packages/cli` target therefore **fails with exit `2`** until you
|
||||
name the test files:
|
||||
outlives any usable timeout. A named `packages/cli` target therefore **fails with exit `2`** until
|
||||
you name the test files:
|
||||
|
||||
```bash
|
||||
$ pnpm mutate packages/cli/src/credentials/external-secrets.utils.ts:32-68
|
||||
@@ -146,6 +148,11 @@ Those runs also get `stryker.cli.mjs` instead of the shared default — same set
|
||||
`vitest.related` turned off, because the explicit list already decides the scope. With it, the
|
||||
example above runs its 36-test file and finishes in seconds.
|
||||
|
||||
In diff mode, the changed CLI test files provide the same explicit list. If a
|
||||
CLI patch changes source without changing a test, the command fails before
|
||||
Stryker starts. Add or update a covering test, or run a named target with the
|
||||
existing covering tests passed through `--test-files`.
|
||||
|
||||
### In-place mutation
|
||||
|
||||
Runs use Stryker's `--inPlace`. Its default sandbox copy breaks on any package whose vitest
|
||||
@@ -184,8 +191,8 @@ Not scored:
|
||||
- `.vue` single-file components — every SFC package crashed Stryker's mutate step in the 2026-06 sweep, and the component layer is low-value to mutate.
|
||||
- Tests, declarations, stories, configs, migrations and build output.
|
||||
|
||||
`packages/cli` is scored, but only with an explicit
|
||||
[`--test-files`](#packagescli-requires-it) list.
|
||||
`packages/cli` is scored with an explicit [`--test-files`](#packagescli-test-scope)
|
||||
list for named targets, or with the changed CLI tests in diff mode.
|
||||
|
||||
## Gate semantics
|
||||
|
||||
|
||||
@@ -16,8 +16,8 @@
|
||||
* Use `--test-files` to name the test files that must kill the mutants. The
|
||||
* value goes to Stryker's `testFiles` config field, thus Stryker runs those
|
||||
* files instead of the whole related-test graph. The flag repeats and it also
|
||||
* takes a comma-separated list. `packages/cli` requires it — see
|
||||
* `cliScopeError` below.
|
||||
* takes a comma-separated list. Named `packages/cli` targets require it;
|
||||
* `--diff` uses the CLI test files changed in the patch.
|
||||
*
|
||||
* Stryker config resolution (first match wins):
|
||||
* 1. --config <path> explicit override
|
||||
@@ -307,6 +307,21 @@ export function isMutableSource(repoRelPath) {
|
||||
return !NON_SOURCE.some((re) => re.test(repoRelPath));
|
||||
}
|
||||
|
||||
export function changedTestFilesForPackage(changedFiles, packageDir) {
|
||||
const prefix = `${toPosix(packageDir).replace(/\/$/, '')}/`;
|
||||
return changedFiles
|
||||
.map((file) => toPosix(file).replace(/^\.\//, ''))
|
||||
.filter((file) => {
|
||||
if (!file.startsWith(prefix)) return false;
|
||||
const relative = file.slice(prefix.length);
|
||||
return (
|
||||
(relative.startsWith('src/') || relative.startsWith('test/unit/')) &&
|
||||
/\.(test|spec)\.ts$/.test(relative) &&
|
||||
!/\.integration\.test\.ts$/.test(relative)
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
// Merge overlapping and adjacent ranges. Stryker then gets one span per region.
|
||||
export function mergeRanges(ranges) {
|
||||
const out = [];
|
||||
@@ -411,9 +426,15 @@ export function strykerCliArgs(config) {
|
||||
|
||||
// Why a cli target may not run, or null when it may. Named so the message says
|
||||
// what to do, not only what went wrong.
|
||||
export function cliScopeError(packageDir, testFiles) {
|
||||
export function cliScopeError(packageDir, testFiles, diffMode = false) {
|
||||
if (toPosix(packageDir) !== CLI_PACKAGE_DIR) return null;
|
||||
if (testFiles.length > 0) return null;
|
||||
if (diffMode) {
|
||||
return (
|
||||
`Mutating ${CLI_PACKAGE_DIR} with --diff needs at least one changed test file.\n` +
|
||||
'Add or update a test that covers the changed source, or run a named target with --test-files.'
|
||||
);
|
||||
}
|
||||
return (
|
||||
`Mutating ${CLI_PACKAGE_DIR} needs --test-files.\n` +
|
||||
'Without it Stryker discovers every related test file in the package, forks a ' +
|
||||
@@ -482,12 +503,13 @@ function planFromDiff(base) {
|
||||
die(2, `git diff against '${base}' failed.\n${names.stderr.trim()}`);
|
||||
}
|
||||
|
||||
const byPackage = new Map();
|
||||
const skipped = [];
|
||||
for (const file of names.stdout
|
||||
const changedFiles = names.stdout
|
||||
.split('\n')
|
||||
.map((s) => s.trim())
|
||||
.filter(Boolean)) {
|
||||
.filter(Boolean);
|
||||
const byPackage = new Map();
|
||||
const skipped = [];
|
||||
for (const file of changedFiles) {
|
||||
if (!isMutableSource(file)) continue;
|
||||
const abs = path.resolve(repoRoot, file);
|
||||
if (!existsSync(abs)) continue; // the branch deleted the file
|
||||
@@ -512,6 +534,13 @@ function planFromDiff(base) {
|
||||
for (const r of ranges) job.targets.push(`${rel}:${r.start}-${r.end}`);
|
||||
byPackage.set(pkgRoot, job);
|
||||
}
|
||||
for (const job of byPackage.values()) {
|
||||
if (toPosix(job.packageDir) === CLI_PACKAGE_DIR) {
|
||||
job.testFiles = changedTestFilesForPackage(changedFiles, job.packageDir).filter((file) =>
|
||||
existsSync(path.resolve(repoRoot, file)),
|
||||
);
|
||||
}
|
||||
}
|
||||
return { jobs: [...byPackage.values()], skipped };
|
||||
}
|
||||
|
||||
@@ -765,13 +794,17 @@ function restoreWorkingTree({ bytes, dirtyBefore }) {
|
||||
return [...restored];
|
||||
}
|
||||
|
||||
async function runJob({ pkgRoot, packageDir, targets }, { configArg, testFiles = [] }) {
|
||||
async function runJob(
|
||||
{ pkgRoot, packageDir, targets, testFiles: plannedTestFiles = [] },
|
||||
{ configArg, testFiles = [] },
|
||||
) {
|
||||
const configPath = resolveConfig(pkgRoot, configArg);
|
||||
const strykerBin = resolveStrykerBin(pkgRoot, packageDir);
|
||||
const mutateArg = formatMutateArg(targets);
|
||||
const selectedTestFiles = testFiles.length > 0 ? testFiles : plannedTestFiles;
|
||||
const strykerConfig = buildStrykerConfig({
|
||||
targets,
|
||||
testFiles: testFiles.map((f) => toPackageRelative(f, packageDir)),
|
||||
testFiles: selectedTestFiles.map((f) => toPackageRelative(f, packageDir)),
|
||||
});
|
||||
|
||||
const reportDir = path.join(pkgRoot, 'reports/mutation');
|
||||
@@ -1062,7 +1095,10 @@ async function main() {
|
||||
);
|
||||
} else {
|
||||
jobs = [planFromTarget(targetArg, packageDirArg)];
|
||||
const scopeError = cliScopeError(jobs[0].packageDir, testFiles);
|
||||
}
|
||||
for (const job of jobs) {
|
||||
const selectedTestFiles = testFiles.length > 0 ? testFiles : (job.testFiles ?? []);
|
||||
const scopeError = cliScopeError(job.packageDir, selectedTestFiles, diffMode);
|
||||
if (scopeError) die(2, scopeError);
|
||||
}
|
||||
|
||||
|
||||
@@ -10,6 +10,7 @@ import {
|
||||
buildNoTestsSummary,
|
||||
buildStrykerConfig,
|
||||
buildSummary,
|
||||
changedTestFilesForPackage,
|
||||
classifyRun,
|
||||
cliScopeError,
|
||||
coverageFromCounts,
|
||||
@@ -257,6 +258,40 @@ describe('isMutableSource', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('changedTestFilesForPackage', () => {
|
||||
it('selects changed test files from the exact package', () => {
|
||||
const changedFiles = [
|
||||
'packages/@n8n/engine/src/runtime/__tests__/create-engine-runtime.test.ts',
|
||||
'packages/cli/src/modules/engine-v2/__tests__/engine-v2.runtime.test.ts',
|
||||
'packages/cli/src/modules/engine-v2/__tests__/in-memory-execution-response.test.ts',
|
||||
'packages/cli/src/modules/engine-v2/engine-v2.runtime.ts',
|
||||
'packages/cli-utils/src/foo.test.ts',
|
||||
];
|
||||
|
||||
assert.deepEqual(changedTestFilesForPackage(changedFiles, 'packages/cli'), [
|
||||
'packages/cli/src/modules/engine-v2/__tests__/engine-v2.runtime.test.ts',
|
||||
'packages/cli/src/modules/engine-v2/__tests__/in-memory-execution-response.test.ts',
|
||||
]);
|
||||
});
|
||||
|
||||
it('matches the CLI unit Vitest config roots and extensions', () => {
|
||||
assert.deepEqual(
|
||||
changedTestFilesForPackage(
|
||||
[
|
||||
'packages/cli/src/a.test.ts',
|
||||
'packages/cli/test/unit/b.spec.ts',
|
||||
'packages/cli/src/c.integration.test.ts',
|
||||
'packages/cli/test/integration/d.test.ts',
|
||||
'packages/cli/src/e.test.mts',
|
||||
'packages/cli/src/f.test.tsx',
|
||||
],
|
||||
'packages/cli/',
|
||||
),
|
||||
['packages/cli/src/a.test.ts', 'packages/cli/test/unit/b.spec.ts'],
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('parseHunkRanges', () => {
|
||||
it('reads new-side ranges out of `git diff -U0` headers', () => {
|
||||
const diff = [
|
||||
@@ -543,6 +578,12 @@ describe('cliScopeError', () => {
|
||||
assert.equal(cliScopeError('packages/cli', ['src/__tests__/foo.test.ts']), null);
|
||||
});
|
||||
|
||||
it('explains how to scope a CLI diff with no changed test file', () => {
|
||||
const error = cliScopeError('packages/cli', [], true);
|
||||
assert.match(error, /changed test file/);
|
||||
assert.match(error, /named target with --test-files/);
|
||||
});
|
||||
|
||||
it('leaves every other package alone', () => {
|
||||
assert.equal(cliScopeError('packages/workflow', []), null);
|
||||
// A prefix match would sweep in an unrelated package.
|
||||
|
||||
@@ -8,10 +8,10 @@
|
||||
* the import graph of the mutated file and finds hundreds of test files in this
|
||||
* package — the dry run alone then outlives any usable timeout.
|
||||
*
|
||||
* With `related: false` the run tests exactly the files `--test-files` names, so
|
||||
* `mutate.mjs` requires that flag for cli targets (see `cliScopeError`). Stryker
|
||||
* also uses the resolved `testFiles` as its global test filter, thus a mutant
|
||||
* with no per-test coverage stays inside the same set.
|
||||
* With `related: false` the run tests exactly the files `--test-files` names.
|
||||
* Named targets require that flag; diff mode uses the CLI tests changed in the
|
||||
* patch. Stryker also uses the resolved `testFiles` as its global test filter,
|
||||
* thus a mutant with no per-test coverage stays inside the same set.
|
||||
*/
|
||||
import defaultConfig from './stryker.default.mjs';
|
||||
|
||||
|
||||
Reference in New Issue
Block a user