Skip to content

Commit d2ae5ec

Browse files
FrawlessFrawless
committed
Add IT and deal with corner cases
Signed-off-by: Jakub Stejskal <[email protected]>
1 parent 4a47a1c commit d2ae5ec

File tree

8 files changed

+392
-38
lines changed

8 files changed

+392
-38
lines changed

‎maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/DefaultReporterFactory.java‎

Lines changed: 35 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -275,24 +275,32 @@ private void mergeTestHistoryResult() {
275275
List<TestMethodStats> testMethodStats = entry.getValue();
276276
String testClassMethodName = entry.getKey();
277277

278-
// Handle @BeforeAll failures (null method names)
279-
if (testClassMethodName == null) {
280-
for (TestMethodStats methodStats : testMethodStats) {
281-
String className = extractClassNameFromStackTrace(methodStats);
278+
// Handle @BeforeAll failures (null, empty, or ends with ".null" method names)
279+
// But only if they actually failed (ERROR or FAILURE), not if they were skipped
280+
if ((testClassMethodName == null || testClassMethodName.isEmpty() || testClassMethodName.endsWith(".null"))
281+
&& (testClassMethodName == null || !testClassMethodName.contains("$"))) {
282+
283+
// Check if this is actually a failure/error (not skipped or success)
284+
boolean isActualFailure = testMethodStats.stream()
285+
.anyMatch(stat -> stat.getResultType() == ReportEntryType.ERROR
286+
|| stat.getResultType() == ReportEntryType.FAILURE);
287+
288+
if (isActualFailure) {
289+
// Extract class name from the test class method name
290+
String className = extractClassNameFromMethodName(testClassMethodName);
282291
if (className != null) {
283292
if (beforeAllFailures.containsKey(className)) {
284293
List<TestMethodStats> previousMethodStats = beforeAllFailures.get(className);
285-
previousMethodStats.add(methodStats);
294+
previousMethodStats.addAll(testMethodStats);
286295
beforeAllFailures.put(className, previousMethodStats);
287296
} else {
288-
List<TestMethodStats> initMethodStats = new ArrayList<>();
289-
initMethodStats.add(methodStats);
290-
beforeAllFailures.put(className, initMethodStats);
297+
beforeAllFailures.put(className, new ArrayList<>(testMethodStats));
291298
}
292299
}
300+
// Skip normal processing of @BeforeAll failures because it needs special care
301+
continue;
293302
}
294-
// Skip normal processing of @BeforeAll failures because it needs special care
295-
continue;
303+
// If it's skipped or success with null method name, fall through to normal processing
296304
}
297305

298306
completedCount++;
@@ -313,15 +321,6 @@ private void mergeTestHistoryResult() {
313321
}
314322
completedCount += successCount - 1;
315323
successTests.put(testClassMethodName, testMethodStats);
316-
317-
// If current test belong to class that failed during beforeAll store that info to proper log info
318-
String className = extractClassNameFromMethodName(testClassMethodName);
319-
if (beforeAllFailures.containsKey(className)) {
320-
List<TestMethodStats> previousMethodStats = beforeAllFailures.get(className);
321-
previousMethodStats.addAll(testMethodStats);
322-
beforeAllFailures.put(className, previousMethodStats);
323-
}
324-
325324
break;
326325
case SKIPPED:
327326
skipped++;
@@ -340,14 +339,30 @@ private void mergeTestHistoryResult() {
340339
}
341340
}
342341

342+
// Loop over all success tests and find those that are passed flakes for beforeAll failures
343+
for (Map.Entry<String, List<TestMethodStats>> entry : successTests.entrySet()) {
344+
List<TestMethodStats> testMethodStats = entry.getValue();
345+
String testClassMethodName = entry.getKey();
346+
// If current test belong to class that failed during beforeAll store that info to proper log info
347+
String className = extractClassNameFromMethodName(testClassMethodName);
348+
if (beforeAllFailures.containsKey(className)) {
349+
List<TestMethodStats> previousMethodStats = beforeAllFailures.get(className);
350+
previousMethodStats.addAll(testMethodStats);
351+
beforeAllFailures.put(className, previousMethodStats);
352+
}
353+
}
354+
343355
// Process @BeforeAll failures after we know which classes have successful tests
344356
for (Map.Entry<String, List<TestMethodStats>> entry : beforeAllFailures.entrySet()) {
345357
String className = entry.getKey();
346358
List<TestMethodStats> testMethodStats = entry.getValue();
347359
String classNameKey = className + ".<beforeAll>";
348360

349361
if (reportConfiguration.getRerunFailingTestsCount() > 0
350-
&& testMethodStats.stream().anyMatch(methodStats -> methodStats.getTestClassMethodName() != null)) {
362+
&& testMethodStats.stream()
363+
.anyMatch(methodStats -> methodStats.getTestClassMethodName() != null
364+
&& !methodStats.getTestClassMethodName().isEmpty()
365+
&& methodStats.getResultType().equals(ReportEntryType.SUCCESS))) {
351366
flakyTests.put(classNameKey, testMethodStats);
352367
} else {
353368
errorTests.put(classNameKey, testMethodStats);

‎maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java‎

Lines changed: 97 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ public void testSetCompleted(WrappedReportEntry testSetReportEntry, TestSetStats
161161
OutputStreamWriter fw = getWriter(outputStream)) {
162162
XMLWriter ppw = new PrettyPrintXMLWriter(new PrintWriter(fw), XML_INDENT, XML_NL, UTF_8.name(), null);
163163

164-
createTestSuiteElement(ppw, testSetReportEntry, testSetStats); // TestSuite
164+
createTestSuiteElement(ppw, testSetReportEntry, classMethodStatistics); // TestSuite
165165

166166
if (enablePropertiesElement) {
167167
showProperties(ppw, testSetReportEntry.getSystemProperties());
@@ -186,9 +186,9 @@ public void testSetCompleted(WrappedReportEntry testSetReportEntry, TestSetStats
186186
}
187187

188188
for (Entry<String, Map<String, List<WrappedReportEntry>>> statistics : classMethodStatistics.entrySet()) {
189-
for (Entry<String, List<WrappedReportEntry>> thisMethodRuns :
190-
statistics.getValue().entrySet()) {
191-
serializeTestClass(outputStream, fw, ppw, thisMethodRuns.getValue());
189+
Map<String, List<WrappedReportEntry>> methodStatistics = statistics.getValue();
190+
for (Entry<String, List<WrappedReportEntry>> thisMethodRuns : methodStatistics.entrySet()) {
191+
serializeTestClass(outputStream, fw, ppw, thisMethodRuns.getValue(), methodStatistics);
192192
}
193193
}
194194

@@ -224,10 +224,14 @@ private Deque<WrappedReportEntry> aggregateCacheFromMultipleReruns(
224224
}
225225

226226
private void serializeTestClass(
227-
OutputStream outputStream, OutputStreamWriter fw, XMLWriter ppw, List<WrappedReportEntry> methodEntries)
227+
OutputStream outputStream,
228+
OutputStreamWriter fw,
229+
XMLWriter ppw,
230+
List<WrappedReportEntry> methodEntries,
231+
Map<String, List<WrappedReportEntry>> methodStatistics)
228232
throws IOException {
229233
if (rerunFailingTestsCount > 0) {
230-
serializeTestClassWithRerun(outputStream, fw, ppw, methodEntries);
234+
serializeTestClassWithRerun(outputStream, fw, ppw, methodEntries, methodStatistics);
231235
} else {
232236
// rerunFailingTestsCount is smaller than 1, but for some reasons a test could be run
233237
// for more than once
@@ -258,10 +262,18 @@ private void serializeTestClassWithoutRerun(
258262
}
259263

260264
private void serializeTestClassWithRerun(
261-
OutputStream outputStream, OutputStreamWriter fw, XMLWriter ppw, List<WrappedReportEntry> methodEntries)
265+
OutputStream outputStream,
266+
OutputStreamWriter fw,
267+
XMLWriter ppw,
268+
List<WrappedReportEntry> methodEntries,
269+
Map<String, List<WrappedReportEntry>> methodStatistics)
262270
throws IOException {
263271
WrappedReportEntry firstMethodEntry = methodEntries.get(0);
264-
switch (getTestResultType(methodEntries)) {
272+
273+
TestResultType resultType =
274+
getTestResultTypeWithBeforeAllHandling(firstMethodEntry.getName(), methodEntries, methodStatistics);
275+
276+
switch (resultType) {
265277
case SUCCESS:
266278
for (WrappedReportEntry methodEntry : methodEntries) {
267279
if (methodEntry.getReportEntryType() == SUCCESS) {
@@ -370,6 +382,40 @@ private TestResultType getTestResultType(List<WrappedReportEntry> methodEntryLis
370382
return DefaultReporterFactory.getTestResultType(testResultTypeList, rerunFailingTestsCount);
371383
}
372384

385+
/**
386+
* Determines the final result type for a test method, applying special handling for @BeforeAll failures.
387+
* If a @BeforeAll fails but any actual test methods succeed, it's classified as a FLAKE.
388+
*
389+
* @param methodName the name of the test method (null or "null" for @BeforeAll)
390+
* @param methodRuns the list of runs for this method
391+
* @param methodStatistics all method statistics for the test class
392+
* @return the final TestResultType
393+
*/
394+
private TestResultType getTestResultTypeWithBeforeAllHandling(
395+
String methodName,
396+
List<WrappedReportEntry> methodRuns,
397+
Map<String, List<WrappedReportEntry>> methodStatistics) {
398+
TestResultType resultType = getTestResultType(methodRuns);
399+
400+
// Special handling for @BeforeAll failures (null method name or method name is "null")
401+
// If @BeforeAll failed but any actual test methods succeeded, treat it as a flake
402+
if ((methodName == null || methodName.equals("null"))
403+
&& (resultType == TestResultType.ERROR || resultType == TestResultType.FAILURE)) {
404+
// Check if any actual test methods (non-null and not "null" names) succeeded
405+
boolean hasSuccessfulTestMethods = methodStatistics.entrySet().stream()
406+
.filter(entry ->
407+
entry.getKey() != null && !entry.getKey().equals("null")) // Only actual test methods
408+
.anyMatch(entry -> entry.getValue().stream()
409+
.anyMatch(reportEntry -> reportEntry.getReportEntryType() == SUCCESS));
410+
411+
if (hasSuccessfulTestMethods) {
412+
resultType = TestResultType.FLAKE;
413+
}
414+
}
415+
416+
return resultType;
417+
}
418+
373419
private Deque<WrappedReportEntry> getAddMethodRunHistoryMap(String testClassName) {
374420
Deque<WrappedReportEntry> methodRunHistory = testClassMethodRunHistoryMap.get(testClassName);
375421
if (methodRunHistory == null) {
@@ -420,7 +466,10 @@ private void startTestElement(XMLWriter ppw, WrappedReportEntry report) throws I
420466
}
421467
}
422468

423-
private void createTestSuiteElement(XMLWriter ppw, WrappedReportEntry report, TestSetStats testSetStats)
469+
private void createTestSuiteElement(
470+
XMLWriter ppw,
471+
WrappedReportEntry report,
472+
Map<String, Map<String, List<WrappedReportEntry>>> classMethodStatistics)
424473
throws IOException {
425474
ppw.startElement("testsuite");
426475

@@ -441,13 +490,46 @@ private void createTestSuiteElement(XMLWriter ppw, WrappedReportEntry report, Te
441490
ppw.addAttribute("time", String.valueOf(report.getElapsed() / ONE_SECOND));
442491
}
443492

444-
ppw.addAttribute("tests", String.valueOf(testSetStats.getCompletedCount()));
445-
446-
ppw.addAttribute("errors", String.valueOf(testSetStats.getErrors()));
447-
448-
ppw.addAttribute("skipped", String.valueOf(testSetStats.getSkipped()));
493+
// Count actual unique test methods and their final results from classMethodStatistics (accumulated across
494+
// reruns)
495+
int actualTestCount = 0;
496+
int errors = 0;
497+
int failures = 0;
498+
int skipped = 0;
499+
int flakes = 0;
500+
501+
for (Map<String, List<WrappedReportEntry>> methodStats : classMethodStatistics.values()) {
502+
actualTestCount += methodStats.size();
503+
for (Map.Entry<String, List<WrappedReportEntry>> methodEntry : methodStats.entrySet()) {
504+
String methodName = methodEntry.getKey();
505+
List<WrappedReportEntry> methodRuns = methodEntry.getValue();
506+
TestResultType resultType = getTestResultTypeWithBeforeAllHandling(methodName, methodRuns, methodStats);
507+
508+
switch (resultType) {
509+
case ERROR:
510+
errors++;
511+
break;
512+
case FAILURE:
513+
failures++;
514+
break;
515+
case SKIPPED:
516+
skipped++;
517+
break;
518+
case FLAKE:
519+
flakes++;
520+
break;
521+
case SUCCESS:
522+
default:
523+
break;
524+
}
525+
}
526+
}
449527

450-
ppw.addAttribute("failures", String.valueOf(testSetStats.getFailures()));
528+
ppw.addAttribute("tests", String.valueOf(actualTestCount));
529+
ppw.addAttribute("errors", String.valueOf(errors));
530+
ppw.addAttribute("skipped", String.valueOf(skipped));
531+
ppw.addAttribute("failures", String.valueOf(failures));
532+
ppw.addAttribute("flakes", String.valueOf(flakes));
451533
}
452534

453535
private static void getTestProblems(

‎maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/report/DefaultReporterFactoryTest.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ public void testMergeTestHistoryResult() throws Exception {
114114
firstRunStats.add(new TestMethodStats(TEST_FIVE, ReportEntryType.SUCCESS, null));
115115
// @BeforeAll failure for a test class that will eventually succeed
116116
firstRunStats.add(new TestMethodStats(
117-
null,
117+
TEST_BEFORE_ALL_FLAKE + ".null",
118118
ReportEntryType.ERROR,
119119
new DummyStackTraceWriter(TEST_BEFORE_ALL_FLAKE + ".null " + TEST_ERROR_SUFFIX)));
120120

@@ -129,7 +129,7 @@ public void testMergeTestHistoryResult() throws Exception {
129129
secondRunStats.add(new TestMethodStats(TEST_BEFORE_ALL_FLAKE + ".testSucceed", ReportEntryType.SUCCESS, null));
130130
// @BeforeAll failure for a different class that will stay as error
131131
secondRunStats.add(new TestMethodStats(
132-
null,
132+
TEST_BEFORE_ALL_ERROR + ".null",
133133
ReportEntryType.ERROR,
134134
new DummyStackTraceWriter(TEST_BEFORE_ALL_ERROR + ".null " + TEST_ERROR_SUFFIX)));
135135

@@ -139,7 +139,9 @@ public void testMergeTestHistoryResult() throws Exception {
139139
thirdRunStats.add(new TestMethodStats(TEST_THREE, ReportEntryType.ERROR, new DummyStackTraceWriter(ERROR)));
140140
// Another @BeforeAll failure for the always-failing class
141141
thirdRunStats.add(new TestMethodStats(
142-
null, ReportEntryType.ERROR, new DummyStackTraceWriter(TEST_BEFORE_ALL_ERROR + ".null")));
142+
TEST_BEFORE_ALL_ERROR + ".null",
143+
ReportEntryType.ERROR,
144+
new DummyStackTraceWriter(TEST_BEFORE_ALL_ERROR + ".null")));
143145

144146
TestSetRunListener firstRunListener = mock(TestSetRunListener.class);
145147
TestSetRunListener secondRunListener = mock(TestSetRunListener.class);

‎surefire-its/src/test/java/org/apache/maven/surefire/its/JUnitPlatformFailingBeforeAllRerunIT.java‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.maven.surefire.its;
20+
21+
import org.apache.maven.surefire.its.fixture.OutputValidator;
22+
import org.apache.maven.surefire.its.fixture.SurefireJUnit4IntegrationTestCase;
23+
import org.junit.Test;
24+
25+
/**
26+
* Integration tests for JUnit Platform @BeforeAll failures with rerun functionality.
27+
* Tests various scenarios where @BeforeAll lifecycle methods fail and are rerun.
28+
*/
29+
public class JUnitPlatformFailingBeforeAllRerunIT extends SurefireJUnit4IntegrationTestCase {
30+
private static final String VERSION = "5.9.1";
31+
32+
private static final String TEST_PROJECT_BASE = "junit-platform-rerun-failing-before-all";
33+
34+
@Test
35+
public void testBeforeAllFailures() {
36+
// Test that @BeforeAll failures are properly handled when they succeed on rerun
37+
OutputValidator outputValidator = unpack(TEST_PROJECT_BASE)
38+
.setJUnitVersion(VERSION)
39+
.maven()
40+
.debugLogging()
41+
.addGoal("-Dsurefire.rerunFailingTestsCount=3")
42+
.withFailure()
43+
.executeTest()
44+
.assertTestSuiteResults(7, 1, 0, 0, 4);
45+
46+
// Verify the @BeforeAll is reported as a flake with proper formatting
47+
outputValidator.verifyTextInLog("junitplatform.FlakyFirstTimeTest.<beforeAll>");
48+
outputValidator.verifyTextInLog("Run 1: FlakyFirstTimeTest.setup:53 IllegalArgument");
49+
outputValidator.verifyTextInLog("Run 2: PASS");
50+
51+
// Verify XML report doesn't contain error testcase with empty name
52+
outputValidator
53+
.getSurefireReportsXmlFile("TEST-junitplatform.FlakyFirstTimeTest.xml")
54+
.assertContainsText("tests=\"4\" errors=\"0\"")
55+
.assertContainsText("name=\"testFailingTestOne\"")
56+
.assertContainsText("name=\"testErrorTestOne\"")
57+
.assertContainsText("name=\"testPassingTest\"");
58+
59+
// Verify @BeforeAll is reported as error
60+
outputValidator.verifyTextInLog("Errors:");
61+
outputValidator.verifyTextInLog("junitplatform.AlwaysFailingTest.<beforeAll>");
62+
outputValidator.verifyTextInLog("Run 3: AlwaysFailingTest.setup:15 IllegalArgument BeforeAll always fails");
63+
}
64+
}

0 commit comments

Comments
 (0)