cashapp / paparazzi

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

Screenshots not showing Coil images correctly #1123

Open guerrerorodrigo opened 11 months ago

guerrerorodrigo commented 11 months ago

Description I have different tests that use Paparazzi. These tests are for a custom Icon that use rememberAsyncImagePainter() and an ImageRequest from Coil. I'm following this approach to setup the tests.

These tests live in different classes. For the sake of this sample, there are three different classes, each with the exact same test.

After executing ./gradlew app:recordPaparazziDebug, I get the different images for the tests. The following are the images I get:

Test #1: image

Test #2: image

Test #3: image

Only the test that is executed first, shows the image. The image is missing from the other tests, even though the tests are exactly the same.

Steps to Reproduce Sample project can be found here. You can run ./gradlew app:recordPaparazziDebug to get the images.

Expected behavior All images should show the icon.

Additional information:

mrmike commented 11 months ago

I think this might be related to https://github.com/cashapp/paparazzi/issues/1087#issuecomment-1728303515

guerrerorodrigo commented 11 months ago

@mrmike I don't think it's related, since I don't use a LazyColumn

yschimke commented 10 months ago

The tests don't look exactly the same. With matching intercept logic, do they match?

guerrerorodrigo commented 10 months ago

The tests don't look exactly the same. With matching intercept logic, do they match?

The three tests in https://github.com/guerrerorodrigo/paparazzicoil/tree/master/app/src/test/java/com/rodrigoguerrero/paparazzicoil are exactly the same. Same tests, testing the same composable. However, only the first screenshot is generated correctly.

yschimke commented 10 months ago

Double check. The intercept calls differ.

guerrerorodrigo commented 10 months ago

that makes no difference. now they are all the same, same result.

colinrtwhite commented 10 months ago

Hi folks I took a look at this and opened an issue on the Coil tracker to track. I'm not 100% sure if this is a Coil issue, a Paparazzi issue, or somewhere in-between. I added some more info in the ticket (including a work-around), but it possibly appears to be a timing issue.

ashley-figueira commented 10 months ago

I get the same result when using Parameterized Tests.

Here is an example:

@RunWith(TestParameterInjector::class)
@Category(ScreenshotTest::class)
class MyScreenshotTest(
    @TestParameter config: Config
) {

    enum class Config(
         val deviceConfig: DeviceConfig
    ) {
        NEXUS_4(deviceConfig = DeviceConfig.NEXUS_4),
        NEXUS_5(deviceConfig = DeviceConfig.NEXUS_5)
    }

   @get:Rule
   val paparazzi = Paparazzi(deviceConfig = config.deviceConfig)

    @Before
    fun setUp() {
        val engine = FakeImageLoaderEngine.Builder()
            .intercept("https://www.google.com/image.jpg", ColorDrawable(Color.RED))
            .default(ColorDrawable(Color.BLUE))
            .build()
        val imageLoader = ImageLoader.Builder(paparazzi.context)
            .components { add(engine) }
            .build()
        Coil.setImageLoader(imageLoader)
    }

    @After
    fun after() {
        Coil.reset()
    }

    @Test
    fun myScreenshotTest() {
        val imageUrl = "https://www.google.com/image.jpg"
        paparazzi.snapshot {
            MyImage(
                imageUrl = imageUrl,
                contentDescription = null
            )
        }
    }
}

This results in only the first screenshot overriding the image while the second does not.

kartik-prakash commented 7 months ago

Any updates here? This has become a huge blocker for us.

xavijimenezmulet commented 7 months ago

Any updates? This is a block for us as well. We found a workaround but for us it's not a good test. If you set a contentscale to None it works.

inktomi commented 5 months ago

This issue is very easy to reproduce with a basic coil test like this with the latest dependencies:

@RunWith(TestParameterInjector::class)
class CoilSnapshotTest {
    // rule setup, etc.. 
    @TestParameter
    val nightMode: Boolean = false

    @Before
    @OptIn(ExperimentalCoilApi::class)
    fun setup() {
        val engine = FakeImageLoaderEngine.Builder()
            .default(ColorDrawable(Color.MAGENTA))
            .build()

        val imageLoader = ImageLoader.Builder(rule.context)
            .components { add(engine) }
            .build()

        Coil.setImageLoader(imageLoader)
    }

    // Tests, only one will show the magenta image. 

}
mtzhisham commented 3 months ago

Was able to bypass this by setting the main dispatchers to Unconfined in the @Before method, more on why here #1087 (comment)

  @Before
  fun before() {
      Dispatchers.setMain(Dispatchers.Unconfined) // setting the dispatcher to Unconfined for tests
      val engine = FakeImageLoaderEngine.Builder()
          ..
richardortiz84 commented 3 months ago

Was able to bypass this by setting the main dispatchers to Unconfined in the @Before method, more on why here #1087 (comment)

  @Before
  fun before() {
      Dispatchers.setMain(Dispatchers.Unconfined) // setting the dispatcher to Unconfined for tests
      val engine = FakeImageLoaderEngine.Builder()
          ..

After implementing this fix we were able to get a lot of screenshot tests to work correctly. Shortly after we had a few edge cases came up where the fix did not work. It would seem this fix works for simple cases where the painter is used directly in an Image. For more complex cases where the setup relies on watching the AsyncImagePainter.State it does not seem to work.

For example:

AnimatedContent(
    painterState,
    label = "Image Load Transition",
) { state ->
    when (state) {
        State.Empty, is State.Loading -> ImageLoadingContent()
        is State.Success -> ImageSuccessContent()
        is State.Error -> ImageErrorContent()
    }
}
julioromano commented 2 months ago

I run into this too in a Junit.Parameterized test that uses Paparazzi to screenshot some views (old fashioned views, not using compose) that use Coil inside. Only the first screenshot of the series shows images loaded by Coil.

The Dispatchers.setMain(Dispatchers.Unconfined) workaround fixes it so far.

geoff-powell commented 3 weeks ago

Yep, looks like Coil has implemented a fix here. Thanks @colinrtwhite!