From c90a1f02bd11018101fb96b9cf710db7670c156a Mon Sep 17 00:00:00 2001 From: quality Date: Wed, 7 Oct 2026 13:06:13 -0400 Subject: [PATCH] fix(coverage): fold e2e regions whose spans cross across artifacts Two branch spans of one source file either nest or stay disjoint, because both are AST extents. A covered span that begins inside a zero span and ends after it is a crossing, which no source can produce; it only appears when two artifacts of the same chunk map one original branch to different spans. getRegionCoverage now carries the start and end columns it already encoded into the region key, and the report drops a zero-count region that any covered region crosses. This folds the three regions #1079 names -- MemberProfile.js 10 and 15, MemberCard.js 75 -- which #1051's multi-line fold and #1066's ordinal key both leave standing because their start columns differ. The rule is narrower than containment on purpose: a covered region that encloses a zero one is the ordinary shape of a branch arm inside an executed block, so enclosure is still never treated as drift and the guard added in #1114 continues to pass unchanged. Signed-off-by: quality --- tests/e2e-coverage-report.test.mjs | 119 ++++++++++++++++++++++++++++ tests/tools/e2e-coverage-report.mjs | 68 +++++++++++++--- 2 files changed, 178 insertions(+), 9 deletions(-) diff --git a/tests/e2e-coverage-report.test.mjs b/tests/e2e-coverage-report.test.mjs index d8794ae3..fc593abe 100644 --- a/tests/e2e-coverage-report.test.mjs +++ b/tests/e2e-coverage-report.test.mjs @@ -513,6 +513,125 @@ test('a single-line zero-count region survives a covered region that encloses it } }); +// The shape above survives; this one must not. Two branch spans of one source +// file either nest or stay disjoint, because both are AST extents -- so a +// covered span that *begins inside* a zero span and *ends after* it is a +// crossing, which no source can produce. It only appears when two artifacts of +// the same chunk map one original branch to different spans (#1079): the +// `''` arm of `useBaseUrl(member.logo || '')` in +// `src/components/MemberDirectory/MemberProfile.js` is reported zero at +// `10:40-10:48` by one artifact while another reports it covered at +// `10:44-15:19`, and `15:19-15:80` crosses `15:70-38:15` the same way. The +// start columns differ, so the ordinal key proposed in #1066 keeps them apart, +// and neither contains the other, so #1051's multi-line fold and the +// enclosure rule the test above pins both leave them standing. +// +// Below, the zero region `1:0-1:6` is crossed by the covered `1:3-3:12`: the +// covered region starts at column 3, strictly inside the zero span, and runs +// past its end. That pair cannot both be real, so the zero half is dropped. +test('a zero-count region crossed by a covered one is treated as source-map drift', async () => { + const fixture = await fixtureRun(); + try { + const lines = ['const a = 1;', 'const b = 2;', 'const c = 3;']; + const source = `${lines.join('\n')}\n`; + const scriptText = `${source}\n//# sourceMappingURL=app.js.map\n`; + const original = join(fixture.root, 'src/components/Example/index.js'); + await mkdir(join(fixture.root, 'src/components/Example'), { + recursive: true, + }); + await writeFile(original, source); + await writeFile(join(fixture.buildDir, 'assets/js/app.js'), scriptText); + await writeFile( + join(fixture.buildDir, 'assets/js/app.js.map'), + JSON.stringify({ + version: 3, + file: 'app.js', + sources: ['webpack://endusers/./src/components/Example/index.js'], + sourcesContent: [source], + names: [], + // Line 1 carries mappings at columns 0, 3 and 6 so that two block + // boundaries inside it resolve to distinct original columns. Without + // the middle one the two spans could only nest or coincide, and the + // crossing this test is about would be unrepresentable. + mappings: [ + [ + [vlq(0), vlq(0), vlq(0), vlq(0)].join(''), + [vlq(3), vlq(0), vlq(0), vlq(3)].join(''), + [vlq(3), vlq(0), vlq(0), vlq(3)].join(''), + ].join(','), + [vlq(0), vlq(0), vlq(1), vlq(-6)].join(''), + [vlq(0), vlq(0), vlq(1), vlq(0)].join(''), + ].join(';'), + }), + ); + + // One page runs the whole script, and a block from column 3 onwards twice + // over -- a loop body. Its differing count is what makes V8 emit the + // boundary at column 3 at all. + await writeCoverageArtifact(fixture.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: scriptText.length, count: 1 }, + { startOffset: 3, endOffset: scriptText.length, count: 2 }, + ], + }, + ], + }, + ], + }); + // A second page maps the same original branch to a span that starts + // earlier and ends earlier, and reports it unexecuted. + await writeCoverageArtifact(fixture.runDir, 'worker-0-page-1', { + schemaVersion: 1, + kind: 'endusers.playwright.v8-coverage', + runId: 'run-1', + result: [ + { + url: 'http://localhost:3000/assets/js/app.js', + scriptId: '2', + functions: [ + { + functionName: '', + isBlockCoverage: true, + ranges: [ + { startOffset: 0, endOffset: scriptText.length, count: 1 }, + { startOffset: 0, endOffset: 6, count: 0 }, + ], + }, + ], + }, + ], + }); + await sealCoverageRun(fixture.runDir, 'passed'); + + const report = await collectE2ECoverage(fixture.runDir, { + root: fixture.root, + buildDir: fixture.buildDir, + }); + assert.equal(report.status, 'ok'); + const [entry] = report.sources; + assert.equal(entry.file, 'src/components/Example/index.js'); + assert.deepEqual(entry.uncoveredLines, []); + assert.deepEqual(entry.uncoveredRegions, []); + assert.equal(entry.regions, entry.coveredRegions); + // The drifted half is dropped, not counted as covered: the denominator + // loses it too, so the file cannot be credited for a region nobody saw. + assert.equal(entry.regions, 3); + } finally { + await rm(fixture.root, { recursive: true, force: true }); + } +}); + // `npm run build:e2e:coverage` compiles the site twice: once from the real // data, and once with tests/e2e/fixtures/data-variants/** layered on, into // build/e2e-coverage-variant under its own base URL. That is what makes a diff --git a/tests/tools/e2e-coverage-report.mjs b/tests/tools/e2e-coverage-report.mjs index bbd07173..b09314ff 100644 --- a/tests/tools/e2e-coverage-report.mjs +++ b/tests/tools/e2e-coverage-report.mjs @@ -341,6 +341,56 @@ function isPhantomRegion(region, lines) { return true; } +function comparePositions(lineA, columnA, lineB, columnB) { + if (lineA !== lineB) return lineA < lineB ? -1 : 1; + if (columnA !== columnB) return columnA < columnB ? -1 : 1; + return 0; +} + +// Two branch spans of one source file either nest or stay disjoint. Both are +// derived from AST node extents, so one cannot begin inside another and end +// after it -- a *crossing* pair is not a shape the language can produce. When +// the union does hold one, the two halves were mapped through source maps that +// placed the same original branch at different spans, which is exactly how +// #1079's drifted twins appear: `MemberProfile.js` contributes a zero region +// at `10:40-10:48` from one artifact and a covered `10:44-15:19` from another +// artifact of the *same* chunk, and `15:19-15:80` against `15:70-38:15` +// likewise. Both pairs cross; neither contains the other, so neither #1051's +// multi-line fold nor an enclosure rule reaches them. +// +// Crossing is the whole test, and it is deliberately narrower than +// containment. A covered region that *encloses* a zero one is the ordinary +// shape of a branch arm inside an executed block -- GroupLinkStatus's +// `checkedAt` guard and RadarReports' empty-corpus arms are enclosed exactly +// that way -- so enclosure is never treated as drift. +function crossesRegion(zero, covered) { + return ( + comparePositions(covered.line, covered.column, zero.line, zero.column) > + 0 && + comparePositions( + covered.line, + covered.column, + zero.endLine, + zero.endColumn, + ) < 0 && + comparePositions( + covered.endLine, + covered.endColumn, + zero.endLine, + zero.endColumn, + ) > 0 + ); +} + +function isDriftedRegion(region, regions) { + if (region.count > 0) return false; + for (const other of regions) { + if (other.count <= 0) continue; + if (crossesRegion(region, other)) return true; + } + return false; +} + // A region is one branch location from the istanbul object convertScript // already builds -- the arm of a ternary, a short-circuit operand, a default // parameter -- data the line map cannot see because several of them share a @@ -356,16 +406,13 @@ function getRegionCoverage(coverageData) { const line = location?.start?.line; if (!Number.isInteger(line)) continue; const endLine = location.end?.line ?? line; - const key = [ - line, - location.start?.column ?? 0, - endLine, - location.end?.column ?? 0, - ].join(':'); + const column = location.start?.column ?? 0; + const endColumn = location.end?.column ?? 0; + const key = [line, column, endLine, endColumn].join(':'); const count = counts?.[index] ?? 0; const existing = regions.get(key); if (!existing || count > existing.count) { - regions.set(key, { line, endLine, count }); + regions.set(key, { line, column, endLine, endColumn, count }); } } } @@ -582,8 +629,11 @@ export async function collectE2ECoverage( const coveredLines = [...coverage.lines.values()].filter( (count) => count > 0, ).length; - const regionValues = [...coverage.regions.values()].filter( - (region) => !isPhantomRegion(region, coverage.lines), + const allRegions = [...coverage.regions.values()]; + const regionValues = allRegions.filter( + (region) => + !isPhantomRegion(region, coverage.lines) && + !isDriftedRegion(region, allRegions), ); const regions = regionValues.length; const coveredRegions = regionValues.filter(