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.

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

Simplify serialization/deserialization of elapsed time (SUREFIRE-2164 + SUREFIRE-2167) #645

Merged
merged 2 commits into from May 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Expand Up @@ -48,42 +48,45 @@
import static org.apache.maven.plugin.surefire.report.ReportEntryType.SUCCESS;
import static org.apache.maven.surefire.shared.utils.StringUtils.isBlank;

@SuppressWarnings({"javadoc", "checkstyle:javadoctype"})
// CHECKSTYLE_OFF: LineLength
/*
/**
* XML format reporter writing to <code>TEST-<i>reportName</i>[-<i>suffix</i>].xml</code> file like written and read
* by Ant's <a href="http://ant.apache.org/manual/Tasks/junit.html"><code>&lt;junit&gt;</code></a> and
* <a href="http://ant.apache.org/manual/Tasks/junitreport.html"><code>&lt;junitreport&gt;</code></a> tasks,
* then supported by many tools like CI servers.
* <br>
* <pre>&lt;?xml version="1.0" encoding="UTF-8"?>
* &lt;testsuite name="<i>suite name</i>" [group="<i>group</i>"] tests="<i>0</i>" failures="<i>0</i>" errors="<i>0</i>" skipped="<i>0</i>" time="<i>0,###.###</i>">
* &lt;properties>
* &lt;property name="<i>name</i>" value="<i>value</i>"/>
* <pre>&lt;?xml version="1.0" encoding="UTF-8"?&gt;
* &lt;testsuite name="<i>suite name</i>" [group="<i>group</i>"] tests="<i>0</i>" failures="<i>0</i>" errors="<i>0</i>" skipped="<i>0</i>" time="<i>{float}</i>"&gt;
* &lt;properties&gt;
* &lt;property name="<i>name</i>" value="<i>value</i>"/&gt;
* [...]
* &lt;/properties>
* &lt;testcase time="<i>0,###.###</i>" name="<i>test name</i> [classname="<i>class name</i>"] [group="<i>group</i>"]"/>
* &lt;testcase time="<i>0,###.###</i>" name="<i>test name</i> [classname="<i>class name</i>"] [group="<i>group</i>"]">
* &lt;<b>error</b> message="<i>message</i>" type="<i>exception class name</i>"><i>stacktrace</i>&lt;/error>
* &lt;system-out><i>system out content (present only if not empty)</i>&lt;/system-out>
* &lt;system-err><i>system err content (present only if not empty)</i>&lt;/system-err>
* &lt;/testcase>
* &lt;testcase time="<i>0,###.###</i>" name="<i>test name</i> [classname="<i>class name</i>"] [group="<i>group</i>"]">
* &lt;<b>failure</b> message="<i>message</i>" type="<i>exception class name</i>"><i>stacktrace</i>&lt;/failure>
* &lt;system-out><i>system out content (present only if not empty)</i>&lt;/system-out>
* &lt;system-err><i>system err content (present only if not empty)</i>&lt;/system-err>
* &lt;/testcase>
* &lt;testcase time="<i>0,###.###</i>" name="<i>test name</i> [classname="<i>class name</i>"] [group="<i>group</i>"]">
* &lt;<b>skipped</b>/>
* &lt;/testcase>
* &lt;/properties&gt;
* &lt;testcase time="<i>{float}</i>" name="<i>test name</i> [classname="<i>class name</i>"] [group="<i>group</i>"]"/&gt;
* &lt;testcase time="<i>{float}</i>" name="<i>test name</i> [classname="<i>class name</i>"] [group="<i>group</i>"]"&gt;
* &lt;<b>error</b> message="<i>message</i>" type="<i>exception class name</i>"&gt;<i>stacktrace</i>&lt;/error&gt;
* &lt;system-out&gt;<i>system out content (present only if not empty)</i>&lt;/system-out&gt;
* &lt;system-err&gt;<i>system err content (present only if not empty)</i>&lt;/system-err&gt;
* &lt;/testcase&gt;
* &lt;testcase time="<i>{float}</i>" name="<i>test name</i> [classname="<i>class name</i>"] [group="<i>group</i>"]"&gt;
* &lt;<b>failure</b> message="<i>message</i>" type="<i>exception class name</i>"&gt;<i>stacktrace</i>&lt;/failure&gt;
* &lt;system-out&gt;<i>system out content (present only if not empty)</i>&lt;/system-out&gt;
* &lt;system-err&gt;<i>system err content (present only if not empty)</i>&lt;/system-err&gt;
* &lt;/testcase&gt;
* &lt;testcase time="<i>{float}</i>" name="<i>test name</i> [classname="<i>class name</i>"] [group="<i>group</i>"]"&gt;
* &lt;<b>skipped</b>/&gt;
* &lt;/testcase&gt;
* [...]</pre>
*
* @author Kristian Rosenvold
* @see <a href="http://wiki.apache.org/ant/Proposals/EnhancedTestReports">Ant's format enhancement proposal</a>
* @see <a href="https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=115528872">Ant's format enhancement proposal</a>
* (not yet implemented by Ant 1.8.2)
* @see <a href="https://github.com/apache/ant/blob/6f68fbdefad521741b37304cfdfc6d8646cf1839/src/main/org/apache/tools/ant/taskdefs/optional/junit/XMLJUnitResultFormatter.java">Ant's <code>XMLJUnitResultFormatter</code></a>
*/
// todo this is no more stateless due to existence of testClassMethodRunHistoryMap since of 2.19.
@SuppressWarnings({"javadoc", "checkstyle:javadoctype"})
// TODO this is no more stateless due to existence of testClassMethodRunHistoryMap since of 2.19.
public class StatelessXmlReporter implements StatelessReportEventListener<WrappedReportEntry, TestSetStats> {
private static final float ONE_SECOND = 1000.0f;

private static final String XML_INDENT = " ";

private static final String XML_NL = "\n";
Expand Down Expand Up @@ -387,7 +390,9 @@ private void startTestElement(XMLWriter ppw, WrappedReportEntry report) throws I
ppw.addAttribute("classname", extraEscapeAttribute(className));
}

ppw.addAttribute("time", report.elapsedTimeAsString());
slawekjaranowski marked this conversation as resolved.
Show resolved Hide resolved
if (report.getElapsed() != null) {
ppw.addAttribute("time", String.valueOf(report.getElapsed() / ONE_SECOND));
Copy link
Member

Choose a reason for hiding this comment

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

This can depend on user locales like use dot or comma as separator - should we take care about it?

Copy link
Member Author

@michael-o michael-o May 25, 2023

Choose a reason for hiding this comment

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

No, it won't. That's the good point about it. It is a straight locale-neutral serialization of a float.

}
}

private void createTestSuiteElement(XMLWriter ppw, WrappedReportEntry report, TestSetStats testSetStats)
Expand All @@ -407,7 +412,9 @@ private void createTestSuiteElement(XMLWriter ppw, WrappedReportEntry report, Te
ppw.addAttribute("group", report.getGroup());
}

ppw.addAttribute("time", report.elapsedTimeAsString());
if (report.getElapsed() != null) {
ppw.addAttribute("time", String.valueOf(report.getElapsed() / ONE_SECOND));
}

ppw.addAttribute("tests", String.valueOf(testSetStats.getCompletedCount()));

Expand Down
Expand Up @@ -26,8 +26,6 @@
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStreamReader;
import java.text.NumberFormat;
import java.text.ParseException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
Expand All @@ -40,15 +38,12 @@
import org.xml.sax.helpers.DefaultHandler;

import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Locale.ENGLISH;
import static org.apache.maven.shared.utils.StringUtils.isBlank;

/**
*
*/
public final class TestSuiteXmlParser extends DefaultHandler {
private final NumberFormat numberFormat = NumberFormat.getInstance(ENGLISH);

private final ConsoleLogger consoleLogger;

private ReportTestSuite defaultSuite;
Expand Down Expand Up @@ -111,13 +106,11 @@ public void startElement(String uri, String localName, String qName, Attributes
case "testsuite":
defaultSuite = new ReportTestSuite();
currentSuite = defaultSuite;

try {
Number time = numberFormat.parse(attributes.getValue("time"));

defaultSuite.setTimeElapsed(time.floatValue());
} catch (NullPointerException e) {
consoleLogger.error("WARNING: no time attribute found on testsuite element");
String timeStr = attributes.getValue("time");
if (timeStr != null) {
defaultSuite.setTimeElapsed(Float.parseFloat(timeStr));
} else {
consoleLogger.warning("No time attribute found on testsuite element");
}

final String name = attributes.getValue("name");
Expand Down Expand Up @@ -151,13 +144,12 @@ public void startElement(String uri, String localName, String qName, Attributes
}
}

String timeAsString = attributes.getValue("time");
Number time = isBlank(timeAsString) ? 0 : numberFormat.parse(timeAsString);
timeStr = attributes.getValue("time");

testCase.setFullClassName(currentSuite.getFullClassName())
.setClassName(currentSuite.getName())
.setFullName(currentSuite.getFullClassName() + "." + testCase.getName())
.setTime(time.floatValue());
.setTime(timeStr != null ? Float.parseFloat(timeStr) : 0.0f);

if (currentSuite != defaultSuite) {
currentSuite.setTimeElapsed(testCase.getTime() + currentSuite.getTimeElapsed());
Expand Down Expand Up @@ -193,8 +185,8 @@ public void startElement(String uri, String localName, String qName, Attributes
default:
break;
}
} catch (ParseException e) {
throw new SAXException(e.getMessage(), e);
} catch (NumberFormatException e) {
throw new SAXException("Failed to parse time value", e);
}
}
}
Expand All @@ -215,10 +207,9 @@ public void endElement(String uri, String localName, String qName) throws SAXExc
break;
case "time":
try {
defaultSuite.setTimeElapsed(
numberFormat.parse(currentElement.toString()).floatValue());
} catch (ParseException e) {
throw new SAXException(e.getMessage(), e);
defaultSuite.setTimeElapsed(Float.parseFloat(currentElement.toString()));
} catch (NumberFormatException e) {
throw new SAXException("Failed to parse time value", e);
}
break;
default:
Expand Down