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.

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

Fix class-level retry with Gradle 5.0 and Suite engine or @Nested test classes #231

Merged
merged 4 commits into from
Nov 3, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,13 @@
import org.gradle.testretry.internal.filter.ClassRetryMatcher;
import org.gradle.testretry.internal.filter.RetryFilter;
import org.gradle.testretry.internal.testsreader.TestsReader;
import org.gradle.util.GradleVersion;

import java.lang.reflect.Method;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;

import static org.gradle.api.tasks.testing.TestResult.ResultType.SKIPPED;

Expand All @@ -47,6 +50,7 @@ final class RetryTestResultProcessor implements TestResultProcessor {

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

private final Set<String> testClassesSeenInCurrentRound = new HashSet<>();
private TestNames currentRoundFailedTests = new TestNames();
private TestNames previousRoundFailedTests = new TestNames();

Expand Down Expand Up @@ -76,6 +80,7 @@ public void started(TestDescriptorInternal descriptor, TestStartEvent testStartE
delegate.started(descriptor, testStartEvent);
} else if (!descriptor.getId().equals(rootTestDescriptorId)) {
activeDescriptorsById.put(descriptor.getId(), descriptor);
registerSeenTestClass(descriptor);
delegate.started(descriptor, testStartEvent);
}
}
Expand Down Expand Up @@ -130,6 +135,14 @@ private boolean isLifecycleFailure(String className, String name) {
return testFrameworkStrategy.isLifecycleFailureTest(testsReader, className, name);
}

private void registerSeenTestClass(TestDescriptorInternal descriptor) {
String maybeTestClassName = descriptor.getClassName();

if (maybeTestClassName != null) {
testClassesSeenInCurrentRound.add(maybeTestClassName);
}
}

private void addRetry(String className, String name) {
if (classRetryMatcher.retryWholeClass(className)) {
currentRoundFailedTests.addClass(className);
Expand Down Expand Up @@ -207,7 +220,53 @@ private boolean currentRoundFailedTestsExceedsMaxFailures() {
}

public RoundResult getResult() {
return new RoundResult(currentRoundFailedTests, previousRoundFailedTests, lastRun(), hasRetryFilteredFailures);
return new RoundResult(currentRoundFailedTests,
cleanedUpFailedTestsOfPreviousRound(),
lastRun(),
hasRetryFilteredFailures
);
}

/**
* When running tests via the JUnit's suite engine or using {@code @Nested} test classes,
* Gradle 5.0 does not report the intermediate test class nodes. This leads to a problem when such
* test classes are configured to be retried on class-level, as we cannot properly remove them
* from the previous round of failed tests without a proper test event from Gradle.
* <p/>
* For Gradle 5.0, we manually remove all test classes with no registers test methods,
* if we saw the test class during this round. We assume here, that those entries are
* just here because of the missing event from Gradle for the intermediate node.
* <p/>
* For Gradle version 5.1 and above we don't do this, as we expect events for those intermediate
* nodes.
* <p/>
* This solution is not perfect but still allows users to use classRetry with Gradle 5
* together with the suite engine and/or nested test classes.
*
* @return cleaned up failed test names of previous round
*/
private TestNames cleanedUpFailedTestsOfPreviousRound() {
boolean isGradle50 = GradleVersion.current().getBaseVersion().equals(GradleVersion.version("5.0"));

if (isGradle50 && !testClassesSeenInCurrentRound.isEmpty() || previousRoundFailedTests.hasClassesWithoutTestNames()) {
pshevche marked this conversation as resolved.
Show resolved Hide resolved
TestNames testNames = new TestNames();
previousRoundFailedTests.stream().forEach(entry -> {
String testClass = entry.getKey();
Set<String> testMethods = entry.getValue();

if (testMethods.isEmpty()) {
if (!testClassesSeenInCurrentRound.contains(testClass)) {
testNames.addClass(testClass);
}
} else {
testNames.addAll(testClass, testMethods);
}
});

return testNames;
}

return previousRoundFailedTests;
}

public void reset(boolean lastRetry) {
Expand All @@ -216,6 +275,7 @@ public void reset(boolean lastRetry) {
}

this.lastRetry = lastRetry;
this.testClassesSeenInCurrentRound.clear();
this.previousRoundFailedTests = currentRoundFailedTests;
this.currentRoundFailedTests = new TestNames();
this.activeDescriptorsById.clear();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ public void add(String className, String testName) {
map.computeIfAbsent(className, ignored -> new HashSet<>()).add(testName);
}

public void addAll(String className, Set<String> testNames) {
map.computeIfAbsent(className, ignored -> new HashSet<>()).addAll(testNames);
}

public void addClass(String className) {
map.put(className, emptySet());
}
Expand Down Expand Up @@ -62,6 +66,11 @@ public boolean remove(String className, String testName) {
}
}

public boolean hasClassesWithoutTestNames() {
return map.values().stream()
.anyMatch(Set::isEmpty);
}

public Stream<Map.Entry<String, Set<String>>> stream() {
return map.entrySet().stream();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*
* Copyright 2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.gradle.testretry.internal.executer

import spock.lang.Specification
import spock.lang.Subject

class TestNamesTest extends Specification {

@Subject
def testNames = new TestNames()

def "removing testName works properly"() {
given:
testNames.addAll("TestClass", ["test1()", "test2()", "test10()"] as Set)

when:
testNames.remove("TestClass", "test2()")

then:
methodsFor("TestClass") ==~ ["test1()", "test10()"]
}

def "removing testName via predicate works properly"() {
given:
testNames.addAll("TestClass", ["test1()", "test2()", "test10()"] as Set)

when:
testNames.remove("TestClass", testMethod -> testMethod.contains("test1"))

then:
methodsFor("TestClass") ==~ ["test2()"]
}

def "hasClassesWithoutTestNames works properly"() {
when:
testNames.add("TestClass", "test()")

then:
!testNames.hasClassesWithoutTestNames()

when:
testNames.addClass("TestClassWithNoMethods")

then:
testNames.hasClassesWithoutTestNames()
}

private Set<String> methodsFor(String testClass) {
def entry = testNames.stream()
.filter { it.key == testClass }
.findFirst()

assert entry.isPresent()

entry.get().value as Set
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,36 @@ package org.gradle.testretry.testframework

import org.gradle.testretry.AbstractFrameworkFuncTest

import javax.annotation.Nullable

class JUnit5FuncTest extends AbstractFrameworkFuncTest {
@Override
String getLanguagePlugin() {
return 'java'
}

protected String afterClassErrorTestMethodName(String gradleVersion) {
private static String afterClassErrorTestMethodName(String gradleVersion) {
gradleVersion == "5.0" ? "classMethod" : "executionError"
}

protected String beforeClassErrorTestMethodName(String gradleVersion) {
private static String beforeClassErrorTestMethodName(String gradleVersion) {
gradleVersion == "5.0" ? "classMethod" : "initializationError"
}

private static String classAndMethodForSuite(String className, String testName, String gradleVersion) {
gradleVersion == "5.0" ? "${className}.${testName}" : "${className} > ${testName}"
}

private static String classAndMethodForNested(String parentClassName, @Nullable String nestedClassName, String testName, String gradleVersion) {
if (nestedClassName == null) {
"${parentClassName} > ${testName}"
} else {
gradleVersion == "5.0"
? "${nestedClassName}.${testName}"
: "${nestedClassName} > ${testName}"
}
}

def "handles failure in #lifecycle - exhaustive #exhaust (gradle version #gradleVersion)"(String gradleVersion, String lifecycle, boolean exhaust) {
given:
buildFile << """
Expand Down Expand Up @@ -518,14 +534,15 @@ class JUnit5FuncTest extends AbstractFrameworkFuncTest {

then:
with(result.output) {
it.count('Test1 > testOk() PASSED') == 2
it.count('Test1 > testFlaky() FAILED') == 1
it.count('Test1 > testFlaky() PASSED') == 1
it.count("${classAndMethodForSuite("Test1", "testOk()", gradleVersion)} PASSED") == 2
it.count("${classAndMethodForSuite("Test1", "testFlaky()", gradleVersion)} FAILED") == 1
it.count("${classAndMethodForSuite("Test1", "testFlaky()", gradleVersion)} PASSED") == 1

// Test2 is retried on method level
it.count('Test2 > testOk() PASSED') == 1
it.count('Test2 > testFlaky() FAILED') == 1
it.count('Test2 > testFlaky() PASSED') == 1
it.count("${classAndMethodForSuite("Test2", "testOk()", gradleVersion)} PASSED") == 1
it.count("${classAndMethodForSuite("Test2", "testFlaky()", gradleVersion)} FAILED") == 1
it.count("${classAndMethodForSuite("Test2", "testFlaky()", gradleVersion)} PASSED") == 1

}

where:
Expand Down Expand Up @@ -580,14 +597,14 @@ class JUnit5FuncTest extends AbstractFrameworkFuncTest {
then:
with(result.output) {
// only failing methods of TopLevelTest should be retried
it.count('TopLevelTest > testOk() PASSED') == 1
it.count('TopLevelTest > testFlaky() FAILED') == 1
it.count('TopLevelTest > testFlaky() PASSED') == 1
it.count("${classAndMethodForNested('TopLevelTest', null, 'testOk()', gradleVersion)} PASSED") == 1
it.count("${classAndMethodForNested('TopLevelTest', null, 'testFlaky()', gradleVersion)} FAILED") == 1
it.count("${classAndMethodForNested('TopLevelTest', null, 'testFlaky()', gradleVersion)} PASSED") == 1

// all methods of NestedTest1 should be retried
it.count('NestedTest > testOk() PASSED') == 2
it.count('NestedTest > testFlaky() FAILED') == 1
it.count('NestedTest > testFlaky() PASSED') == 1
it.count("${classAndMethodForNested('TopLevelTest', 'NestedTest', 'testOk()', gradleVersion)} PASSED") == 2
it.count("${classAndMethodForNested('TopLevelTest', 'NestedTest', 'testFlaky()', gradleVersion)} FAILED") == 1
it.count("${classAndMethodForNested('TopLevelTest', 'NestedTest', 'testFlaky()', gradleVersion)} PASSED") == 1
}

where:
Expand Down Expand Up @@ -644,13 +661,13 @@ class JUnit5FuncTest extends AbstractFrameworkFuncTest {
then:
with(result.output) {
// all methods of TopLevelTest are rerun
it.count('TopLevelTest > testOk() PASSED') == 2
it.count('TopLevelTest > testFlaky() FAILED') == 1
it.count('TopLevelTest > testFlaky() PASSED') == 1
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('NestedTest1 > testOk() PASSED') == 2
it.count('NestedTest2 > testOk() PASSED') == 2
it.count("${classAndMethodForNested('TopLevelTest', 'NestedTest1', 'testOk()', gradleVersion)} PASSED") == 2
it.count("${classAndMethodForNested('TopLevelTest', 'NestedTest2', 'testOk()', gradleVersion)} PASSED") == 2
}

where:
Expand Down