From 275a2b32531fe2d6fba88ee8d25da8d450bbcc49 Mon Sep 17 00:00:00 2001 From: Adam Murdoch Date: Fri, 1 May 2020 12:13:13 +1000 Subject: [PATCH] Fix flaky test and add some assertions to prevent a similar problem in the future. --- ...ractInstantExecutionIntegrationTest.groovy | 8 +++++ ...antExecutionEnablingIntegrationTest.groovy | 32 ++----------------- ...ToolingApiInvocationIntegrationTest.groovy | 6 +++- 3 files changed, 15 insertions(+), 31 deletions(-) diff --git a/subprojects/instant-execution/src/integTest/groovy/org/gradle/instantexecution/AbstractInstantExecutionIntegrationTest.groovy b/subprojects/instant-execution/src/integTest/groovy/org/gradle/instantexecution/AbstractInstantExecutionIntegrationTest.groovy index 96bb3f6c6e96..6423d530d314 100644 --- a/subprojects/instant-execution/src/integTest/groovy/org/gradle/instantexecution/AbstractInstantExecutionIntegrationTest.groovy +++ b/subprojects/instant-execution/src/integTest/groovy/org/gradle/instantexecution/AbstractInstantExecutionIntegrationTest.groovy @@ -31,9 +31,17 @@ class AbstractInstantExecutionIntegrationTest extends AbstractIntegrationSpec { protected InstantExecutionProblemsFixture problems def setup() { + // Verify that the previous test cleaned up state correctly + assert System.getProperty(SystemProperties.isEnabled) == null problems = new InstantExecutionProblemsFixture(executer, testDirectory) } + @Override + def cleanup() { + // Verify that the test (or fixtures) has cleaned up state correctly + assert System.getProperty(SystemProperties.isEnabled) == null + } + void buildKotlinFile(@Language("kotlin") String script) { buildKotlinFile << script } diff --git a/subprojects/instant-execution/src/integTest/groovy/org/gradle/instantexecution/InstantExecutionEnablingIntegrationTest.groovy b/subprojects/instant-execution/src/integTest/groovy/org/gradle/instantexecution/InstantExecutionEnablingIntegrationTest.groovy index 1fc70802d67d..5f901e90a503 100644 --- a/subprojects/instant-execution/src/integTest/groovy/org/gradle/instantexecution/InstantExecutionEnablingIntegrationTest.groovy +++ b/subprojects/instant-execution/src/integTest/groovy/org/gradle/instantexecution/InstantExecutionEnablingIntegrationTest.groovy @@ -16,14 +16,9 @@ package org.gradle.instantexecution -import org.gradle.integtests.fixtures.executer.AbstractGradleExecuter import org.gradle.util.ToBeImplemented - -import javax.annotation.Nullable - import spock.lang.Ignore - class InstantExecutionEnablingIntegrationTest extends AbstractInstantExecutionIntegrationTest { def "can enable instant execution from the command line"() { @@ -39,11 +34,9 @@ class InstantExecutionEnablingIntegrationTest extends AbstractInstantExecutionIn } def "can enable instant execution from the client jvm"() { - setup: - AbstractGradleExecuter.propagateSystemProperty(SystemProperties.isEnabled) - def previousProp = System.getProperty(SystemProperties.isEnabled) - System.setProperty(SystemProperties.isEnabled, "true") + executer.withCommandLineGradleOpts(INSTANT_EXECUTION_PROPERTY) + executer.requireGradleDistribution() and: def fixture = newInstantExecutionFixture() @@ -53,22 +46,12 @@ class InstantExecutionEnablingIntegrationTest extends AbstractInstantExecutionIn then: fixture.assertStateStored() - - cleanup: - setOrClearProperty(SystemProperties.isEnabled, previousProp) - AbstractGradleExecuter.doNotPropagateSystemProperty(SystemProperties.isEnabled) } @Ignore @ToBeImplemented def "can enable instant execution from gradle.properties"() { setup: - def previousProp = System.getProperty(SystemProperties.isEnabled) - if (GradleContextualExecuter.isEmbedded()) { - System.clearProperty(SystemProperties.isEnabled) - } - - and: file('gradle.properties') << """ systemProp.${SystemProperties.isEnabled}=true """ @@ -81,16 +64,5 @@ class InstantExecutionEnablingIntegrationTest extends AbstractInstantExecutionIn then: 'instant execution is enabled' fixture.assertStateStored() - - cleanup: - setOrClearProperty(SystemProperties.isEnabled, previousProp) - } - - private static void setOrClearProperty(String name, @Nullable String value) { - if (value != null) { - System.setProperty(name, value) - } else { - System.clearProperty(name) - } } } diff --git a/subprojects/instant-execution/src/integTest/groovy/org/gradle/instantexecution/InstantExecutionToolingApiInvocationIntegrationTest.groovy b/subprojects/instant-execution/src/integTest/groovy/org/gradle/instantexecution/InstantExecutionToolingApiInvocationIntegrationTest.groovy index 2df2fa44d4cb..cb9040c6cc3b 100644 --- a/subprojects/instant-execution/src/integTest/groovy/org/gradle/instantexecution/InstantExecutionToolingApiInvocationIntegrationTest.groovy +++ b/subprojects/instant-execution/src/integTest/groovy/org/gradle/instantexecution/InstantExecutionToolingApiInvocationIntegrationTest.groovy @@ -27,7 +27,7 @@ class InstantExecutionToolingApiInvocationIntegrationTest extends AbstractInstan buildFile << """ plugins { id("java") - } + } """ when: @@ -39,6 +39,7 @@ class InstantExecutionToolingApiInvocationIntegrationTest extends AbstractInstan } ExecutionResult runWithInstantExecutionViaToolingApi(String... tasks) { + // TODO - move this to a GradleExecuter implementation def output = new ByteArrayOutputStream() def error = new ByteArrayOutputStream() def context = new IntegrationTestBuildContext() @@ -62,6 +63,9 @@ class InstantExecutionToolingApiInvocationIntegrationTest extends AbstractInstan .run() } finally { connection.close() + if (GradleContextualExecuter.embedded) { + System.clearProperty(SystemProperties.isEnabled) + } } result = OutputScrapingExecutionResult.from(output.toString(), error.toString()) return result