From e3c8734d3a9eeaae5791bd5a1cc6179413603675 Mon Sep 17 00:00:00 2001 From: TurtleWolfe Date: Thu, 20 Aug 2026 05:18:49 +0000 Subject: [PATCH] feat(#396): name every test that finished having run zero assertions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #396's last unticked item, and the one it calls highest-value: "a spec that runs zero assertions is visibly different from one that runs twelve". Every instance in that catalogue shares one symptom — a green result that measured nothing — and every one was found by a person becoming suspicious of a specific test. This makes the symptom visible without anyone having to suspect anything first. Not hypothetical. Two specs found on 2026-08-20 guarded every assertion behind `if (elements.length > 0)` while visiting a page with none of those elements (#842); fixing them immediately exposed a real 40px touch target on production. Both would have printed here as zero-assertion tests the first time they ran. WHY A REPORTER AND NOT THE JSON OUTPUT. Checked before building: the `json` reporter's result objects carry annotations, attachments, duration, errors, status and stdout — and NO `steps`. Assertion counts are reachable only through the reporter API's step callbacks, so post-processing `results.json` cannot do this. IT ONLY PRINTS, deliberately, following this repo's own pattern of landing a gate in annotate mode first (E2E_BUDGET_MODE, FLAKY_GATE_MODE). A zero-assertion test is not always a defect — a spec may assert via `toPass`, a fixture, or a thrown helper — so failing on it today would redden the REQUIRED lane for reasons nobody has triaged. Turn it into a gate once the list is known and empty. Measured on a real batch before wiring it in: 19 passing tests across the mobile and colorblind specs, all of which asserted. The suite is in better shape than the catalogue implies, largely because #843 fixed the two that were not — so this ships producing a clean signal rather than a backlog of noise. THE REPORTER REFUSES TO GIVE A CLEAN BILL OF HEALTH HAVING OBSERVED NOTHING. If a run sees no tests — a bad shard filter, a crashed setup — it says so rather than printing "all good". A tool built to detect this family committing it would be its own #396 entry, and it is asserted, not just intended. Mutation-verified, mutant confirmed present in the file first, three ways: counting every step instead of only `expect` steps, disabling the observed-nothing guard, and not recording zeros so silent tests vanish from the map. Each is caught. Stated in the file so a green run is not over-read: it cannot see assertions made by a raw `throw` inside a helper, and a nonzero count does not prove a test asserts anything USEFUL. vitest 4724/4724 (447 files); test:scripts 424/424; type-check and lint clean. Refs #396 --- playwright.config.ts | 5 + .../e2e/reporters/assertion-count-reporter.ts | 97 ++++++++++++++++ tests/unit/assertion-count-reporter.test.ts | 105 ++++++++++++++++++ 3 files changed, 207 insertions(+) create mode 100644 tests/e2e/reporters/assertion-count-reporter.ts create mode 100644 tests/unit/assertion-count-reporter.test.ts diff --git a/playwright.config.ts b/playwright.config.ts index a11c6a77..9bdf362f 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -93,6 +93,11 @@ export default defineConfig({ process.env.CI ? ['github'] : ['line'], ['json', { outputFile: 'test-results/results.json' }], ['junit', { outputFile: 'test-results/junit.xml' }], + // Names any test that finished having run ZERO assertions (#396). Reports + // only — it cannot fail a run. The `json` reporter above cannot do this: its + // result objects carry no `steps`, so assertion counts are reachable only + // through the reporter API. + ['./tests/e2e/reporters/assertion-count-reporter.ts'], ], /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */ use: { diff --git a/tests/e2e/reporters/assertion-count-reporter.ts b/tests/e2e/reporters/assertion-count-reporter.ts new file mode 100644 index 00000000..9366e48f --- /dev/null +++ b/tests/e2e/reporters/assertion-count-reporter.ts @@ -0,0 +1,97 @@ +/** + * Report tests that finished having run ZERO assertions (#396). + * + * WHY. #396 catalogues "gates that could not fail" and names this as the highest-value + * remaining fix: *"a spec that runs zero assertions is visibly different from one that + * runs twelve."* Every instance in that catalogue shares one symptom — a green result + * that measured nothing — and every one of them was found by a person becoming + * suspicious of a specific test. This makes the symptom visible without anyone having + * to suspect anything. + * + * It is not hypothetical. On 2026-08-20 two specs were found guarding every assertion + * behind `if (elements.length > 0)` while visiting a page with none of those elements + * (#842); fixing them immediately exposed a real 40px touch target on production. Both + * would have shown up here as `expects=0` the first time they ran. + * + * WHY A REPORTER AND NOT THE JSON OUTPUT. The `json` reporter's result objects carry + * `annotations`, `attachments`, `duration`, `errors`, `status`, `stdout`… and **no + * `steps`**. Assertion counts are only reachable through the reporter API's step + * callbacks. Verified by generating a report and inspecting its keys — post-processing + * `results.json` cannot do this. + * + * IT ONLY PRINTS. Deliberately, and following this repo's own pattern of landing a + * gate in annotate mode first (`E2E_BUDGET_MODE`, `FLAKY_GATE_MODE`). A zero-assertion + * test is not always a defect — a spec may legitimately assert via `toPass`, a fixture, + * or a thrown helper — so failing on it immediately would redden the required lane for + * reasons nobody has triaged yet. Turn it into a gate once the list is known and empty. + * + * WHAT IT CANNOT SEE, said plainly: assertions made inside a helper that does not go + * through `expect` (a raw `throw`), and `soft` assertions are counted like any other. + * A test with a nonzero count is not thereby proven to assert anything USEFUL. + */ +import type { + Reporter, + TestCase, + TestResult, + TestStep, +} from '@playwright/test/reporter'; + +/** Steps Playwright tags as assertions. */ +const EXPECT = 'expect'; + +class AssertionCountReporter implements Reporter { + private counts = new Map(); + + onStepEnd(test: TestCase, _result: TestResult, step: TestStep): void { + if (step.category !== EXPECT) return; + const key = this.key(test); + this.counts.set(key, (this.counts.get(key) ?? 0) + 1); + } + + onTestEnd(test: TestCase, result: TestResult): void { + const key = this.key(test); + // Record a zero explicitly so a test that ran and asserted nothing is + // distinguishable from one that never ran at all. + if (!this.counts.has(key) && result.status === 'passed') { + this.counts.set(key, 0); + } + } + + onEnd(): void { + const silent = [...this.counts.entries()] + .filter(([, n]) => n === 0) + .map(([k]) => k) + .sort(); + + if (this.counts.size === 0) { + // Non-vacuity. A reporter that observed nothing must say so rather than + // printing a clean bill of health — that would be the very shape #396 is about. + console.log( + '\n[assertion-count] observed no tests; this run proves nothing about assertion coverage.' + ); + return; + } + + if (silent.length === 0) { + console.log( + `\n[assertion-count] ${this.counts.size} passing test(s), all ran at least one assertion.` + ); + return; + } + + console.log( + `\n[assertion-count] ${silent.length} of ${this.counts.size} passing test(s) ran ZERO assertions (#396):` + ); + for (const k of silent) console.log(` ${k}`); + console.log( + ' A green result here measured nothing. Usually the page does not contain what ' + + 'the spec guards on — see #842. Reporting only; this does not fail the run.' + ); + } + + private key(test: TestCase): string { + return `${test.location.file.replace(`${process.cwd()}/`, '')}:${test.location.line} › ${test.title}`; + } +} + +export default AssertionCountReporter; diff --git a/tests/unit/assertion-count-reporter.test.ts b/tests/unit/assertion-count-reporter.test.ts new file mode 100644 index 00000000..956ced66 --- /dev/null +++ b/tests/unit/assertion-count-reporter.test.ts @@ -0,0 +1,105 @@ +/** + * The zero-assertion reporter must name silent tests and stay quiet otherwise (#396). + * + * #396's highest-value remaining item was making "a spec that runs zero assertions + * visibly different from one that runs twelve". Every entry in that catalogue was found + * by a person becoming suspicious of one specific test; this makes the symptom visible + * without anyone having to suspect anything. + * + * The three ways a reporter like this is useless, all pinned below: + * - it misses a silent test (the whole point) + * - it slanders a test that DID assert (nobody would trust it twice) + * - it prints a clean bill of health having observed nothing — which is the exact + * shape #396 catalogues, committed by the tool built to detect it + */ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import Reporter from '../e2e/reporters/assertion-count-reporter'; +import type { TestCase, TestResult, TestStep } from '@playwright/test/reporter'; + +const testCase = (file: string, line: number, title: string) => + ({ location: { file, line }, title }) as unknown as TestCase; + +const step = (category: string) => ({ category }) as unknown as TestStep; +const passed = () => ({ status: 'passed' }) as unknown as TestResult; + +describe('assertion-count reporter (#396)', () => { + let out: string[]; + let spy: ReturnType; + + beforeEach(() => { + out = []; + spy = vi.spyOn(console, 'log').mockImplementation((...a: unknown[]) => { + out.push(a.join(' ')); + }); + }); + afterEach(() => spy.mockRestore()); + + const report = () => out.join('\n'); + + it('names a test that ran no assertions', () => { + const r = new Reporter(); + const silent = testCase( + '/repo/tests/e2e/quiet.spec.ts', + 12, + 'asserts nothing' + ); + r.onTestEnd?.(silent, passed()); + r.onEnd?.(); + + expect(report()).toContain('ran ZERO assertions'); + expect(report()).toContain('quiet.spec.ts:12'); + expect(report()).toContain('asserts nothing'); + }); + + it('does not slander a test that did assert', () => { + const r = new Reporter(); + const real = testCase('/repo/tests/e2e/loud.spec.ts', 3, 'asserts twice'); + r.onStepEnd?.(real, passed(), step('expect')); + r.onStepEnd?.(real, passed(), step('expect')); + r.onTestEnd?.(real, passed()); + r.onEnd?.(); + + expect(report()).toContain('all ran at least one assertion'); + expect(report()).not.toContain('ZERO'); + }); + + it('counts only expect steps, not every step', () => { + // A test whose only steps are navigations has asserted nothing, however busy + // it looked. This is exactly the #842 shape: goto, locate, loop, return. + const r = new Reporter(); + const busy = testCase('/repo/tests/e2e/busy.spec.ts', 7, 'navigates a lot'); + for (const c of ['pw:api', 'hook', 'fixture', 'pw:api']) { + r.onStepEnd?.(busy, passed(), step(c)); + } + r.onTestEnd?.(busy, passed()); + r.onEnd?.(); + + expect(report()).toContain('ran ZERO assertions'); + expect(report()).toContain('busy.spec.ts:7'); + }); + + it('refuses to give a clean bill of health having seen nothing', () => { + // The reporter committing the very sin it detects. If a run observes no tests — + // a bad shard filter, a crashed setup — "all good" would be a lie of the exact + // kind #396 exists to catalogue. + const r = new Reporter(); + r.onEnd?.(); + + expect(report()).toContain('proves nothing'); + expect(report()).not.toContain('all ran at least one assertion'); + }); + + it('reports only, and cannot fail a run', () => { + // It must never throw and never signal failure — it is wired into every lane, + // including the required one, and a reporter that throws breaks the run it is + // only supposed to describe. + const r = new Reporter(); + const silent = testCase('/repo/tests/e2e/quiet.spec.ts', 1, 'x'); + expect(() => { + r.onStepEnd?.(silent, passed(), step('expect')); + r.onTestEnd?.(silent, passed()); + r.onEnd?.(); + }).not.toThrow(); + expect(r.onEnd?.()).toBeUndefined(); + }); +});