search

Shopify / android-testify

Add screenshots to your Android tests
https://testify.dev
MIT License
231 stars 23 forks source link

Screenshot is taken before GlideImage drawable is loaded #264

Closed jennatauro closed 2 years ago

jennatauro commented 2 years ago

Describe the bug I'm not sure that this is a bug, but am wondering if there is recommended way for the screenshot to have to wait to be taken until after an image is loaded.

This issue relates to:

To Reproduce I have previously been simply loading a drawable into a compose Image like this:

Image(
                painter = painterResource(id = imageModel.drawableResId),
                contentDescription = imageModel.contentDescription,
                contentScale = ContentScale.Fit,
                modifier = Modifier
                    .clip(CircleShape)
                    .size(dimensionResource(id = avatarSize.diameter))
            )

Running screenshot tests with this worked and gave the result on the left in the screenshots. But we want to be able to switch over to use GlideImage to allow the image to be either loaded from a url OR a drawable like this (specifically, we are using this library https://github.com/skydoves/landscapist which uses https://github.com/bumptech/glide):

GlideImage(
            imageModel = avatarOverlayBadge.image.imageModel, // imageModel could be of type drawable or a String url
            ...
        )

This works in the app as expected and loads the image, however on the screenshot tests it seems now that the screenshot is taken before the image is loaded and gives the result on the right most of the time. We did not change anything in the screenshot test and are still using a drawable, it just seems the library takes slightly longer to show the drawable than directly using a Compose Image.

I attempted to fix this using a CountingIdlingResource with something like:

val idlingResource = CountingIdlingResource("AvatarImageLoadCompletionResource", true).apply {
            increment()
}
IdlingRegistry.getInstance().register(idlingResource)

and then in the implementation code I decrement the idling resource on success of the image loading. This seemed to make the test work a rare amount of times.

Expected behavior

Screenshots

Screen Shot 2022-03-02 at 6 08 44 PM

Additional context Any help would be very much appreciated thank you!!

DanielJette commented 2 years ago

Hi @jennatauro As image loading libraries like Coil, Picasso or Glide typically load the bitmaps asynchronously in the background, race conditions with Testify are common. For this reason, Testify was designed to synchronize with the Espresso idling resources.

I believe you've got the right idea with trying to integrate an IdlingResource with your image loading.

The trick will be to ensure you register, go idle, and unregister your resource at precisely the right time.

One way I've found to see if you're on the right track is to set up your idling resource such that you do not emit true from isIdleNow (or similarly do not call decrement() on your CountingIdlingResource). If you do this, Testify should pause and timeout waiting for Espresso to go idle. The logs should also confirm that your resource was the one that did not go idle. This will indicate if you are correctly registering the idling resource at the right time. Then, you can work on adding the decrement()/isIdleNow() logic to the right place. Most likely this will be by hooking into a RequestListener.onResourceReady.

Another point that I have found to be problematic is that you must ensure that the key you use to register the idling resource is unique for each instance. If you have multiple images on screen, you'll want to use something like the hashCode() of your object as a key.

Finally, if you're so inclined, feel free to open a PR where you integrate Glide into the Sample application. I'd be happy to try to set up something directly in Testify to support your usecase. I feel like adding support for common image loading libraries in a companion library would be a very good idea.

jennatauro commented 2 years ago

Hi @DanielJette would you be able to take a look at https://github.com/Shopify/android-testify/pull/265 ?

I've added a draft pull request that uses the same sort of logic we're using in our app and I'm still not able to get the image to show up on the screenshot.

I tried following your advice and if I remove the decrement() the screenshot test does timeout waiting for Espresso. However, calling decrement stops the test from timing out but the screenshot still doesn't include the image loaded. I'm wondering if the timing is still wrong with where the CountingIdlingResource is being registered versus when the screenshot is taken?

Any help would be greatly appreciated!

DanielJette commented 2 years ago

@jennatauro Thanks for providing that Sample. I'll take a look and see if I can offer any better solutions.

DanielJette commented 2 years ago

@jennatauro I've opened a PR using a synchronization mechanism that seems to be working for me https://github.com/Shopify/android-testify/pull/266

The main change that I have made is to couple the idling resource directly to the GlideImage in the app itself. I create, increment and register the idling resource when the requestListener is instantiated and decrement it and unregister it in onResourceReady.

jennatauro commented 2 years ago

@DanielJette Thanks for looking into this. I just took a look at your branch, I tried using the same approach and it still isn't working 100% of the time.

It's strange, on my implementation it does work very rarely (approximately 1/10 times). I tried running the image test on your branch many times and for me at least, it fails (still shows a blank screenshot) about 4/10 times. If you run your test locally multiple times, does it pass every time?

DanielJette commented 2 years ago

Yes, @jennatauro I ran the test many times both alone and as part of the entire class of tests and it passed (rendered the avatar) each time.

I'm not sure why this would continue to be flaky. Perhaps my changes simply added enough delay to allow the image to load in my configuration, on my machine.

I wonder if the landscapist callback is invoked prior to the composition actually rendering the results. As in, the callback is telling Testify that the image resource is loaded, not necessarily that the image is rendered. Perhaps adding an AndroidComposeTestRule and a call to composeTestRule.waitForIdle() would help.

DanielJette commented 2 years ago

I was thinking about this a bit more. Is it possible that Glide uses a default animation/transition between the empty state and the loaded state? This seems to imply that there is some kind of built-in transition. I'm using the emulator configuration as specified in the RECIPES.md which has animations disabled. Perhaps that's contributing to my being able to consistently run the test without errors.

DanielJette commented 2 years ago

I ran the image test 50 times in a loop and it passed every time

~/DevSource/android-testify: for i in {1..50}; do adb shell am instrument -w -e class 'com.shopify.testify.sample.compose.ComposableScreenshotTest#image' com.shopify.testify.sample.test/androidx.test.runner.AndroidJUnitRunner; done

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.074
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.465
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 1.971
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.324
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.212
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 1.987
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 1.966
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.191
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 1.948
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.257
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.205
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.019
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.297
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.215
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.234
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 1.978
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.434
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.006
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.245
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.277
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.231
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 1.969
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.265
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.243
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.237
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.263
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 1.968
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.457
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.288
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 1.981
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 1.969
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.302
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.221
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.291
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.207
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.269
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.484
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.266
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.32
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.237
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.284
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 1.979
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 1.986
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.004
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 1.964
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.679
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.234
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.004
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 1.975
OK (1 test)

com.shopify.testify.sample.compose.ComposableScreenshotTest:.
Time: 2.27
OK (1 test)
DanielJette commented 2 years ago

I also experimented with using a ComposeTestRule.waitForIdle() here https://github.com/Shopify/android-testify/pull/266/commits/67efd1f45bd5e69a55c7ed48993edbaaaa259033

jennatauro commented 2 years ago

@DanielJette thank you for continuing to look into this. From my early testing so far it seems like adding the changes you added in https://github.com/Shopify/android-testify/commit/67efd1f45bd5e69a55c7ed48993edbaaaa259033 is seeming to make this work for me! I will update here if there's anything else useful that I come up with.

DanielJette commented 2 years ago

That's good to hear @jennatauro. Please let me know if that works out for you as I'd like to incorporate this kind of support directly into Testify

DanielJette commented 2 years ago

Moved to https://github.com/ndtp/android-testify/issues/93