diff --git a/docs/agents/testing.md b/docs/agents/testing.md index 44d5e6841..ccda0feee 100644 --- a/docs/agents/testing.md +++ b/docs/agents/testing.md @@ -188,9 +188,9 @@ hand-maintained path map: implicitly. - **Always-on gates** (`lint`, `typecheck`, `layering`, `fallow`, `format`) fire for their input categories and are never silently skipped. Legacy - `src/platforms/` source also selects provider-integration and coverage. + `src/platforms/` source also selects provider-integration and reports coverage as CI-owned. `packages/platform-*` source selects the shared runtime-contract unit lane, - provider-integration, and coverage so a package move cannot narrow its evidence. + provider-integration, and CI-owned coverage so a package move cannot narrow its evidence. - **Commands** are resolved from real `package.json` scripts, so a renamed script fails loudly instead of dropping a gate. - A **small explicit build-ownership layer** covers the paths whose owning build @@ -224,11 +224,10 @@ docs-only short-circuit its path would otherwise take. If the matrix moves again, move that entry with it. The plan documents the rule and changed path behind every selected check. -Local coverage reuses the affected Vitest run as its LCOV producer and applies the changed-line -coverage gate to that report. It does not run the full instrumented suite; global coverage -thresholds and full unit/provider matrices remain authoritative in GitHub CI. When coverage is -selected, `vitest-related` is folded into this one affected coverage run, and full unit/provider -aggregates are not repeated locally. +Coverage is never instrumented by `check:affected --run`. The plan still reports the coverage +obligation and its authoritative GitHub job, while the local path runs plain `vitest related` and +deduplicates full unit/provider aggregates. When CI reports a coverage failure, reproduce it in +isolation with the coverage command named by that job; do not make every pre-push loop pay for LCOV. Model and catalog live under `scripts/check-affected/`; the derivation is guarded by `pnpm check:affected:test` (the `Affected-check Selector` CI job). diff --git a/scripts/check-affected/checks.ts b/scripts/check-affected/checks.ts index 088801cb9..514e8255e 100644 --- a/scripts/check-affected/checks.ts +++ b/scripts/check-affected/checks.ts @@ -49,7 +49,7 @@ export const CHECK_CATALOG: readonly CheckSpec[] = [ localRunnable: true, }, gate('unit', 'Unit + smoke suite', 'check:unit'), - gate('coverage', 'Affected LCOV + changed-line coverage', 'check:coverage-changed'), + gate('coverage', 'Changed-line coverage', 'check:coverage-changed', false), gate('provider-integration', 'Provider-backed integration suite', 'test:integration:provider'), gate( 'integration-progress', @@ -78,8 +78,9 @@ export const CHECK_CATALOG: readonly CheckSpec[] = [ // steps; before the registry became canonical, nothing in the repo could name // them, so nothing could ask whether they still ran. // - // `unit-ci` is the CI form of the unit suite. Locally you run `unit` and - // `coverage`, which together repeat it. + // `unit-ci` is the CI form of the unit suite under coverage. Local affected + // checks use Vitest's related graph without instrumentation; CI owns the + // full coverage run and changed-line verdict. gate('unit-ci', 'CI unit suite under coverage', 'test:coverage:ci', false), gate('affected-selector', 'Affected-check selector model', 'check:affected:test'), gate('gate-manifest', 'Gate manifest — every gate owned and wired', 'check:gate-manifest'), diff --git a/scripts/check-affected/model.ts b/scripts/check-affected/model.ts index 5cc9e2145..77632eade 100644 --- a/scripts/check-affected/model.ts +++ b/scripts/check-affected/model.ts @@ -97,7 +97,7 @@ export const ALL_CHECKS: readonly CheckId[] = [ 'build', 'package', // Real daemon/process integration owns host-global lifecycle state and must - // run before the high-parallelism coverage workload heats the host. + // run before Vitest's related-project workload heats the host. 'integration-node', 'vitest-related', 'unit', diff --git a/scripts/check-affected/run.test.ts b/scripts/check-affected/run.test.ts index 336030eb8..4bae52d5b 100644 --- a/scripts/check-affected/run.test.ts +++ b/scripts/check-affected/run.test.ts @@ -10,7 +10,6 @@ import path from 'node:path'; import { test } from 'node:test'; import { runCmdSync } from '../../src/utils/exec.ts'; import { CHECK_CATALOG } from './checks.ts'; -import { DEFAULT_VITEST_MAX_WORKERS } from '../lib/vitest-concurrency.ts'; import { selectChecks } from './model.ts'; import { type CommandExecutor, readChangedFiles, runChecks } from './run.ts'; @@ -171,26 +170,29 @@ test('runChecks skips GitHub-authoritative checks and passes when locals succeed } }); -test('runChecks combines related tests with lightweight changed-line coverage', async () => { +test('runChecks leaves coverage to CI and runs plain related tests once', async () => { const executed: string[][] = []; const execute: CommandExecutor = async (command) => { executed.push(command); return 0; }; const plan = selectChecks({ changedFiles: ['unknown/path.xyz'], packageEntryFiles: [] }); + assert.equal( + CHECK_CATALOG.find((spec) => spec.id === 'coverage')?.localRunnable, + false, + 'coverage must stay visible in the plan but must not run locally', + ); const code = await runChecks(plan, { scripts: ALL_SCRIPTS }, ARGS, { execute, cwd: '.' }); assert.equal(code, 0); const related = executed.filter((command) => command.includes('related')); assert.equal(related.length, 1); - assert.ok(related[0]?.includes('--coverage')); - assert.ok(related[0]?.includes('--coverage.reporter=lcov')); - assert.ok(related[0]?.includes(`--maxWorkers=${DEFAULT_VITEST_MAX_WORKERS}`)); + assert.equal(related[0]?.includes('--coverage'), false); assert.ok( executed.findIndex((command) => command.includes('test:integration:node')) < executed.findIndex((command) => command.includes('related')), - 'process-lifecycle integration must run before high-parallelism affected coverage', + 'process-lifecycle integration must run before the related-project workload', ); assert.equal( executed.some((command) => command.includes('test:coverage')), @@ -206,6 +208,6 @@ test('runChecks combines related tests with lightweight changed-line coverage', ); assert.equal( executed.some((command) => command.includes('check:coverage-changed')), - true, + false, ); }); diff --git a/scripts/check-affected/run.ts b/scripts/check-affected/run.ts index 469a0e7f7..d31f5bf2a 100644 --- a/scripts/check-affected/run.ts +++ b/scripts/check-affected/run.ts @@ -11,7 +11,6 @@ import { pathToFileURL } from 'node:url'; import { runCmdStreaming, runCmdSync } from '../../src/utils/exec.ts'; import { parseScriptArgs } from '../lib/cli-args.ts'; import { runEntrypoint } from '../lib/cli-entrypoint.ts'; -import { DEFAULT_VITEST_MAX_WORKERS } from '../lib/vitest-concurrency.ts'; import { assertCatalogComplete, CHECK_CATALOG, @@ -191,24 +190,22 @@ export async function runChecks( const execute = options.execute ?? streamingExecutor; const runnable = plan.checks.map(getCheckSpec).filter((spec: CheckSpec) => spec.localRunnable); const skipped = plan.checks.map(getCheckSpec).filter((spec: CheckSpec) => !spec.localRunnable); - const coverageSelected = plan.checks.includes('coverage'); + const relatedSelected = plan.checks.includes('vitest-related'); const ciJobs = skipped.length > 0 ? ciJobsByCheck() : new Map(); for (const spec of skipped) { process.stdout.write(`\n[skip] ${spec.id} — ${describeOwner(spec.id, ciJobs)}\n`); } for (const spec of runnable) { - if (isCoveredByAffectedCoverage(spec, coverageSelected)) { - process.stdout.write(`\n[dedupe] ${spec.id} — covered by affected LCOV or GitHub CI\n`); + if (isCoveredByRelatedTests(spec, relatedSelected)) { + process.stdout.write(`\n[dedupe] ${spec.id} — covered by related tests or GitHub CI\n`); continue; } - const commands = resolveCheckCommands(spec, pkg, args, options.changedFiles ?? []); - for (const command of commands) { - process.stdout.write(`\n[run] ${spec.id}: ${command.join(' ')}\n`); - const exitCode = await execute(command, cwd); - if (exitCode !== 0) { - process.stderr.write(`\ncheck:affected: ${spec.id} failed.\n`); - return 1; - } + const command = resolveCommand(spec, pkg.scripts, args.base, options.changedFiles ?? []); + process.stdout.write(`\n[run] ${spec.id}: ${command.join(' ')}\n`); + const exitCode = await execute(command, cwd); + if (exitCode !== 0) { + process.stderr.write(`\ncheck:affected: ${spec.id} failed.\n`); + return 1; } } process.stdout.write('\ncheck:affected: all runnable checks passed.\n'); @@ -223,54 +220,8 @@ function describeOwner(id: CheckId, ciJobs: ReadonlyMap): str return `GitHub-authoritative (jobs: ${(ciJobs.get(id) ?? []).join(', ')})`; } -function isCoveredByAffectedCoverage(spec: CheckSpec, coverageSelected: boolean): boolean { - return ( - coverageSelected && - (spec.id === 'vitest-related' || spec.id === 'unit' || spec.id === 'provider-integration') - ); -} - -function resolveCheckCommands( - spec: CheckSpec, - pkg: PackageJson, - args: Args, - changedFiles: readonly string[], -): string[][] { - return spec.id === 'coverage' - ? resolveAffectedCoverageCommands(pkg.scripts, args.base, changedFiles) - : [resolveCommand(spec, pkg.scripts, args.base, changedFiles)]; -} - -function resolveAffectedCoverageCommands( - scripts: Readonly>, - base: string, - changedFiles: readonly string[], -): string[][] { - if (!('check:coverage-changed' in scripts)) { - throw new Error('Required package.json script "check:coverage-changed" does not exist.'); - } - return [ - [ - 'pnpm', - 'exec', - 'vitest', - 'related', - '--run', - '--passWithNoTests', - // `related` spans every configured Vitest project for broad diffs. The - // machine-derived default can start enough projects concurrently to - // starve otherwise-green subprocess/provider tests past their exact - // timeout budgets. Bound this aggregate feedback lane without changing - // the suites' own timeout or serialization contracts. - `--maxWorkers=${DEFAULT_VITEST_MAX_WORKERS}`, - '--coverage', - '--coverage.reporter=lcov', - '--coverage.thresholds.statements=0', - '--coverage.thresholds.lines=0', - ...changedFiles, - ], - ['pnpm', 'run', 'check:coverage-changed', '--base', base], - ]; +function isCoveredByRelatedTests(spec: CheckSpec, relatedSelected: boolean): boolean { + return relatedSelected && (spec.id === 'unit' || spec.id === 'provider-integration'); } async function main(argv = process.argv.slice(2)): Promise {