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

Update BackgroundMatcher to compare GradientDrawables #384

Conversation

sarn0ld
Copy link
Contributor

@sarn0ld sarn0ld commented Mar 9, 2021

At the moment, when you use assertHasBackground() with a drawable like this:

<?xml version="1.0" encoding="utf-8"?>
<shape
    xmlns:android="http://schemas.android.com/apk/res/android"
    android:shape="oval">
    <solid android:color="@color/yellow" />
</shape>

then an exception is thrown because the drawable height and width are zero.

Instead of using DrawableToBitmapConverter a ShapeDrawable (=GradientDrawable) should be checked directly like a ColorDrawable.

@Sloy
Copy link
Member

Sloy commented Mar 9, 2021

Thank you for the fix @sarn0ld!
Could you add a test for this scenario to the suite? There are some existing tests for other kinds of backgrounds using assertHasBackground in the AssertionsTest file.

@rocboronat
Copy link
Member

Hello @sarn0ld !

Thanks a lot for the PR!

As Barista is becoming a project used by thousands, may you mind adding a test that reproduces the issue you describe?

Thanks a lot again! :·)

@sarn0ld
Copy link
Contributor Author

sarn0ld commented Mar 9, 2021

Hey @Sloy @rocboronat !
Sure! I added tests that are analoge to the ColorDrawable Tests. Thanks for the hint about the AssertionsTest file!

Please feel free to edit my changes if something is wrong or different than you'd expect it to be!
My lunch break is over so this is all I can contribute for now. :-)

@alorma
Copy link
Member

alorma commented Jul 1, 2021

Hi @sarn0ld

We updated the library package, so now this PR has some conflicts that needs to be fixed before merge.

We will solve the conflicts before merge.

@Sloy Sloy enabled auto-merge (squash) July 5, 2021 07:29
@Sloy Sloy merged commit 1b61a36 into AdevintaSpain:master Jul 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants