test_runner: remove failure attribute in junit_report testcase element#59685
test_runner: remove failure attribute in junit_report testcase element#59685devholic22 wants to merge 2 commits intonodejs:mainfrom
Conversation
|
Review requested:
|
| children: [inspectWithNoCustomRetry(error, inspectOptions)], | ||
| }); | ||
| currentTest.failures = 1; | ||
| currentTest.attrs.failure = error?.message ?? ''; |
There was a problem hiding this comment.
While the change the good, I wonder if we have to treat this as potentially breaking? @nodejs/test_runner @nodejs/tsc any thoughts? I'm fine landing as a semver-patch but want to be sure.
MoLow
left a comment
There was a problem hiding this comment.
LGTM. also not sure rgarding semver-major vs patch
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #59685 +/- ##
==========================================
- Coverage 89.91% 89.87% -0.04%
==========================================
Files 667 667
Lines 196600 196855 +255
Branches 38601 38646 +45
==========================================
+ Hits 176780 176933 +153
- Misses 12269 12359 +90
- Partials 7551 7563 +12
🚀 New features to boost your workflow:
|
|
Hello @atlowChemi #59593 is auto-closed because #60274 has been merged, thank you. Probably this PR also needs a ping, or should we reopen the issue #59593? |
|
The commit message here needs an amend |
|
Hello @devholic22 Are you planning to amend the commit message? Thank you. |
Fixes: #59593
This PR aligns the Node.js test runner’s JUnit XML output with the official JUnit specification by removing the non-standard failure attribute from elements and replacing it with a proper nested tag.
Changes
• Remove non-compliant failure attribute from elements
• Update snapshot tests accordingly
Background
The JUnit XML specification mandates that failure details must be represented via a nested (child)
<failure>element inside a<testcase>, rather than as an attribute.However, the Node.js test runner previously:
• Incorrectly embedded the failure message as a failure="..." attribute on , which is not spec-compliant
• This caused compatibility issues with tools like GitLab, Jenkins, or any CI/CD pipeline that parses JUnit reports strictly
Before:
After:
Testing