From a8c4ddeabfdfcbf74031d6d4bebeeb3ae810875b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 29 Oct 2026 10:30:06 +0100 Subject: [PATCH 1/2] perf(check-affected): keep the related run out of the unit projects when the unit suite runs Since #4025 every production source change runs `unit` (`check:unit`), which runs every unit-core and fuzz-worker test. The `vitest related` run still searched all projects, so it ran the same unit tests a second time: 1,556 duplicate test files for packages/kernel/src/errors.ts, 38 for src/cli.ts. scripts/lib/vitest-projects.ts now declares the projects `test:unit` runs. When a narrowed plan executes `unit` in the same run, the related command adds `--project=!` for each one. Fail-open plans and test-only plans keep today's related command. A contract test fails when `test:unit` runs other projects or adds a filter, when `check:unit` stops running `test:unit`, or when a declared name is not a project in vitest.config.ts. Co-Authored-By: Claude Opus 5.5 --- package.json | 2 +- scripts/check-affected/checks.ts | 19 +++++++++++-- scripts/check-affected/model.test.ts | 33 +++++++++++++--------- scripts/check-affected/run.test.ts | 39 ++++++++++++++++++++------ scripts/check-affected/run.ts | 10 ++++++- scripts/lib/vitest-projects.test.ts | 41 ++++++++++++++++++++++++++++ scripts/lib/vitest-projects.ts | 6 ++++ 7 files changed, 125 insertions(+), 25 deletions(-) create mode 100644 scripts/lib/vitest-projects.test.ts create mode 100644 scripts/lib/vitest-projects.ts diff --git a/package.json b/package.json index 66118c5543..2f3887a1ee 100644 --- a/package.json +++ b/package.json @@ -151,7 +151,7 @@ "native-dupes:report": "jscpd --config .jscpd-native.json --baseline native-dupes-baseline.json --fail-on-empty", "native-dupes:baseline": "jscpd --config .jscpd-native.json --baseline native-dupes-baseline.json --update-baseline --fail-on-empty", "check:affected": "node --experimental-strip-types scripts/check-affected/run.ts", - "check:affected:test": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/check-affected/*.test.ts", + "check:affected:test": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/check-affected/*.test.ts scripts/lib/vitest-projects.test.ts", "gate": "node --experimental-strip-types scripts/gate/run.ts", "check:gate-manifest": "node --experimental-strip-types scripts/gate/check.ts", "check:gate-manifest:test": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/gate/*.test.ts", diff --git a/scripts/check-affected/checks.ts b/scripts/check-affected/checks.ts index 77402f2ce7..7b991faa60 100644 --- a/scripts/check-affected/checks.ts +++ b/scripts/check-affected/checks.ts @@ -9,6 +9,7 @@ // workflows by scripts/gate/model.ts, so the "GitHub-authoritative" claim an // agent reads before skipping a check locally cannot go stale. +import { UNIT_TEST_PROJECTS } from '../lib/vitest-projects.ts'; import { ALL_CHECKS, type CheckId } from './model.ts'; export type CheckKind = @@ -205,6 +206,13 @@ export const CHECK_CATALOG: readonly CheckSpec[] = [ gate('replay-android', 'Android replay suite', 'test:replay:android', false), ]; +type RelatedRunScope = { + readonly changedFiles: readonly string[]; + // The same run executes `unit`, which runs every test of UNIT_TEST_PROJECTS. The related run + // then leaves those projects out instead of running their tests twice. + readonly unitSuiteRuns: boolean; +}; + export function getCheckSpec(id: CheckId): CheckSpec { const spec = CHECK_CATALOG.find((entry) => entry.id === id); if (!spec) throw new Error(`No catalog entry for check "${id}".`); @@ -218,10 +226,17 @@ export function resolveCommand( spec: CheckSpec, scripts: Readonly>, base: string, - changedFiles: readonly string[] = [], + related: RelatedRunScope = { changedFiles: [], unitSuiteRuns: false }, ): string[] { if (spec.kind.type === 'vitest-related') { - return ['pnpm', 'exec', 'vitest', 'related', '--run', '--passWithNoTests', ...changedFiles]; + const unitProjects = related.unitSuiteRuns + ? UNIT_TEST_PROJECTS.map((name) => `--project=!${name}`) + : []; + return [ + ...['pnpm', 'exec', 'vitest', 'related', '--run', '--passWithNoTests'], + ...unitProjects, + ...related.changedFiles, + ]; } const { script } = spec.kind; if (!(script in scripts)) { diff --git a/scripts/check-affected/model.test.ts b/scripts/check-affected/model.test.ts index 1cc090d110..3f29c1c2ec 100644 --- a/scripts/check-affected/model.test.ts +++ b/scripts/check-affected/model.test.ts @@ -455,20 +455,27 @@ test('unit and coverage checks preserve their package-script owners', () => { test('vitest-related delegates changed paths to Vitest instead of modeling projects', () => { const related = CHECK_CATALOG.find((entry) => entry.id === 'vitest-related')!; - assert.deepEqual(resolveCommand(related, {}, 'origin/main', ['src/a.ts', 'test/fixture.ts']), [ - 'pnpm', - 'exec', - 'vitest', - 'related', - '--run', - '--passWithNoTests', - 'src/a.ts', - 'test/fixture.ts', - ]); + assert.deepEqual( + resolveCommand(related, {}, 'origin/main', { + changedFiles: ['src/a.ts', 'test/fixture.ts'], + unitSuiteRuns: false, + }), + [ + 'pnpm', + 'exec', + 'vitest', + 'related', + '--run', + '--passWithNoTests', + 'src/a.ts', + 'test/fixture.ts', + ], + ); assert.equal( - resolveCommand(related, {}, 'origin/main', ['src/a.ts']).some((arg) => - arg.startsWith('--maxWorkers='), - ), + resolveCommand(related, {}, 'origin/main', { + changedFiles: ['src/a.ts'], + unitSuiteRuns: true, + }).some((arg) => arg.startsWith('--maxWorkers=')), false, 'worker sizing belongs to vitest.config.ts', ); diff --git a/scripts/check-affected/run.test.ts b/scripts/check-affected/run.test.ts index 0665a6f16d..ae6a33cee3 100644 --- a/scripts/check-affected/run.test.ts +++ b/scripts/check-affected/run.test.ts @@ -145,23 +145,42 @@ test('runChecks runs local checks in order and stops on the first failure', asyn ); }); -test('runChecks passes the selector change set to Vitest related', async () => { +async function runPassing(changedFiles: string[]): Promise { const executed: string[][] = []; - const execute: CommandExecutor = async (command) => { - executed.push(command); - return 0; - }; - const changedFiles = ['packages/selectors/src/index.ts', 'packages/selectors/src/index.test.ts']; const plan = selectChecks({ changedFiles, packageEntryFiles: [] }); + assert.equal(plan.failOpen, false, changedFiles.join(', ')); const code = await runChecks(plan, { scripts: ALL_SCRIPTS }, ARGS, { - execute, + execute: async (command) => { + executed.push(command); + return 0; + }, cwd: '.', changedFiles, }); assert.equal(code, 0); + return executed; +} + +const RELATED = ['pnpm', 'exec', 'vitest', 'related', '--run', '--passWithNoTests']; + +// check:unit runs every unit-core and fuzz-worker test, so the related run must not repeat them. +test('a production source change leaves the unit projects to the unit suite it runs', async () => { + const changedFiles = ['packages/selectors/src/index.ts', 'packages/selectors/src/index.test.ts']; + const executed = await runPassing(changedFiles); + assert.ok(executed.some((command) => command.includes('check:unit'))); assert.deepEqual( executed.find((command) => command.includes('related')), - ['pnpm', 'exec', 'vitest', 'related', '--run', '--passWithNoTests', ...changedFiles], + [...RELATED, '--project=!unit-core', '--project=!fuzz-worker', ...changedFiles], + ); +}); + +test('a test-only change keeps the unit projects in the related run', async () => { + const changedFiles = ['packages/daemon/src/selectors.test.ts']; + const executed = await runPassing(changedFiles); + assert.ok(!executed.some((command) => command.includes('check:unit'))); + assert.deepEqual( + executed.find((command) => command.includes('related')), + [...RELATED, ...changedFiles], ); }); @@ -196,6 +215,10 @@ test('runChecks skips CI checks, runs every local check of a fail-open plan, and } const related = executed.filter((command) => command.includes('related')); assert.equal(related.length, 1); + assert.ok( + !related[0]?.some((arg) => arg.startsWith('--project')), + 'a fail-open related run keeps every Vitest project', + ); assert.ok( !related[0]?.includes('--coverage'), 'coverage instrumentation stays GitHub-authoritative; the local run must not add it', diff --git a/scripts/check-affected/run.ts b/scripts/check-affected/run.ts index bc7e85f5a5..556582ab1f 100644 --- a/scripts/check-affected/run.ts +++ b/scripts/check-affected/run.ts @@ -213,6 +213,14 @@ export async function runChecks( const skipped = plan.checks.map(getCheckSpec).filter((spec: CheckSpec) => !spec.localRunnable); // A fail-open plan could not narrow the change, so no check may stand in for another. const relatedSelected = !plan.failOpen && plan.checks.includes('vitest-related'); + const related = { + changedFiles: options.changedFiles ?? [], + unitSuiteRuns: + !plan.failOpen && + runnable.some( + (spec) => spec.id === 'unit' && !isCoveredByRelatedTests(spec, relatedSelected), + ), + }; const ciJobs = skipped.length > 0 ? ciJobsByCheck() : new Map(); for (const spec of skipped) { const reason = describeOwner(spec.id, ciJobs); @@ -224,7 +232,7 @@ export async function runChecks( options.skip?.(spec.id, 'covered by related tests', true); continue; } - const command = resolveCommand(spec, pkg.scripts, args.base, options.changedFiles ?? []); + const command = resolveCommand(spec, pkg.scripts, args.base, related); const exitCode = await execute(command, cwd, spec.id); if (exitCode !== 0) { process.stderr.write(`\ncheck:affected: ${spec.id} failed.\n`); diff --git a/scripts/lib/vitest-projects.test.ts b/scripts/lib/vitest-projects.test.ts new file mode 100644 index 0000000000..1f3c984a91 --- /dev/null +++ b/scripts/lib/vitest-projects.test.ts @@ -0,0 +1,41 @@ +// The related run of `check:affected --run` leaves UNIT_TEST_PROJECTS to the unit suite. That is +// sound only while `test:unit` runs every test of exactly those projects, so drift fails here. + +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { test } from 'node:test'; +import { UNIT_TEST_PROJECTS } from './vitest-projects.ts'; + +const repoRoot = path.resolve(import.meta.dirname, '../..'); +const scripts = ( + JSON.parse(fs.readFileSync(path.join(repoRoot, 'package.json'), 'utf8')) as { + scripts: Record; + } +).scripts; + +test('test:unit runs every test of exactly UNIT_TEST_PROJECTS', () => { + const [runner, mode, ...flags] = (scripts['test:unit'] ?? '').trim().split(/\s+/); + assert.deepEqual([runner, mode], ['vitest', 'run']); + const projects: string[] = []; + for (let i = 0; i < flags.length; i++) { + const flag = flags[i] ?? ''; + if (flag === '--project') projects.push(flags[++i] ?? ''); + else if (flag.startsWith('--project=')) projects.push(flag.slice('--project='.length)); + else assert.fail(`test:unit must not filter its projects, found "${flag}"`); + } + assert.deepEqual(projects.toSorted(), [...UNIT_TEST_PROJECTS].toSorted()); +}); + +test('the unit check runs test:unit', () => { + const steps = (scripts['check:unit'] ?? '').split('&&').map((step) => step.trim()); + assert.ok(steps.includes('pnpm test:unit'), `check:unit is "${scripts['check:unit']}"`); +}); + +test('every UNIT_TEST_PROJECTS entry is a project of vitest.config.ts', async () => { + const { default: config } = (await import('../../vitest.config.ts')) as { + default: { test: { projects: { test: { name: string } }[] } }; + }; + const names = config.test.projects.map((project) => project.test.name); + for (const name of UNIT_TEST_PROJECTS) assert.ok(names.includes(name), `${name} in ${names}`); +}); diff --git a/scripts/lib/vitest-projects.ts b/scripts/lib/vitest-projects.ts new file mode 100644 index 0000000000..774c944b4a --- /dev/null +++ b/scripts/lib/vitest-projects.ts @@ -0,0 +1,6 @@ +/** + * The Vitest projects that `pnpm test:unit` runs in full. package.json spells the same list as + * `--project` flags; scripts/lib/vitest-projects.test.ts fails when the two differ. This module + * stays dependency-free: importing vitest.config.ts consumes its validation-report variable. + */ +export const UNIT_TEST_PROJECTS = ['unit-core', 'fuzz-worker'] as const; From 92b98727ccaf56df9b7f11cdb72409aa8bb6fd12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 29 Oct 2026 10:33:23 +0100 Subject: [PATCH 2/2] fix(check-affected): list the related command words without a spread oxlint no-useless-spread rejected the spread array literal. Co-Authored-By: Claude Opus 5.5 --- scripts/check-affected/checks.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/scripts/check-affected/checks.ts b/scripts/check-affected/checks.ts index 7b991faa60..3833520ea2 100644 --- a/scripts/check-affected/checks.ts +++ b/scripts/check-affected/checks.ts @@ -233,7 +233,12 @@ export function resolveCommand( ? UNIT_TEST_PROJECTS.map((name) => `--project=!${name}`) : []; return [ - ...['pnpm', 'exec', 'vitest', 'related', '--run', '--passWithNoTests'], + 'pnpm', + 'exec', + 'vitest', + 'related', + '--run', + '--passWithNoTests', ...unitProjects, ...related.changedFiles, ];