Add failOnSkippedAfterRetry option

Signed-off-by: Alexander Boyarshinov <[email protected]>
gradle · Sep 10, 2024 · 99cb279 · 99cb279
1 parent 05138d3
commit 99cb279
Show file tree

Hide file tree

Showing 10 changed files with 137 additions and 3 deletions.
diff --git a/README.adoc b/README.adoc
@@ -47,6 +47,7 @@ test {
         maxRetries = 2
         maxFailures = 20
         failOnPassedAfterRetry = true
+        failOnSkippedAfterRetry = true
     }
 }
 ----
@@ -60,6 +61,7 @@ test {
         maxRetries.set(2)
         maxFailures.set(20)
         failOnPassedAfterRetry.set(true)
+        failOnSkippedAfterRetry.set(true)
     }
 }
 ----
@@ -79,6 +81,7 @@ test {
             maxFailures = 20
         }
         failOnPassedAfterRetry = true
+        failOnSkippedAfterRetry = true
     }
 }
 ----
@@ -120,6 +123,18 @@ public interface TestRetryTaskExtension {
      */
     Property<Boolean> getFailOnPassedAfterRetry();
 
+    /**
+     * Whether tests that initially fail and then are skipped on retry should fail the task.
+     * <p>
+     * This setting defaults to {@code true} (for backward compatibility),
+     * which results in the task failing if any of tests skip on retry.
+     * <p>
+     * This setting has no effect if {@link Test#getIgnoreFailures()} is set to true.
+     *
+     * @return whether tests that initially fails and then are skipped on retry should fail the task
+     */
+    Property<Boolean> getFailOnSkippedAfterRetry();
+
     /**
      * The maximum number of times to retry an individual test.
      * <p>

diff --git a/plugin/src/main/java/org/gradle/testretry/TestRetryTaskExtension.java b/plugin/src/main/java/org/gradle/testretry/TestRetryTaskExtension.java
@@ -44,6 +44,18 @@ public interface TestRetryTaskExtension {
      */
     Property<Boolean> getFailOnPassedAfterRetry();
 
+    /**
+     * Whether tests that initially fail and then are skipped on retry should fail the task.
+     * <p>
+     * This setting defaults to {@code true} (for backward compatibility),
+     * which results in the task failing if any of tests skip on retry.
+     * <p>
+     * This setting has no effect if {@link Test#getIgnoreFailures()} is set to true.
+     *
+     * @return whether tests that initially fails and then are skipped on retry should fail the task
+     */
+    Property<Boolean> getFailOnSkippedAfterRetry();
+
     /**
      * The maximum number of times to retry an individual test.
      * <p>

diff --git a/plugin/src/main/java/org/gradle/testretry/internal/config/DefaultTestRetryTaskExtension.java b/plugin/src/main/java/org/gradle/testretry/internal/config/DefaultTestRetryTaskExtension.java
@@ -26,6 +26,7 @@
 public class DefaultTestRetryTaskExtension implements TestRetryTaskExtension {
 
     private final Property<Boolean> failOnPassedAfterRetry;
+    private final Property<Boolean> failOnSkippedAfterRetry;
     private final Property<Integer> maxRetries;
     private final Property<Integer> maxFailures;
     private final Filter filter;
@@ -35,6 +36,7 @@ public class DefaultTestRetryTaskExtension implements TestRetryTaskExtension {
     @Inject
     public DefaultTestRetryTaskExtension(ObjectFactory objects) {
         this.failOnPassedAfterRetry = objects.property(Boolean.class);
+        this.failOnSkippedAfterRetry = objects.property(Boolean.class);
         this.maxRetries = objects.property(Integer.class);
         this.maxFailures = objects.property(Integer.class);
         this.filter = new FilterImpl(objects);
@@ -45,6 +47,10 @@ public Property<Boolean> getFailOnPassedAfterRetry() {
         return failOnPassedAfterRetry;
     }
 
+    public Property<Boolean> getFailOnSkippedAfterRetry() {
+        return failOnSkippedAfterRetry;
+    }
+
     public Property<Integer> getMaxRetries() {
         return maxRetries;
     }

diff --git a/...in/src/main/java/org/gradle/testretry/internal/config/TestRetryTaskExtensionAccessor.java b/...in/src/main/java/org/gradle/testretry/internal/config/TestRetryTaskExtensionAccessor.java
@@ -21,6 +21,8 @@ public interface TestRetryTaskExtensionAccessor {
 
     boolean getFailOnPassedAfterRetry();
 
+    boolean getFailOnSkippedAfterRetry();
+
     int getMaxRetries();
 
     int getMaxFailures();

diff --git a/plugin/src/main/java/org/gradle/testretry/internal/config/TestRetryTaskExtensionAdapter.java b/plugin/src/main/java/org/gradle/testretry/internal/config/TestRetryTaskExtensionAdapter.java
@@ -36,6 +36,7 @@ public final class TestRetryTaskExtensionAdapter implements TestRetryTaskExtensi
     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_FAIL_ON_SKIPPED_AFTER_RETRY = true;
 
     private final ProviderFactory providerFactory;
     private final TestRetryTaskExtension extension;
@@ -62,6 +63,7 @@ private static void initialize(TestRetryTaskExtension extension, boolean gradle5
             extension.getMaxRetries().convention(DEFAULT_MAX_RETRIES);
             extension.getMaxFailures().convention(DEFAULT_MAX_FAILURES);
             extension.getFailOnPassedAfterRetry().convention(DEFAULT_FAIL_ON_PASSED_AFTER_RETRY);
+            extension.getFailOnSkippedAfterRetry().convention(DEFAULT_FAIL_ON_SKIPPED_AFTER_RETRY);
             filter.getIncludeClasses().convention(emptySet());
             filter.getIncludeAnnotationClasses().convention(emptySet());
             filter.getExcludeClasses().convention(emptySet());
@@ -93,11 +95,30 @@ Callable<Provider<Boolean>> getFailOnPassedAfterRetryInput() {
         }
     }
 
+    Callable<Provider<Boolean>> getFailOnSkippedAfterRetryInput() {
+        if (useConventions) {
+            return extension::getFailOnSkippedAfterRetry;
+        } else {
+            return () -> {
+                if (extension.getFailOnSkippedAfterRetry().isPresent()) {
+                    return extension.getFailOnSkippedAfterRetry();
+                } else {
+                    return providerFactory.provider(TestRetryTaskExtensionAdapter.this::getFailOnSkippedAfterRetry);
+                }
+            };
+        }
+    }
+
     @Override
     public boolean getFailOnPassedAfterRetry() {
         return read(extension.getFailOnPassedAfterRetry(), DEFAULT_FAIL_ON_PASSED_AFTER_RETRY);
     }
 
+    @Override
+    public boolean getFailOnSkippedAfterRetry() {
+        return read(extension.getFailOnSkippedAfterRetry(), DEFAULT_FAIL_ON_SKIPPED_AFTER_RETRY);
+    }
+
     @Override
     public int getMaxRetries() {
         return read(extension.getMaxRetries(), DEFAULT_MAX_RETRIES);

diff --git a/plugin/src/main/java/org/gradle/testretry/internal/config/TestTaskConfigurer.java b/plugin/src/main/java/org/gradle/testretry/internal/config/TestTaskConfigurer.java
@@ -54,6 +54,8 @@ public static void configureTestTask(Test test, ObjectFactory objectFactory, Pro
 
         test.getInputs().property("retry.failOnPassedAfterRetry", adapter.getFailOnPassedAfterRetryInput());
 
+        test.getInputs().property("retry.failOnSkippedAfterRetry", adapter.getFailOnSkippedAfterRetryInput());
+
         Provider<Boolean> shouldReplaceTestExecutor = shouldReplaceTestExecutor(test, objectFactory, providerFactory, gradleVersion);
         test.getInputs().property("isDeactivatedByTestDistributionPlugin", shouldReplaceTestExecutor);
 

diff --git a/plugin/src/main/java/org/gradle/testretry/internal/executer/RetryTestExecuter.java b/plugin/src/main/java/org/gradle/testretry/internal/executer/RetryTestExecuter.java
@@ -72,6 +72,7 @@ public void execute(JvmTestExecutionSpec spec, TestResultProcessor testResultPro
         int maxRetries = extension.getMaxRetries();
         int maxFailures = extension.getMaxFailures();
         boolean failOnPassedAfterRetry = extension.getFailOnPassedAfterRetry();
+        boolean failOnSkippedAfterRetry = extension.getFailOnSkippedAfterRetry();
 
         if (maxRetries <= 0) {
             delegate.execute(spec, testResultProcessor);
@@ -106,7 +107,8 @@ public void execute(JvmTestExecutionSpec spec, TestResultProcessor testResultPro
             classRetryMatcher,
             frameworkTemplate.testsReader,
             testResultProcessor,
-            maxFailures
+            maxFailures,
+            failOnSkippedAfterRetry
         );
 
         int retryCount = 0;

diff --git a/plugin/src/main/java/org/gradle/testretry/internal/executer/RetryTestResultProcessor.java b/plugin/src/main/java/org/gradle/testretry/internal/executer/RetryTestResultProcessor.java
@@ -47,6 +47,7 @@ final class RetryTestResultProcessor implements TestResultProcessor {
     private final TestResultProcessor delegate;
 
     private final int maxFailures;
+    private final boolean failOnSkippedAfterRetry;
     private boolean lastRetry;
     private boolean hasRetryFilteredFailures;
     private Method failureMethod;
@@ -66,14 +67,16 @@ final class RetryTestResultProcessor implements TestResultProcessor {
         ClassRetryMatcher classRetryMatcher,
         TestsReader testsReader,
         TestResultProcessor delegate,
-        int maxFailures
+        int maxFailures,
+        boolean failOnSkippedAfterRetry
     ) {
         this.testFrameworkStrategy = testFrameworkStrategy;
         this.filter = filter;
         this.classRetryMatcher = classRetryMatcher;
         this.testsReader = testsReader;
         this.delegate = delegate;
         this.maxFailures = maxFailures;
+        this.failOnSkippedAfterRetry = failOnSkippedAfterRetry;
     }
 
     @Override
@@ -107,7 +110,8 @@ public void completed(Object testId, TestCompleteEvent testCompleteEvent) {
                 String name = descriptor.getName();
 
                 boolean failedInPreviousRound = previousRoundFailedTests.remove(className, name);
-                if (failedInPreviousRound && testCompleteEvent.getResultType() == SKIPPED) {
+                boolean shouldRetrySkippedTestThatPreviouslyFailed = failedInPreviousRound && testCompleteEvent.getResultType() == SKIPPED && failOnSkippedAfterRetry;
+                if (shouldRetrySkippedTestThatPreviouslyFailed) {
                     addRetry(descriptor);
                 }
 

diff --git a/plugin/src/test/groovy/org/gradle/testretry/testframework/JUnit4FuncTest.groovy b/plugin/src/test/groovy/org/gradle/testretry/testframework/JUnit4FuncTest.groovy
@@ -375,6 +375,39 @@ class JUnit4FuncTest extends AbstractFrameworkFuncTest {
         gradleVersion << GRADLE_VERSIONS_UNDER_TEST
     }
 
+    def "test that is skipped after failure with option 'failOnSkippedAfterRetry = false' is passes (gradle version #gradleVersion)"() {
+        given:
+        buildFile << """
+            test.retry.maxRetries = 1
+            test.retry.failOnSkippedAfterRetry = false
+        """
+
+        writeJavaTestSource """
+            package acme;
+            import java.nio.file.*;
+
+            public class FlakyTests {
+                @org.junit.Test
+                public void flakyAssumeTest() {
+                   ${flakyAssert()};
+                   org.junit.Assume.assumeFalse(${markerFileExistsCheck()});
+                }
+            }
+        """
+
+        when:
+        def result = gradleRunner(gradleVersion).build()
+
+        then:
+        with(result.output) {
+            it.count('flakyAssumeTest FAILED') == 1
+            it.count('flakyAssumeTest SKIPPED') == 1
+        }
+
+        where:
+        gradleVersion << GRADLE_VERSIONS_UNDER_TEST
+    }
+
     def "handles failure in rule before = #failBefore (gradle version #gradleVersion)"(String gradleVersion, boolean failBefore) {
         given:
         buildFile << """

diff --git a/plugin/src/test/groovy/org/gradle/testretry/testframework/JUnit5FuncTest.groovy b/plugin/src/test/groovy/org/gradle/testretry/testframework/JUnit5FuncTest.groovy
@@ -280,6 +280,43 @@ class JUnit5FuncTest extends AbstractFrameworkFuncTest {
         gradleVersion << GRADLE_VERSIONS_UNDER_TEST
     }
 
+    def "test that is skipped after failure with option 'failOnSkippedAfterRetry = false' is passes (gradle version #gradleVersion)"() {
+        given:
+        buildFile << """
+            test.retry.maxRetries = 1
+            test.retry.failOnSkippedAfterRetry = false
+        """
+
+        writeJavaTestSource """
+            package acme;
+
+            import org.junit.jupiter.api.*;
+            import java.nio.file.*;
+
+            import static org.junit.jupiter.api.Assumptions.assumeFalse;
+
+            class FlakyTests {
+                @Test
+                void flakyAssumeTest() {
+                    ${flakyAssert()}
+                    Assumptions.assumeFalse(${markerFileExistsCheck()});
+                }
+            }
+        """
+
+        when:
+        def result = gradleRunner(gradleVersion).build()
+
+        then:
+        with(result.output) {
+            it.count('flakyAssumeTest() FAILED') == 1
+            it.count('flakyAssumeTest() SKIPPED') == 1
+        }
+
+        where:
+        gradleVersion << GRADLE_VERSIONS_UNDER_TEST
+    }
+
     def "can rerun on whole class via className (gradle version #gradleVersion)"() {
         given:
         buildFile << """