cashapp / paparazzi

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

Decouple snapshot tests from unit tests. #1161

Open KatieBarnett opened 7 months ago

KatieBarnett commented 7 months ago

I would like to be able to just run the snapshot tests and not also the unit tests. On my project's CI we have parallel build workflows to run verifications and tests and I would like the snapshot tests to run on a parallel workflow and not require the unit tests to have run first (so they can be run separately).

I have been able to exclude the snapshot tests from the general unit test task using the following:

afterEvaluate {
    if (!(props["runSpecificUnitTestsOnly"] as Boolean)) {
        tasks.named<Test>("testAlphaDebugUnitTest") {
            dependsOn(getModuleTaskList("feature", "testDebugUnitTest"))
            dependsOn(getModuleTaskList("base", "testDebugUnitTest"))
            filter {
                // Exclude snapshot tests - they will be run separately as part of verifyPaparazzi
                excludeTestsMatching("*.snapshot.*")
            }
        }
    }
}

I have also tried to just run the snapshot tests using ./gradlew recordPaparazziAlphaDebug --tests "*.snapshot.*" but it is not really achieving what I want.

TWiStErRob commented 7 months ago

I had the same problem, I solved it by a property too, but instead of name matching I used JUnit categories: https://github.com/search?q=repo%3ATWiStErRob%2Fnet.twisterrob.sun%20net.twisterrob.build.screenshot-tests&type=code

KatieBarnett commented 7 months ago

@TWiStErRob Thanks for that, it achieves half of what I want, but I also want to be able to run JUST the snapshot tests (so I can run them as parallel workflows.

TWiStErRob commented 7 months ago

Yes, that's what record/verify job does on GHA CI. If you pass the flag with true, it's Paparazzi-only. If you don't, it's only unit tests (because of default in gradle.properties). See the PaparazziPlugin.kt if-else.

TWiStErRob commented 7 months ago

Or by "parallel workflows" do you mean parallel tasks within a single Gradle execution process?

KatieBarnett commented 7 months ago

@TWiStErRob Thanks, yes, I ended up implementing as you did and have it working now.

ln-12 commented 5 months ago

If someone comes across this issue and does not want to (or cannot) implement the custom plugin approach shown above, here is how I did it:

// custom property to manually exclude snapshot tests, usage:
// - only unit test:         testDebugUnitTest -Ptests.exclude="com.example.tests.SnapshotTest"
// - only snapshot tests:    verifyPaparazziDebug --tests "com.example.tests.SnapshotTest"
def testsToExclude = properties["tests.exclude"]

tasks.configureEach { task ->
    if (task.name == "testDebugUnitTest" && testsToExclude != null && testsToExclude != "") {
        filter {
            excludeTestsMatching(testsToExclude)
        }
    }
}

In the case of verifying/recording snaphots, we can simply use the --tests option. As gradle does not yet have an option to exclude pattern (see https://github.com/gradle/gradle/issues/6505#issuecomment-415960830), I am using the custom property tests.exclude to only run the unit tests.

However, I am wondering if the PaparazziTask.setTestNameIncludePatterns option might be useful for that?

peterdk commented 2 months ago

I tried the solution of @TWiStErRob , but can't seem to get it to run only the screenshot tests that are marked with a Category. (I just want to run the screenshot tests) It will just run everything.

  tasks.withType(Test::class.java).configureEach {
            useJUnitPlatform()

            useJUnit {
                includeCategories("foo.bar.PaparazziTest")
                testLogging {
                    events(TestLogEvent.PASSED, TestLogEvent.FAILED, TestLogEvent.SKIPPED)
                }
            }
        }

@Category(PaparazziTest::class)
internal class AScreenTest {
}

It would really be useful to somehow only run the screenshot tests. (Only solution I see is to add a specific identifier in the package name or test class name).

I would love to see a @PaparazziTest annotation instead of @Test and then filter on that by default when running verify/recordPaparazzi.

peterdk commented 2 months ago

Never mind, finally got it working. Probably due to the Kotlin syntax I did something wrong:

tasks.withType(Test::class.java).configureEach {
            useJUnitPlatform(){
                includeTags.add("PaparazziTest")
            }
        }

This only runs test that are marked with @Category(PaparazziTest::class) (Note that you need to use includeTags but those handle the @Category classes in Junit4)

TWiStErRob commented 2 months ago

useJUnit (4) and useJUnitPlatform (5) are exclusive, you can only use one of them. 5 is capable of running 4, but that's hidden inside the vintage engine, so from Gradle's perspective you're running 5.

peterdk commented 2 months ago

We shaved 6 minutes of our CI build time by excluding paparazzi tests in normal unittest and running the screenshot tests only on the paparazzi tests.

TWiStErRob commented 1 month ago

@jrodbx based on how the new Compose screenshot testing works (separate source set, separate dependencies, separate compilation, separate task, supported experimentally by AGP), is there a chance of getting similar UX in Paparazzi using a sameish approach (even as an opt-in experimental feature)? This would probably solve recent issues with mocking frameworks, Robolectric by providing classpath isolation (even the ability to run on different JDK than normal unit tests), and it would nicely decouple things. I think the only problem is that it's experimental, and only available in AGP 8.5.