Skip to content

Commit 05322d9

Browse files
Ramon Bisswangermichael-o
Ramon Bisswanger
authored andcommitted
[SUREFIRE-2212] OutOfMemoryError raised when parsing files with huge stderr/stdout output in surefire-report-parser
This closes #687
1 parent 55ccd06 commit 05322d9

File tree

2 files changed

+87
-5
lines changed

2 files changed

+87
-5
lines changed

surefire-report-parser/src/main/java/org/apache/maven/plugins/surefire/report/TestSuiteXmlParser.java

+7-1
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,8 @@ public final class TestSuiteXmlParser extends DefaultHandler {
6060

6161
private boolean valid;
6262

63+
private boolean parseContent;
64+
6365
public TestSuiteXmlParser(ConsoleLogger consoleLogger) {
6466
this.consoleLogger = consoleLogger;
6567
}
@@ -157,12 +159,14 @@ public void startElement(String uri, String localName, String qName, Attributes
157159
break;
158160
case "failure":
159161
currentElement = new StringBuilder();
162+
parseContent = true;
160163

161164
testCase.setFailure(attributes.getValue("message"), attributes.getValue("type"));
162165
currentSuite.incrementNumberOfFailures();
163166
break;
164167
case "error":
165168
currentElement = new StringBuilder();
169+
parseContent = true;
166170

167171
testCase.setError(attributes.getValue("message"), attributes.getValue("type"));
168172
currentSuite.incrementNumberOfErrors();
@@ -181,6 +185,7 @@ public void startElement(String uri, String localName, String qName, Attributes
181185
break;
182186
case "time":
183187
currentElement = new StringBuilder();
188+
parseContent = true;
184189
break;
185190
default:
186191
break;
@@ -215,6 +220,7 @@ public void endElement(String uri, String localName, String qName) throws SAXExc
215220
default:
216221
break;
217222
}
223+
parseContent = false;
218224
// TODO extract real skipped reasons
219225
}
220226

@@ -225,7 +231,7 @@ public void endElement(String uri, String localName, String qName) throws SAXExc
225231
public void characters(char[] ch, int start, int length) {
226232
assert start >= 0;
227233
assert length >= 0;
228-
if (valid && isNotBlank(start, length, ch)) {
234+
if (valid && parseContent && isNotBlank(start, length, ch)) {
229235
currentElement.append(ch, start, length);
230236
}
231237
}

surefire-report-parser/src/test/java/org/apache/maven/plugins/surefire/report/TestSuiteXmlParserTest.java

+80-4
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,9 @@
1818
*/
1919
package org.apache.maven.plugins.surefire.report;
2020

21-
import java.io.ByteArrayInputStream;
22-
import java.io.File;
23-
import java.io.InputStream;
24-
import java.io.InputStreamReader;
21+
import java.io.*;
22+
import java.nio.file.Files;
23+
import java.nio.file.Path;
2524
import java.util.ArrayList;
2625
import java.util.Collection;
2726
import java.util.List;
@@ -632,4 +631,81 @@ public void shouldTestIsNumeric() {
632631
assertTrue(TestSuiteXmlParser.isNumeric(new StringBuilder("0?51M2"), 2, 4));
633632
assertFalse(TestSuiteXmlParser.isNumeric(new StringBuilder("0?51M2"), 2, 5));
634633
}
634+
635+
@Test
636+
public void shouldParseLargeFile() throws Exception {
637+
// Create test file
638+
Path tempFile = Files.createTempFile("largeReport", ".xml");
639+
640+
try (BufferedWriter w = Files.newBufferedWriter(tempFile)) {
641+
w.write("<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>\n"
642+
+ "<testsuite failures=\"1\" time=\"2.413\" errors=\"0\" skipped=\"0\" tests=\"2\" name=\"largeFile.TestSurefire3\">\n"
643+
+ " <properties></properties>\n"
644+
+ " <testcase time=\"0.005\" classname=\"largeFile.TestSurefire3\" name=\"testSuccess\" />\n"
645+
+ " <testcase time=\"2.20\" classname=\"largeFile.TestSurefire3\" name=\"testFailure\">\n"
646+
+ " <failure message=\"test failure\" type=\"junit.framework.AssertionFailedError\">junit.framework.AssertionFailedError: \n"
647+
+ "\tat junit.framework.Assert.fail(Assert.java:47)\n"
648+
+ "\tat largeFile.TestSurefire3.testFailure(TestSurefire3.java:40)\n"
649+
+ "</failure>\n"
650+
+ " <system-err><![CDATA[\n");
651+
652+
// Adding CDATA which should be ignored during parsing and not cause memory issues
653+
for (int i = 0; i < 100000; i++) {
654+
w.write(
655+
"Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.\n");
656+
w.flush();
657+
}
658+
w.write("]]></system-err>\n" + " </testcase>\n" + "</testsuite>\n");
659+
w.flush();
660+
}
661+
662+
System.err.println(Files.size(tempFile));
663+
664+
// Parse test file
665+
TestSuiteXmlParser testSuiteXmlParser = new TestSuiteXmlParser(consoleLogger);
666+
667+
try (InputStreamReader is = new InputStreamReader(Files.newInputStream(tempFile), UTF_8)) {
668+
List<ReportTestSuite> parse = testSuiteXmlParser.parse(is);
669+
670+
assertThat(parse.size(), is(1));
671+
ReportTestSuite report = parse.get(0);
672+
assertThat(report.getFullClassName(), is("largeFile.TestSurefire3"));
673+
assertThat(report.getName(), is("TestSurefire3"));
674+
assertThat(report.getPackageName(), is("largeFile"));
675+
assertThat(report.getNumberOfTests(), is(2));
676+
assertThat(report.getNumberOfSkipped(), is(0));
677+
assertThat(report.getNumberOfErrors(), is(0));
678+
assertThat(report.getNumberOfFailures(), is(1));
679+
assertThat(report.getNumberOfFlakes(), is(0));
680+
assertThat(report.getTimeElapsed(), is(2.413f));
681+
assertThat(report.getTestCases().size(), is(2));
682+
683+
List<ReportTestCase> tests = report.getTestCases();
684+
assertThat(tests.get(0).getFullClassName(), is("largeFile.TestSurefire3"));
685+
assertThat(tests.get(0).getName(), is("testSuccess"));
686+
assertNull(tests.get(0).getFailureDetail());
687+
assertThat(tests.get(0).getClassName(), is("TestSurefire3"));
688+
assertThat(tests.get(0).getTime(), is(0.005f));
689+
assertThat(tests.get(0).getFullName(), is("largeFile.TestSurefire3.testSuccess"));
690+
assertThat(tests.get(0).hasError(), is(false));
691+
692+
assertThat(tests.get(1).getFullClassName(), is("largeFile.TestSurefire3"));
693+
assertThat(tests.get(1).getName(), is("testFailure"));
694+
assertThat(
695+
tests.get(1).getFailureDetail(),
696+
is("junit.framework.AssertionFailedError: \n"
697+
+ "\tat junit.framework.Assert.fail(Assert.java:47)\n"
698+
+ "\tat largeFile.TestSurefire3.testFailure(TestSurefire3.java:40)\n"));
699+
assertThat(tests.get(1).getClassName(), is("TestSurefire3"));
700+
assertThat(tests.get(1).getTime(), is(2.20f));
701+
assertThat(tests.get(1).getFailureErrorLine(), is("40"));
702+
assertThat(tests.get(1).getFailureMessage(), is("test failure"));
703+
assertThat(tests.get(1).getFullName(), is("largeFile.TestSurefire3.testFailure"));
704+
assertThat(tests.get(1).getFailureType(), is("junit.framework.AssertionFailedError"));
705+
assertThat(tests.get(1).hasError(), is(false));
706+
}
707+
708+
// Delete test file
709+
Files.delete(tempFile);
710+
}
635711
}

0 commit comments

Comments
 (0)