Skip to content

Commit

Permalink
Retry new or modified tests based on git diff
Browse files Browse the repository at this point in the history
The best time to resolve flaky test is once you wrote that test. This is
how Google looks for flakes in their monorepo. At that moment you have
the best knowledge to solve such tests. Otherwise after month of running
such test the cognitive load to fix it is very high.

This PR adds a way to discover all changed test classes with diff to
master branch and retry them until the maxRetry or to the first fail
execution.

Signed-off-by: Michał Rowicki <[email protected]>
  • Loading branch information
wyhasany committed May 26, 2021
1 parent cbc7a97 commit 7f1c5ff
Show file tree
Hide file tree
Showing 9 changed files with 439 additions and 9 deletions.
2 changes: 2 additions & 0 deletions plugin/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,14 @@ configurations.getByName("compileOnly").extendsFrom(plugin)

dependencies {
plugin("org.ow2.asm:asm:9.0")
plugin("org.eclipse.jgit:org.eclipse.jgit:5.11.1.202105131744-r")

testImplementation(gradleTestKit())
testImplementation(localGroovy())
testImplementation("org.spockframework:spock-core:1.3-groovy-2.5")
testImplementation("net.sourceforge.nekohtml:nekohtml:1.9.22")
testImplementation("org.ow2.asm:asm:8.0.1")
testImplementation("org.eclipse.jgit:org.eclipse.jgit:5.11.1.202105131744-r")

codenarc("org.codenarc:CodeNarc:1.0")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,22 @@ public interface TestRetryTaskExtension {
*/
Property<Integer> getMaxFailures();

/**
* Whether tests that has been modified should be retried.
* <p>
* Discovers modification of test by git diff with local master branch.
* All test cases in modified class are rerun until rich maxRetries or
* to the first fail execution.
* <p>
* This setting defaults to {@code false},
* which disables that feature.
* <p>
* This setting has no effect if gradle isn't run in git repository context.
*
* @return whether modified/new tests should be retried
*/
Property<Boolean> getModifiedTestRetry();

/**
* The filter for specifying which tests may be retried.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,15 @@
public class DefaultTestRetryTaskExtension implements TestRetryTaskExtension {

private final Property<Boolean> failOnPassedAfterRetry;
private final Property<Boolean> modifiedTestRetry;
private final Property<Integer> maxRetries;
private final Property<Integer> maxFailures;
private final Filter filter;

@Inject
public DefaultTestRetryTaskExtension(ObjectFactory objects) {
this.failOnPassedAfterRetry = objects.property(Boolean.class);
this.modifiedTestRetry = objects.property(Boolean.class);
this.maxRetries = objects.property(Integer.class);
this.maxFailures = objects.property(Integer.class);
this.filter = new FilterImpl(objects);
Expand All @@ -42,6 +44,11 @@ public Property<Boolean> getFailOnPassedAfterRetry() {
return failOnPassedAfterRetry;
}

@Override
public Property<Boolean> getModifiedTestRetry() {
return modifiedTestRetry;
}

public Property<Integer> getMaxRetries() {
return maxRetries;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public final class TestRetryTaskExtensionAdapter {
private static final int DEFAULT_MAX_RETRIES = 0;
private static final int DEFAULT_MAX_FAILURES = 0;
private static final boolean DEFAULT_FAIL_ON_PASSED_AFTER_RETRY = false;
private static final boolean DEFAULT_MODIFIED_TEST_RETRY = false;

private final ProviderFactory providerFactory;
private final TestRetryTaskExtension extension;
Expand Down Expand Up @@ -63,6 +64,7 @@ private void initialize(TestRetryTaskExtension extension, boolean gradle51OrLate
if (gradle51OrLater) {
extension.getMaxRetries().convention(DEFAULT_MAX_RETRIES);
extension.getMaxFailures().convention(DEFAULT_MAX_FAILURES);
extension.getModifiedTestRetry().convention(DEFAULT_MODIFIED_TEST_RETRY);
extension.getFailOnPassedAfterRetry().convention(DEFAULT_FAIL_ON_PASSED_AFTER_RETRY);
extension.getFilter().getIncludeClasses().convention(Collections.emptySet());
extension.getFilter().getIncludeAnnotationClasses().convention(Collections.emptySet());
Expand Down Expand Up @@ -103,6 +105,10 @@ public int getMaxFailures() {
return read(extension.getMaxFailures(), DEFAULT_MAX_FAILURES);
}

public boolean getModifiedTestRetry() {
return read(extension.getModifiedTestRetry(), DEFAULT_MODIFIED_TEST_RETRY);
}

public Set<String> getIncludeClasses() {
return read(extension.getFilter().getIncludeClasses(), Collections.emptySet());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.gradle.util.VersionNumber;
import org.jetbrains.annotations.NotNull;

import java.io.File;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;

Expand All @@ -47,14 +48,14 @@ public static void configureTestTask(Test test, ObjectFactory objectFactory, Pro
test.getInputs().property("retry.failOnPassedAfterRetry", adapter.getFailOnPassedAfterRetryInput());

test.getExtensions().add(TestRetryTaskExtension.class, TestRetryTaskExtension.NAME, extension);
test.doFirst(new ConditionalTaskAction(new InitTaskAction(adapter, objectFactory)));
test.doFirst(new ConditionalTaskAction(new InitTaskAction(adapter, objectFactory, test.getProject().getRootDir())));
test.doLast(new ConditionalTaskAction(new FinalizeTaskAction()));
}

private static RetryTestExecuter createRetryTestExecuter(Test task, TestRetryTaskExtensionAdapter extension, ObjectFactory objectFactory) {
private static RetryTestExecuter createRetryTestExecuter(Test task, TestRetryTaskExtensionAdapter extension, ObjectFactory objectFactory, File rootDir) {
TestExecuter<JvmTestExecutionSpec> delegate = getTestExecuter(task);
Instantiator instantiator = invoke(declaredMethod(AbstractTestTask.class, "getInstantiator"), task);
return new RetryTestExecuter(task, extension, delegate, instantiator, objectFactory);
return new RetryTestExecuter(task, extension, delegate, instantiator, objectFactory, rootDir);
}

private static TestExecuter<JvmTestExecutionSpec> getTestExecuter(Test task) {
Expand Down Expand Up @@ -117,15 +118,17 @@ private static class InitTaskAction implements Action<Test> {

private final TestRetryTaskExtensionAdapter adapter;
private final ObjectFactory objectFactory;
private final File rootDir;

public InitTaskAction(TestRetryTaskExtensionAdapter adapter, ObjectFactory objectFactory) {
public InitTaskAction(TestRetryTaskExtensionAdapter adapter, ObjectFactory objectFactory, File rootDir) {
this.adapter = adapter;
this.objectFactory = objectFactory;
this.rootDir = rootDir;
}

@Override
public void execute(@NotNull Test task) {
RetryTestExecuter retryTestExecuter = createRetryTestExecuter(task, adapter, objectFactory);
RetryTestExecuter retryTestExecuter = createRetryTestExecuter(task, adapter, objectFactory, rootDir);
setTestExecuter(task, retryTestExecuter);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/*
* Copyright 2019 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 org.eclipse.jgit.api.Git;
import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.diff.DiffEntry;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.treewalk.AbstractTreeIterator;
import org.eclipse.jgit.treewalk.CanonicalTreeParser;
import org.gradle.testretry.internal.config.TestRetryTaskExtensionAdapter;

import java.io.File;
import java.io.IOException;
import java.util.List;

import static java.util.stream.Collectors.toList;

public class GitRepository {

private final List<String> modifiedFilesPaths;

private GitRepository(List<String> modifiedFilesPaths) {
this.modifiedFilesPaths = modifiedFilesPaths;
}

public boolean wasModified(String path) {
return modifiedFilesPaths.contains(path);
}

static class NoRepository extends GitRepository {
private NoRepository() {
super(null);
}

@Override
public boolean wasModified(String path) {
return false;
}
}

static GitRepository create(File rootDir, TestRetryTaskExtensionAdapter extension) {
if (!extension.getModifiedTestRetry()) {
return new NoRepository();
}
try (
Git git = Git.open(rootDir)
) {
Repository repository = git.getRepository();
AbstractTreeIterator oldTree = getTreeIterator("refs/heads/master", repository);
AbstractTreeIterator newTree = getTreeIterator("HEAD", repository);
List<DiffEntry> diffs = git.diff()
.setOldTree(oldTree)
.setNewTree(newTree)
.call();
return new GitRepository(
diffs.stream()
.map(DiffEntry::getNewPath)
.map(it -> { //src/test/java/acme/SuccessfulTests.java
final int firstSlash = it.indexOf('/'); //src/
final int secondSlash = it.indexOf('/', firstSlash + 1); //test/
final int thirdSlash = it.indexOf('/', secondSlash + 1); //java/
final int extensionIndex = it.lastIndexOf('.'); //.java
final String packageClass = it.substring(thirdSlash + 1, extensionIndex); // acme/SuccessfulTests
return packageClass.replace('/', '.'); //acme.SuccessfulTests
}
)
.collect(toList())
);
} catch (IOException | GitAPIException e) {
return new NoRepository();
}
}

private static AbstractTreeIterator getTreeIterator(String name, Repository repository)
throws IOException {
final ObjectId id = repository.resolve(name);
if (id == null) {
throw new IllegalArgumentException(name);
}
final CanonicalTreeParser p = new CanonicalTreeParser();
try (ObjectReader or = repository.newObjectReader(); RevWalk walk = new RevWalk(repository)) {
p.reset(or, walk.parseTree(id));
return p;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.gradle.testretry.internal.filter.AnnotationInspectorImpl;
import org.gradle.testretry.internal.filter.RetryFilter;

import java.io.File;
import java.util.stream.Collectors;

public final class RetryTestExecuter implements TestExecuter<JvmTestExecutionSpec> {
Expand All @@ -35,6 +36,7 @@ public final class RetryTestExecuter implements TestExecuter<JvmTestExecutionSpe
private final TestExecuter<JvmTestExecutionSpec> delegate;
private final Test testTask;
private final TestFrameworkTemplate frameworkTemplate;
private final GitRepository gitRepository;

private RoundResult lastResult;

Expand All @@ -43,11 +45,13 @@ public RetryTestExecuter(
TestRetryTaskExtensionAdapter extension,
TestExecuter<JvmTestExecutionSpec> delegate,
Instantiator instantiator,
ObjectFactory objectFactory
ObjectFactory objectFactory,
File rootDir
) {
this.extension = extension;
this.delegate = delegate;
this.testTask = task;
this.gitRepository = GitRepository.create(rootDir, extension);
this.frameworkTemplate = new TestFrameworkTemplate(
testTask,
instantiator,
Expand Down Expand Up @@ -81,7 +85,8 @@ public void execute(JvmTestExecutionSpec spec, TestResultProcessor testResultPro
filter,
frameworkTemplate.testsReader,
testResultProcessor,
maxFailures
maxFailures,
gitRepository
);

int retryCount = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,15 @@ final class RetryTestResultProcessor implements TestResultProcessor {
private final TestResultProcessor delegate;

private final int maxFailures;
private final GitRepository gitRepository;
private boolean lastRetry;
private boolean hasRetryFilteredFailures;

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

private TestNames currentRoundFailedTests = new TestNames();
private TestNames previousRoundFailedTests = new TestNames();
private TestNames currentRoundFilteredTests = new TestNames();

private Object rootTestDescriptorId;

Expand All @@ -52,13 +54,15 @@ final class RetryTestResultProcessor implements TestResultProcessor {
RetryFilter filter,
TestsReader testsReader,
TestResultProcessor delegate,
int maxFailures
int maxFailures,
GitRepository gitRepository
) {
this.testFrameworkStrategy = testFrameworkStrategy;
this.filter = filter;
this.testsReader = testsReader;
this.delegate = delegate;
this.maxFailures = maxFailures;
this.gitRepository = gitRepository;
}

@Override
Expand Down Expand Up @@ -89,6 +93,10 @@ public void completed(Object testId, TestCompleteEvent testCompleteEvent) {
if (failedInPreviousRound && testCompleteEvent.getResultType() == SKIPPED) {
currentRoundFailedTests.add(className, name);
}
if (gitRepository.wasModified(className) && !isClassDescriptor(descriptor)
&& !currentRoundFilteredTests.remove(className, name)) {
currentRoundFailedTests.add(className, name);
}

if (isClassDescriptor(descriptor)) {
previousRoundFailedTests.remove(className, n -> {
Expand Down Expand Up @@ -130,9 +138,10 @@ public void failure(Object testId, Throwable throwable) {
if (descriptor != null) {
String className = descriptor.getClassName();
if (className != null) {
if (filter.canRetry(className)) {
if (filter.canRetry(className) && !gitRepository.wasModified(className)) {
currentRoundFailedTests.add(className, descriptor.getName());
} else {
currentRoundFilteredTests.add(className, descriptor.getName());
hasRetryFilteredFailures = true;
}
}
Expand Down
Loading

0 comments on commit 7f1c5ff

Please sign in to comment.