Improve coverage in permalinks package

[jmartinesp]

Type of change

• Feature
• Bugfix
• Technical
• Other : Improve coverage in io.element.android.libraries.matrix.api.permalink (it was 9% before).

Content

Added some tests to check our handling of permalinks is correct, and also improve coverage.

Motivation and context

Improving reliability, bumping coverage.

Checklist

• Changes have been tested on an Android device or Android emulator with API 23
• UI change has been tested on both light and dark themes
• Accessibility has been taken into account. See https://github.com/vector-im/element-x-android/blob/develop/CONTRIBUTING.md#accessibility
• Pull request is based on the develop branch
• Pull request includes a new file under ./changelog.d. See https://github.com/vector-im/element-x-android/blob/develop/CONTRIBUTING.md#changelog
• Pull request includes screenshots or videos if containing UI changes
• Pull request includes a sign off
• You've made a self review of your PR

[github-actions]

📱 Scan the QR code below to install the build (arm64 only) for this PR.

If you can't scan the QR code you can install the build via this link: https://i.diawi.com/vSDhRz

[codecov]

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (5d49196) 58.39% compared to head (28ac632) 58.71%.
Report is 2 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1502      +/-   ##
===========================================
+ Coverage    58.39%   58.71%   +0.32%     
===========================================
  Files         1133     1133              
  Lines        30298    30298              
  Branches      6214     6214              
===========================================
+ Hits         17692    17789      +97     
+ Misses        9951     9838     -113     
- Partials      2655     2671      +16     

see 6 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[julioromano]

LGTM!

[julioromano]

This is what I call "API renaming", I used to do quite a good amount of this in another codebase and then started wondering if I really should:

• On one side I like it because it make things more idiomatic.
• On the other it makes the code less readable to new comers (every Android dev knows what BuildConfig.DEBUG is but nobody knows what isInDebug() so it could potentially do anything).

[jmartinesp]

I just did this because BuildConfig.DEBUG didn't seem to be accessible in unit tests. Maybe by generating resources it would work?

[julioromano]

BuildConfig generation is optional since AGP8, maybe that's the issue?
But even better question, why do you need BuildConfig in a unit test, what are you trying to do?

[jmartinesp]

No, the BuildConfig generation is already enabled for that module. As for why I need this, it's because UserId("foo") will crash in debug mode so we can track invalid ids but will work fine in release mode. Since we have some code that needs to check for invalid ids in release mode in these classes we need to take into account both cases.

Another option would be to remove this check.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information