From 05322d992778dd33e2f2e41661a2e66ef7539a68 Mon Sep 17 00:00:00 2001 From: Ramon Bisswanger Date: Tue, 14 Nov 2023 13:19:16 +0100 Subject: [PATCH] [SUREFIRE-2212] OutOfMemoryError raised when parsing files with huge stderr/stdout output in surefire-report-parser This closes #687 --- .../surefire/report/TestSuiteXmlParser.java | 8 +- .../report/TestSuiteXmlParserTest.java | 84 ++++++++++++++++++- 2 files changed, 87 insertions(+), 5 deletions(-) diff --git a/surefire-report-parser/src/main/java/org/apache/maven/plugins/surefire/report/TestSuiteXmlParser.java b/surefire-report-parser/src/main/java/org/apache/maven/plugins/surefire/report/TestSuiteXmlParser.java index eabe10046b..d651b3e64c 100644 --- a/surefire-report-parser/src/main/java/org/apache/maven/plugins/surefire/report/TestSuiteXmlParser.java +++ b/surefire-report-parser/src/main/java/org/apache/maven/plugins/surefire/report/TestSuiteXmlParser.java @@ -60,6 +60,8 @@ public final class TestSuiteXmlParser extends DefaultHandler { private boolean valid; + private boolean parseContent; + public TestSuiteXmlParser(ConsoleLogger consoleLogger) { this.consoleLogger = consoleLogger; } @@ -157,12 +159,14 @@ public void startElement(String uri, String localName, String qName, Attributes break; case "failure": currentElement = new StringBuilder(); + parseContent = true; testCase.setFailure(attributes.getValue("message"), attributes.getValue("type")); currentSuite.incrementNumberOfFailures(); break; case "error": currentElement = new StringBuilder(); + parseContent = true; testCase.setError(attributes.getValue("message"), attributes.getValue("type")); currentSuite.incrementNumberOfErrors(); @@ -181,6 +185,7 @@ public void startElement(String uri, String localName, String qName, Attributes break; case "time": currentElement = new StringBuilder(); + parseContent = true; break; default: break; @@ -215,6 +220,7 @@ public void endElement(String uri, String localName, String qName) throws SAXExc default: break; } + parseContent = false; // TODO extract real skipped reasons } @@ -225,7 +231,7 @@ public void endElement(String uri, String localName, String qName) throws SAXExc public void characters(char[] ch, int start, int length) { assert start >= 0; assert length >= 0; - if (valid && isNotBlank(start, length, ch)) { + if (valid && parseContent && isNotBlank(start, length, ch)) { currentElement.append(ch, start, length); } } diff --git a/surefire-report-parser/src/test/java/org/apache/maven/plugins/surefire/report/TestSuiteXmlParserTest.java b/surefire-report-parser/src/test/java/org/apache/maven/plugins/surefire/report/TestSuiteXmlParserTest.java index a76b6356ab..01a1842139 100644 --- a/surefire-report-parser/src/test/java/org/apache/maven/plugins/surefire/report/TestSuiteXmlParserTest.java +++ b/surefire-report-parser/src/test/java/org/apache/maven/plugins/surefire/report/TestSuiteXmlParserTest.java @@ -18,10 +18,9 @@ */ package org.apache.maven.plugins.surefire.report; -import java.io.ByteArrayInputStream; -import java.io.File; -import java.io.InputStream; -import java.io.InputStreamReader; +import java.io.*; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.ArrayList; import java.util.Collection; import java.util.List; @@ -632,4 +631,81 @@ public void shouldTestIsNumeric() { assertTrue(TestSuiteXmlParser.isNumeric(new StringBuilder("0?51M2"), 2, 4)); assertFalse(TestSuiteXmlParser.isNumeric(new StringBuilder("0?51M2"), 2, 5)); } + + @Test + public void shouldParseLargeFile() throws Exception { + // Create test file + Path tempFile = Files.createTempFile("largeReport", ".xml"); + + try (BufferedWriter w = Files.newBufferedWriter(tempFile)) { + w.write("\n" + + "\n" + + " \n" + + " \n" + + " \n" + + " junit.framework.AssertionFailedError: \n" + + "\tat junit.framework.Assert.fail(Assert.java:47)\n" + + "\tat largeFile.TestSurefire3.testFailure(TestSurefire3.java:40)\n" + + "\n" + + " \n" + " \n" + "\n"); + w.flush(); + } + + System.err.println(Files.size(tempFile)); + + // Parse test file + TestSuiteXmlParser testSuiteXmlParser = new TestSuiteXmlParser(consoleLogger); + + try (InputStreamReader is = new InputStreamReader(Files.newInputStream(tempFile), UTF_8)) { + List parse = testSuiteXmlParser.parse(is); + + assertThat(parse.size(), is(1)); + ReportTestSuite report = parse.get(0); + assertThat(report.getFullClassName(), is("largeFile.TestSurefire3")); + assertThat(report.getName(), is("TestSurefire3")); + assertThat(report.getPackageName(), is("largeFile")); + assertThat(report.getNumberOfTests(), is(2)); + assertThat(report.getNumberOfSkipped(), is(0)); + assertThat(report.getNumberOfErrors(), is(0)); + assertThat(report.getNumberOfFailures(), is(1)); + assertThat(report.getNumberOfFlakes(), is(0)); + assertThat(report.getTimeElapsed(), is(2.413f)); + assertThat(report.getTestCases().size(), is(2)); + + List tests = report.getTestCases(); + assertThat(tests.get(0).getFullClassName(), is("largeFile.TestSurefire3")); + assertThat(tests.get(0).getName(), is("testSuccess")); + assertNull(tests.get(0).getFailureDetail()); + assertThat(tests.get(0).getClassName(), is("TestSurefire3")); + assertThat(tests.get(0).getTime(), is(0.005f)); + assertThat(tests.get(0).getFullName(), is("largeFile.TestSurefire3.testSuccess")); + assertThat(tests.get(0).hasError(), is(false)); + + assertThat(tests.get(1).getFullClassName(), is("largeFile.TestSurefire3")); + assertThat(tests.get(1).getName(), is("testFailure")); + assertThat( + tests.get(1).getFailureDetail(), + is("junit.framework.AssertionFailedError: \n" + + "\tat junit.framework.Assert.fail(Assert.java:47)\n" + + "\tat largeFile.TestSurefire3.testFailure(TestSurefire3.java:40)\n")); + assertThat(tests.get(1).getClassName(), is("TestSurefire3")); + assertThat(tests.get(1).getTime(), is(2.20f)); + assertThat(tests.get(1).getFailureErrorLine(), is("40")); + assertThat(tests.get(1).getFailureMessage(), is("test failure")); + assertThat(tests.get(1).getFullName(), is("largeFile.TestSurefire3.testFailure")); + assertThat(tests.get(1).getFailureType(), is("junit.framework.AssertionFailedError")); + assertThat(tests.get(1).hasError(), is(false)); + } + + // Delete test file + Files.delete(tempFile); + } }