cashapp / paparazzi

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

Support alternate test suites #1520

Open yogurtearl opened 3 months ago

yogurtearl commented 3 months ago

Gradle supports multiple test suites: https://docs.gradle.org/current/userguide/jvm_test_suite_plugin.html

AGP doesn't support this yet: https://issuetracker.google.com/issues/307694643

If/when AGP does support a test suite concept, paparazzi would need to allow for configuring with test suites it will run in.

As of version 1.3.4, it looks like it only supports a default test suite named "test".

https://github.com/cashapp/paparazzi/blob/6bbe276138ece88769c3ae8046d5afd5ef7bfb34/paparazzi-gradle-plugin/src/main/java/app/cash/paparazzi/gradle/PaparazziPlugin.kt#L289

https://github.com/cashapp/paparazzi/blob/6bbe276138ece88769c3ae8046d5afd5ef7bfb34/paparazzi-gradle-plugin/src/main/java/app/cash/paparazzi/gradle/PaparazziPlugin.kt#L178

jrodbx commented 3 months ago

Gradle supports multiple test suites: https://docs.gradle.org/current/userguide/jvm_test_suite_plugin.html

Gradle per se doesn't; that link points to documentation for the JVM test suite plugin. Even Xav mentions that here: https://issuetracker.google.com/issues/307694643#comment7

Does Gradle intend to provide core platform for this?

Taking a step back, what's the value proposition of supporting this? It's not clear to me why the "very useful" part of the feature request in AGP would be: https://issuetracker.google.com/issues/307694643#comment1.

If it's because of AGP/Robolectric conflicting with each other re: bytecode manipulation, then I'd encourage fixing that instead. If it's to run a subset of tests, using --tests with a regex or test annotation markers would be preferred here.

Mind elaborating more on the use cases?

TWiStErRob commented 3 months ago

The point of test suites is to separate the suites. In Android this comes with different tech, in JVM too, but only sometimes.

Screenshot tests are a very specific type of tests, just like unit tests, instrumentation tests, etc. I could imagine an integration tests as a suite as well. Each of these groups of tests would have different dependencies, and different approaches, which could go as far as one running TestNG and another JUnit.

Yes, it is possible (potentially with hacks and workarounds) to have all of these in one source set, but why would that be the norm if we can do better? Simply splitting up tests into source sets solves a ton of problems. You can have different resources, annotation processors, even languages!

Also how much more ergonomic is gradlew :foo:testDebugPaparazziTest than gradlew :foo:testDebugUnitTest --test "com.example.**.*ScreenshotTest" -DscreenshotTest=on. Much less implied knowledge, improved compilation, dependency management, separation of concerns etc.

yogurtearl commented 3 months ago

Does Gradle intend to provide core platform for this?

The jvm-test-suite plugin is included in gradle, similar to how the java-library or java-test-fixtures plugins are included with gradle, but yeah it is implemented as a plugin.

AGP doesn't currently support the jvm-test-suite plugin or concepts, but they could add support in a similar way to how they added support for the java-test-fixtures concepts.

Mind elaborating more on the use cases?

Having layoutlib on the classpath provides different implementations of android.* class compared to what you normally get from android.jar. (We have tests that fail just by the presence of layoutlib on the class path before android.jar)

We need to support tests that work in a "plain old" android /test setup without layoutlib (or roboletric) on the classpath.

With a separate test suite, tests that require the android.jar versions of android.* stubs would work if paparazzi is kept to a separate test suite.

I opened this issue more as a placeholder for if/when AGP ever decides to support the jvm-test-suite plugin or concepts.

Our current workaround will be to have plain old unit tests in :feature:foo:ui (in /test) and put Paparazzi tests in :feature:foo:screenshots (in /test) and in feature/foo/screenshots/build.gradle.kts we will have testImplementation(projects.feature.foo.ui)

yogurtearl commented 1 month ago

can we remove the waiting on user tag on this one?