takahirom / roborazzi

Make JVM Android integration test visible 🤖📸
https://takahirom.github.io/roborazzi/
Apache License 2.0
649 stars 24 forks source link

[WIP] Only use output dir as an input when roborazzi runs in compare or verify modes #386

Closed lukas-mercari closed 1 month ago

lukas-mercari commented 1 month ago

This is an attempt to make the roborazzi gradle tasks fully cacheable.

Problem

Currently, Roborazzi's gradle plugin declares the output directory (where the golden screenshot files are stored) as an input. This is correct when the task runs in compare and verify modes, but it is not necessary when running record mode only. The side-effect of this is that it is much more difficult to find an existing build cache entry.

Solution

The basic idea is that we need to detect whether we run in verify/compare or record mode and then selectively set the input files. The complication is that (when running recordRoborazzi*) we cannot know whether we are recording, because the provider does not contain a value at the time when inputs are configured.

Attempt 1

Generally it seems that we can provide properties that are lazily evaluated. (Such as isRecordRun.) I attempted to create one (using project.objects.directoryProperty()), which I would then be able set similar to how isRecordRun's value is assigned.

However, this was not successful:

> Cannot fingerprint input property : value 'file collection' cannot be serialized.

It seems that setting complex properties as input is unusual. See: https://discuss.gradle.org/t/task-with-a-complex-object-as-input/41385

Attempt 2

Instead, I thought that we don't actually need to provide the file collection -- but a hash of the file directory should be good enough to detect whether it has changed or not. Essentially I declared a property named goldenFilesHash, which I set to a hash of the output directory if roborazzi is running in verify/compare mode -- and otherwise to "".

lukas-mercari commented 1 month ago

@takahirom I think I might have finally found a solution for the gradle cache issue. It's not super straightforward unfortunately, but I'd be happy for hear any feedback!

takahirom commented 1 month ago

Using hashes is one option, but I thought we could avoid using hashes for the record task by using empty files. So, I made a PR for checking https://github.com/takahirom/roborazzi/pull/389

takahirom commented 1 month ago

@lukas-mercari I think Gradle might be calculating the hash as well, and I believe it is better if we can rely on Gradle for this. So what do you think of https://github.com/takahirom/roborazzi/pull/389 ?

takahirom commented 1 month ago

I'll close this PR as the fix is released!