[WIP] Cleanup intermediate dir usage, since there seem to be some problems restoring it when caches are used

[lukas-mercari]

Thank you as always for your work and release 1.16.0! When there were no changes affecting screenshot tests, it reduced the CI/CD build time of the VRT task to about 30% of what it used to be. 👏

I noticed one problem when I was checking with a local cache. This can be reproduced using the following commands:

./gradlew :sample-android:recordRoborazziDebug --build-cache
// Note: ./sample-android/build/outputs/roborazzi exists
rm -rf ./sample-android/build
./gradlew :sample-android:recordRoborazziDebug --build-cache // Cached build
// Note: Sometimes ./sample-android/build/outputs/roborazzi doesn't exist. 😱
./gradlew :sample-android:recordRoborazziDebug --build-cache // Read again from cache without deleting build dir.
// Note: ./sample-android/build/outputs/roborazzi exists!

It seems that there is some flakiness when restoring screenshots from the intermediate to the output dir. This was added in #128 (but it's not immediate clear to me why it was needed).

[github-actions[bot]]

Snapshot diff report	File name	Image
manual_small_view_bu tton_compare.png
manual_all_4_compare .png
manual_view_a11y_dum p_compare.png
com.github.takahirom .roborazzi.sample.Co mposeTest.accessibil ityExplanation_2_com pare.png
manual_view_first_sc reen_with_query_view _compare.png
com.github.takahirom .roborazzi.sample.Co mposeTest.accessibil ityExplanation_compa re.png
com.github.takahirom .roborazzi.sample.Ma nualTest.moveToNextP ageWithEspresso_comp are.png
manual_viewwithout window_compare.png
manual_view_first_sc reen_with_query_comp ose_custom_compare.p ng
manual_bitmap_compar e.png
com.github.takahirom .roborazzi.sample.Ma nualTest.captureScre enLevelImageWithEspr esso_compare.png
com.github.takahirom .roborazzi.sample.Wi ndowCaptureTest.noDi alog_compare.png
custom-com.github.ta kahirom.roborazzi.sa mple.RuleTestWithAll Image.captureRoboGif Sample.3_compare.png
com.github.takahirom .roborazzi.sample.Ma nualTest.captureScre enLevelTabletWithEsp resso_compare.png
manual_view_on_windo w_compare.png
com.github.takahirom .roborazzi.sample.Fr agmentActivityWindow CaptureTest.bottomSh eetDialog_compare.pn g
com.github.takahirom .roborazzi.sample.Ma nualTest.captureScre enLevelImageWithEspr essoAndScaleOptions_ compare.png
com.github.takahirom .roborazzi.sample.Wi ndowCaptureTest.andr oidDialog_compare.pn g
com.github.takahirom .roborazzi.sample.Co mposeTest.roundTrans parentResizeCompose_ 2_compare.png
com.github.takahirom .roborazzi.sample.Ru leTestWithLastImage. captureRoboGifSample _compare.png
custom-com.github.ta kahirom.roborazzi.sa mple.RuleTestWithAll Image.captureRoboGif Sample.4_compare.png
com.github.takahirom .roborazzi.sample.Co mparisonStyleTest.di ffNoLabel_compare.pn g
com.github.takahirom .roborazzi.sample.Ma nualTest.captureScre enWithMetadata_compa re.png
custom-com.github.ta kahirom.roborazzi.sa mple.RuleTestWithAll Image.captureRoboGif Sample.2_compare.png
com.github.takahirom .roborazzi.sample.Sd k25DontWorkScreensho tTest.sdk25DontWorkS creenshot_compare.pn g
com.github.takahirom .roborazzi.sample.Wi ndowCaptureTest.comp oseDialog_compare.pn g
com.github.takahirom .roborazzi.sample.Wi ndowCaptureTest.dump _compare.png
com.github.takahirom .roborazzi.sample.Co mposeTest.roundTrans parentResizeCompose_ compare.png
com.github.takahirom .roborazzi.sample.Wi ndowCaptureTest.comp oseBottomSheet_compa re.png
com.github.takahirom .roborazzi.sample.Co mparisonStyleTest.di ffDefault_compare.pn g
com.github.takahirom .roborazzi.sample.Co mparisonStyleTest.di ffNoSmallLineSpace_c ompare.png
com.github.takahirom .roborazzi.sample.Co mparisonStyleTest.si mple_compare.png
com.github.takahirom .roborazzi.sample.Ma nualTest.captureScre enLevelNightWithEspr esso_compare.png
customoutputFilePro vider-com.github.tak ahirom.roborazzi.sam ple.RuleTestWithPath .roboOutputNameTest compare.png
custom_outputFilePro vider-com.github.tak ahirom.roborazzi.sam ple.RuleTestWithPath .captureRoboImage_co mpare.png
custom_outputFilePro vider-com.github.tak ahirom.roborazzi.sam ple.RuleTestWithPath .captureRoboImageWit hPath_compare.png
custom_file_compare. png
com.github.takahirom .roborazzi.sample.Co mposeTest.wearCompos able_2_compare.png
com.github.takahirom .roborazzi.sample.Co mparisonStyleTest.di ffNoBigLineSpace_com pare.png
manual_all_0_compare .png
custom-com.github.ta kahirom.roborazzi.sa mple.RuleTestWithAll Image.captureRoboGif Sample.1_compare.png
com.github.takahirom .roborazzi.sample.Co mposeTest.composable _compare.png
manual_compose_compa re.png
com.github.takahirom .roborazzi.sample.Ma nualTest.captureScre enLevelJapaneseWithE spresso_compare.png
manual_all_2_compare .png
custom-com.github.ta kahirom.roborazzi.sa mple.RuleTestWithAll Image.captureRoboGif Sample.0_compare.png
com.github.takahirom .roborazzi.sample.Co mposeTest.wearCompos able_compare.png
manual_all_1_compare .png
manual_last_compare. png
com.github.takahirom .roborazzi.sample.Ma nualTest.captureComp oseImage_compare.png
manual_all_3_compare .png
manual_view_first_sc reen_with_query_comp ose_compare.png
MainKmpTest.test_2_c ompare.png
MainKmpTest.test_com pare.png
MainJvmTest.test_com pare.png
MainJvmTest.test_2_c ompare.png
com.github.takahirom .roborazzi.sample.No ComposeManualTest.ca ptureRoboImageSample _compare.png

[takahirom]

Thanks to the output from this PR, I discovered a bug that our integration tests had not covered. Therefore, I added tests and reverted the removal of test task inputs. https://github.com/takahirom/roborazzi/pull/364

[takahirom]

We might be able to modify the input dir for either recording or for comparing and verifying.

[lukas-mercari]

@takahirom Nice, thanks for adding the test and reverting the change! Indeed, after looking at it again it seems that making the cache work isn't going to be as simple as we had hoped. The other problem that I ran into (why I started to make changes) was that even with the caching enabled we relied on copying files from the intermediate dir to the output dir. In my local runs, isTestSkipped was not always detected correctly, so sometimes the output dir ended up being empty. (Maybe due to the configuration cache?)

[takahirom]

We might need to add additional logs for further investigation.

[lukas-mercari]

Yes, I added some locally and it seemed that TestTaskSkipEventsServiceProvider.expectingTestNames wasn't initialised properly. (So, the skipped test run never gets added to skippedTestTaskMap.)

[takahirom]

Thanks. I'm thinking of removing TestTaskSkipEventsServiceProvider and isTestSkipped. Consistency is more important, and I don't think the copy operation is that heavy of a task. What do you think about this?

              // if (isTestSkipped) {
              // If the test is skipped, we need to use cached files
              val startCopy = System.currentTimeMillis()
              intermediateDir.get().asFile.mkdirs()
              intermediateDir.get().asFile.copyRecursively(
                target = outputDir.get().asFile,
                overwrite = true
              )
              finalizeTestTask.infoln("Roborazzi: finalizeTestRoborazziTask Copy files from ${intermediateDir.get()} to ${outputDir.get()} end ${System.currentTimeMillis() - startCopy}ms")
              // }

[takahirom]

I'll make a PR to check integration tests

→ https://github.com/takahirom/roborazzi/pull/366

[lukas-mercari]

@takahirom Sorry for the late reply. I found some time to do a bit more research today.

build/outputs/roborazzi not being restored from cache

Since #364 I got the following error instead:

* What went wrong:
A problem was found with the configuration of task ':sample-android:testDebugUnitTest' (type 'AndroidUnitTest').
  - Type 'com.android.build.gradle.tasks.factory.AndroidUnitTest' property '$1' specifies directory '/Users/lukas/Dev/test/roborazzi/sample-android/build/outputs/roborazzi' which doesn't exist.

    Reason: An input file was expected to be present but it doesn't exist.

    Possible solutions:
      1. Make sure the directory exists before the task is called.
      2. Make sure that the task which produces the directory is declared as an input.

    For more information, please refer to https://docs.gradle.org/8.4/userguide/validation_problems.html#input_file_does_not_exist in the Gradle documentation.

It's a bit hard to reproduce, since I believe the configuration cache needs to kick in, but the steps are roughly:

// Make a change to the tests in `sample-android`.
./gradlew :sample-android:recordRoborazziDebug --build-cache
rm -rf ./sample-android/build
./gradlew :sample-android:recordRoborazziDebug --build-cache
rm -rf ./sample-android/build
./gradlew :sample-android:recordRoborazziDebug --build-cache

I think the error is essentially what is described in this issue: https://github.com/gradle/gradle/issues/2016 I made a fix like this, and this brought back the problem with restoring the output directory instead. (Edit: It doesn't seem to be working well in all cases though -- see #374.) Finally #366 fixes that issue.

Modifying the input dir depending on if we run in record or compare/verify mode

It seems that we cannot read isRecordRun in configureEach:

> Could not create task ':sample-android:testDebugUnitTest'.
 > Cannot query the value of this property because it has no value available`.

We could read test.systemProperties["roborazzi.test.record"] directly, but in that case the caching behaviour would only change when the user runs testDebugUnitTest directly (and not using recordRoborazziDebug).

I'm a bit out of ideas on how to approach this -- this seems to be a general limitation in the interaction between gradle and how our tasks are set up. (E.g. paparazzi seems to have the problem reversed since they don't declare the output dir as an input.)