fix(test-results): parse the reports real runners actually emit - #283
Open
colek42 wants to merge 1 commit into
Open
fix(test-results): parse the reports real runners actually emit#283colek42 wants to merge 1 commit into
colek42 wants to merge 1 commit into
Conversation
The attestor could not read the JUnit output of `node --test`, the runner that ships with the language. A JS project using it produced no test-results attestation at all, so any policy requiring one was unsatisfiable by that project — while the tests passed and a valid report sat on disk. Found by running the attestor against a real project rather than against fixtures. Three shapes were rejected, each for its own reason. **Testcases directly under <testsuites>.** Node emits `<testsuites><testcase/>...</testsuites>` with no intervening `<testsuite>`. The parser handled the nested form and a bare `<testsuite>` root, so this document was correctly DETECTED as JUnit and then refused with "JUnit document contains no testsuite elements". Root-level cases are now wrapped in one synthetic suite. It is left unnamed rather than given an invented name: the document did not name it, and a fabricated name would appear in the attestation as though the emitter had supplied it. **An unnamed <testsuite> root.** The fallback required `single.Name != ""`, so whether a run counted depended on an optional attribute. Cases now qualify it too. **Failure text containing ANSI escapes.** Runners embed the failing assertion verbatim, and Node writes the error's `cause` with colour codes still in it — 56 raw ESC bytes in the report from a 2-failure run here. XML 1.0 forbids those, so the document was not well-formed and no conforming parser would read it. That made the failure path worse than the missing-message it looks like: a policy asking for passing tests was satisfied only by runs that PASSED, and refused runs that FAILED with "no evidence" — the same answer it gives when no tests ran at all. Those are different facts and a gate should not conflate them. Bytes XML forbids are now dropped before unmarshalling; nothing is re-encoded and no structure is repaired, so a document malformed for any other reason still fails. The empty-document guard is unchanged and still tested: widening the shapes we accept must not turn "no tests ran" into a passing report. Verified against a real project (9 tests, node --test, JUnit reporter): passing run -> total 9, passed 9, failed 0 failing run -> total 9, passed 7, failed 2, naming both failing tests Six new unit tests cover all three shapes plus the two things that must still be refused — an empty <testsuites>, and a structurally malformed document. Each fails against the unfixed parser.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
The
test-resultsattestor could not read the JUnit output ofnode --test—the runner that ships with the language. A JS project using it produced no
test-results attestation at all, so any policy requiring one was unsatisfiable
by that project, while the tests passed and a valid report sat on disk.
Found by running the attestor against a real project rather than against
fixtures. The failure surfaced as:
which points at product capture. Products were fine —
product/v0.3leavescontained exactly
junit.xml. The parser was rejecting it.Three shapes, three reasons
Testcases directly under
<testsuites>. Node emits<testsuites><testcase/>…</testsuites>with no intervening<testsuite>. Theparser handled the nested form and a bare
<testsuite>root, so the document wascorrectly detected as JUnit and then refused with "contains no testsuite
elements". Root-level cases are now wrapped in one synthetic suite — left
unnamed, because the document did not name it and a fabricated name would appear
in the attestation as though the emitter had supplied it.
An unnamed
<testsuite>root. The fallback requiredsingle.Name != "", sowhether a run counted at all depended on an optional attribute. Cases now
qualify it too.
ANSI escapes in failure text. Runners embed the failing assertion verbatim,
and Node writes the error's
causewith colour codes still in it — 56 raw ESCbytes in the report from a 2-failure run here. XML 1.0 forbids those, so the
document is not well-formed and no conforming parser will read it.
That last one made the failure path worse than a missing message: a policy asking
for passing tests was satisfied only by runs that passed, and refused runs
that failed with "no evidence" — the same answer it gives when no tests ran
at all. Those are different facts and a gate should not conflate them. Bytes XML
forbids are now dropped before unmarshalling; nothing is re-encoded and no
structure is repaired, so a document malformed for any other reason still fails.
Verified against a real project
A 9-test Node project,
node --testwith the JUnit reporter, evidence producedby a cilock built from this branch:
The two it named are exactly the tests that catch the planted regression
(lexical instead of numeric version compare).
Tests
Six new unit tests: all three shapes, plus the two things that must still be
refused — an empty
<testsuites>, and a structurally malformed document.Widening what the parser accepts must not turn "no tests ran" into a passing
report. Each fails against the unfixed parser.
Existing tests in the package are unchanged and still pass.