Skip to content
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

Correctly handle retries of classes with nested classes #235

Merged
merged 2 commits into from
Nov 20, 2023
Merged

Correctly handle retries of classes with nested classes #235

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3f3d5c3
Update test to actually exercise desired behavior
pshevche Nov 15, 2023
437a5e8
Detect the first class in the test class hierarchy to match the class…
pshevche Nov 15, 2023
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
48 changes: 41 additions & 7 deletions plugin/src/main/java/org/gradle/testretry/internal/executer/RetryTestResultProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Optional;
import java.util.Set;

import static org.gradle.api.tasks.testing.TestResult.ResultType.SKIPPED;
Expand All @@ -49,6 +50,7 @@ final class RetryTestResultProcessor implements TestResultProcessor {
private Method failureMethod;

private final Map<Object, TestDescriptorInternal> activeDescriptorsById = new HashMap<>();
private final Map<Object, Object> parentIdByDescriptorId = new HashMap<>();

private final Set<String> testClassesSeenInCurrentRound = new HashSet<>();
private TestNames currentRoundFailedTests = new TestNames();
Expand Down Expand Up @@ -80,6 +82,7 @@ public void started(TestDescriptorInternal descriptor, TestStartEvent testStartE
delegate.started(descriptor, testStartEvent);
} else if (!descriptor.getId().equals(rootTestDescriptorId)) {
activeDescriptorsById.put(descriptor.getId(), descriptor);
parentIdByDescriptorId.put(descriptor.getId(), testStartEvent.getParentId());
registerSeenTestClass(descriptor);
delegate.started(descriptor, testStartEvent);
}
Expand All @@ -99,7 +102,7 @@ public void completed(Object testId, TestCompleteEvent testCompleteEvent) {

boolean failedInPreviousRound = previousRoundFailedTests.remove(className, name);
if (failedInPreviousRound && testCompleteEvent.getResultType() == SKIPPED) {
addRetry(className, name);
addRetry(descriptor);
}

// class-level lifecycle failures do not guarantee that all methods that failed in the previous round will be re-executed (e.g. due to class setup failure)
Expand All @@ -108,7 +111,7 @@ public void completed(Object testId, TestCompleteEvent testCompleteEvent) {
if (isLifecycleFailure(className, name)) {
previousRoundFailedTests.remove(className, n -> {
if (isLifecycleFailure(className, n)) {
addRetry(className, n);
currentRoundFailedTests.add(className, n);
}
return true;
});
Expand Down Expand Up @@ -143,14 +146,44 @@ private void registerSeenTestClass(TestDescriptorInternal descriptor) {
}
}

private void addRetry(String className, String name) {
if (classRetryMatcher.retryWholeClass(className)) {
currentRoundFailedTests.addClass(className);
private void addRetry(TestDescriptorInternal descriptor) {
Optional<TestDescriptorInternal> classMatchingClassRetryFilter = firstClassMatchingClassRetryFilter(descriptor);
if (classMatchingClassRetryFilter.isPresent()) {
currentRoundFailedTests.addClass(classMatchingClassRetryFilter.get().getClassName());
} else {
currentRoundFailedTests.add(className, name);
currentRoundFailedTests.add(descriptor.getClassName(), descriptor.getName());
}
}

private Optional<TestDescriptorInternal> firstClassMatchingClassRetryFilter(TestDescriptorInternal descriptor) {
// top-level descriptor describes a test worker which cannot match the class retry filter
Object parentId = parentIdByDescriptorId.get(descriptor.getId());
if (parentId == null) {
return Optional.empty();
}

// if the parent is not tracked for any reason, then it also cannot match the class retry filter
TestDescriptorInternal parentDescriptor = activeDescriptorsById.get(parentId);
if (parentDescriptor == null) {
return Optional.empty();
}

// check if any of the parent classes matches the class retry filter
Optional<TestDescriptorInternal> parentClassToRetryEntirely = firstClassMatchingClassRetryFilter(parentDescriptor);
if (parentClassToRetryEntirely.isPresent()) {
return parentClassToRetryEntirely;
}

// check if the class on the current level matches the class retry filter
String className = descriptor.getClassName();
if (className != null && classRetryMatcher.retryWholeClass(className)) {
return Optional.of(descriptor);
}

// no classes in the descriptor hierarchy should be retried as a whole
return Optional.empty();
}

private void emitFakePassedEvent(TestDescriptorInternal parent, TestCompleteEvent parentEvent, String name) {
Object syntheticTestId = new Object();
TestDescriptorInternal syntheticDescriptor = new TestDescriptorImpl(syntheticTestId, parent, name);
Expand Down Expand Up @@ -201,7 +234,7 @@ private void failure(Object testId) {
String className = descriptor.getClassName();
if (className != null) {
if (filter.canRetry(className)) {
addRetry(className, descriptor.getName());
addRetry(descriptor);
} else {
hasRetryFilteredFailures = true;
}
Expand Down Expand Up @@ -279,6 +312,7 @@ public void reset(boolean lastRetry) {
this.previousRoundFailedTests = currentRoundFailedTests;
this.currentRoundFailedTests = new TestNames();
this.activeDescriptorsById.clear();
this.parentIdByDescriptorId.clear();
}

}
15 changes: 8 additions & 7 deletions plugin/src/test/groovy/org/gradle/testretry/testframework/JUnit5FuncTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -634,16 +634,16 @@ class JUnit5FuncTest extends AbstractFrameworkFuncTest {
void testOk() {
}

@Test
void testFlaky() {
${flakyAssert("topLevel")}
}

@Nested
class NestedTest1 {
@Test
void testOk() {
}

@Test
void testFlaky() {
${flakyAssert("topLevel")}
}
}

@Nested
Expand All @@ -662,11 +662,12 @@ class JUnit5FuncTest extends AbstractFrameworkFuncTest {
with(result.output) {
// all methods of TopLevelTest are rerun
it.count("${classAndMethodForNested('TopLevelTest', null, 'testOk()', gradleVersion)} PASSED") == 2
it.count("${classAndMethodForNested('TopLevelTest', null, 'testFlaky()', gradleVersion)} FAILED") == 1
it.count("${classAndMethodForNested('TopLevelTest', null, 'testFlaky()', gradleVersion)} PASSED") == 1

// all methods of nested classes are retried
it.count("${classAndMethodForNested('TopLevelTest', 'NestedTest1', 'testOk()', gradleVersion)} PASSED") == 2
it.count("${classAndMethodForNested('TopLevelTest', 'NestedTest1', 'testFlaky()', gradleVersion)} FAILED") == 1
it.count("${classAndMethodForNested('TopLevelTest', 'NestedTest1', 'testFlaky()', gradleVersion)} PASSED") == 1

it.count("${classAndMethodForNested('TopLevelTest', 'NestedTest2', 'testOk()', gradleVersion)} PASSED") == 2
}

Expand Down