Fix flaky test and add some assertions to prevent a similar problem i…

…n the future.
gradle · May 1, 2020 · 275a2b3 · 275a2b3
1 parent 2f4e3e5
commit 275a2b3
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 31 deletions.
diff --git a/...tegTest/groovy/org/gradle/instantexecution/AbstractInstantExecutionIntegrationTest.groovy b/...tegTest/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/...tegTest/groovy/org/gradle/instantexecution/InstantExecutionEnablingIntegrationTest.groovy b/...tegTest/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/...vy/org/gradle/instantexecution/InstantExecutionToolingApiInvocationIntegrationTest.groovy b/...vy/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