cashapp / paparazzi

Render your Android screens without a physical device or emulator
https://cashapp.github.io/paparazzi/
Apache License 2.0
2.24k stars 212 forks source link

Gradle build cache issue with snapshots folder #1334

Open kozaxinan opened 4 months ago

kozaxinan commented 4 months ago

Description A clear and concise description of what the bug is.

Since we started using, we end up having cache issues for the all the modules with paparazzi test. At first, we didnt notice but now with lots of modules with Paparazzi, our test duration increased a lot.

Gradle reports following for cacheability of the Test task.

Overlapping outputs: Gradle does not know how file 'src/test/snapshots/images' was created 
(output property '$2'). Task output caching requires exclusive access to output paths to 
guarantee correctness (i.e. multiple tasks are not allowed to produce output in the same location).

I tried adding all subfolders and files as output. That didnt work.

I noticed that "videos" folder is empty and not checked out to git. I tried deleting "videos" folder in doLast to delete empty folder. That didn't help. Empty output folder should not affect the cache.

I checked all tasks and their outputs to find out if any other tasks has same output. It showed debug and release Test has same output directory. I disabled unitTests for release variant. testReleaseUnitTest stopped getting created. But that didn't solve the issue. Now I know that only one task has "src/test/snapshots/images" as output.

I have tried running test with -Dorg.gradle.caching.debug=true. That didn't give any more details that Gradle Scan.

The cache debug about outputs.

Appending output property name to build cache key: $1
Appending output property name to build cache key: $2
Appending output property name to build cache key: binaryResultsDirectory
Appending output property name to build cache key: jvmArgumentProviders.jacocoAgent$1.jacoco.destinationFile
Appending output property name to build cache key: reports.enabledReports.html.outputLocation
Appending output property name to build cache key: reports.enabledReports.junitXml.outputLocation
Non-cacheable because Gradle does not know how file 'src/test/snapshots/images' was created 
(output property '$2'). Task output caching requires exclusive access to output paths to guarantee 
correctness (i.e. multiple tasks are not allowed to produce output in the same location). 
[OVERLAPPING_OUTPUTS]

Steps to Reproduce Provide a sample project, failing test, or steps to reproduce.

I dont have reproducible case. It is always non cacheable on CI. Locally sometimes task become cacheable.

Expected behavior A clear and concise description of what you expected to happen.

The snapshot folder should not be output of the Test and Verify tasks. They dont produce anything in that folder and there is no reason I can think of gradle to track that folder as output.

I created the Paparazzi Plugin in out codebase. I removed the snapshot folder from outputs. Now test task is cacheable.

Our build time changes without any code change:

testDebugUnitTest ~25m verifyPaparazziDebug ~11m jacoco ~12m

testDebugUnitTest ~2m verifyPaparazziDebug ~50 seconds jacoco ~1m

Without setting snapshot as output, we are able to save ~45m from out CI builds.

Additional information:

Screenshots If applicable, add screenshots to help explain your problem.

Screenshot 2024-03-10 at 21 26 05
kozaxinan commented 4 months ago

I locally moved snapshots folder to input as it is actually input for test and verify. record could have that folder as output without setting up test task output.

but after those changes, running unit test again showed a new issue. the report produce new file every time it runs. that is not good for cache as output of the test will change every time we run test.

Screenshot 2024-03-11 at 16 18 22

AlexanderGH commented 4 months ago

@kozaxinan do you by chance have code snippets for how you modified those tasks (at runtime) or did you just modify the paparazzi code/plugin directly?

Somehow coincidentally we both started investigating this issue at the same time (on the weekend nonetheless, though you're ahead of me in terms of solutions since i moved onto other cache issues in our build) so there's a theory that this caching issue is a recent regression.

kozaxinan commented 4 months ago

I copied the plugin code internally and modified it.

At the moment the snapshot folder is input for the test task rather than output. Also I gave reports/paparazzi/debug/images as output instead of the whole paparazzi folder. Every test run modified the run folder in the build/report and produced unreproducible output for cached tasks.

I would say the test task should do verification and fail. There needs to be a different non cachable task that does iterations in html report. And records should handle creating or moving screenshots instead of test runs.

kozaxinan commented 4 months ago

I internally added plugin code and changed what needed to be input and output.

Our findings and changes:

Our current setup has the following changes.

These are suggestions for changing the task input/output setup paparazzi library.

  1. The snapshots folder is input to test.

    test.inputs.files(snapshotOutputDir.asFileTree)
  2. Only testDebugUnitTest/verification is cacheable. recordPaparazi and reportPaparazzi (Our new task that generates HTML reports) are non-cacheable.

    test.outputs.cacheIf { !isReportRun.get() && !isRecordRun.get() }
  3. The report folder is removed from output.

This approach makes the Test task cacheable for test runs. Also, allow us to do the verification in a single test run instead of testDebugUnitTest + verifyPapazazzi.

If you want to see changes in a PR, I can open a pr with suggested changes around input and output changes.