From f20857772c4ca22b69736995de8935a93552dee5 Mon Sep 17 00:00:00 2001 From: operator Date: Mon, 24 Aug 2026 08:44:18 +0900 Subject: [PATCH] A guard whose test was renamed away reported the same summary as a healthy one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `vitest run -t ` exits 0 when the name matches nothing. It skips every test in the file and reports success. The ratchet read only that exit code, so a registration pointing at a test that no longer exists was indistinguishable from a mutation nothing reacted to, and both came back `inert`. Renaming a test is routine. A/B on the same broken registry, with the test name of an inert-baselined guard changed to one that does not resolve: old exit 0 64 bound, 1 inert, 1 unavailable, 13 uncovered no failures new exit 1 64 bound, 0 inert, 1 unresolved, 1 unavailable REGISTRATION DEFECT The old summary is character-for-character what a healthy run prints. Nothing anywhere said the guard had stopped being checked. The second gap has already cost a session. Yesterday four v6 guards were rejected and two were reported `inert`; applying those mutations by hand failed a test both times, a different test than the registry named. The ratchet's message says the property has no test defending it, but what it observes is that the *named* test did not fail — the same output for an uncovered guard and for a misfiled one, and the misfiled one looks fine on inspection because the mutation is real and the test is real. So the reading now distinguishes four things where it used to see two: bound the registered test failed misfiled it passed, but the mutation failed some other test — named unresolved the registered name matched nothing, so nothing was measured inert the mutation ran and no test in the file failed `misfiled` and `unresolved` are not coverage gaps, they are broken registrations, so `bench/cdeb/guards/baseline.json` may not hold them and measuring one fails on sight. A recorded gap says how far coverage reaches and can be carried with a reason; a broken registration says the recorded coverage cannot be checked at all, and ratcheting that in defeats the baseline. The unfiltered second run costs one extra Vitest process, and only on the path where the named test survived — two of sixty-six here. Bound mutations, the common case, still cost one. The reading moved to `scripts/guard-outcomes.mjs` because the ratchet spawns a Vitest process per mutation and cannot be exercised from inside the suite it runs. The decision table is pure, so `test/guard-ratchet-outcomes.test.ts` covers it in two milliseconds. Removing the `unresolved` branch fails two of those tests and removing `misfiled` fails a third. Record-Id: r-ratchetdefects Provenance: authored Certainty: firm Blast: system Undo: easy Ruled-out: always running the file unfiltered | it doubles the cost of every bound mutation to sharpen a diagnosis that only matters when the named test survived Ruled-out: recording misfiled and unresolved in the baseline like the other gaps | the baseline records how far coverage reaches, and a registration that cannot be checked has no reach to record; carrying one would make the guard look accounted for Ruled-out: parsing the test file for the registered name | describe blocks compose names at runtime, so a static read would reject valid registrations and accept a name that only appears to exist Ruled-out: exporting the decision table from the ratchet itself | importing that script runs it, so the test would spawn sixty-six Vitest processes Limit: the whole-file run attributes any failure to the mutation, which holds because the clean tree is green but would misread a flaky test as misfiled. That is a loud wrong answer rather than a silent one Limit: this repairs how a mutation is read and repairs no guard. Thirteen exclusion-index properties remain uncovered and one scan remains genuinely inert, unchanged Limit: nothing here checks that a registered claim matches what its test asserts. A mutation can be bound to the right test for the wrong property and this still reads bound Verified: A/B recorded above, both directions, same registry; guard-mutations exits 0 on the clean tree with 64 bound, 1 inert, 1 unavailable, 13 uncovered — identical to before the change; injected unresolved and misfiled registrations both produce REGISTRATION DEFECT and exit 1; vitest 3744 passed, 13 skipped, 0 failed; tsc --noEmit clean on the root and bench tsconfigs --- scripts/guard-mutations.mjs | 128 ++++++++++++++++++++-------- scripts/guard-outcomes.mjs | 51 +++++++++++ test/guard-ratchet-outcomes.test.ts | 86 +++++++++++++++++++ 3 files changed, 229 insertions(+), 36 deletions(-) create mode 100644 scripts/guard-outcomes.mjs create mode 100644 test/guard-ratchet-outcomes.test.ts diff --git a/scripts/guard-mutations.mjs b/scripts/guard-mutations.mjs index 8991276a..7f436e94 100644 --- a/scripts/guard-mutations.mjs +++ b/scripts/guard-mutations.mjs @@ -11,6 +11,13 @@ import { copyFileSync, existsSync, mkdtempSync, readFileSync, rmSync, writeFileS import { spawnSync } from "node:child_process"; import { tmpdir } from "node:os"; import { dirname, isAbsolute, relative, resolve } from "node:path"; +import { + ALL_OUTCOMES, + BASELINE_OUTCOMES, + REGISTRATION_DEFECTS, + classifyRun, + severestOutcome, +} from "./guard-outcomes.mjs"; const ROOT = resolve(dirname(new URL(import.meta.url).pathname), ".."); const REGISTRY_PATH = resolve(ROOT, "bench/cdeb/guards/registry.json"); @@ -75,7 +82,7 @@ const readRegistry = () => { }); }; -const OUTCOMES = new Set(["bound", "inert", "unavailable", "uncovered"]); +const OUTCOMES = BASELINE_OUTCOMES; const readBaseline = () => { const parsed = JSON.parse(readFileSync(BASELINE_PATH, "utf8")); @@ -131,18 +138,59 @@ const applyMutation = (mutation) => { } }; -const runTest = (testFile, testName) => - spawnSync("npx", ["vitest", "run", testFile, "-t", testName], { - cwd: ROOT, - encoding: "utf8", - maxBuffer: 16 * 1024 * 1024, - }); +let runCounter = 0; + +// Returns how many tests actually executed, not just the exit code. `vitest run +// -t ` exits 0 when the name matches nothing -- it skips the whole file and +// reports success -- so the exit code alone cannot tell a mutation nothing +// reacted to from a test name that no longer resolves. Renaming a test is +// routine, and under the old reading that silently downgraded its guard. +const runTest = (testFile, testName) => { + runCounter += 1; + const outputFile = resolve(backupRoot, `vitest-${String(runCounter)}.json`); + const args = ["vitest", "run", testFile, "--reporter=json", `--outputFile=${outputFile}`]; + if (testName !== null) args.push("-t", testName); + const spawned = spawnSync("npx", args, { cwd: ROOT, encoding: "utf8", maxBuffer: 16 * 1024 * 1024 }); + if (spawned.error !== undefined) return { started: false, reason: spawned.error.message }; + if (!existsSync(outputFile)) { + return { started: false, reason: `vitest exited ${String(spawned.status)} without writing a report` }; + } + let report; + try { + report = JSON.parse(readFileSync(outputFile, "utf8")); + } catch (error) { + return { started: false, reason: `vitest report was not readable JSON: ${error instanceof Error ? error.message : String(error)}` }; + } + const assertions = Array.isArray(report.testResults) + ? report.testResults.flatMap((file) => (Array.isArray(file.assertionResults) ? file.assertionResults : [])) + : []; + const executed = assertions.filter((assertion) => assertion.status !== "skipped" && assertion.status !== "pending"); + const failed = executed.filter((assertion) => assertion.status === "failed"); + return { + started: true, + executed: executed.length, + failed: failed.length, + failedNames: failed.map((assertion) => assertion.fullName ?? assertion.title ?? "(unnamed test)"), + }; +}; + +const describe = (outcome, mutation, property, named, filtered, whole) => { + if (outcome === "unavailable") { + return `${mutation.id}: Vitest could not start — ${filtered.reason}; ${mutation.why}`; + } + if (outcome === "unresolved") { + return `${mutation.id}: no test in ${property.testFile} matches "${named}", so nothing was run; the registration names a test that does not exist`; + } + if (outcome === "bound") return `${mutation.id}: mutation applied, test failed — ${mutation.why}`; + if (outcome === "misfiled") { + return `${mutation.id}: "${named}" passed, but the mutation failed ${whole.failedNames.join(", ")}; register it against the test that actually fails`; + } + return `${mutation.id}: mutation applied, no test in ${property.testFile} failed — ${mutation.why}`; +}; const measurements = []; let total = 0; -let boundControls = 0; -let inertControls = 0; -let unavailableControls = 0; +const controlTally = new Map(ALL_OUTCOMES.map((outcome) => [outcome, 0])); try { for (const property of readRegistry()) { @@ -156,39 +204,34 @@ try { } const controlOutcomes = []; const details = []; + const record = (outcome, detail) => { + controlTally.set(outcome, controlTally.get(outcome) + 1); + controlOutcomes.push(outcome); + details.push(detail); + }; for (const mutation of property.mutations) { total += 1; const applied = applyMutation(mutation); if (!applied.applied) { - unavailableControls += 1; - controlOutcomes.push("unavailable"); - details.push(`${mutation.id}: mutation could not be applied — ${applied.reason}; ${mutation.why}`); + record("unavailable", `${mutation.id}: mutation could not be applied — ${applied.reason}; ${mutation.why}`); continue; } try { - const result = runTest(property.testFile, mutation.testName ?? property.testName); - if (result.error !== undefined) { - unavailableControls += 1; - controlOutcomes.push("unavailable"); - details.push(`${mutation.id}: Vitest could not start — ${result.error.message}; ${mutation.why}`); - } else if (result.status !== 0) { - boundControls += 1; - controlOutcomes.push("bound"); - details.push(`${mutation.id}: mutation applied, test failed — ${mutation.why}`); - } else { - inertControls += 1; - controlOutcomes.push("inert"); - details.push(`${mutation.id}: mutation applied, test passed — ${mutation.why}`); - } + const named = mutation.testName ?? property.testName; + const filtered = runTest(property.testFile, named); + // The unfiltered run is only needed to tell misfiled from inert, and the + // clean tree is green, so a failure in it is caused by the mutation. + const whole = + filtered.started && filtered.executed > 0 && filtered.failed === 0 + ? runTest(property.testFile, null) + : undefined; + const outcome = classifyRun(filtered, whole); + record(outcome, describe(outcome, mutation, property, named, filtered, whole)); } finally { restoreActive(); } } - const outcome = controlOutcomes.includes("unavailable") - ? "unavailable" - : controlOutcomes.includes("inert") - ? "inert" - : "bound"; + const outcome = severestOutcome(controlOutcomes); measurements.push({ ...property, outcome, detail: details.join("; ") }); } } finally { @@ -197,24 +240,37 @@ try { } const baseline = readBaseline(); -const byOutcome = new Map([...OUTCOMES].map((outcome) => [outcome, []])); +const byOutcome = new Map(ALL_OUTCOMES.map((outcome) => [outcome, []])); for (const measurement of measurements) byOutcome.get(measurement.outcome).push(measurement); process.stdout.write("OUTCOME TABLE:\n"); -for (const outcome of ["bound", "inert", "unavailable", "uncovered"]) { +for (const outcome of ["bound", "misfiled", "unresolved", "inert", "unavailable", "uncovered"]) { const rows = byOutcome.get(outcome); process.stdout.write(`${outcome.toUpperCase()} (${String(rows.length)}):\n`); for (const row of rows) { - const baselineReason = baseline.get(row.guardId)?.reason; + // A registration defect has no legitimate baseline entry, so its own detail + // is the only account of it; for the recorded gaps the baseline reason is + // the considered one and supersedes the generated line. + const baselineReason = REGISTRATION_DEFECTS.has(outcome) ? undefined : baseline.get(row.guardId)?.reason; const suffix = baselineReason === undefined ? row.detail : baselineReason; process.stdout.write(` ${row.guardId}: ${row.claim} — ${suffix}\n`); } } -process.stdout.write(`CONTROL SUMMARY: ${String(boundControls)} bound, ${String(inertControls)} inert, ${String(unavailableControls)} unavailable, ${String(byOutcome.get("uncovered").length)} uncovered, ${String(total)} mutations run\n`); +const tallyText = ["bound", "misfiled", "unresolved", "inert", "unavailable"] + .map((outcome) => `${String(controlTally.get(outcome))} ${outcome}`) + .join(", "); +process.stdout.write(`CONTROL SUMMARY: ${tallyText}, ${String(byOutcome.get("uncovered").length)} uncovered, ${String(total)} mutations run\n`); const failures = []; const measuredIds = new Set(measurements.map((measurement) => measurement.guardId)); for (const measurement of measurements) { + // A registration defect fails on sight and is never reconciled against the + // baseline. Recording one would ratchet in a guard whose stated coverage + // cannot be checked -- exactly the state the baseline exists to make visible. + if (REGISTRATION_DEFECTS.has(measurement.outcome)) { + failures.push(`REGISTRATION DEFECT: ${measurement.guardId}: ${measurement.detail}`); + continue; + } const expected = baseline.get(measurement.guardId); if (expected === undefined) { if (measurement.outcome === "uncovered") { diff --git a/scripts/guard-outcomes.mjs b/scripts/guard-outcomes.mjs new file mode 100644 index 00000000..26829878 --- /dev/null +++ b/scripts/guard-outcomes.mjs @@ -0,0 +1,51 @@ +/** + * How a mutation run is read, kept apart from the runner that produces it. + * + * The ratchet spawns one Vitest process per mutation, so the only way to test + * its reading inside the suite is to separate the reading from the running. + * Everything here is pure. + */ + +// bound, inert, unavailable and uncovered are states a baseline may record: they +// describe how far coverage reaches, and a known gap can be carried. +export const BASELINE_OUTCOMES = new Set(["bound", "inert", "unavailable", "uncovered"]); + +// These two are not gaps, they are broken registrations, so the baseline may not +// hold them and measuring one always fails. +export const REGISTRATION_DEFECTS = new Set(["misfiled", "unresolved"]); + +export const ALL_OUTCOMES = [...BASELINE_OUTCOMES, ...REGISTRATION_DEFECTS]; + +// Worst first. unresolved leads because nothing was measured at all, so the +// property is neither shown defended nor shown undefended. inert outranks +// misfiled because inert means no test anywhere reacted, whereas misfiled means +// the property is defended and only the name recorded against it is wrong. +export const OUTCOME_SEVERITY = ["unresolved", "unavailable", "inert", "misfiled", "bound"]; + +/** + * Read one mutation from its runs. + * + * `named` is the run filtered to the registered test name; `whole` is the same + * file unfiltered and is only consulted when the named test survived. + * + * The `executed === 0` branch is the one that matters. `vitest run -t ` + * exits 0 when the name matches nothing -- it skips the whole file and reports + * success -- so an exit code alone cannot separate a mutation nothing reacted to + * from a test name that no longer resolves. Renaming a test is routine, and + * under that reading its guard degrades to inert without anything saying so. + */ +export const classifyRun = (named, whole) => { + if (!named.started) return "unavailable"; + if (named.executed === 0) return "unresolved"; + if (named.failed > 0) return "bound"; + // The named test survived. Before calling the property undefended, ask whether + // anything else in the file reacted: a mutation registered against the wrong + // test looks identical to one nothing catches, and only the second is a + // coverage gap. + if (whole !== undefined && whole.started && whole.failed > 0) return "misfiled"; + return "inert"; +}; + +/** A property is represented by the worst outcome among its mutations. */ +export const severestOutcome = (outcomes) => + OUTCOME_SEVERITY.find((candidate) => outcomes.includes(candidate)) ?? "bound"; diff --git a/test/guard-ratchet-outcomes.test.ts b/test/guard-ratchet-outcomes.test.ts new file mode 100644 index 00000000..46766222 --- /dev/null +++ b/test/guard-ratchet-outcomes.test.ts @@ -0,0 +1,86 @@ +import { describe, expect, it } from "vitest"; + +import { + ALL_OUTCOMES, + BASELINE_OUTCOMES, + OUTCOME_SEVERITY, + REGISTRATION_DEFECTS, + classifyRun, + severestOutcome, + // @ts-expect-error -- the ratchet and its helpers are plain ESM, not typed sources +} from "../scripts/guard-outcomes.mjs"; + +const ran = (failed: number) => ({ started: true, executed: 1, failed, failedNames: [] as string[] }); +const failing = (...names: string[]) => ({ started: true, executed: names.length, failed: names.length, failedNames: names }); +const nothingMatched = { started: true, executed: 0, failed: 0, failedNames: [] as string[] }; +const crashed = { started: false, reason: "vitest exited 1 without writing a report" }; + +describe("how the mutation ratchet reads a run", () => { + it("calls a mutation bound when the registered test fails", () => { + expect(classifyRun(ran(1), undefined)).toBe("bound"); + }); + + it("calls a mutation inert when the registered test and every other test survive", () => { + expect(classifyRun(ran(0), ran(0))).toBe("inert"); + }); + + it("does not call a mutation inert when a test other than the registered one fails", () => { + // The failure this exists to stop: a real mutation and a real test, paired + // wrongly. Reading it as inert reports the property undefended when it is + // defended, and points the repair at the mutation instead of the name. + expect(classifyRun(ran(0), failing("holds the floors at the values the preregistration fixed"))).toBe("misfiled"); + }); + + it("does not call a mutation inert when the registered name matches no test", () => { + // `vitest run -t ` skips the whole file and exits 0 when nothing + // matches, so an exit code alone reads a renamed test as a mutation nothing + // reacted to. Renaming a test is routine; its guard must not go quiet. + expect(classifyRun(nothingMatched, undefined)).toBe("unresolved"); + }); + + it("does not consult the unfiltered run before the registered name has resolved", () => { + // Whatever else is failing, an unresolved name was not measured, so the + // unfiltered run cannot upgrade it to a statement about coverage. + expect(classifyRun(nothingMatched, failing("some other test"))).toBe("unresolved"); + }); + + it("reports a run that never started as unavailable rather than as a gap", () => { + expect(classifyRun(crashed, undefined)).toBe("unavailable"); + }); +}); + +describe("which outcome represents a property", () => { + it("takes the worst outcome among a property's mutations", () => { + expect(severestOutcome(["bound", "inert", "bound"])).toBe("inert"); + expect(severestOutcome(["bound", "misfiled"])).toBe("misfiled"); + expect(severestOutcome(["inert", "unresolved"])).toBe("unresolved"); + }); + + it("ranks every outcome it can be handed", () => { + for (const outcome of ALL_OUTCOMES.filter((name: string) => name !== "uncovered")) { + expect(OUTCOME_SEVERITY).toContain(outcome); + } + }); + + it("reports bound only when nothing worse was measured", () => { + expect(severestOutcome(["bound", "bound"])).toBe("bound"); + expect(severestOutcome([])).toBe("bound"); + }); +}); + +describe("what a baseline may record", () => { + it("refuses to treat a broken registration as a carryable gap", () => { + // bound/inert/unavailable/uncovered say how far coverage reaches and can be + // carried with a reason. misfiled and unresolved say the registration itself + // is wrong, and recording one would ratchet in a guard whose stated coverage + // cannot be checked. + for (const defect of REGISTRATION_DEFECTS) { + expect(BASELINE_OUTCOMES.has(defect)).toBe(false); + } + }); + + it("keeps the two sets disjoint and complete", () => { + expect(ALL_OUTCOMES.length).toBe(BASELINE_OUTCOMES.size + REGISTRATION_DEFECTS.size); + expect(new Set(ALL_OUTCOMES).size).toBe(ALL_OUTCOMES.length); + }); +});