From ea6eb85162fc94d59a5cc64953274971faee3d36 Mon Sep 17 00:00:00 2001 From: imccyu <276526105+imccyu@users.noreply.github.com> Date: Fri, 31 Jul 2026 02:53:54 +0800 Subject: [PATCH] fix(ci): review follow-ups for the coverage lane split - Split DSH_COVERAGE_MAX_WORKERS between the two parallel gates (instrumented gets ~2/3, exempt gets ~1/3, both at least 1) so the lane never exceeds the budget the failover pool's 8 x 6-instance bound assumes; a budget of 1 pairs with DSH_GATE_CONCURRENCY=1 on the serial reference lanes, which already prevents gate overlap. - Fail loud on a set-but-not-'1' DSH_COVERAGE_EXEMPT_HEAVY value in vitest.config.ts instead of silently ignoring it. - Add coverage-exempt.spec.ts: each roster entry's filter and exclude must select the same non-empty spec set and entries must not overlap, so a renamed suite breaks the gate instead of silently returning to the instrumented run with a stale roster. --- scripts/coverage-exempt.spec.ts | 55 +++++++++++++++++++++++++++++++++ scripts/run-gates.ts | 25 +++++++++++++-- vitest.config.ts | 7 ++++- 3 files changed, 84 insertions(+), 3 deletions(-) create mode 100644 scripts/coverage-exempt.spec.ts diff --git a/scripts/coverage-exempt.spec.ts b/scripts/coverage-exempt.spec.ts new file mode 100644 index 0000000000..5ee854e954 --- /dev/null +++ b/scripts/coverage-exempt.spec.ts @@ -0,0 +1,55 @@ +/** + * Mechanical guard for the coverage-exempt roster: each entry's positional + * filter and exclude glob must select the same non-empty file set out of the + * repository's spec inventory, so a renamed suite cannot silently fall out of + * the uninstrumented gate while its exclude goes stale. + */ + +import { globSync } from 'node:fs' +import { resolve } from 'node:path' +import { describe, expect, it } from 'vitest' +import { coverageExemptHeavySuites } from './coverage-exempt.ts' + +const root = resolve(import.meta.dirname, '..') + +/** The spec inventory mirrored from vitest.config.ts testIncludes. */ +const allSpecs = new Set([ + ...globSync('packages/*/*/tests/**/*.spec.ts', { cwd: root }), + ...globSync('packages/*/*/tests/**/*.spec.tsx', { cwd: root }), + ...globSync('apps/*/tests/**/*.spec.ts', { cwd: root }), + ...globSync('examples/*/tests/**/*.spec.ts', { cwd: root }), + ...globSync('scripts/**/*.spec.ts', { cwd: root }), +].map(path => path.replaceAll('\\', '/'))) + +function excludeMatches(exclude: string): string[] { + return globSync(exclude, { cwd: root }) + .map(path => path.replaceAll('\\', '/')) + .filter(path => allSpecs.has(path)) + .sort() +} + +function filterMatches(filter: string): string[] { + return [...allSpecs].filter(spec => spec.startsWith(filter)).sort() +} + +describe('coverage-exempt roster', () => { + it.each(coverageExemptHeavySuites.map(suite => [suite.filter, suite] as const))( + 'filter and exclude select the same non-empty spec set for %s', + (_filter, suite) => { + const fromExclude = excludeMatches(suite.exclude) + const fromFilter = filterMatches(suite.filter) + expect(fromExclude.length).toBeGreaterThan(0) + expect(fromFilter).toEqual(fromExclude) + }, + ) + + it('entries never overlap, so no suite is double-run or double-excluded', () => { + const seen = new Map() + for (const suite of coverageExemptHeavySuites) { + for (const spec of excludeMatches(suite.exclude)) { + expect(seen.get(spec), `${spec} matched by ${seen.get(spec) ?? ''} and ${suite.exclude}`).toBeUndefined() + seen.set(spec, suite.exclude) + } + } + }) +}) diff --git a/scripts/run-gates.ts b/scripts/run-gates.ts index 0289625959..5d5eb46285 100644 --- a/scripts/run-gates.ts +++ b/scripts/run-gates.ts @@ -412,13 +412,34 @@ function lintGate(): Gate { // compiler- and subprocess-bound fixtures pay a multiple of their runtime // under v8 instrumentation while contributing nothing the thresholds need // (membership contract in scripts/coverage-exempt.ts). +// +// DSH_COVERAGE_MAX_WORKERS is the lane's worker budget, so the two parallel +// gates split it instead of each claiming it whole (the failover pool's +// 8 x 6-instance bound assumes one lane never exceeds its value). The exempt +// gate's wall clock is dominated by its longest single file, so it takes the +// small share. A budget of 1 gives each gate 1 worker; lanes that need a +// strict total of one (the serial reference jobs) also set +// DSH_GATE_CONCURRENCY=1, which keeps the gates from overlapping at all. +function coverageWorkerArgs(): { instrumented: string[]; exempt: string[] } { + const [flag] = positiveIntArg('DSH_COVERAGE_MAX_WORKERS', '--maxWorkers') + if (flag === undefined) return { instrumented: [], exempt: [] } + const total = Number.parseInt(flag.split('=')[1] ?? '', 10) + const exempt = Math.max(1, Math.floor(total / 3)) + const instrumented = Math.max(1, total - exempt) + return { + instrumented: [`--maxWorkers=${String(instrumented)}`], + exempt: [`--maxWorkers=${String(exempt)}`], + } +} + function coverageGates(): Gate[] { + const workers = coverageWorkerArgs() return [ pnpmExec('coverage', [ 'vitest', 'run', '--coverage', - ...positiveIntArg('DSH_COVERAGE_MAX_WORKERS', '--maxWorkers'), + ...workers.instrumented, ], { label: 'test:coverage', env: { [COVERAGE_EXEMPT_ENV]: '1' }, @@ -427,7 +448,7 @@ function coverageGates(): Gate[] { 'vitest', 'run', ...coverageExemptHeavySuites.map(suite => suite.filter), - ...positiveIntArg('DSH_COVERAGE_MAX_WORKERS', '--maxWorkers'), + ...workers.exempt, ], { label: 'test:coverage-exempt-heavy', }), diff --git a/vitest.config.ts b/vitest.config.ts index d31dca5d3f..db44166e6c 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -40,7 +40,12 @@ const testIncludes = [ // The instrumented coverage gate sets this env; the exempt heavy suites then // run beside it uninstrumented (membership contract in scripts/coverage-exempt.ts). -const coverageExemptExcludes = process.env[COVERAGE_EXEMPT_ENV] === '1' +// A set-but-not-'1' value is a misconfiguration, not a silent no-op. +const coverageExemptRaw = process.env[COVERAGE_EXEMPT_ENV] +if (coverageExemptRaw !== undefined && coverageExemptRaw !== '' && coverageExemptRaw !== '1') { + throw new Error(`vitest config: ${COVERAGE_EXEMPT_ENV} must be '1' or unset, got ${JSON.stringify(coverageExemptRaw)}.`) +} +const coverageExemptExcludes = coverageExemptRaw === '1' ? coverageExemptHeavySuites.map(suite => suite.exclude) : []