paparazzi icon indicating copy to clipboard operation
paparazzi copied to clipboard

Tests running Paparazzi are not cacheable

Open cesar1000 opened this issue 2 years ago • 2 comments

Description Gradle disables caching of unit tests when running Paparazzi, declares the snapshots directory output property as "overlapping."

Steps to Reproduce This happens consistently when running tests, provided that the snapshots directory is not empty. Just check the cacheability of the task in a build scan.

Expected behavior A Gradle task should own its outputs entirely in order to be able to cache them and restore them. As a result, pointing an output to a location in the repo causes the Gradle to disable caching for the task. In my opinion, this bug would be fixed by:

  • Updating the Paparazzi to write snapshots to a temporary location in the build directory.
  • Update the task output to point at this temporary location, and add a finalizer task to sync snapshots from there to the final location.
  • Add a new input for the execution mode (recording or verifying), since otherwise the cache will use outputs collected in the wrong execution mode.

For reference, this is a hack I put in in our codebase to reenable caching:

if (gradle.startParameter.taskNames.any {name -> name.contains('recordPaparazzi') }) {
    // Add an input for the mode of execution since, otherwise, recording will be satisfied from the cache by prior
    // verification runs.
    tasks.withType(Test).configureEach {
        inputs.property("mode", "recording")
    }
} else {
    // If not recording snapshots, rewrite the snapshot task output to point at an empty directly.
    def snapshotDirectory = project.layout.projectDirectory.dir("src/test/snapshots")
    def emptySnapshotDirectory = project.layout.buildDirectory.dir("paparazzi-snapshots").get()
    emptySnapshotDirectory.asFile.delete()
    afterEvaluate {
        tasks.withType(Test).configureEach {
            outputs.visitRegisteredProperties(
                    new PropertyVisitor.Adapter() {
                        @Override
                        void visitOutputFileProperty(String propertyName, boolean optional, PropertyValue value,
                                OutputFilePropertyType filePropertyType) {
                            if (value.call() == snapshotDirectory) {
                                // Use reflection to modify the value.
                                setValue(value, emptySnapshotDirectory)
                            }
                        }

                        private def setValue(staticValue, newValue) {
                            def valueField = staticValue.getClass().getDeclaredField('value')
                            valueField.setAccessible(true)

                            // Strip the final modifier.
                            def modifiersField = Field.getDeclaredField("modifiers")
                            modifiersField.setAccessible(true)
                            modifiersField.setInt(valueField, valueField.modifiers & ~Modifier.FINAL)

                            valueField.set(staticValue, newValue)
                        }
                    }
            )
        }
    }
}

Additional information:

  • Paparazzi Version: 1.0.0

cesar1000 avatar Sep 23 '22 17:09 cesar1000

Note that this workaround doesn't work in java 12 (you can't modify a final field anymore) so it'd be great to get this properly fixed. https://bugs.openjdk.org/browse/JDK-8210522

edenman avatar Dec 19 '22 23:12 edenman

Hi! I also encountered this issue and I was wondering what the status of this is.

wwchung avatar Apr 25 '23 04:04 wwchung