perf(check-affected): keep the related run out of the unit projects when the unit suite runs

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",

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,22 @@ export function resolveCommand(
   spec: CheckSpec,
   scripts: Readonly<Record<string, string>>,
   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)) {

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',
   );

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<string[][]> {
   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',

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<CheckId, string[]>();
   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`);

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<string, string>;
+  }
+).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}`);
+});

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;