Gradle build cache issue with snapshots folder

[kozaxinan]

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:

• Paparazzi Version: 1.3.3
• OS:
• Compile SDK: 34
• Gradle Version: 8.6
• Android Gradle Plugin Version: 8.3

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

[kozaxinan]

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.

[AlexanderGH]

@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]

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]

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

Our findings and changes:

• Report folder -> This shouldn't be output. Because with every new run, there will be new results. That prevents caching from working properly as the same input can create different outputs/reports.

• snapshots folder -> This should be input at best. Test and Verify use snapshots as input and don't produce any new snapshots in that folder. The record task could have the snapshots folder as output but I think it is best to have a noncacheable record task, but that is a different topic.

• Test should run tests and fail -> With the current setup, running testDebugUnitTest generates HTML report but does not fail in case of snapshot failure. That doesn't align with the test intention. We now have a new reportPaparazzi task that generates HTML reports. Running testDebugUnitTest verifies the snapshots and fails. (I am not sure how much this change is open to discussion for Paparazzi)

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.

[jrodbx]

A few comments here:

The original issue is a duplicate of the issue attempted to be address in #852, but most recently in progress here: #1522. That should land very soon 🤞

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.

i'm a bit confused by this, since reportOutputDir is a task output, it should be cached/not rerun each time. can you elaborate more? (I also wonder if that bug goes away after #1522 lands.)

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.

This was proposed in #1365 which was closed as a duplicate. That being said, I think #1522 fixes this.

Our findings and changes:
Report folder -> This shouldn't be output. Because with every new run, there will be new results. That prevents caching from working properly as the same input can create different outputs/reports.

The report folder is definitely an task output, but...subsequent runs shouldn't produce new results unless there are new changes. I'd love more insights into this, as this is likely a bug!

snapshots folder -> This should be input at best. Test and Verify use snapshots as input and don't produce any new snapshots in that folder. The record task could have the snapshots folder as output but I think it is best to have a noncacheable record task, but that is a different topic.

Agreed on changing the inputs/outputs, to be address in #1522. A separate record task isn't ideal as it relies on Gradle more, vs tackling the root of the issue.

Test should run tests and fail -> With the current setup, running testDebugUnitTest generates HTML report but does not fail in case of snapshot failure. That doesn't align with the test intention. We now have a new reportPaparazzi task that generates HTML reports. Running testDebugUnitTest verifies the snapshots and fails. (I am not sure how much this change is open to discussion for Paparazzi)

Duplicate of #1365. I think a solution might be to eliminate direct invocations of testDebug entirely. My comment here explains why: #421 (comment)

[kozaxinan]

I reviewed recent changes. I am happy to see that there are significant changes. I am on vacation now without a pc.

I will verify changes with snapshot builds in mid September