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

Can I run paparazzi with Robolectric? #425

Closed takahirom closed 2 years ago

takahirom commented 2 years ago

When run with Robolectric, the error "class redefinition failed: attempted to change schema (add/remove fields)" occurs.

To reproduce the procedure, simply insert Robolectric and run it as follows.

@RunWith(AndroidJUnit4::class)
class ExampleUnitTest {
    @get:Rule
    val paparazzi = Paparazzi()

This is the difference from the default file when the project was created. https://github.com/takahirom/paparazzi-with-robolectric/commit/ac47b23e89b62c28969fa40298f794d3d1238ecd

I checked on Android Studio's JDK and JDK 16

stacktrace

java.lang.UnsupportedOperationException: class redefinition failed: attempted to change the schema (add/remove fields)
    at java.instrument/sun.instrument.InstrumentationImpl.retransformClasses0(Native Method)
    at java.instrument/sun.instrument.InstrumentationImpl.retransformClasses(InstrumentationImpl.java:168)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:567)
    at net.bytebuddy.utility.Invoker$Dispatcher.invoke(Unknown Source)
    at net.bytebuddy.utility.dispatcher.JavaDispatcher$Dispatcher$ForNonStaticMethod.invoke(JavaDispatcher.java:1013)
    at net.bytebuddy.utility.dispatcher.JavaDispatcher$ProxiedInvocationHandler.invoke(JavaDispatcher.java:1142)
    at net.bytebuddy.dynamic.loading.$Proxy52.retransformClasses(Unknown Source)
    at net.bytebuddy.dynamic.loading.ClassReloadingStrategy$Strategy$2.apply(ClassReloadingStrategy.java:408)
    at net.bytebuddy.dynamic.loading.ClassReloadingStrategy.load(ClassReloadingStrategy.java:236)
    at net.bytebuddy.dynamic.TypeResolutionStrategy$Passive.initialize(TypeResolutionStrategy.java:100)
    at net.bytebuddy.dynamic.DynamicType$Default$Unloaded.load(DynamicType.java:6154)
    at app.cash.paparazzi.agent.InterceptorRegistrar$addMethodInterceptors$1.invoke(InterceptorRegistrar.kt:34)
    at app.cash.paparazzi.agent.InterceptorRegistrar$addMethodInterceptors$1.invoke(InterceptorRegistrar.kt:22)
    at app.cash.paparazzi.agent.InterceptorRegistrar.registerMethodInterceptors(InterceptorRegistrar.kt:39)
    at app.cash.paparazzi.agent.AgentTestRule$apply$1.evaluate(AgentTestRule.kt:15)
    at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
    at org.robolectric.RobolectricTestRunner$HelperTestRunner$1.evaluate(RobolectricTestRunner.java:591)
    at org.robolectric.internal.SandboxTestRunner$2.lambda$evaluate$0(SandboxTestRunner.java:274)
    at org.robolectric.internal.bytecode.Sandbox.lambda$runOnMainThread$0(Sandbox.java:88)
    at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1130)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:630)
    at java.base/java.lang.Thread.run(Thread.java:831)
sdoward commented 2 years ago

@takahirom As far as I understand Robolectric and Paparazzi solve different problems. I don't know of a usecase where both would exist. Perhaps you are following an incorrect approach?

takahirom commented 2 years ago

I would like to explain my use case. To solve the Flaky test in Integration test, I ideally wanted to run Dagger Hilt and paparazzi together in the JVM. The Dagger Hilt test requires Android Instrumentation, i.e. Robolectric in the JVM for accessing the Application class. Therefore, this is currently not possible. Is there any alternative way to do this?

sdoward commented 2 years ago

@takahirom what are you trying to test exactly? Paparazzi allows us to render a layout on the local machine rather than on an emulator/device. It also provides a mechanism to assert against pngs.

It sounds like you want to run your application on a device but you still want the assertion mechanism. Is that correct?

If that is the case there might be solutions that are more suited for your usecase...

https://github.com/pedrovgs/Shot https://github.com/facebook/screenshot-tests-for-android

takahirom commented 2 years ago

Thanks for all the suggestions.

What I want to test is the behavior of the application. I find that running tests on real devices and emulators often makes the tests flaky because there are more aspects that the developer has no control over. Therefore I feel that I would like to run the tests on the JVM.

takahirom commented 2 years ago

For example, suppose you have an app that maps data retrieved from the server's API to a model in the app and displays it on the screen. In the test, we replace the API client class with Hilt and use paparazzi to check what is displayed. If the mapping is incorrect, the display will change and you will notice.

However, considering that paparazzi is using Android Studio preview and as a library to test its contents, I feel that this use case may not be the correct way to use this library.

sdoward commented 2 years ago

However, considering that paparazzi is using Android Studio preview and as a library to test its contents, I feel that this use case may not be the correct way to use this library.

I think you are correct. It sounds like you just need the assertion part. So I think the libraries that I linked to might help

jrodbx commented 2 years ago

The Dagger Hilt test requires Android Instrumentation, i.e. Robolectric in the JVM for accessing the Application class. Therefore, this is currently not possible.

IIUC, this means your test would run in /androidTest which is not supported by Paparazzi, as those tests are pushed and ran on a device/emulator along with an APK.

In theory, a Robolectric test running in /test should work with Paparazzi, but as @sdoward mentions, part of the goal of Paparazzi is to replace mainly of the use cases that one would use to justify a Robolectric test.

but...that all being said, I don't think the original issue comment points to Robolectric being the issue, but rather using JDK 16. Assuming I'm understanding that correctly, then this ticket is a duplicate of https://github.com/cashapp/paparazzi/issues/384.

takahirom commented 2 years ago

your test would run in /androidTes

If you mean to make it work with Roblectric, that sounds good, but there seems to be one misunderstanding that I would like to share. The hilt test can be run in the JVM because Android Instrumentation is provided by Roborectric.🙏 Instrumentation refers to the android.app.Instrumentation class, and This is implemented in the JVM by Robolectric and in the Emulator and Android runtime environment by androidx.test.

This is explained in Hilt's documentation and I use it in my own projects.

Hilt only supports Android instrumentation and Robolectric tests

https://dagger.dev/hilt/testing.html

Examples of projects in use https://github.com/airbnb/mavericks/blob/4daa6add8a0daf094d6cefaa390737c83fcdb168/mvrx-hilt/src/test/kotlin/com/airbnb/mvrx/hilt/ViewModelTest.kt#L18

takahirom commented 2 years ago

@jrodbx Maybe my problem is not related to the JDK version. I wrote a comment here so you can take a look if you have time. https://github.com/cashapp/paparazzi/issues/384#issuecomment-1133510913

Thomodachi commented 1 year ago

@takahirom As far as I understand Robolectric and Paparazzi solve different problems. I don't know of a usecase where both would exist. Perhaps you are following an incorrect approach?

I am running in to the exact same problem as @takahirom. For me the reason to use Robolectric is that I have to use a library that calls DisplayManager.getDisplay(DEFAULT_DISPLAY).flags. However, getDisplay() returns null. I also do not want to test with an Activity but isolated Views instead (such as buttons or list items). I would like to mock Android's Context to make DisplayManager.getDisplay() return something. Would you have any suggestions here how I could achieve this without using Robolectric?

takahirom commented 1 year ago

If all you want to do is that, I think you may use the approach used in paparazzi which mocks the android framework. https://github.com/cashapp/paparazzi/blob/a76702744a7f380480f323ffda124e845f2733aa/paparazzi/paparazzi-agent/src/main/java/app/cash/paparazzi/agent/InterceptorRegistrar.kt#L4

sergio-sastre commented 10 months ago

I'm sorry to say this, but it is not possible to use Robolectric with Paparazzi in the very same test.

The matter is that for Robolectric to run, you need the RobolectricTestRunner. It's also important to know that AndroidJunitTestRunner uses it under the hood when running on the JVM.

However, RobolectricTestRunner prepares the Test Environment and, from my understanding, it also downloads some resources.

PaparazziRule does something similar and the JVM complains because it is trying to redefine sth that Robolectric already did.

I think this is hard to fix due to the way Robolectric and Paparazzi work, which interfere with each other.

IMO, this should not be closed, since I can confirm the issue still exists with Paparazzi 1.3.1

jrodbx commented 10 months ago

@sergio-sastre thanks for the update. can you please file a new issue with a minimal sample project? In previous investigations, we've found the opposite, but it's possible we were missing something. i'm interested in fixing this, once and for all, if we can figure out a good repro case.

sergio-sastre commented 9 months ago

@sergio-sastre thanks for the update. can you please file a new issue with a minimal sample project? In previous investigations, we've found the opposite, but it's possible we were missing something. i'm interested in fixing this, once and for all, if we can figure out a good repro case.

@jrodbx Sure, give me a couple of days to branch out a repo where I can reproduce it.

It actually happens if setting includeAndroidResources = true in gradle, used by Robolectric 4.x, otherwise tests are skipped