Support application modules

[jrodbx]

Currently, the plugin can only parse merged resources from library modules since they're in XML format, while application modules output merged resources to a binary format. This limits Paparazzi to library modules for now.

[erawhctim]

@jrodbx I'm curious if you have any thoughts or updates on this? This issue seems like a bit of a hurdle to adopting Paparazzi in non-modularized projects. I'm starting to dig into the implementation and figure out what sort of effort is required here; I'd love to hear your thoughts if you have suggestions on the what and how of the changes (or if there are any non-obvious blockers that prevent work on this issue from even completing).

[jrodbx]

I don't have any suggestions or put much time yet into thinking about how to implement this, since Paparazzi currently expects xml/text resources on the file system.

A workaround for now would be to create a new module and move your views there, referencing it from :app.

[RainNapper]

Is it too much scope creep to also support dynamic feature modules (plugin: com.android.dynamic-feature)?

I have a feeling it would be a similar fix to application modules, but let me know if I should file a separate issue for this.

Thanks!

[jrodbx]

Given my current lack of familiarity with dynamic features, I'd favor a separate issue. If anything, we'll close both with one PR 😸

[TWiStErRob]

On AGP 4.2.2 this is the stack trace:

``` null cannot be cast to non-null type V of app.cash.paparazzi.Paparazzi.inflate java.lang.NullPointerException: null cannot be cast to non-null type V of app.cash.paparazzi.Paparazzi.inflate at app.cash.paparazzi.Paparazzi.inflate(Paparazzi.kt:164) at net.twisterrob.sun.android.logic.VerifyTest.test(VerifyTest.java:20) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:566) at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59) at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12) at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56) at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17) at app.cash.paparazzi.Paparazzi$apply$statement$1.evaluate(Paparazzi.kt:107) at app.cash.paparazzi.agent.AgentTestRule$apply$1.evaluate(AgentTestRule.kt:17) at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306) at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100) at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366) at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103) at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63) at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331) at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79) at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329) at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66) at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293) at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306) at org.junit.runners.ParentRunner.run(ParentRunner.java:413) at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.runTestClass(JUnitTestClassExecutor.java:110) at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.execute(JUnitTestClassExecutor.java:58) at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.execute(JUnitTestClassExecutor.java:38) at org.gradle.api.internal.tasks.testing.junit.AbstractJUnitTestClassProcessor.processTestClass(AbstractJUnitTestClassProcessor.java:62) at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.processTestClass(SuiteTestClassProcessor.java:51) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:566) at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36) at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24) at org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:33) at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:94) at com.sun.proxy.$Proxy2.processTestClass(Unknown Source) at org.gradle.api.internal.tasks.testing.worker.TestWorker.processTestClass(TestWorker.java:119) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:566) at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36) at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24) at org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection$DispatchWrapper.dispatch(MessageHubBackedObjectConnection.java:182) at org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection$DispatchWrapper.dispatch(MessageHubBackedObjectConnection.java:164) at org.gradle.internal.remote.internal.hub.MessageHub$Handler.run(MessageHub.java:414) at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:64) at org.gradle.internal.concurrent.ManagedExecutorImpl$1.run(ManagedExecutorImpl.java:48) at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) at org.gradle.internal.concurrent.ThreadFactoryImpl$ManagedThreadRunnable.run(ThreadFactoryImpl.java:56) at java.base/java.lang.Thread.run(Thread.java:834) ```

The difference in file structure:

app/build/intermediates/merged/debug/ * layout_activity_config.xml.flat * layout_activity_screenshot.xml.flat * values_values.arsc.flat * values-v11_values-v11.arsc.flat lib/build/intermediates/merges/debug * layout * activity_config.xml * activity_screenshot.xml * values * values.xml * values-v11 * values-v11.xml

@jrodbx for app modules does it need to use the merged resources, using sources is not "good enough"?

[gumil]

I've recently saw the PR for rendering compose snapshots. I have not dug deep into the code, but would the merging of resources still revelant for paparazzi if the app only uses compose? Or testing only on compose UI.

[BsBrabi]

Any updates on this issue? Currently this is a dealbreaker for us to implement Paparazzi in our existing projects.

[TWiStErRob]

Can you please expand on that cyclic dependency thing? @erawhctim

[erawhctim]

Actually, I'm realizing that what I originally wrote isn't actually true (disregard that - my apologies!). There is a way to set up a gradle module dependency cycle between an app module and a library module, but only if you're using a test-specific dependency declaration (e.g. app module depends on a library module, and then the library module has a test or androidTest dependency back on the app module). This is explained in the context of a real-world testing issue in this Google bug ticket, for anyone curious.

However, I do have an alternative solution for you! Change your top-level app module to be a library module, set up Paparazzi there, and then create a new app module that depends on your library module.

There's an example of this here over in the slackhq/circuit repo sample app 🙂

[Mittchel]

Any updates on this issue? Currently this is a dealbreaker for us to implement Paparazzi in our existing projects.

Same here, we would also love to hear about any updates regarding this issue. Can't wait to start using Paparazzi :-) !

[Burtan]

I've recently saw the PR for rendering compose snapshots. I have not dug deep into the code, but would the merging of resources still revelant for paparazzi if the app only uses compose? Or testing only on compose UI.

There seems no reason to block paparazzi on application modules when only using composables?

[Burtan]

I added a pull request: https://github.com/cashapp/paparazzi/pull/755

[jrodbx]

but would the merging of resources still revelant for paparazzi if the app only uses compose? Or testing only on compose UI.

Yes, it's still relevant. In the app module, resource merging will also entail converting to proto flat format, which isn't parseable by Paparazzi.

There seems no reason to block paparazzi on application modules when only using composables?

Yes, because even though layout resources are no longer needed, other resources (namely strings/drawables) may be included, either directly or transitively.

[jrodbx]

Fixed in https://github.com/cashapp/paparazzi/pull/960

[Burtan]

The check for application/library modules is still enabled in the gradle plugin when new resources handling is disabled (which is currently the default?). Maybe improve the error message to inform about the new resource handling.

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

[TWiStErRob]

@Burtan There's a PR to make the new one the default: https://github.com/cashapp/paparazzi/pull/985