Force test re-runs if generated report or snapshot dirs are deleted

[jrodbx]

Similar to #204, but with task outputs. Closes #178.

[alashow]

After running ./gradlew testDebugUnitTest and then running ./gradlew verifyPaparazziDebug doesn't do anything, by saying tasks all tasks are uptodate. This was happening every time on CI and only sometimes locally. After seeing this PR & linked issues, I added rm -rf **/build/reports/paparazzi step after running testDebugUnitTest and before verifyPaparazziDebug, and now it works properly.
It's working but I find having to remove previous reports a little hacky and cleaning the cache or using --rerun-tasks causes longer builds.

Is there anyway to improve this caching behavior?

[TWiStErRob]

potential other workaround:
In recent Gradle releases there's --rerun-task verifyPaparazziDebug, similar to -x. This could be used instead of manually cleaning.

[alashow]

@TWiStErRob You linked to --rerun, which doesn't seem to work. But I had already tried --rerun-tasks, it works same as using clean verifyPaparazziDebug, which I'm trying to avoid:

This will force test and all task dependencies of test to execute. It’s a little like running gradle clean test, but without the build’s generated output being deleted.

[TWiStErRob]

Oh, you are right about the rerun option, it's not like -x 🤦‍♂️. Note that there's also a clean* task for every task. So gradlew cleanTestDebugUnitTest verifyPaparazziDebug should also rerun.

Can you please report a new issue with a full repro? Including version numbers. According to the code there's a property added to the test task, that should trigger a rerun. Do you use Gradle predictive test selection or similar?

[alashow]

According to the code there's a property added to the test task, that should trigger a rerun.

What property is that?

Do you use Gradle predictive test selection or similar?

No

I wasn't able to reproduce this properly yet. Deleting reports trick doesn't even work with paparazzi/sample. It just re-creates the report & but tests are not run

For example:

1. Clone this repo
2. Change some test to fail when verifying. For ex, val text = "Hello, Paparazzi" -> val text = "Hello, Paparazzi World"
3. Run ./gradlew sample:testDebugUnitTest - it will create the reports with "Hello, Paparazzi World"
4. Then running ./gradlew sample:verifyPaparazziDebug doesn't do anything with all up-to-date tasks.

Deleting the report doesn't work in this case - verifyPaparazziDebug just "recreates" the report (doesn't actually print "See the Paparazzi report at:" but the report files are re-created or copied from somewhere maybe because the task runs very fast, 2 seconds )

[TWiStErRob]

It just re-creates the report & but tests are not run
verifyPaparazziDebug just "recreates" the report

That's probably because caching is enabled:

paparazzi/gradle.properties

Line 1 in 17e3538

org.gradle.caching=true

According to the code there's a property added to the test task, that should trigger a rerun.

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

Lines 173 to 174 in 17e3538

	test.systemProperties["paparazzi.test.record"] = isRecordRun.get()
	test.systemProperties["paparazzi.test.verify"] = isVerifyRun.get()

Which means testDebugUnitTest should have those values false, and verifyPaparazziDebug should have those values true, which would normally invalidate the cache and cause a re-run.

Deleting the report doesn't work in this case - verifyPaparazziDebug just "recreates" the report

Also, here, you must be aware that verifyPaparazziDebug doesn't actually do anything other than configuring the test task. You can see this by adding --console=verbose to gradlew:

> Task :sample:testDebugUnitTest UP-TO-DATE
> Task :sample:verifyPaparazziDebug UP-TO-DATE

and just as I was writing this, I realized the problem...
the property setting above is added via test.doFirst(object : Action<Task> { which runs AFTER the caching key (including UP-TO-DATE check) is calculated, so it would affect the execution, if the incremental build calculations allowed it.

All the systemProperties manipulation is eager, and the isRecordRun value is only set after afterEvaluate completes, so there's no chance to do afterEvaluate { test.systemProperties.... This is why it was necessary to do it in doFirst, but this is incorrect and exhibits your problem. It needs to be made lazy in a different way.

I have a feeling as quick solution we might be able to add:

test.inputs.property("paparazzi.test.record", isRecordRun)
test.inputs.property("paparazzi.test.verify", isVerifyRun)

so that when it comes to UP-TO-DATE check, it has the providers as dependencies.

[TWiStErRob]

Created unit test and fix based on above findings #690