diff --git a/tests/coverage-gate-thresholds.test.mjs b/tests/coverage-gate-thresholds.test.mjs index 4ae2427b..6a427887 100644 --- a/tests/coverage-gate-thresholds.test.mjs +++ b/tests/coverage-gate-thresholds.test.mjs @@ -48,15 +48,16 @@ const FLOORS = { // entered none of the ratios above, which are all built from // isSourceFile(). These are the same three shapes over its own row. // Measured on the suite this commit ships: harness files 100.00% lines / - // 99.32% regions (3890/3890 lines, 1471/1481 regions), and the lowest - // harness file is tests/tools/e2e-coverage-report.mjs at 97.98%, floored + // 99.53% regions (3925/3925 lines, 1467/1474 regions), and the lowest + // harness file is tests/tools/e2e-coverage-report.mjs at 98.57%, floored // to 97. Every other harness file is at 100.00%. // - // 97.98% is also that file's ceiling, not a backlog: its 10 uncovered + // 98.57% is also that file's ceiling, not a backlog: its 7 uncovered // regions are the defensive fallbacks the note at the top of the file lists - // as unreachable (#1253), so no test can lift it. The per-file floor must - // stay at or below 97 until that list shrinks; a probe that lands one of - // them should raise the floor and shorten the list in the same change. + // as unreachable (#1253; three more were deleted rather than listed in + // #1304), so no test can lift it. The per-file floor must stay at or below + // 98 until that list shrinks; a probe that lands one of them should raise + // the floor and shorten the list in the same change. '--check-harness': 100, '--check-harness-regions': 97, '--check-harness-file-regions': 97, diff --git a/tests/e2e-coverage-attribution.test.mjs b/tests/e2e-coverage-attribution.test.mjs new file mode 100644 index 00000000..44852585 --- /dev/null +++ b/tests/e2e-coverage-attribution.test.mjs @@ -0,0 +1,238 @@ +// `convertScript` (tests/tools/e2e-coverage-report.mjs) decides which of a +// bundle's original sources a script's coverage may be written to: +// +// const attributedPaths = new Set( +// Object.entries(converter.branches) +// .filter(([, ranges]) => ranges.length > 0) +// .map(([path]) => path), +// ); +// +// and `collectE2ECoverage` drops any `src/` path missing from that set when +// the bundle maps to more than one source. So "branches alone decide +// attribution" is a load-bearing claim about v8-to-istanbul, not a style +// choice: a release that stopped populating `branches` for a source would not +// fail anything -- the report would quietly stop counting that file, and the +// e2e gate would pass on a smaller denominator. +// +// These tests state the facts the single-arm form rests on. They drive the +// installed converter directly rather than a report, because a report can only +// show the consequence and these are the premises: +// +// 1. `branches` and `functions` are initialised to `{}` by the converter's +// constructor, so neither needs a `?? {}` guard; +// 2. `functions` is populated only for a *named* block, and then only for +// the source its first range lands in, which makes it a strictly weaker +// signal than `branches`; and +// 3. the records `convertScript` applies are always anonymous, because +// `normalizeRanges` rebuilds every script's coverage as one block with +// `functionName: ''` -- which is why a `functions` arm could never have +// contributed a path that `branches` had not. +// +// The last test is the end-to-end consequence, kept beside the premises: a +// two-source bundle attributes both of its sources, including one whose +// coverage is a single uninterrupted covered range. +import assert from 'node:assert/strict'; +import test from 'node:test'; +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { dirname, join } from 'node:path'; + +import v8ToIstanbul from 'v8-to-istanbul'; + +import { collectE2ECoverage } from './tools/e2e-coverage-report.mjs'; +import { + initCoverageRun, + sealCoverageRun, + writeCoverageArtifact, +} from './tools/e2e-coverage-run.mjs'; + +const ALPHA = 'const alpha1 = 1;\nconst alpha2 = 2;\n'; +const BETA = 'const beta1 = 1;\nconst beta2 = 2;\n'; +const MAPPED_SOURCE = `${ALPHA}${BETA}`; +const SCRIPT_TEXT = `${MAPPED_SOURCE}\n//# sourceMappingURL=app.js.map\n`; +const ALPHA_PATH = 'src/components/Alpha/index.js'; +const BETA_PATH = 'src/components/Beta/index.js'; + +// Four generated lines, all at column 0. 'AAAA' opens source 0 line 1, 'AACA' +// advances the original line, 'ACDA' switches to source 1 and steps the +// original line back to 1 (source +1, originalLine -1), 'AACA' advances it +// again. Spelled out rather than produced by a helper so the fixture states +// exactly where the source boundary falls. +const MAPPINGS = 'AAAA;AACA;ACDA;AACA'; + +// `normalizeSourceMap` rewrites every reference to the resolved absolute path +// of the file on disk before the converter sees it, so the premise tests use +// that shape rather than the `webpack://` form the bundler emits. +const ABSOLUTE_SOURCES = ['/virtual/src/alpha.js', '/virtual/src/beta.js']; + +function premiseMap() { + return { + version: 3, + file: 'app.js', + sourceRoot: '', + sources: [...ABSOLUTE_SOURCES], + sourcesContent: [ALPHA, BETA], + names: [], + mappings: MAPPINGS, + }; +} + +// One range per source, which is what `splitRangeAtBoundaries` guarantees +// `convertScript` hands to `applyCoverage`: a range that straddles the +// boundary resolves its endpoints into two different originals, and +// v8-to-istanbul abandons it, recording nothing for either source. +function blockCovering(functionName) { + return [ + { + functionName, + isBlockCoverage: true, + ranges: [ + { startOffset: 0, endOffset: ALPHA.length - 1, count: 1 }, + { + startOffset: ALPHA.length, + endOffset: MAPPED_SOURCE.length, + count: 1, + }, + ], + }, + ]; +} + +function newConverter() { + return v8ToIstanbul('/virtual/build/assets/js/app.js', 0, { + source: MAPPED_SOURCE, + sourceMap: { sourcemap: premiseMap() }, + }); +} + +async function applied(functionName) { + const converter = newConverter(); + await converter.load(); + converter.applyCoverage(blockCovering(functionName)); + return converter; +} + +// The same fold `convertScript` applies, so these tests read the maps exactly +// as attribution does rather than merely asserting they are non-empty. +const attributable = (map) => + Object.entries(map) + .filter(([, ranges]) => ranges.length > 0) + .map(([path]) => path) + .sort(); + +test('the converter initialises branches and functions, so neither needs a ?? guard', async () => { + const converter = newConverter(); + await converter.load(); + assert.deepEqual(converter.branches, {}); + assert.deepEqual(converter.functions, {}); + + // Still object-valued once coverage has been applied. The `?? {}` guards + // `convertScript` used to carry could only ever have fired if one of these + // were reassigned to a nullish value, which nothing in the converter does. + converter.applyCoverage(blockCovering('')); + assert.equal(typeof converter.branches, 'object'); + assert.notEqual(converter.branches, null); + assert.equal(typeof converter.functions, 'object'); + assert.notEqual(converter.functions, null); +}); + +test('an anonymous block attributes every source through branches and none through functions', async () => { + const converter = await applied(''); + assert.deepEqual(attributable(converter.branches), ABSOLUTE_SOURCES); + // The whole reason the `functions` arm of `attributedPaths` was removable: + // `normalizeRanges` names no block, so this map is never written to. + assert.deepEqual(converter.functions, {}); +}); + +test('a named block populates functions, and then only for its first range', async () => { + // Non-vacuity for the test above -- `functions` is not simply always empty -- + // and the reason it would remain the weaker arm even if it were reachable: a + // named block records one function, against the source its first range lands + // in, while `branches` already names both. + const converter = await applied('render'); + assert.deepEqual(attributable(converter.functions), [ABSOLUTE_SOURCES[0]]); + assert.deepEqual(attributable(converter.branches), ABSOLUTE_SOURCES); +}); + +async function fixtureRun() { + const root = await mkdtemp(join(tmpdir(), 'endusers-attribution-')); + const runDir = join(root, 'coverage'); + const buildDir = join(root, 'build'); + for (const [path, content] of [ + [ALPHA_PATH, ALPHA], + [BETA_PATH, BETA], + ]) { + await mkdir(dirname(join(root, path)), { recursive: true }); + await writeFile(join(root, path), content); + } + await mkdir(join(buildDir, 'assets/js'), { recursive: true }); + await writeFile(join(buildDir, 'assets/js/app.js'), SCRIPT_TEXT); + await writeFile( + join(buildDir, 'assets/js/app.js.map'), + JSON.stringify({ + version: 3, + file: 'app.js', + sources: [ + `webpack://endusers/./${ALPHA_PATH}`, + `webpack://endusers/./${BETA_PATH}`, + ], + sourcesContent: [ALPHA, BETA], + names: [], + mappings: MAPPINGS, + }), + ); + await initCoverageRun(runDir, 'run-1', { project: 'chromium' }); + + // A zero-count range inside Alpha's line 2, which is what the browser + // records for a block that did not run. Beta is left as one uninterrupted + // covered range, so it is the source that tests whether attribution + // survives a file the run never split. + const alphaSplitStart = ALPHA.indexOf('const alpha2'); + await writeCoverageArtifact(runDir, 'worker-0-page-0', { + schemaVersion: 1, + kind: 'endusers.playwright.v8-coverage', + runId: 'run-1', + result: [ + { + url: 'http://localhost:3000/assets/js/app.js', + scriptId: '1', + functions: [ + { + functionName: '', + isBlockCoverage: true, + ranges: [ + { startOffset: 0, endOffset: SCRIPT_TEXT.length, count: 1 }, + { + startOffset: alphaSplitStart, + endOffset: alphaSplitStart + 'const alpha2'.length, + count: 0, + }, + ], + }, + ], + }, + ], + }); + await sealCoverageRun(runDir, 'passed'); + return { root, runDir, buildDir }; +} + +test('both sources of a two-source bundle are attributed, including the unsplit one', async () => { + const fixture = await fixtureRun(); + try { + const report = await collectE2ECoverage(fixture.runDir, { + root: fixture.root, + buildDir: fixture.buildDir, + }); + assert.equal(report.status, 'ok'); + // `sourceFiles.length > 1` here, so a path missing from `attributedPaths` + // is dropped from the report outright -- silently, with no diagnostic. + assert.deepEqual( + report.sources.map((source) => source.file).sort(), + [ALPHA_PATH, BETA_PATH].sort(), + ); + assert.deepEqual(report.diagnostics.errors, []); + } finally { + await rm(fixture.root, { recursive: true, force: true }); + } +}); diff --git a/tests/tools/e2e-coverage-report.mjs b/tests/tools/e2e-coverage-report.mjs index 014569d4..a5e2c75e 100644 --- a/tests/tools/e2e-coverage-report.mjs +++ b/tests/tools/e2e-coverage-report.mjs @@ -45,10 +45,6 @@ const ORIGINAL_SCRIPT = /\.(?:c|m)?(?:js|jsx|ts|tsx)$/u; // `c` satisfies `c < range.endOffset` by the filter above it, `start` // equals `c` when the loop ends, so the final `parts.push` always runs // and `parts` is never empty. -// convertScript `converter.branches ?? {}` and `converter.functions -// ?? {}`, and the `.map(([path]) => path)` arm that only runs when one -// of those is absent -- `v8-to-istanbul` always defines both after -// `load()`, so the fallback needs a shape the library does not emit. // collectE2ECoverage `converted.coverage ?? {}` -- `convertScript` returns // `converter.toIstanbul()` on every path and that never returns nullish; // and `if (lineCoverage.size === 0) continue`, which needs an istanbul @@ -59,7 +55,10 @@ const ORIGINAL_SCRIPT = /\.(?:c|m)?(?:js|jsx|ts|tsx)$/u; // // Reachability was established empirically rather than by reading: probe tests // were written against each candidate and these are the ones no probe could -// close. Together they are 10 regions, which caps this file at 97.98% regions; +// close. (The `convertScript` guards that used to sit in this list were +// removed outright once tests/e2e-coverage-attribution.test.mjs pinned the +// converter shape they defended against, #1304.) Together they are 7 regions, +// which caps this file at 98.57% regions; // tests/coverage-gate-thresholds.test.mjs records that ceiling beside // `--check-harness-file-regions` so the floor is never set above it. Raise the // floor rather than this list when a probe does land, and extend this list @@ -682,14 +681,28 @@ async function convertScript(scriptCoverage, root, scriptRoots) { } converter.applyCoverage(usableFunctions); const coverage = converter.toIstanbul(); - const attributedPaths = new Set([ - ...Object.entries(converter.branches ?? {}) + // `converter.branches` is the whole attribution signal, and the two things + // this does not also read are not an oversight: + // + // * `converter.functions` is permanently `{}` here. v8-to-istanbul writes + // `this.functions[path]` only under `if (block.functionName ...)` + // (lib/v8-to-istanbul.js:167,178), and `normalizeRanges` above rewrites + // every script's record into exactly one block with `functionName: ''`. + // A `functions` arm would therefore contribute no path that `branches` + // has not already contributed. + // * neither needs a `?? {}` guard: both are initialised to `{}` in the + // converter's constructor (lib/v8-to-istanbul.js:36-37) and are never + // reassigned. + // + // tests/e2e-coverage-attribution.test.mjs pins both facts against the + // installed converter, so a release that started naming blocks or stopped + // initialising the maps fails there rather than silently dropping a source + // from a multi-source bundle. + const attributedPaths = new Set( + Object.entries(converter.branches) .filter(([, ranges]) => ranges.length > 0) .map(([path]) => path), - ...Object.entries(converter.functions ?? {}) - .filter(([, ranges]) => ranges.length > 0) - .map(([path]) => path), - ]); + ); return { sourceFiles, coverage,