From e66abbcecc52adbe13a5ad45a507644f7301238b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc=20Pap?= Date: Mon, 31 May 2021 19:18:38 +0200 Subject: [PATCH 1/4] Keep test distribution presence with config cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Lőrinc Pap --- .../internal/config/TestTaskConfigurer.java | 56 ++++++++++--------- .../ConfigCachingPluginFuncTest.groovy | 40 +++++++++++-- 2 files changed, 67 insertions(+), 29 deletions(-) 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 index cfaf2665..97a34dba 100644 --- a/plugin/src/main/java/org/gradle/testretry/internal/config/TestTaskConfigurer.java +++ b/plugin/src/main/java/org/gradle/testretry/internal/config/TestTaskConfigurer.java @@ -20,6 +20,8 @@ import org.gradle.api.internal.tasks.testing.JvmTestExecutionSpec; import org.gradle.api.internal.tasks.testing.TestExecuter; import org.gradle.api.model.ObjectFactory; +import org.gradle.api.provider.Property; +import org.gradle.api.provider.Provider; import org.gradle.api.provider.ProviderFactory; import org.gradle.api.tasks.testing.AbstractTestTask; import org.gradle.api.tasks.testing.Test; @@ -47,8 +49,31 @@ 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.doLast(new ConditionalTaskAction(new FinalizeTaskAction())); + + Provider isDeactivatedByTestDistributionPlugin = shouldTestRetryPluginBeDeactivated(test, objectFactory, providerFactory); + test.doFirst(new ConditionalTaskAction(isDeactivatedByTestDistributionPlugin, new InitTaskAction(adapter, objectFactory))); + test.doLast(new ConditionalTaskAction(isDeactivatedByTestDistributionPlugin, new FinalizeTaskAction())); + } + + private static Provider shouldTestRetryPluginBeDeactivated(Test test, ObjectFactory objectFactory, ProviderFactory providerFactory) { + Provider provider = providerFactory.provider(() -> callShouldTestRetryPluginBeDeactivated(test)); + Property result = objectFactory.property(Boolean.class).convention(provider); + result.finalizeValueOnRead(); + return result; + } + + private static boolean callShouldTestRetryPluginBeDeactivated(Test test) { + Object distributionExtension = test.getExtensions().findByName("distribution"); + if (distributionExtension == null) { + return false; + } + try { + Method result = makeAccessible(distributionExtension.getClass().getMethod("shouldTestRetryPluginBeDeactivated")); + return invoke(result, distributionExtension); + } catch (Exception e) { + test.getLogger().warn("Failed to determine whether test-retry plugin should be deactivated from distribution extension", e); + return false; + } } private static RetryTestExecuter createRetryTestExecuter(Test task, TestRetryTaskExtensionAdapter extension, ObjectFactory objectFactory) { @@ -67,33 +92,22 @@ private static void setTestExecuter(Test task, RetryTestExecuter retryTestExecut private static class ConditionalTaskAction implements Action { + private final Provider isDeactivatedByTestDistributionPlugin; private final Action delegate; - public ConditionalTaskAction(Action delegate) { + public ConditionalTaskAction(Provider isDeactivatedByTestDistributionPlugin, Action delegate) { + this.isDeactivatedByTestDistributionPlugin = isDeactivatedByTestDistributionPlugin; this.delegate = delegate; } @Override public void execute(@NotNull Task task) { - if (isDeactivatedByTestDistributionPlugin(task)) { + if (isDeactivatedByTestDistributionPlugin.get()) { task.getLogger().info("Test execution via the test-retry plugin is deactivated. Retries are handled by the test-distribution plugin."); } else { delegate.execute((Test) task); } } - - private boolean isDeactivatedByTestDistributionPlugin(Task task) { - Object distributionExtension = task.getExtensions().findByName("distribution"); - if (distributionExtension == null) { - return false; - } - try { - return invoke(method(distributionExtension.getClass(), "shouldTestRetryPluginBeDeactivated"), distributionExtension); - } catch (Exception e) { - task.getLogger().warn("Failed to determine whether test-retry plugin should be deactivated from distribution extension", e); - return false; - } - } } private static class FinalizeTaskAction implements Action { @@ -134,14 +148,6 @@ private static Method declaredMethod(Class type, String methodName, Class. } } - private static Method method(Class type, String methodName, Class... paramTypes) { - try { - return makeAccessible(type.getMethod(methodName, paramTypes)); - } catch (NoSuchMethodException e) { - throw new RuntimeException(e); - } - } - private static Method makeAccessible(Method method) { method.setAccessible(true); return method; diff --git a/plugin/src/test/groovy/org/gradle/testretry/ConfigCachingPluginFuncTest.groovy b/plugin/src/test/groovy/org/gradle/testretry/ConfigCachingPluginFuncTest.groovy index 05e3ed9e..32050806 100644 --- a/plugin/src/test/groovy/org/gradle/testretry/ConfigCachingPluginFuncTest.groovy +++ b/plugin/src/test/groovy/org/gradle/testretry/ConfigCachingPluginFuncTest.groovy @@ -44,7 +44,7 @@ class ConfigCachingPluginFuncTest extends AbstractGeneralPluginFuncTest { result = gradleRunnerWithConfigurationCache(gradleVersion).build() then: - assertConfigurationCacheIsReused(result, gradleVersion) + configurationCacheIsReused(result, gradleVersion) where: gradleVersion << GRADLE_VERSIONS_UNDER_TEST @@ -71,7 +71,39 @@ class ConfigCachingPluginFuncTest extends AbstractGeneralPluginFuncTest { result = gradleRunnerWithConfigurationCache(gradleVersion).build() then: - assertConfigurationCacheIsReused(result, gradleVersion) + configurationCacheIsReused(result, gradleVersion) + + where: + gradleVersion << GRADLE_VERSIONS_UNDER_TEST + } + + def "compatible with configuration cache when Test Distribution is also present (gradle version #gradleVersion)"() { + shouldTestConfigCache(gradleVersion) + buildFile << """ + interface TestDistributionExtension {} + class DefaultTestDistributionExtension implements TestDistributionExtension { + boolean shouldTestRetryPluginBeDeactivated() { + true + } + } + test.extensions.create(TestDistributionExtension, "distribution", DefaultTestDistributionExtension) + """ + + failedTest() + + when: + def result = gradleRunnerWithConfigurationCache(gradleVersion, 'test', '--info').buildAndFail() + + then: + !configurationCacheIsReused(result, gradleVersion) + result.output.contains('handled by the test-distribution plugin') + + when: + result = gradleRunnerWithConfigurationCache(gradleVersion, 'test', '--info').buildAndFail() + + then: + configurationCacheIsReused(result, gradleVersion) + result.output.contains('handled by the test-distribution plugin') where: gradleVersion << GRADLE_VERSIONS_UNDER_TEST @@ -86,8 +118,8 @@ class ConfigCachingPluginFuncTest extends AbstractGeneralPluginFuncTest { GradleVersion.version(gradleVersion) >= GradleVersion.version("6.1") } - static void assertConfigurationCacheIsReused(BuildResult result, String gradleVersion) { - assert result.output.contains(getConfigurationCacheMessage(gradleVersion)) + private static boolean configurationCacheIsReused(BuildResult result, String gradleVersion) { + result.output.contains(getConfigurationCacheMessage(gradleVersion)) } static String[] withConfigurationCacheArguments(String gradleVersion, String[] arguments) { From 356c7f7453515771638f7df62aeb480539b16e1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc=20Pap?= Date: Tue, 1 Jun 2021 14:46:30 +0200 Subject: [PATCH 2/4] Is deactivated when decorated distribution extension changes to true MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Lőrinc Pap --- ...TestDistributionIntegrationFuncTest.groovy | 39 ++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/plugin/src/test/groovy/org/gradle/testretry/TestDistributionIntegrationFuncTest.groovy b/plugin/src/test/groovy/org/gradle/testretry/TestDistributionIntegrationFuncTest.groovy index e502fa25..a6c7265c 100644 --- a/plugin/src/test/groovy/org/gradle/testretry/TestDistributionIntegrationFuncTest.groovy +++ b/plugin/src/test/groovy/org/gradle/testretry/TestDistributionIntegrationFuncTest.groovy @@ -23,11 +23,11 @@ class TestDistributionIntegrationFuncTest extends AbstractGeneralPluginFuncTest buildFile << """ test.retry.maxRetries = 1 """ - failedTest() } def "is deactivated when decorated distribution extension returns true (gradle version #gradleVersion)"() { given: + failedTest() buildFile << """ interface TestDistributionExtension {} class DefaultTestDistributionExtension implements TestDistributionExtension { @@ -49,8 +49,43 @@ class TestDistributionIntegrationFuncTest extends AbstractGeneralPluginFuncTest gradleVersion << GRADLE_VERSIONS_UNDER_TEST } + def "is deactivated when decorated distribution extension changes to true (gradle version #gradleVersion)"() { + given: + successfulTest() // a failing one prohibit task outputs from being cached + buildFile << """ + interface TestDistributionExtension {} + class DefaultTestDistributionExtension implements TestDistributionExtension { + boolean shouldTestRetryPluginBeDeactivated() { + Boolean.getBoolean("shouldTestRetryPluginBeDeactivated") + } + } + test.extensions.create(TestDistributionExtension.class, "distribution", DefaultTestDistributionExtension.class) + """ + + when: + System.setProperty("shouldTestRetryPluginBeDeactivated", "${true}") + def result = gradleRunner(gradleVersion, 'test', '--info').build() + + then: + result.output.contains("handled by the test-distribution plugin") + + when: + System.setProperty("shouldTestRetryPluginBeDeactivated", "${false}") + result = gradleRunner(gradleVersion, 'test', '--info').build() + + then: + with(result.output) { + !contains("handled by the test-distribution plugin") + !contains("> Task :test UP-TO-DATE") + } + + where: + gradleVersion << GRADLE_VERSIONS_UNDER_TEST + } + def "is deactivated when undecorated distribution extension returns true (gradle version #gradleVersion)"() { given: + failedTest() buildFile << """ class TestDistributionExtension { boolean shouldTestRetryPluginBeDeactivated() { @@ -72,6 +107,7 @@ class TestDistributionIntegrationFuncTest extends AbstractGeneralPluginFuncTest def "is not deactivated when distribution extension returns false (gradle version #gradleVersion)"() { given: + failedTest() buildFile << """ interface TestDistributionExtension {} class DefaultTestDistributionExtension implements TestDistributionExtension { @@ -94,6 +130,7 @@ class TestDistributionIntegrationFuncTest extends AbstractGeneralPluginFuncTest def "is not deactivated when distribution extension does not declare the expected method (gradle version #gradleVersion)"() { given: + failedTest() buildFile << """ class TestDistributionExtension { } From dc0ffda6adb1d759dc9de49deba5eb50648cd07b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc=20Pap?= Date: Tue, 1 Jun 2021 14:47:50 +0200 Subject: [PATCH 3/4] Make TD extension state an input of the plugin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Lőrinc Pap --- .../gradle/testretry/internal/config/TestTaskConfigurer.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 index 97a34dba..bb57acc3 100644 --- a/plugin/src/main/java/org/gradle/testretry/internal/config/TestTaskConfigurer.java +++ b/plugin/src/main/java/org/gradle/testretry/internal/config/TestTaskConfigurer.java @@ -48,9 +48,11 @@ public static void configureTestTask(Test test, ObjectFactory objectFactory, Pro test.getInputs().property("retry.failOnPassedAfterRetry", adapter.getFailOnPassedAfterRetryInput()); + Provider isDeactivatedByTestDistributionPlugin = shouldTestRetryPluginBeDeactivated(test, objectFactory, providerFactory); + test.getInputs().property("isDeactivatedByTestDistributionPlugin", isDeactivatedByTestDistributionPlugin); + test.getExtensions().add(TestRetryTaskExtension.class, TestRetryTaskExtension.NAME, extension); - Provider isDeactivatedByTestDistributionPlugin = shouldTestRetryPluginBeDeactivated(test, objectFactory, providerFactory); test.doFirst(new ConditionalTaskAction(isDeactivatedByTestDistributionPlugin, new InitTaskAction(adapter, objectFactory))); test.doLast(new ConditionalTaskAction(isDeactivatedByTestDistributionPlugin, new FinalizeTaskAction())); } From 5d339c8af67f3229cc2c55ac71cacc3b7fe1ad56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc=20Pap?= Date: Tue, 1 Jun 2021 15:18:09 +0200 Subject: [PATCH 4/4] Disable config cache for old Gradle versions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Lőrinc Pap --- .../config/TestRetryTaskExtensionAdapter.java | 6 ++-- .../internal/config/TestTaskConfigurer.java | 29 +++++++++++++++---- .../internal/executer/RetryTestExecuter.java | 4 ++- .../TestNgTestFrameworkStrategy.java | 4 ++- 4 files changed, 32 insertions(+), 11 deletions(-) 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 index e661ad80..0bfdbfa5 100644 --- a/plugin/src/main/java/org/gradle/testretry/internal/config/TestRetryTaskExtensionAdapter.java +++ b/plugin/src/main/java/org/gradle/testretry/internal/config/TestRetryTaskExtensionAdapter.java @@ -26,6 +26,8 @@ import java.util.Set; import java.util.concurrent.Callable; +import static org.gradle.testretry.internal.config.TestTaskConfigurer.supportsPropertyConventions; + public final class TestRetryTaskExtensionAdapter { // for testing only @@ -53,10 +55,6 @@ public TestRetryTaskExtensionAdapter( initialize(extension, this.useConventions); } - private static boolean supportsPropertyConventions(VersionNumber gradleVersion) { - return gradleVersion.compareTo(VersionNumber.parse("5.1")) >= 0; - } - private static void initialize(TestRetryTaskExtension extension, boolean gradle51OrLater) { if (gradle51OrLater) { extension.getMaxRetries().convention(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 index bb57acc3..a1d2e31b 100644 --- a/plugin/src/main/java/org/gradle/testretry/internal/config/TestTaskConfigurer.java +++ b/plugin/src/main/java/org/gradle/testretry/internal/config/TestTaskConfigurer.java @@ -48,7 +48,8 @@ public static void configureTestTask(Test test, ObjectFactory objectFactory, Pro test.getInputs().property("retry.failOnPassedAfterRetry", adapter.getFailOnPassedAfterRetryInput()); - Provider isDeactivatedByTestDistributionPlugin = shouldTestRetryPluginBeDeactivated(test, objectFactory, providerFactory); + Provider isDeactivatedByTestDistributionPlugin = + shouldTestRetryPluginBeDeactivated(test, objectFactory, providerFactory, gradleVersion); test.getInputs().property("isDeactivatedByTestDistributionPlugin", isDeactivatedByTestDistributionPlugin); test.getExtensions().add(TestRetryTaskExtension.class, TestRetryTaskExtension.NAME, extension); @@ -57,13 +58,31 @@ public static void configureTestTask(Test test, ObjectFactory objectFactory, Pro test.doLast(new ConditionalTaskAction(isDeactivatedByTestDistributionPlugin, new FinalizeTaskAction())); } - private static Provider shouldTestRetryPluginBeDeactivated(Test test, ObjectFactory objectFactory, ProviderFactory providerFactory) { - Provider provider = providerFactory.provider(() -> callShouldTestRetryPluginBeDeactivated(test)); - Property result = objectFactory.property(Boolean.class).convention(provider); - result.finalizeValueOnRead(); + private static Provider shouldTestRetryPluginBeDeactivated( + Test test, + ObjectFactory objectFactory, + ProviderFactory providerFactory, + VersionNumber gradleVersion + ) { + Provider result = providerFactory.provider(() -> callShouldTestRetryPluginBeDeactivated(test)); + if (supportsPropertyConventions(gradleVersion)) { + Property property = objectFactory.property(Boolean.class).convention(result); + if (supportsFinalizeValueOnRead(gradleVersion)) { + property.finalizeValueOnRead(); + } + result = property; + } return result; } + public static boolean supportsPropertyConventions(VersionNumber gradleVersion) { + return gradleVersion.compareTo(VersionNumber.parse("5.1")) >= 0; + } + + private static boolean supportsFinalizeValueOnRead(VersionNumber gradleVersion) { + return gradleVersion.compareTo(VersionNumber.parse("6.1")) >= 0; + } + private static boolean callShouldTestRetryPluginBeDeactivated(Test test) { Object distributionExtension = test.getExtensions().findByName("distribution"); if (distributionExtension == null) { 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 index 36244039..406d93bd 100644 --- a/plugin/src/main/java/org/gradle/testretry/internal/executer/RetryTestExecuter.java +++ b/plugin/src/main/java/org/gradle/testretry/internal/executer/RetryTestExecuter.java @@ -29,6 +29,8 @@ import java.util.stream.Collectors; +import static org.gradle.testretry.internal.executer.framework.TestFrameworkStrategy.gradleVersionIsAtLeast; + public final class RetryTestExecuter implements TestExecuter { private final TestRetryTaskExtensionAdapter extension; @@ -121,7 +123,7 @@ public void failWithNonRetriedTestsIfAny() { } private JvmTestExecutionSpec createRetryJvmExecutionSpec(JvmTestExecutionSpec spec, TestFramework retryTestFramework) { - if (TestFrameworkStrategy.gradleVersionIsAtLeast("6.4")) { + if (gradleVersionIsAtLeast("6.4")) { // This constructor is in Gradle 6.4+ return new JvmTestExecutionSpec( retryTestFramework, diff --git a/plugin/src/main/java/org/gradle/testretry/internal/executer/framework/TestNgTestFrameworkStrategy.java b/plugin/src/main/java/org/gradle/testretry/internal/executer/framework/TestNgTestFrameworkStrategy.java index 0dd2e82c..290a8205 100644 --- a/plugin/src/main/java/org/gradle/testretry/internal/executer/framework/TestNgTestFrameworkStrategy.java +++ b/plugin/src/main/java/org/gradle/testretry/internal/executer/framework/TestNgTestFrameworkStrategy.java @@ -37,6 +37,8 @@ import java.util.Map; import java.util.Optional; +import static org.gradle.testretry.internal.executer.framework.TestFrameworkStrategy.gradleVersionIsAtLeast; + final class TestNgTestFrameworkStrategy implements TestFrameworkStrategy { private static final Logger LOGGER = LoggerFactory.getLogger(TestNgTestFrameworkStrategy.class); @@ -73,7 +75,7 @@ public TestFramework createRetrying(TestFrameworkTemplate template, TestNames fa } private TestNGTestFramework createTestFramework(TestFrameworkTemplate template, DefaultTestFilter retriedTestFilter) { - if (TestFrameworkStrategy.gradleVersionIsAtLeast("6.6")) { + if (gradleVersionIsAtLeast("6.6")) { return new TestNGTestFramework(template.task, template.task.getClasspath(), retriedTestFilter, template.objectFactory); } else { try {