Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[SUREFIRE-2224] StatelessXmlReporter#getTestProblems() does not properly reflect report schema structure #702

Merged
merged 1 commit into from Dec 22, 2023
Merged

[SUREFIRE-2224] StatelessXmlReporter#getTestProblems() does not properly reflect report schema structure #702

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 6 additions & 14 deletions ...re-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java
View file
Open in desktop
Expand Up @@ -432,7 +432,7 @@ private static void getTestProblems(
boolean trimStackTrace,
OutputStream fw,
String testErrorType,
boolean createOutErrElementsInside)
boolean enableNestedOutErrElements)
throws IOException {
ppw.startElement(testErrorType);

Expand All @@ -456,21 +456,13 @@ private static void getTestProblems(
}
}

boolean hasNestedElements = createOutErrElementsInside & stackTrace != null;

if (stackTrace != null) {
if (hasNestedElements) {
ppw.startElement("stackTrace");
if (enableNestedOutErrElements) {
ppw.startElement("stackTrace");
if (stackTrace != null) {
extraEscapeElementValue(stackTrace, outputStreamWriter, ppw, fw);
}
ppw.endElement();

extraEscapeElementValue(stackTrace, outputStreamWriter, ppw, fw);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still needs to be called if stackTrace != null and !enableNestedOutErrElements. Otherwise, error and failure elements won't contain the stack trace as their content.

Copy link
Member Author

@michael-o michael-o Jan 2, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I understand what you mean. Let me rephrase and I guess you are right here, the schema is poorly designed. According to https://maven.apache.org/surefire/maven-surefire-plugin/xsd/surefire-test-report.xsd both failure and error aren't complex elements, thus do not contain stackTrace. Their element body contains the strack trace? Is that the case, if so I will cancel the vote and work on a fix and let you know.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's right.

Before (with 3.2.3 and earlier)

<testcase name="addsTwoNumbers" classname="com.example.project.CalculatorTests" time="0.014">
  <failure message="1 + 1 should equal 2 ==&gt; expected: &lt;3&gt; but was: &lt;2&gt;" type="org.opentest4j.AssertionFailedError"><![CDATA[org.opentest4j.AssertionFailedError: 1 + 1 should equal 2 ==> expected: <3> but was: <2>
  at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
  at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
  at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
  at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:150)
  at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:563)
  at com.example.project.CalculatorTests.addsTwoNumbers(CalculatorTests.java:26)
  at java.base/java.lang.reflect.Method.invoke(Method.java:578)
  at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
  at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
]]></failure>
</testcase>

After this change

<testcase name="addsTwoNumbers" classname="com.example.project.CalculatorTests" time="0.014">
  <failure message="1 + 1 should equal 2 ==&gt; expected: &lt;3&gt; but was: &lt;2&gt;" type="org.opentest4j.AssertionFailedError"/>
</testcase>

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the confirmation. I will cancel the vote today, create an improvement request to change the schema in the next major to a complex element with stackTrace to align with the rest and another one to fix this regression. I will ping you as soon as I have something to test.

Copy link
Member Author

@michael-o michael-o Jan 3, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vote canceled and repo dropped.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix: #709


if (hasNestedElements) {
ppw.endElement();
}
}

if (createOutErrElementsInside) {
createOutErrElements(outputStreamWriter, ppw, report, fw);
}

Expand Down