agoda-com / Kakao

This repo is no longer supported. Please visit a https://github.com/KakaoCup/Kakao
Apache License 2.0
1.11k stars 102 forks source link

Issue with hasDrawable when ImageView is using scaleType #232

Closed lucas34 closed 3 years ago

lucas34 commented 4 years ago

Steps to reproduce:

  1. Use latest Kakao library
  2. Create ImageView and set android:scaleType="fitXY"
  3. Try to match the drawable using hasDrawable()

Observed Results:

Test fail. Drawable are different.

Expected Results:

Not sure if in this case the test should actually pass, because if you are using scaleType=crop the final image will indeed be different from the original image. Is there any solution to match this kind of image ?

Relevant Code:

I can provide test cases for it if required.

Vacxe commented 4 years ago

@lucas34 could u please confirm that issue represented only on API 28 and above. Please check my PR.

lucas34 commented 4 years ago

My test still faill on API 25 with same error.

com.agoda.sample.DrawableRecyclerTest > matchDrawablesInList[Pixel_4_XL_API_25(AVD) - 7.1.1] FAILED 
        androidx.test.espresso.base.DefaultFailureHandler$AssertionFailedWithCauseError: 'with drawable id 2131165292 or provided instance' doesn't match the selected view.
        Expected: with drawable id 2131165292 or provided instance
Unlimity commented 4 years ago

@lucas34 are you providing resource id to the hasDrawable() assertion or some custom bitmap? If the latter is correct, then you need to provide pixel perfect matching bitmap to the bitmap being output to the user in ImageView, otherwise it will not match. In case of drawable resource id all internal alterations should be repeated by Kakao before matching bitmaps.

lucas34 commented 4 years ago

@Unlimity I'm providing all the images with

hasDrawable(R.drawable.video_overlay)
Vacxe commented 4 years ago

@lucas34 I tried to replicate same issue on API 25 and looks like it works. But on 28+ it failed. Could u please represent issue with PR sample. It definitely help us to understand your case and move forward quickly.