Make sure record/verify/properties actually affect test tasks (#690)

* Add failing tests that reproduce the problem where the record and verify parameters are not playing in up to date checks and fix by registering inputs explicitly.

Co-authored-by: John Rodriguez <john.rodriguez@gmail.com>

* Move paparazzi properties earlier, so they are playing in incremental builds.

It's a standard HashMap not a DomainObjectCollection, so there won't be any further items inserted between filterKeys and doFirst.

* Move and remove comments

* Document that we're doing something funky

* Merge two tests

* Move systemProperties together

---------

Co-authored-by: John Rodriguez <john.rodriguez@gmail.com>

Author: TWiStErRob

paparazzi/paparazzi-gradle-plugin/src/main/java/app/cash/paparazzi/gradle/PaparazziPlugin.kt

@@ -158,6 +158,11 @@ class PaparazziPlugin : Plugin<Project> {
           writeResourcesTask.flatMap { it.paparazziResources.asFile }.get().path
         test.systemProperties["paparazzi.build.dir"] =
           buildDirectory.get().toString()
+        test.systemProperties["kotlinx.coroutines.main.delay"] = true
+        test.systemProperties.putAll(project.properties.filterKeys { it.startsWith("app.cash.paparazzi") })
+
+        test.inputs.property("paparazzi.test.record", isRecordRun)
+        test.inputs.property("paparazzi.test.verify", isVerifyRun)
 
         test.inputs.dir(mergeResourcesOutputDir)
         test.inputs.dir(mergeAssetsOutputDir)
@@ -168,18 +173,16 @@ class PaparazziPlugin : Plugin<Project> {
         test.outputs.dir(reportOutputDir)
         test.outputs.dir(snapshotOutputDir)
 
-        val paparazziProperties = project.properties.filterKeys { it.startsWith("app.cash.paparazzi") }
-
         @Suppress("ObjectLiteralToLambda")
         // why not a lambda?  See: https://docs.gradle.org/7.2/userguide/validation_problems.html#implementation_unknown
         test.doFirst(object : Action<Task> {
           override fun execute(t: Task) {
+            // Note: these are lazy properties that are not resolvable in the Gradle configuration phase.
+            // They need special handling, so they're added as inputs.property above, and systemProperty here.
             test.systemProperties["paparazzi.platform.data.root"] =
               nativePlatformFileCollection.singleFile.absolutePath
             test.systemProperties["paparazzi.test.record"] = isRecordRun.get()
             test.systemProperties["paparazzi.test.verify"] = isVerifyRun.get()
-            test.systemProperties["kotlinx.coroutines.main.delay"] = true
-            test.systemProperties.putAll(paparazziProperties)
           }
         })
       }

paparazzi/paparazzi-gradle-plugin/src/test/java/app/cash/paparazzi/gradle/PaparazziPluginTest.kt

@@ -521,6 +521,44 @@ class PaparazziPluginTest {
     snapshotsDir.deleteRecursively()
   }
 
+  @Test
+  fun rerunTestsOnPropertyChange() {
+    val fixtureRoot = File("src/test/projects/rerun-property-change")
+
+    // Take 1
+    val firstRunResult = gradleRunner
+      .withArguments("testDebugUnitTest", "--stacktrace")
+      .forwardOutput()
+      .runFixture(fixtureRoot) { build() }
+
+    with(firstRunResult.task(":testDebugUnitTest")) {
+      assertThat(this).isNotNull()
+      assertThat(this!!.outcome).isEqualTo(SUCCESS)
+    }
+
+    // Take 2
+    val secondRunResult = gradleRunner
+      .withArguments("recordPaparazziDebug", "--stacktrace")
+      .forwardOutput()
+      .runFixture(fixtureRoot) { build() }
+
+    with(secondRunResult.task(":testDebugUnitTest")) {
+      assertThat(this).isNotNull()
+      assertThat(this!!.outcome).isEqualTo(SUCCESS) // not UP-TO-DATE
+    }
+
+    // Take 3
+    val thirdRunResult = gradleRunner
+      .withArguments("verifyPaparazziDebug", "--stacktrace")
+      .forwardOutput()
+      .runFixture(fixtureRoot) { build() }
+
+    with(thirdRunResult.task(":testDebugUnitTest")) {
+      assertThat(this).isNotNull()
+      assertThat(this!!.outcome).isEqualTo(SUCCESS) // not UP-TO-DATE
+    }
+  }
+
   @Test
   fun verifySuccess() {
     val fixtureRoot = File("src/test/projects/verify-mode-success")

paparazzi/paparazzi-gradle-plugin/src/test/projects/rerun-property-change/build.gradle

@@ -0,0 +1,22 @@
+plugins {
+  id 'com.android.library'
+  id 'kotlin-android'
+  id 'app.cash.paparazzi'
+}
+
+repositories {
+  maven {
+    url "file://${projectDir.absolutePath}/../../../../../build/localMaven"
+  }
+  mavenCentral()
+  //mavenLocal()
+  google()
+}
+
+android {
+  namespace 'app.cash.paparazzi.plugin.test'
+  compileSdk libs.versions.compileSdk.get() as int
+  defaultConfig {
+    minSdk libs.versions.minSdk.get() as int
+  }
+}

paparazzi/paparazzi-gradle-plugin/src/test/projects/rerun-property-change/src/test/java/app/cash/paparazzi/sample/RecordTest.kt

@@ -0,0 +1,16 @@
+package app.cash.paparazzi.plugin.test
+
+import android.view.View
+import app.cash.paparazzi.Paparazzi
+import org.junit.Rule
+import org.junit.Test
+
+class RecordTest {
+  @get:Rule
+  val paparazzi = Paparazzi()
+
+  @Test
+  fun record() {
+    paparazzi.snapshot(View(paparazzi.context))
+  }
+}