Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 55 additions & 41 deletions tests/coverage-gate-thresholds.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -66,23 +66,25 @@ const FLOORS = {
// Measured on the suite this commit ships: harness files 100.00% lines /
// 99.79% regions (5742/5742 lines, 1938/1942 regions), taken twice -- on
// node v22.23.3, the major ci.yml pins, and on node v26.10.0 -- with the two
// runs agreeing to the region. tests/tools/e2e-coverage-report.mjs is the
// only harness file below 100.00%; every other one is at 100.00%.
// runs agreeing to the region. The harness files below 100.00% are
// tests/tools/e2e-coverage-report.mjs and tests/tools/e2e-coverage-run.mjs;
// every other one is at 100.00%.
//
// That file's percentage is its ceiling, not a backlog: its uncovered
// regions are the defensive fallbacks the note at the top of the file lists
// as unreachable, so no test can lift it. The per-file floor must stay at
// or below that ceiling until the list shrinks; a probe that lands one of
// them should raise the floor and shorten the list in the same change.
// Those percentages are ceilings, not a backlog: the uncovered regions in
// each are the defensive fallbacks the residual note at the top of that
// file lists as unreachable, so no test can lift them. The per-file floor
// must stay at or below the lowest of those ceilings until a list shrinks;
// a probe that lands one of them should raise the floor and shorten that
// file's note in the same change.
//
// The ceiling itself is deliberately not written out here. It is asserted
// below against the note instead, because every attempt to restate it in
// this comment has gone stale while the note moved on: the comment said "6
// uncovered regions" where the note said 7, and it later said the file was
// capped at 98.57% -- the 7-region figure -- after three of them had been
// deleted and the note had been rewritten to 4 regions. A second test below
// now fails on that shape directly, so the next restatement cannot drift
// silently either.
// The ceilings themselves are deliberately not written out here. They are
// asserted below against the notes instead, because every attempt to restate
// one in this comment has gone stale while the note moved on: the comment
// said "6 uncovered regions" where the note said 7, and it later said the
// file was capped at 98.57% -- the 7-region figure -- after three of them
// had been deleted and the note had been rewritten to 4 regions. A second
// test below now fails on that shape directly, so the next restatement
// cannot drift silently either.
//
// Both harness floors moved up with this commit. They were previously held
// at 97 by tests/helpers-script-sandbox.mjs, whose single uncovered region
Expand All @@ -109,13 +111,20 @@ function parseGate(command) {

// The `--check-harness-file-regions` floor is the only one in this file that
// is not set by the suite's measurement but by another file's *claim*: the
// residual note at the top of tests/tools/e2e-coverage-report.mjs enumerates
// the fallbacks no probe could close, totals them, and states the percentage
// they cap that file at. The comment above read that total back by hand and
// went stale -- it said 6 while the note said 7 -- which is the same
// drift-by-omission tests/docs-threshold-parity.test.mjs closes between the
// gate and the contributor docs. Reading it instead of restating it is what
// keeps the two in step.
// residual note at the top of a harness module enumerates the fallbacks no
// probe could close, totals them, and states the percentage they cap that
// file at. The comment above read that total back by hand and went stale --
// it said 6 while the note said 7 -- which is the same drift-by-omission
// tests/docs-threshold-parity.test.mjs closes between the gate and the
// contributor docs. Reading it instead of restating it is what keeps the two
// in step.
//
// Every harness file that carries such a note is read, not just the lowest
// one of the day. A floor argued from one file alone silently stops being
// argued at all the moment a probe closes that file's residuals: the next
// raise would clear the file it was measured against and land above a ceiling
// nothing here had ever looked at. The lowest ceiling among the notes is the
// one the floor has to clear.
// Matched across the comment's own line wrapping: the note is prose in a `//`
// block, so any word boundary in it may carry a newline and a `// ` prefix.
const RESIDUAL_NOTE = new RegExp(
Expand All @@ -124,47 +133,52 @@ const RESIDUAL_NOTE = new RegExp(
'they',
'are',
'(\\d+)',
'regions,',
// Singular for a note that is down to its last one.
'regions?,',
'which',
'caps',
'this',
'file',
'at',
'(\\d+(?:\\.\\d+)?)%',
'regions',
'regions?',
].join('(?:\\s*\\n\\s*//)?\\s+'),
'u',
);
const RESIDUAL_FILE = 'tests/tools/e2e-coverage-report.mjs';
const RESIDUAL_FILES = [RESIDUAL_FILE, 'tests/tools/e2e-coverage-run.mjs'];

function residualCeiling() {
const source = readFileSync(`${repoRoot}${RESIDUAL_FILE}`, 'utf8');
function residualCeiling(file = RESIDUAL_FILE) {
const source = readFileSync(`${repoRoot}${file}`, 'utf8');
const match = RESIDUAL_NOTE.exec(source);
assert.ok(
match,
`${RESIDUAL_FILE} must keep its residual note in the form "Together they are ` +
`${file} must keep its residual note in the form "Together they are ` +
'N regions, which caps this file at P% regions"; the --check-harness-file-regions ' +
'floor below is argued from that sentence, so a rewrite that drops it leaves the ' +
'floor with nothing behind it',
);
return { regions: Number(match[1]), ceiling: Number(match[2]) };
}

test(`${RESIDUAL_FILE} still caps the harness per-file floor where this file says`, () => {
const { regions, ceiling } = residualCeiling();
assert.ok(
regions > 0,
`${RESIDUAL_FILE} now lists no residual regions, so it is no longer the lowest ` +
'harness file; re-measure the harness row and raise --check-harness-file-regions',
);
test('the harness per-file floor clears every documented residual ceiling', () => {
const floor = FLOORS['--check-harness-file-regions'];
assert.ok(
floor <= Math.floor(ceiling),
`--check-harness-file-regions is floored at ${floor}, above the ${ceiling}% ceiling ` +
`${RESIDUAL_FILE} documents for its ${regions} unreachable regions; the gate can ` +
'only be raised past that by closing one of them and shortening the note in the ' +
'same change',
);
for (const file of RESIDUAL_FILES) {
const { regions, ceiling } = residualCeiling(file);
assert.ok(
regions > 0,
`${file} now lists no residual regions, so it no longer caps the harness ` +
'per-file gate; drop it from RESIDUAL_FILES, re-measure the harness row, ' +
'and raise --check-harness-file-regions',
);
assert.ok(
floor <= Math.floor(ceiling),
`--check-harness-file-regions is floored at ${floor}, above the ${ceiling}% ceiling ` +
`${file} documents for its ${regions} unreachable regions; the gate can ` +
'only be raised past that by closing one of them and shortening the note in the ' +
'same change',
);
}
});

// Any percentage this file attributes to the residual file by hand, with the
Expand Down
122 changes: 122 additions & 0 deletions tests/e2e-coverage-run-residual-arm.test.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
// The residual note at the top of tests/tools/e2e-coverage-run.mjs names one
// arm no probe could reach:
//
// readProvenance `if (!(error instanceof SyntaxError)) throw error`
//
// It is dead for a reason that lives entirely outside the arm. The `try` it
// sits under wraps a single call, `JSON.parse(raw)`; `raw` is bound from a
// `'utf8'` read, so it is always a string; and `JSON.parse` of a string throws
// nothing but `SyntaxError`. Each of those is a claim about code that can
// change without the arm changing -- widen the `try` over the `readFile`, or
// over a validator called on the parsed payload, and the re-throw goes live in
// an edit that never touches its line.
//
// An unreachable arm asserted by nothing is indistinguishable from an arm
// whose fixture is merely missing, so these tests state the premise directly,
// the way tests/e2e-coverage-error-arms.test.mjs states the premises behind
// the reporter's residuals. The consequence of getting it wrong is not only
// the region count: the re-throw abandons the whole provenance walk, so one
// unexpected failure on one capture file would lose the runtime records of
// every capture after it, where the `SyntaxError` path deliberately degrades
// to a warning and continues.
//
// The reachable half of the same `catch` -- the warning a malformed capture
// produces -- is exercised against the real function by
// tests/e2e-coverage-runtime.test.mjs and is not repeated here.
import assert from 'node:assert/strict';
import test from 'node:test';
import { readFile } from 'node:fs/promises';
import { fileURLToPath } from 'node:url';

const RUN_SOURCE = fileURLToPath(
new URL('./tools/e2e-coverage-run.mjs', import.meta.url),
);

// `readProvenance`'s body, from its signature to the closing brace in the
// first column. Every function in the module is written at top level, so that
// brace is unambiguous without parsing.
async function readProvenanceSource() {
const source = await readFile(RUN_SOURCE, 'utf8');
const start = source.indexOf('async function readProvenance(');
assert.notEqual(
start,
-1,
'tests/tools/e2e-coverage-run.mjs no longer declares readProvenance at top ' +
'level; the residual note at the top of that file names the arm by this ' +
'function, so rename the note with the function',
);
const end = source.indexOf('\n}\n', start);
assert.notEqual(end, -1, 'readProvenance has no top-level closing brace');
return source.slice(start, end);
}

test('readProvenance guards nothing but the JSON.parse call', async () => {
const body = await readProvenanceSource();
const guarded = /\btry\s*\{([\s\S]*?)\}\s*catch\s*\(/u.exec(body);
assert.ok(guarded, 'readProvenance still guards its parse with try/catch');
const statements = guarded[1]
.split('\n')
.map((line) => line.trim())
.filter((line) => line !== '' && !line.startsWith('//'));
assert.deepEqual(
statements,
['payload = JSON.parse(raw);'],
'the try block in readProvenance now guards more than the JSON.parse call, ' +
'so its catch can see an error JSON.parse did not throw and the ' +
'`throw error` arm the residual note calls unreachable is live; either ' +
'narrow the try back or cover the arm and shorten the note',
);
});

test('the argument readProvenance parses is always a string', async () => {
const body = await readProvenanceSource();
assert.match(
body,
/const raw = await readFile\([\s\S]*?,\s*'utf8'\);/u,
"readProvenance no longer binds `raw` from a 'utf8' read; a Buffer or a " +
'non-string argument widens what JSON.parse can throw past SyntaxError, ' +
'which is the premise the residual note rests on',
);
// The same binding must stay outside the guard, or the read's own failures
// reach a catch that only knows how to describe a parse failure.
assert.ok(
body.indexOf('const raw = await readFile(') < body.indexOf('try {'),
'the `raw` binding moved inside the try block in readProvenance; a read ' +
'error would then reach the catch, which is exactly what the ' +
'`throw error` arm re-throws',
);
});

// Empirical half: the residual note claims no probe could make `JSON.parse`
// fail with anything but a SyntaxError. These are the probes.
const HOSTILE_INPUTS = new Map([
['empty', ''],
['truncated object', '{"runId":'],
['truncated array', '[1,2,'],
['trailing comma', '{"a":1,}'],
['lone surrogate', '"\uD800"'],
['NUL bytes', '\u0000\u0000\u0000'],
['a bare identifier', 'undefined'],
['deeply nested arrays', `${'['.repeat(200000)}1${']'.repeat(200000)}`],
['deeply nested objects', `${'{"a":'.repeat(200000)}1${'}'.repeat(200000)}`],
['unterminated deep nesting', '['.repeat(200000)],
]);

test('JSON.parse of a string throws only SyntaxError', () => {
for (const [name, input] of HOSTILE_INPUTS) {
let thrown = null;
try {
JSON.parse(input);
} catch (error) {
thrown = error;
}
if (thrown === null) continue;
assert.ok(
thrown instanceof SyntaxError,
`JSON.parse threw a ${thrown?.constructor?.name} for ${name}; the ` +
'`throw error` arm in readProvenance is reachable after all, so cover ' +
'it and shorten the residual note at the top of ' +
'tests/tools/e2e-coverage-run.mjs',
);
}
});
28 changes: 28 additions & 0 deletions tests/tools/e2e-coverage-run.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,34 @@ import { captureRunScripts } from './e2e-coverage-scripts.mjs';

const repoRoot = fileURLToPath(new URL('../..', import.meta.url));

// Residual uncovered regions, recorded the way tests/tools/e2e-coverage-report.mjs
// records its own so this file's region percentage is not re-investigated every
// cycle. Named by function and expression rather than by line, so the note does
// not go stale when the file moves:
//
// readProvenance `if (!(error instanceof SyntaxError)) throw error` -- the
// re-throw arm needs `JSON.parse` to fail with something other than a
// `SyntaxError`. The `try` wraps that one call and nothing else, and
// `raw` is bound from a `'utf8'` read, so the argument is always a
// string; `JSON.parse` of a string throws only `SyntaxError`. The
// `SyntaxError` half of the same `catch` is reached by
// tests/e2e-coverage-runtime.test.mjs.
//
// That premise is a claim about the code around the arm, not about the arm: a
// later edit that widens the `try` -- over the `readFile`, or over a validator
// -- makes the re-throw live without touching the line. It is asserted
// directly by tests/e2e-coverage-run-residual-arm.test.mjs, the way
// tests/e2e-coverage-error-arms.test.mjs asserts the premises behind the
// reporter's residuals, so a change that makes the arm reachable fails on its
// own name rather than merely moving the region count. Together they are 1
// region, which caps this file at 99.15% regions;
// tests/coverage-gate-thresholds.test.mjs reads that ceiling beside
// `--check-harness-file-regions` so the floor is never set above it. The
// denominator is V8's, so it drifts a little with the Node major -- 1/117 was
// measured on Node 26 while CI pins Node 22 -- which is why the floor sits
// below the ceiling rather than against it. Close the arm and shorten this
// note in the same change; do not raise the floor past it alone.

export const COVERAGE_RUN_SCHEMA_VERSION = 1;
export const COVERAGE_RUN_KIND = 'endusers.e2e.coverage-run';
export const COVERAGE_ARTIFACT_KIND = 'endusers.playwright.v8-coverage';
Expand Down
Loading