Skip to content

Commit 735963d

Browse files
FrawlessFrawless
committed
Properly work with test failures caused during beforeAll phase
Signed-off-by: Jakub Stejskal <xstejs24@gmail.com>
1 parent 1ec30aa commit 735963d

File tree

2 files changed

+126
-6
lines changed

2 files changed

+126
-6
lines changed

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

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,9 @@ public class DefaultReporterFactory implements ReporterFactory, ReportsMerger {
7272

7373
private RunStatistics globalStats = new RunStatistics();
7474

75+
// from "<testclass>.<testmethod>" -> statistics about all the runs for success tests
76+
private Map<String, List<TestMethodStats>> successTests;
77+
7578
// from "<testclass>.<testmethod>" -> statistics about all the runs for flaky tests
7679
private Map<String, List<TestMethodStats>> flakyTests;
7780

@@ -243,6 +246,7 @@ static TestResultType getTestResultType(List<ReportEntryType> reportEntries, int
243246
*/
244247
private void mergeTestHistoryResult() {
245248
globalStats = new RunStatistics();
249+
successTests = new TreeMap<>();
246250
flakyTests = new TreeMap<>();
247251
failedTests = new TreeMap<>();
248252
errorTests = new TreeMap<>();
@@ -265,10 +269,32 @@ private void mergeTestHistoryResult() {
265269

266270
// Update globalStatistics by iterating through mergedTestHistoryResult
267271
int completedCount = 0, skipped = 0;
272+
Map<String, List<TestMethodStats>> beforeAllFailures = new HashMap<>();
268273

269274
for (Map.Entry<String, List<TestMethodStats>> entry : mergedTestHistoryResult.entrySet()) {
270275
List<TestMethodStats> testMethodStats = entry.getValue();
271276
String testClassMethodName = entry.getKey();
277+
278+
// Handle @BeforeAll failures (null method names)
279+
if (testClassMethodName == null) {
280+
for (TestMethodStats methodStats : testMethodStats) {
281+
String className = extractClassNameFromStackTrace(methodStats);
282+
if (className != null) {
283+
if (beforeAllFailures.containsKey(className)) {
284+
List<TestMethodStats> previousMethodStats = beforeAllFailures.get(className);
285+
previousMethodStats.add(methodStats);
286+
beforeAllFailures.put(className, previousMethodStats);
287+
} else {
288+
List<TestMethodStats> initMethodStats = new ArrayList<>();
289+
initMethodStats.add(methodStats);
290+
beforeAllFailures.put(className, initMethodStats);
291+
}
292+
}
293+
}
294+
// Skip normal processing of @BeforeAll failures because it needs special care
295+
continue;
296+
}
297+
272298
completedCount++;
273299

274300
List<ReportEntryType> resultTypes = new ArrayList<>();
@@ -286,6 +312,16 @@ private void mergeTestHistoryResult() {
286312
}
287313
}
288314
completedCount += successCount - 1;
315+
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+
289325
break;
290326
case SKIPPED:
291327
skipped++;
@@ -304,6 +340,21 @@ private void mergeTestHistoryResult() {
304340
}
305341
}
306342

343+
// Process @BeforeAll failures after we know which classes have successful tests
344+
for (Map.Entry<String, List<TestMethodStats>> entry : beforeAllFailures.entrySet()) {
345+
String className = entry.getKey();
346+
List<TestMethodStats> testMethodStats = entry.getValue();
347+
String classNameKey = className + ".<beforeAll>";
348+
349+
if (reportConfiguration.getRerunFailingTestsCount() > 0
350+
&& testMethodStats.stream().anyMatch(methodStats -> methodStats.getTestClassMethodName() != null)) {
351+
flakyTests.put(classNameKey, testMethodStats);
352+
} else {
353+
errorTests.put(classNameKey, testMethodStats);
354+
completedCount++;
355+
}
356+
}
357+
307358
globalStats.set(completedCount, errorTests.size(), failedTests.size(), skipped, flakyTests.size());
308359
}
309360

@@ -362,6 +413,41 @@ boolean printTestFailures(TestResultType type) {
362413
return printed;
363414
}
364415

416+
/**
417+
* Extract class name from test class method name like "com.example.TestClass.methodName"
418+
*/
419+
private static String extractClassNameFromMethodName(String testClassMethodName) {
420+
if (testClassMethodName == null || testClassMethodName.isEmpty()) {
421+
return null;
422+
}
423+
int lastDotIndex = testClassMethodName.lastIndexOf('.');
424+
if (lastDotIndex > 0) {
425+
return testClassMethodName.substring(0, lastDotIndex);
426+
}
427+
return null;
428+
}
429+
430+
/**
431+
* Extract class name from stack trace when method name is null (e.g., @BeforeAll failures)
432+
*/
433+
private static String extractClassNameFromStackTrace(TestMethodStats stats) {
434+
if (stats.getStackTraceWriter() == null) {
435+
return null;
436+
}
437+
String stackTrace = stats.getStackTraceWriter().smartTrimmedStackTrace();
438+
if (stackTrace == null || stackTrace.isEmpty()) {
439+
return null;
440+
}
441+
442+
// Strip everything after the first whitespace character
443+
int firstWhitespace = stackTrace.indexOf(' ');
444+
if (firstWhitespace > 0) {
445+
stackTrace = stackTrace.substring(0, firstWhitespace);
446+
}
447+
448+
return extractClassNameFromMethodName(stackTrace);
449+
}
450+
365451
// Describe the result of a given test
366452
enum TestResultType {
367453
ERROR("Errors: "),

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

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,12 @@ public class DefaultReporterFactoryTest extends TestCase {
6868

6969
private static final String ERROR = "error";
7070

71+
private static final String TEST_BEFORE_ALL_FLAKE = "com.example.FlakyClass";
72+
73+
private static final String TEST_BEFORE_ALL_ERROR = "com.example.AlwaysFailClass";
74+
75+
private static final String TEST_ERROR_SUFFIX = "-- Time elapsed: 292.2 s <<< ERROR!";
76+
7177
public void testMergeTestHistoryResult() throws Exception {
7278
MessageUtils.setColorEnabled(false);
7379
File target = new File(System.getProperty("user.dir"), "target");
@@ -97,7 +103,7 @@ public void testMergeTestHistoryResult() throws Exception {
97103

98104
DefaultReporterFactory factory = new DefaultReporterFactory(reportConfig, reporter);
99105

100-
// First run, four tests failed and one passed
106+
// First run, four tests failed and one passed, plus @BeforeAll failure
101107
Queue<TestMethodStats> firstRunStats = new ArrayDeque<>();
102108
firstRunStats.add(new TestMethodStats(TEST_ONE, ReportEntryType.ERROR, new DummyStackTraceWriter(ERROR)));
103109
firstRunStats.add(new TestMethodStats(TEST_TWO, ReportEntryType.ERROR, new DummyStackTraceWriter(ERROR)));
@@ -106,6 +112,11 @@ public void testMergeTestHistoryResult() throws Exception {
106112
firstRunStats.add(
107113
new TestMethodStats(TEST_FOUR, ReportEntryType.FAILURE, new DummyStackTraceWriter(ASSERTION_FAIL)));
108114
firstRunStats.add(new TestMethodStats(TEST_FIVE, ReportEntryType.SUCCESS, null));
115+
// @BeforeAll failure for a test class that will eventually succeed
116+
firstRunStats.add(new TestMethodStats(
117+
null,
118+
ReportEntryType.ERROR,
119+
new DummyStackTraceWriter(TEST_BEFORE_ALL_FLAKE + ".null " + TEST_ERROR_SUFFIX)));
109120

110121
// Second run, two tests passed
111122
Queue<TestMethodStats> secondRunStats = new ArrayDeque<>();
@@ -114,11 +125,21 @@ public void testMergeTestHistoryResult() throws Exception {
114125
secondRunStats.add(new TestMethodStats(TEST_TWO, ReportEntryType.SUCCESS, null));
115126
secondRunStats.add(new TestMethodStats(TEST_THREE, ReportEntryType.ERROR, new DummyStackTraceWriter(ERROR)));
116127
secondRunStats.add(new TestMethodStats(TEST_FOUR, ReportEntryType.SUCCESS, null));
128+
// Successful test from the class that had @BeforeAll failure
129+
secondRunStats.add(new TestMethodStats(TEST_BEFORE_ALL_FLAKE + ".testSucceed", ReportEntryType.SUCCESS, null));
130+
// @BeforeAll failure for a different class that will stay as error
131+
secondRunStats.add(new TestMethodStats(
132+
null,
133+
ReportEntryType.ERROR,
134+
new DummyStackTraceWriter(TEST_BEFORE_ALL_ERROR + ".null " + TEST_ERROR_SUFFIX)));
117135

118136
// Third run, another test passed
119137
Queue<TestMethodStats> thirdRunStats = new ArrayDeque<>();
120138
thirdRunStats.add(new TestMethodStats(TEST_ONE, ReportEntryType.SUCCESS, null));
121139
thirdRunStats.add(new TestMethodStats(TEST_THREE, ReportEntryType.ERROR, new DummyStackTraceWriter(ERROR)));
140+
// Another @BeforeAll failure for the always-failing class
141+
thirdRunStats.add(new TestMethodStats(
142+
null, ReportEntryType.ERROR, new DummyStackTraceWriter(TEST_BEFORE_ALL_ERROR + ".null")));
122143

123144
TestSetRunListener firstRunListener = mock(TestSetRunListener.class);
124145
TestSetRunListener secondRunListener = mock(TestSetRunListener.class);
@@ -134,17 +155,21 @@ public void testMergeTestHistoryResult() throws Exception {
134155
invokeMethod(factory, "mergeTestHistoryResult");
135156
RunStatistics mergedStatistics = factory.getGlobalRunStatistics();
136157

137-
// Only TEST_THREE is a failing test, other three are flaky tests
138-
assertEquals(5, mergedStatistics.getCompletedCount());
139-
assertEquals(1, mergedStatistics.getErrors());
158+
// TEST_THREE and AlwaysFailClass.null are failing tests, regular tests + FlakyClass.null are flaky
159+
assertEquals(7, mergedStatistics.getCompletedCount());
160+
assertEquals(2, mergedStatistics.getErrors());
140161
assertEquals(0, mergedStatistics.getFailures());
141-
assertEquals(3, mergedStatistics.getFlakes());
162+
assertEquals(4, mergedStatistics.getFlakes());
142163
assertEquals(0, mergedStatistics.getSkipped());
143164

144165
// Now test the result will be printed out correctly
145166
factory.printTestFailures(TestResultType.FLAKE);
146167
String[] expectedFlakeOutput = {
147168
"Flakes: ",
169+
TEST_BEFORE_ALL_FLAKE + ".<beforeAll>",
170+
" Run 1: " + TEST_BEFORE_ALL_FLAKE + ".null " + TEST_ERROR_SUFFIX,
171+
" Run 2: PASS",
172+
"",
148173
TEST_FOUR,
149174
" Run 1: " + ASSERTION_FAIL,
150175
" Run 2: PASS",
@@ -164,7 +189,16 @@ public void testMergeTestHistoryResult() throws Exception {
164189
reporter.reset();
165190
factory.printTestFailures(TestResultType.ERROR);
166191
String[] expectedFailureOutput = {
167-
"Errors: ", TEST_THREE, " Run 1: " + ASSERTION_FAIL, " Run 2: " + ERROR, " Run 3: " + ERROR, ""
192+
"Errors: ",
193+
TEST_BEFORE_ALL_ERROR + ".<beforeAll>",
194+
" Run 1: " + TEST_BEFORE_ALL_ERROR + ".null " + TEST_ERROR_SUFFIX,
195+
" Run 2: " + TEST_BEFORE_ALL_ERROR + ".null",
196+
"",
197+
TEST_THREE,
198+
" Run 1: " + ASSERTION_FAIL,
199+
" Run 2: " + ERROR,
200+
" Run 3: " + ERROR,
201+
""
168202
};
169203
assertEquals(asList(expectedFailureOutput), reporter.getMessages());
170204

0 commit comments

Comments
 (0)