Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Screenshots not showing Coil images correctly #1123

Closed
guerrerorodrigo opened this issue Oct 3, 2023 · 15 comments
Closed

Screenshots not showing Coil images correctly #1123

guerrerorodrigo opened this issue Oct 3, 2023 · 15 comments
Labels
bug Something isn't working

Comments

@guerrerorodrigo
Copy link

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:

  • Paparazzi Version: 1.3.1
  • OS: API 33
  • Compile SDK: 33 and 34
  • Gradle Version: 8.0, also had the issue with 8.3
  • Android Gradle Plugin Version: 8.1.1 and 8.1.2
@guerrerorodrigo guerrerorodrigo added the bug Something isn't working label Oct 3, 2023
@mrmike
Copy link

mrmike commented Oct 11, 2023

I think this might be related to #1087 (comment)

@guerrerorodrigo
Copy link
Author

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

@yschimke
Copy link
Contributor

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

@guerrerorodrigo
Copy link
Author

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
Copy link
Contributor

Double check. The intercept calls differ.

@guerrerorodrigo
Copy link
Author

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

@colinrtwhite
Copy link
Member

colinrtwhite commented Oct 30, 2023

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
Copy link

ashley-figueira commented Nov 9, 2023

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
Copy link

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

@xavijimenezmulet
Copy link

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
Copy link

inktomi commented Mar 20, 2024

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
Copy link

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
Copy link

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
Copy link

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
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests