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

Use Shadows for permission mocking instead of mocks #36

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ArnasSmicius
Copy link
Contributor

I'm trying to simplify all the mocks in the tests. I use Shadows to replace our created mocks in CoperTestActivity. The only issue is that I can't find how to replace PermissionChecker.PERMISSION_DENIED_APP_OP with shadows. This result can be returned on devices whose SDK level is below 23. On those devices, there are no runtime permissions. They always get PERMISSION_GRANTED or PERMISSION_DENIED_APP_OP if the user disables permission through app settings. But still this constant means that permission is denied. Sadly, as I debug, Shadows doesn't handle this thing under the hood. It still returns PERMISSION_DENIED on SDK 21. So, https://github.com/vinted/coper/blob/master/library/src/main/java/com/vinted/coper/CoperFragment.kt#L240-L241 this place is not tested that well in this case. But I'm not sure if it's worth writing our own mocks just to test this scenario. What do you think?

@ArnasSmicius ArnasSmicius requested a review from a team September 15, 2023 11:46
@neworld
Copy link
Contributor

neworld commented Sep 15, 2023

This result can be returned on devices whose SDK level is below 23. On those devices, there are no runtime permissions. They always get PERMISSION_GRANTED or PERMISSION_DENIED_APP_OP if the user disables permission through app settings.

Nowadays it is safe to assume that most of app development happens in >23 minSdk. So I recommend just bump minSdk. It is not very good idea to release not well tested functionality anyways.

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.

2 participants