From fb3e228a0d19bdd17e1687beb7c56e4273e7e595 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 17 Sep 2026 11:30:23 -0400 Subject: [PATCH] fix(#4724): classify Surefire/Failsafe XML as RED evidence (#4825) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(#4724): add failing-first coverage for Surefire XML RED evidence * fix(#4724): classify Surefire/Failsafe XML as RED evidence check tdd-red-evidence parsed only node:test TAP, so a JVM project's genuine Maven red scored INVALID_RED while hand-written synthetic TAP scored RED_EVIDENCE_OK — the gate was passable only by fabricating its input (issue #4724's measured repro). classifyRedEvidence detects Surefire/Failsafe XML (a element) and parses it by TAG-BOUNDARY scanning: each owns its own tag (self-closing) or the segment up to its closer, so the issue's warned-about spanning trap (a lazy lazy match from a green self-closing case to the next closing tag) cannot misreport names. A or child marks the case failing; the target matches at class granularity (exact classname, dotted-suffix, or method name). Any parse anomaly degrades to not-failing — the module stays fail-closed and PURE (no fs/clock; report freshness remains the workflow's run-start check per the issue's implementation notes). TAP classification is byte-identical: all existing fixtures stay green. * test(#4724): pin the scanner hardening — truncation, TAP-message flip, CDATA phantom * docs(#4724): backfill changeset PR number --------- Co-authored-by: sim --- .changeset/bold-deer-click.md | 5 + src/tdd-red-evidence.cts | 97 ++++++++++++++++-- tests/tdd-red-evidence.test.cjs | 173 ++++++++++++++++++++++++++++++++ 3 files changed, 268 insertions(+), 7 deletions(-) create mode 100644 .changeset/bold-deer-click.md diff --git a/.changeset/bold-deer-click.md b/.changeset/bold-deer-click.md new file mode 100644 index 000000000..b933c4649 --- /dev/null +++ b/.changeset/bold-deer-click.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4825 +--- +**TDD red evidence accepts Surefire/Failsafe XML** — the tdd-red-evidence gate parsed only node:test TAP, so a JVM project's genuine Maven red scored INVALID_RED while hand-written synthetic TAP scored RED_EVIDENCE_OK (the gate was passable only by fabricating its input). Surefire/Failsafe XML reports now classify by tag-boundary scanning: a testcase with a or child for the target class is a real red; self-closing passing cases are never spanned into failing names. (#4724) diff --git a/src/tdd-red-evidence.cts b/src/tdd-red-evidence.cts index 49372d6f0..65c597172 100644 --- a/src/tdd-red-evidence.cts +++ b/src/tdd-red-evidence.cts @@ -46,9 +46,12 @@ export interface RedEvidenceInput { command: unknown; /** The command's exit code. */ exitCode: unknown; - /** The command's combined stdout (TAP for node --test). */ + /** The command's combined stdout (TAP for node --test; Surefire/Failsafe XML for Maven). */ output: unknown; - /** Identity of the target test named by the plan (its `test('...')` name). */ + /** + * Identity of the target the plan named: the `test('...')` name for + * node:test runs, or the target CLASS for Surefire/Failsafe runs (#4724). + */ targetTest: unknown; /** Path of the test file the target test lives in (file-named failures are crashes). */ targetFile?: unknown; @@ -90,6 +93,59 @@ function baseOf(p: unknown): string { return typeof p === 'string' ? (p.split(/[\\/]/).pop() ?? p) : ''; } + +/** + * #4724 — Surefire/Failsafe XML summary parsed by TAG-BOUNDARY scanning, NOT + * by a lazy `(.*?)<\/testcase>` regex: Surefire writes PASSING cases as + * self-closing `` elements, so a lazy span from a green case to + * the next closing tag swallows every case between and misreports names beside + * the right failure message. The scanner walks `` closer (testcases never nest). A `', idx); + // #4724 review (HIGH): a `` (truncated/stream-cut + // output) must terminate the scan — resuming the search at -1 clamps to 0 + // and re-finds the same tag forever. Degrade to what was scanned so far + // (fail-closed: an incomplete report proves nothing). + if (tagEnd === -1) break; + tests++; + const tagText = output.slice(idx, tagEnd + 1); + // Self-closing (`/>`): the case has NO body — a failing sibling after it + // must never be attributed to this case (the #4724 spanning trap). + const selfClosing = output[tagEnd - 1] === '/'; + const closeIdx = selfClosing ? -1 : output.indexOf('', tagEnd); + // CDATA sections are verbatim captured output (e.g. System.out echoing + // test source) — their content must never be scanned for failure tags + // (#4724 review: CDATA phantom failures). + const body = (selfClosing + ? '' + : output.slice(tagEnd + 1, closeIdx === -1 ? output.length : closeIdx) + ).replace(//g, ''); + const name = /name="([^"]*)"/.exec(tagText)?.[1] ?? ''; + const classname = /classname="([^"]*)"/.exec(tagText)?.[1] ?? ''; + if (body.includes('` element) is + // parsed by tag-boundary scanning; everything else stays on the proven TAP + // primitives. Unknown formats keep the TAP parse — which finds nothing — and + // fail closed below. + // #4724 review: BOTH element names must appear — a TAP error message that + // merely quotes "" (e.g. "expected was 2") must not + // flip a genuine TAP red into the XML path. + const isSurefireXml = output.includes(' { + const cls = entry.split('#')[0] ?? ''; + const nm = entry.split('#')[1] ?? ''; + return cls === targetTest || cls.endsWith(`.${targetTest}`) || nm === targetTest; + }) + : failing.includes(targetTest); const evidence = { command, exit_code: exitCode, @@ -153,14 +235,15 @@ export function classifyRedEvidence(input: RedEvidenceInput): RedEvidenceResult if (summary.tests === 0) { return { verdict: 'INVALID_RED', reason: 'zero_tests_discovered', evidence }; } - // Nonzero exit but TAP reports no failing test: harness/setup/parser crash - // whose failure never reached a test assertion (or unparseable output). + // Nonzero exit but the report shows no failing test: harness/setup/parser + // crash whose failure never reached a test assertion (or unparseable output). if (summary.fail === 0 || failing.length === 0) { return { verdict: 'INVALID_RED', reason: 'nonzero_exit_without_test_failure', evidence }; } // Fixture/load failure: every failing entry is named like the target FILE — // node reports a load-time crash (throw-on-require, syntax error, ENOENT // fixture) as a file-named `not ok 1 - `, never the target test. + // (TAP-only shape: Surefire failures carry class#method names.) const targetBase = baseOf(input?.targetFile ?? ''); const distinctlyNamed = failing.filter((n) => (targetBase ? baseOf(n) !== targetBase : true)); if (distinctlyNamed.length === 0) { @@ -168,7 +251,7 @@ export function classifyRedEvidence(input: RedEvidenceInput): RedEvidenceResult } // Unrelated failure: real tests ran and failed, but none is the target test // the plan named — an unrelated assertion must not authorize GREEN. - if (!distinctlyNamed.includes(targetTest)) { + if (!(isSurefireXml ? surefireTargetMatches : distinctlyNamed.includes(targetTest))) { return { verdict: 'INVALID_RED', reason: 'no_target_test_failure', evidence }; } return { verdict: 'RED_EVIDENCE_OK', reason: 'target_test_failed', evidence }; diff --git a/tests/tdd-red-evidence.test.cjs b/tests/tdd-red-evidence.test.cjs index 1d25fecbc..d7fddfe07 100644 --- a/tests/tdd-red-evidence.test.cjs +++ b/tests/tdd-red-evidence.test.cjs @@ -247,3 +247,176 @@ describe('executor spec requires intentional RED evidence before GREEN (#3770)', 'execute-mvp-tdd.md must halt GREEN on INVALID_RED'); }); }); + +// ── #4724 — Surefire/Failsafe XML RED evidence ──────────────────────────────── +// A JVM project's genuine red is a Surefire/Failsafe XML report, not node:test +// TAP. The gate used to parse only TAP, so a real Maven red scored +// INVALID_RED while hand-written synthetic TAP scored RED_EVIDENCE_OK — the +// gate was passable only by fabricating its input. The XML scanner walks +// tag boundaries (NOT a lazy spanning regex: Surefire writes +// passing cases self-closing, so `(.*?)` spans from a green case +// to the next closing tag and reports wrong method names). + +describe('#4724 — Surefire/Failsafe XML RED evidence', () => { + const XML_HEAD = '\n'; + + function surefire(cases) { + const body = cases.join('\n'); + return `${XML_HEAD}\n${body}\n`; + } + + const failingCase = (cls, name) => + `1 != 2`; + const errorCase = (cls, name) => + `NullPointerException`; + const greenCase = (cls, name) => + ``; + + const INPUT = { + command: 'mvn -Dtest=AppTest test', + exitCode: 1, + targetTest: 'AppTest', + targetFile: 'src/test/java/com/example/AppTest.java', + }; + + test('a genuine Surefire red with the target class failing classifies RED_EVIDENCE_OK (#4724)', () => { + const result = classifyRedEvidence({ + ...INPUT, + output: surefire([failingCase('com.example.AppTest', 'divides_by_zero')]), + }); + assert.equal(result.verdict, 'RED_EVIDENCE_OK'); + assert.equal(result.reason, 'target_test_failed'); + assert.equal(result.evidence.fail, 1); + assert.ok( + result.evidence.failing_tests.some((n) => n.includes('AppTest')), + 'failing_tests must carry the classname so the target matches', + ); + }); + + test('an unrelated Surefire failure is not the target test', () => { + const result = classifyRedEvidence({ + ...INPUT, + output: surefire([failingCase('com.other.UnrelatedTest', 'unrelated_case')]), + }); + assert.equal(result.verdict, 'INVALID_RED'); + assert.equal(result.reason, 'no_target_test_failure'); + }); + + test('an all-green self-closing Surefire report is unexpected_green', () => { + const result = classifyRedEvidence({ + ...INPUT, + exitCode: 0, + output: surefire([greenCase('com.example.AppTest', 'passes'), greenCase('com.example.AppTest', 'passes2')]), + }); + assert.equal(result.verdict, 'INVALID_RED'); + assert.equal(result.reason, 'unexpected_green'); + }); + + test('self-closing passing cases are not spanned into failing names (#4725-class trap)', () => { + // The issue's trap: a lazy /(.*?)<\/testcase>/ regex starting at the green + // self-closing case spans to the NEXT closing tag, misreporting the green + // case's name beside the failing one. Boundary scanning must not. + const result = classifyRedEvidence({ + ...INPUT, + output: surefire([ + greenCase('com.example.AppTest', 'green_before_failure'), + failingCase('com.example.AppTest', 'the_real_failure'), + ]), + }); + assert.equal(result.verdict, 'RED_EVIDENCE_OK'); + assert.deepEqual( + result.evidence.failing_tests, + ['com.example.AppTest#the_real_failure'], + 'exactly the failing case is reported — no spanned green-case names', + ); + }); + + test('an child counts as a failing testcase', () => { + const result = classifyRedEvidence({ + ...INPUT, + output: surefire([errorCase('com.example.AppTest', 'explodes')]), + }); + assert.equal(result.verdict, 'RED_EVIDENCE_OK'); + assert.equal(result.evidence.fail, 1); + }); + + test('a testcase-free Surefire report is zero_tests_discovered', () => { + const result = classifyRedEvidence({ + ...INPUT, + output: `${XML_HEAD}`, + }); + assert.equal(result.verdict, 'INVALID_RED'); + assert.equal(result.reason, 'zero_tests_discovered'); + }); + + test('Failsafe XML (same schema) classifies like Surefire', () => { + const result = classifyRedEvidence({ + command: 'mvn -Dtest=AppIT verify', + exitCode: 1, + targetTest: 'AppIT', + targetFile: 'src/test/java/com/example/AppIT.java', + output: `${XML_HEAD}\n${failingCase('com.example.AppIT', 'integration_red')}\n`, + }); + assert.equal(result.verdict, 'RED_EVIDENCE_OK'); + assert.equal(result.reason, 'target_test_failed'); + }); +}); + +// ── #4724 review hardening — scanner edge cases ────────────────────────────── + +test('#4724: a (truncated output) terminates and fails closed', () => { + // Pre-hardening this hung forever: indexOf(' stays on the TAP path', () => { + // Format detection requires BOTH " must not flip to the + // XML path (which would misread it as zero_tests_discovered). + const tap = [ + 'TAP version 13', + '# Subtest: the test', + 'not ok 1 - expected was 2', + ' ---', + ' error: |-', + ' expected was 2', + ' ...', + '# tests 1', + '# pass 0', + '# fail 1', + ].join('\n'); + const result = classifyRedEvidence({ + command: 'node --test', + exitCode: 1, + targetTest: 'the test', + output: tap, + }); + assert.equal(result.status ?? result.verdict, 'INVALID_RED'); + assert.equal(result.reason, 'no_target_test_failure', + 'the TAP path must classify it (fail=1), not the XML path (zero tests)'); + assert.equal(result.evidence.fail, 1); +}); + +test('#4724: CDATA sections in a passing case are never scanned as failures', () => { + // A passing case whose captured System.out (CDATA) echoes an + // literal must not count as failing — CDATA is verbatim content. + const cd = ']]>'; + const fl = '1 != 2'; + const result = classifyRedEvidence({ + command: 'mvn test', + exitCode: 1, + targetTest: 'UnrelatedTest', + output: `\n\n${cd}\n${fl}\n`, + }); + assert.equal(result.reason, 'no_target_test_failure', + 'the CDATA phantom must not flip the unrelated failure into a target match'); + assert.equal(result.evidence.fail, 1, 'only the real failing case counts'); +});