From af05ccbcfb7210a51c7ba110bc7a9de72c6fa21d Mon Sep 17 00:00:00 2001 From: kevinccbsg Date: Sat, 5 Sep 2026 23:39:58 +0200 Subject: [PATCH] feat(diagnostics): surface twd-js failure diagnostics in the run summary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A failing test now reports which mock rules never fired, above the error message: mock rules 6/7 triggered — catalog never requested AssertionError: expected 0 rows (at http://localhost:5173/cg-1/...) Above the error, not below, because a twd-js failure message can carry a full accessible-roles dump that would bury it. The planned approach needed twd-js to expose its formatter on `window` so the in-page `onFail` could reach it — new public surface on the library, and a blocking decision. That is not necessary: the snapshot is plain data, so `onFail` carries `test.diagnostics` out with the result entry and rendering happens in Node. Better split regardless. This package already owns its terminal style and shares no rendering with twd-js — a divergence the upstream sidebar removal confirms is deliberate. The stable contract between the two is the data shape, not the rendering. It also puts the snapshot in the run report as structured data, so shard artifacts and anything reading run.json get it too, not just the terminal. Only the mock-rule signal is rendered. The snapshot's `location` row is dropped: every failure message here already ends in `(at )`, and the full href is strictly more informative than pathname + search + hash. Printing both would say the same thing twice. No schema bump. `diagnostics` is additive and optional on tests[]; runReport spreads it through and mergeReports carries it across the merge without either knowing about it. Verified beyond the mocked suite: the in-page onFail is driven against a stub runner, since page.evaluate is mocked everywhere else and would never otherwise execute it. Output is byte-identical to before when no snapshot is present, and a real failing test against the shipped twd-js 1.9.0 prints exactly as it always has, exit code 1. twd-js has not released diagnostics yet (PR #335 open), so the populated path is unit-tested only and degrades to today's output until it ships. 456 tests pass, coverage 96.13% -> 97.08%. --- src/failureDiagnostics.js | 49 +++++++++++++++++ src/index.js | 12 ++++- src/testSummary.js | 6 +++ tests/failureDiagnostics.test.js | 92 ++++++++++++++++++++++++++++++++ tests/mergeReports.test.js | 50 +++++++++++++++++ tests/runTests.test.js | 66 +++++++++++++++++++++++ tests/testSummary.test.js | 69 ++++++++++++++++++++++++ 7 files changed, 343 insertions(+), 1 deletion(-) create mode 100644 src/failureDiagnostics.js create mode 100644 tests/failureDiagnostics.test.js diff --git a/src/failureDiagnostics.js b/src/failureDiagnostics.js new file mode 100644 index 0000000..09dbeba --- /dev/null +++ b/src/failureDiagnostics.js @@ -0,0 +1,49 @@ +/** + * Renders the diagnostics snapshot twd-js hangs off a failed test. + * + * The snapshot is plain data — `{ location, mockRules }` — so it serialises + * straight out of `page.evaluate()` and is rendered here, in Node. That split + * is the whole point: the in-page `onFail` is a stringified function with no + * module scope, so a formatter shared with twd-js could only reach it through + * new public surface on the library (a `window.__twdFormatDiagnostics`). + * Carrying the data out instead needs nothing from twd-js, and puts the + * rendering where the rest of this package's output already lives. + * + * Only the mock-rule signal is rendered. twd-js's own block also carries a + * `location` row, but every failure message here already ends in + * `(at )`, and the full href is strictly more informative than the + * snapshot's pathname + search + hash. Printing both would say the same thing + * twice. + */ + +// A page with fifteen mocks must not produce fifteen lines. The rules that did +// fire are only interesting as a count; it is the misses that name the bug. +const LIST_CAP = 5; + +// Aligns a continuation row under the value column of `mock rules `. +const CONTINUATION = ' '.repeat(12); + +export function formatFailureDiagnostics(diagnostics) { + const mockRules = diagnostics?.mockRules; + // Omitted by twd-js whenever the test registered no rules, and absent + // entirely on a twd-js that predates diagnostics. Never render `0/0`: + // "this test mocks nothing" is not a diagnostic, it is the default. + if (!mockRules) return []; + + const { registered, triggered } = mockRules; + const untriggered = mockRules.untriggered ?? []; + const summary = `${triggered}/${registered} triggered`; + + if (untriggered.length === 0) return [`mock rules ${summary}`]; + if (untriggered.length === 1) { + return [`mock rules ${summary} — ${untriggered[0]} never requested`]; + } + + const lines = [`mock rules ${summary} — ${untriggered.length} never requested`]; + for (const alias of untriggered.slice(0, LIST_CAP)) { + lines.push(`${CONTINUATION}✗ ${alias}`); + } + const rest = untriggered.length - LIST_CAP; + if (rest > 0) lines.push(`${CONTINUATION}+${rest} more`); + return lines; +} diff --git a/src/index.js b/src/index.js index db025a5..ec15987 100644 --- a/src/index.js +++ b/src/index.js @@ -251,7 +251,17 @@ export async function runTests(options = {}) { }, onFail: (test, err) => { test.status = "done"; - testStatus.push({ id: test.id, status: "fail", error: `${err.message} (at ${window.location.href})` }); + // The raw snapshot travels out; src/failureDiagnostics.js renders + // it in Node. This callback is serialised into the page and has no + // module scope, so it cannot reach a formatter, and duplicating one + // here would be a copy that drifts. `undefined` on a twd-js without + // diagnostics support, and dropped by serialisation. + testStatus.push({ + id: test.id, + status: "fail", + diagnostics: test.diagnostics, + error: `${err.message} (at ${window.location.href})`, + }); }, onSkip: (test) => { test.status = "done"; diff --git a/src/testSummary.js b/src/testSummary.js index f2eb971..0f722bc 100644 --- a/src/testSummary.js +++ b/src/testSummary.js @@ -1,4 +1,5 @@ import { buildTestPath } from './buildTestPath.js'; +import { formatFailureDiagnostics } from './failureDiagnostics.js'; /** * Display name for one test result. @@ -55,6 +56,11 @@ export function formatRunComplete({ for (const failure of failures) { const testPath = resolvePath(failure, handlers); lines.push(` × ${testPath}`); + // Above the error, not below: a twd-js failure message can carry a full + // accessible-roles dump, which would bury the block underneath it. + for (const row of formatFailureDiagnostics(failure.diagnostics)) { + lines.push(` ${row}`); + } if (failure.error) { lines.push(` ${String(failure.error).replace(/\n/g, '\n ')}`); } diff --git a/tests/failureDiagnostics.test.js b/tests/failureDiagnostics.test.js new file mode 100644 index 0000000..c4174f5 --- /dev/null +++ b/tests/failureDiagnostics.test.js @@ -0,0 +1,92 @@ +import { describe, it, expect } from 'vitest'; +import { formatFailureDiagnostics } from '../src/failureDiagnostics.js'; + +describe('formatFailureDiagnostics', () => { + // The snapshot is absent on a passing test, on a test that failed one attempt + // then passed on retry, and on every test run against a twd-js that predates + // diagnostics. All three have to render nothing rather than throw. + it('renders nothing when there is no snapshot', () => { + expect(formatFailureDiagnostics(undefined)).toEqual([]); + expect(formatFailureDiagnostics(null)).toEqual([]); + }); + + // "This test mocks nothing" is the default, not a diagnostic. twd-js omits + // mockRules in that case and the block must not invent a `0/0` row. + it('renders nothing when the test registered no mock rules', () => { + expect(formatFailureDiagnostics({ location: '/checkout' })).toEqual([]); + }); + + it('renders a bare count when every rule was triggered', () => { + expect(formatFailureDiagnostics({ + location: '/checkout', + mockRules: { registered: 6, triggered: 6, untriggered: [] }, + })).toEqual(['mock rules 6/6 triggered']); + }); + + // One miss names itself. That single alias is usually the whole answer, so it + // is worth the inline room. + it('names the alias when exactly one rule was never requested', () => { + expect(formatFailureDiagnostics({ + location: '/cg-1/settings/catalog', + mockRules: { registered: 7, triggered: 6, untriggered: ['catalog'] }, + })).toEqual(['mock rules 6/7 triggered — catalog never requested']); + }); + + it('lists the aliases under a count when several were never requested', () => { + expect(formatFailureDiagnostics({ + location: '/cg-1', + mockRules: { registered: 7, triggered: 4, untriggered: ['catalog', 'profile', 'advisor'] }, + })).toEqual([ + 'mock rules 4/7 triggered — 3 never requested', + ' ✗ catalog', + ' ✗ profile', + ' ✗ advisor', + ]); + }); + + // A page with fifteen mocks must not produce fifteen lines. + it('caps the list at five and counts the remainder', () => { + const untriggered = ['a', 'b', 'c', 'd', 'e', 'f', 'g']; + expect(formatFailureDiagnostics({ + location: '/wide', + mockRules: { registered: 9, triggered: 2, untriggered }, + })).toEqual([ + 'mock rules 2/9 triggered — 7 never requested', + ' ✗ a', + ' ✗ b', + ' ✗ c', + ' ✗ d', + ' ✗ e', + ' +2 more', + ]); + }); + + it('shows no remainder line when the list lands exactly on the cap', () => { + const lines = formatFailureDiagnostics({ + location: '/exact', + mockRules: { registered: 5, triggered: 0, untriggered: ['a', 'b', 'c', 'd', 'e'] }, + }); + expect(lines).toHaveLength(6); + expect(lines.some((l) => l.includes('more'))).toBe(false); + }); + + // A snapshot from a future twd-js that stops sending the array must not throw + // in the middle of reporting a failure. + it('tolerates a missing untriggered array', () => { + expect(formatFailureDiagnostics({ + location: '/partial', + mockRules: { registered: 3, triggered: 3 }, + })).toEqual(['mock rules 3/3 triggered']); + }); + + // The location row is deliberately not rendered: the failure message already + // ends in `(at )`, which is strictly more informative. + it('never renders a location row', () => { + const lines = formatFailureDiagnostics({ + location: '/cg-1/settings/catalog', + mockRules: { registered: 2, triggered: 1, untriggered: ['catalog'] }, + }); + expect(lines.join('\n')).not.toContain('/cg-1/settings/catalog'); + expect(lines.join('\n')).not.toContain('location'); + }); +}); diff --git a/tests/mergeReports.test.js b/tests/mergeReports.test.js index 33f3c89..2bebc4a 100644 --- a/tests/mergeReports.test.js +++ b/tests/mergeReports.test.js @@ -249,3 +249,53 @@ describe('reportTotals', () => { expect(reportTotals(merged)).toEqual({ executed: 2, notRun: 0, expected: 2, consistent: true }); }); }); + +// A failure's diagnostics snapshot is written by the shard that ran the test +// and rendered by whoever prints the merged summary, so it has to survive the +// merge and a JSON round-trip (embedded newlines in `error` included). +describe('mergeRunReports diagnostics', () => { + const diagnostics = { + location: '/cg-1/settings/catalog', + mockRules: { registered: 7, triggered: 6, untriggered: ['catalog'] }, + }; + + it('carries the snapshot through the merge on the failing shard', () => { + const merged = mergeRunReports([ + makeReport(1, { + tests: [{ id: 'r1-t', path: 'Login > a', index: 0, status: 'pass' }], + }), + makeReport(2, { + failed: 1, + tests: [{ + id: 'r2-t', + path: 'Login > b', + index: 1, + status: 'fail', + diagnostics, + error: 'AssertionError: expected 0 rows\n at Object.', + }], + }), + ]); + + const failed = merged.tests.find((t) => t.status === 'fail'); + expect(failed.diagnostics).toEqual(diagnostics); + + const roundTripped = JSON.parse(JSON.stringify(merged)); + const after = roundTripped.tests.find((t) => t.status === 'fail'); + expect(after.diagnostics).toEqual(diagnostics); + expect(after.error).toContain('\n'); + }); + + // Shards produced by a twd-js without diagnostics merge unchanged. + it('merges reports that carry no snapshot at all', () => { + const merged = mergeRunReports([ + makeReport(1, { + failed: 1, + tests: [{ id: 'r1-t', path: 'Login > a', index: 0, status: 'fail', error: 'boom' }], + }), + makeReport(2), + ]); + + expect(merged.tests.find((t) => t.status === 'fail').diagnostics).toBeUndefined(); + }); +}); diff --git a/tests/runTests.test.js b/tests/runTests.test.js index 547af64..fc59cc8 100644 --- a/tests/runTests.test.js +++ b/tests/runTests.test.js @@ -1555,3 +1555,69 @@ describe('runTests sharded behavior changes', () => { expect(report.contracts.results).toEqual([{ alias: 'a' }]); }); }); + +// The in-page onFail is serialised into the browser, so the suite never runs it +// by mocking page.evaluate. These drive the real callback directly against a +// stub runner: it is the only place the diagnostics hand-off is observable. +describe("in-page onFail diagnostics hand-off", () => { + let savedWindow; + + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(loadConfig).mockReturnValue({ ...defaultMockConfig }); + vi.spyOn(console, 'log').mockImplementation(() => {}); + savedWindow = global.window; + }); + + afterEach(() => { + global.window = savedWindow; + vi.restoreAllMocks(); + }); + + // Runs the function twd-cli hands to page.evaluate, with a stub + // window.__testRunner that fails one test. + async function runInPageFn(failingTest) { + const handlers = [{ id: 't1', name: 'test1', type: 'test' }]; + const page = createMockPage({ handlers, testStatus: [] }); + vi.mocked(puppeteer.launch).mockResolvedValue(createMockBrowser(page)); + await runTests(); + + // call 0 is the enumeration pass; call 1 is the first chunk run. + const [inPageFn, retryCount, chunkIds] = page.evaluate.mock.calls[1]; + + global.window = { + location: { href: 'http://localhost:5173/cg-1/settings/catalog' }, + __testRunner: class { + constructor(callbacks) { this.callbacks = callbacks; } + async runByIds() { this.callbacks.onFail(failingTest, new Error('boom')); } + }, + }; + + return inPageFn(retryCount, chunkIds); + } + + it("carries the raw snapshot out on the failure entry", async () => { + const diagnostics = { + location: '/cg-1/settings/catalog', + mockRules: { registered: 7, triggered: 6, untriggered: ['catalog'] }, + }; + + const result = await runInPageFn({ id: 't1', diagnostics }); + + expect(result).toEqual([{ + id: 't1', + status: 'fail', + diagnostics, + error: 'boom (at http://localhost:5173/cg-1/settings/catalog)', + }]); + }); + + // twd-js 1.9.0 and earlier hang nothing off the handler. The entry must still + // be well-formed, and the error text unchanged. + it("leaves the entry intact when twd-js sends no snapshot", async () => { + const result = await runInPageFn({ id: 't1' }); + + expect(result[0].diagnostics).toBeUndefined(); + expect(result[0].error).toBe('boom (at http://localhost:5173/cg-1/settings/catalog)'); + }); +}); diff --git a/tests/testSummary.test.js b/tests/testSummary.test.js index b8a45a2..a0887a4 100644 --- a/tests/testSummary.test.js +++ b/tests/testSummary.test.js @@ -257,3 +257,72 @@ describe('formatRunComplete with shards', () => { expect(output).not.toContain('Shards:'); }); }); + +// The diagnostics snapshot travels out of the page as raw data on the failure +// entry (src/index.js), and is rendered here rather than in twd-js. See +// src/failureDiagnostics.js for why the split sits where it does. +describe('formatRunComplete diagnostics block', () => { + const failing = (diagnostics) => ({ + id: 't1', + status: 'fail', + diagnostics, + error: 'AssertionError: expected 0 rows (at http://localhost:5173/cg-1/settings/catalog)', + }); + + it('prints the mock-rule row above the error message', () => { + const output = formatRunComplete({ + testStatus: [failing({ + location: '/cg-1/settings/catalog', + mockRules: { registered: 7, triggered: 6, untriggered: ['catalog'] }, + })], + handlers, + durationMs: 1000, + }); + expect(output).toContain( + ' × Login > shows error on wrong password\n' + + ' mock rules 6/7 triggered — catalog never requested\n' + + ' AssertionError: expected 0 rows (at http://localhost:5173/cg-1/settings/catalog)' + ); + }); + + it('indents every row of a multi-alias block to the error column', () => { + const output = formatRunComplete({ + testStatus: [failing({ + location: '/cg-1', + mockRules: { registered: 4, triggered: 1, untriggered: ['catalog', 'profile'] }, + })], + handlers, + durationMs: 1000, + }); + expect(output).toContain( + ' mock rules 1/4 triggered — 2 never requested\n' + + ' ✗ catalog\n' + + ' ✗ profile\n' + ); + }); + + // twd-js 1.9.0 and earlier send no snapshot at all. The failure must print + // exactly as it always has. + it('is byte-identical to the old output when no snapshot is present', () => { + const args = { handlers, durationMs: 1000 }; + const withField = formatRunComplete({ testStatus: [failing(undefined)], ...args }); + const withoutField = formatRunComplete({ + testStatus: [{ id: 't1', status: 'fail', error: failing().error }], + ...args, + }); + expect(withField).toBe(withoutField); + expect(withField).not.toContain('mock rules'); + }); + + // A test that failed an attempt then passed carries no snapshot on the pass + // entry, so a retried-then-green run stays clean. + it('prints no block for a test that passed on retry', () => { + const output = formatRunComplete({ + testStatus: [{ id: 't1', status: 'pass', retryAttempt: 2 }], + handlers, + durationMs: 1000, + }); + expect(output).not.toContain('mock rules'); + expect(output).toContain('Retried (1):'); + }); +});