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.
This commit is contained in:
@@ -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<string, string>()
|
||||
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)
|
||||
}
|
||||
}
|
||||
})
|
||||
})
|
||||
+23
-2
@@ -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',
|
||||
}),
|
||||
|
||||
+6
-1
@@ -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)
|
||||
: []
|
||||
|
||||
|
||||
Reference in New Issue
Block a user