Skip to content

Revert PR #3194 that handle beforeAll failures to follow proper contributing rules #3211

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Revert PR #3194 that handle beforeAll failures to follow proper contributing rules #3211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 0 additions & 86 deletions ...-common/src/main/java/org/apache/maven/plugin/surefire/report/DefaultReporterFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,6 @@ public class DefaultReporterFactory implements ReporterFactory, ReportsMerger {

private RunStatistics globalStats = new RunStatistics();

// from "<testclass>.<testmethod>" -> statistics about all the runs for success tests
private Map<String, List<TestMethodStats>> successTests;

// from "<testclass>.<testmethod>" -> statistics about all the runs for flaky tests
private Map<String, List<TestMethodStats>> flakyTests;

Expand Down Expand Up @@ -246,7 +243,6 @@ static TestResultType getTestResultType(List<ReportEntryType> reportEntries, int
*/
private void mergeTestHistoryResult() {
globalStats = new RunStatistics();
successTests = new TreeMap<>();
flakyTests = new TreeMap<>();
failedTests = new TreeMap<>();
errorTests = new TreeMap<>();
Expand All @@ -269,32 +265,10 @@ private void mergeTestHistoryResult() {

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

for (Map.Entry<String, List<TestMethodStats>> entry : mergedTestHistoryResult.entrySet()) {
List<TestMethodStats> testMethodStats = entry.getValue();
String testClassMethodName = entry.getKey();

// Handle @BeforeAll failures (null method names)
if (testClassMethodName == null) {
for (TestMethodStats methodStats : testMethodStats) {
String className = extractClassNameFromStackTrace(methodStats);
if (className != null) {
if (beforeAllFailures.containsKey(className)) {
List<TestMethodStats> previousMethodStats = beforeAllFailures.get(className);
previousMethodStats.add(methodStats);
beforeAllFailures.put(className, previousMethodStats);
} else {
List<TestMethodStats> initMethodStats = new ArrayList<>();
initMethodStats.add(methodStats);
beforeAllFailures.put(className, initMethodStats);
}
}
}
// Skip normal processing of @BeforeAll failures because it needs special care
continue;
}

completedCount++;

List<ReportEntryType> resultTypes = new ArrayList<>();
Expand All @@ -312,16 +286,6 @@ private void mergeTestHistoryResult() {
}
}
completedCount += successCount - 1;
successTests.put(testClassMethodName, testMethodStats);

// If current test belong to class that failed during beforeAll store that info to proper log info
String className = extractClassNameFromMethodName(testClassMethodName);
if (beforeAllFailures.containsKey(className)) {
List<TestMethodStats> previousMethodStats = beforeAllFailures.get(className);
previousMethodStats.addAll(testMethodStats);
beforeAllFailures.put(className, previousMethodStats);
}

break;
case SKIPPED:
skipped++;
Expand All @@ -340,21 +304,6 @@ private void mergeTestHistoryResult() {
}
}

// Process @BeforeAll failures after we know which classes have successful tests
for (Map.Entry<String, List<TestMethodStats>> entry : beforeAllFailures.entrySet()) {
String className = entry.getKey();
List<TestMethodStats> testMethodStats = entry.getValue();
String classNameKey = className + ".<beforeAll>";

if (reportConfiguration.getRerunFailingTestsCount() > 0
&& testMethodStats.stream().anyMatch(methodStats -> methodStats.getTestClassMethodName() != null)) {
flakyTests.put(classNameKey, testMethodStats);
} else {
errorTests.put(classNameKey, testMethodStats);
completedCount++;
}
}

globalStats.set(completedCount, errorTests.size(), failedTests.size(), skipped, flakyTests.size());
}

Expand Down Expand Up @@ -413,41 +362,6 @@ boolean printTestFailures(TestResultType type) {
return printed;
}

/**
* Extract class name from test class method name like "com.example.TestClass.methodName"
*/
private static String extractClassNameFromMethodName(String testClassMethodName) {
if (testClassMethodName == null || testClassMethodName.isEmpty()) {
return null;
}
int lastDotIndex = testClassMethodName.lastIndexOf('.');
if (lastDotIndex > 0) {
return testClassMethodName.substring(0, lastDotIndex);
}
return null;
}

/**
* Extract class name from stack trace when method name is null (e.g., @BeforeAll failures)
*/
private static String extractClassNameFromStackTrace(TestMethodStats stats) {
if (stats.getStackTraceWriter() == null) {
return null;
}
String stackTrace = stats.getStackTraceWriter().smartTrimmedStackTrace();
if (stackTrace == null || stackTrace.isEmpty()) {
return null;
}

// Strip everything after the first whitespace character
int firstWhitespace = stackTrace.indexOf(' ');
if (firstWhitespace > 0) {
stackTrace = stackTrace.substring(0, firstWhitespace);
}

return extractClassNameFromMethodName(stackTrace);
}

// Describe the result of a given test
enum TestResultType {
ERROR("Errors: "),
Expand Down
46 changes: 6 additions & 40 deletions ...mon/src/test/java/org/apache/maven/plugin/surefire/report/DefaultReporterFactoryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,6 @@ public class DefaultReporterFactoryTest extends TestCase {

private static final String ERROR = "error";

private static final String TEST_BEFORE_ALL_FLAKE = "com.example.FlakyClass";

private static final String TEST_BEFORE_ALL_ERROR = "com.example.AlwaysFailClass";

private static final String TEST_ERROR_SUFFIX = "-- Time elapsed: 292.2 s <<< ERROR!";

public void testMergeTestHistoryResult() throws Exception {
MessageUtils.setColorEnabled(false);
File target = new File(System.getProperty("user.dir"), "target");
Expand Down Expand Up @@ -103,7 +97,7 @@ public void testMergeTestHistoryResult() throws Exception {

DefaultReporterFactory factory = new DefaultReporterFactory(reportConfig, reporter);

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

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

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

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

// TEST_THREE and AlwaysFailClass.null are failing tests, regular tests + FlakyClass.null are flaky
assertEquals(7, mergedStatistics.getCompletedCount());
assertEquals(2, mergedStatistics.getErrors());
// Only TEST_THREE is a failing test, other three are flaky tests
assertEquals(5, mergedStatistics.getCompletedCount());
assertEquals(1, mergedStatistics.getErrors());
assertEquals(0, mergedStatistics.getFailures());
assertEquals(4, mergedStatistics.getFlakes());
assertEquals(3, mergedStatistics.getFlakes());
assertEquals(0, mergedStatistics.getSkipped());

// Now test the result will be printed out correctly
factory.printTestFailures(TestResultType.FLAKE);
String[] expectedFlakeOutput = {
"Flakes: ",
TEST_BEFORE_ALL_FLAKE + ".<beforeAll>",
" Run 1: " + TEST_BEFORE_ALL_FLAKE + ".null " + TEST_ERROR_SUFFIX,
" Run 2: PASS",
"",
TEST_FOUR,
" Run 1: " + ASSERTION_FAIL,
" Run 2: PASS",
Expand All @@ -189,16 +164,7 @@ public void testMergeTestHistoryResult() throws Exception {
reporter.reset();
factory.printTestFailures(TestResultType.ERROR);
String[] expectedFailureOutput = {
"Errors: ",
TEST_BEFORE_ALL_ERROR + ".<beforeAll>",
" Run 1: " + TEST_BEFORE_ALL_ERROR + ".null " + TEST_ERROR_SUFFIX,
" Run 2: " + TEST_BEFORE_ALL_ERROR + ".null",
"",
TEST_THREE,
" Run 1: " + ASSERTION_FAIL,
" Run 2: " + ERROR,
" Run 3: " + ERROR,
""
"Errors: ", TEST_THREE, " Run 1: " + ASSERTION_FAIL, " Run 2: " + ERROR, " Run 3: " + ERROR, ""
};
assertEquals(asList(expectedFailureOutput), reporter.getMessages());

Expand Down