Improve code coverage metrics #3450
Conversation
|
📱 Scan the QR code below to install the build (arm64 only) for this PR. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3450 +/- ##
===========================================
- Coverage 82.67% 82.66% -0.01%
===========================================
Files 1731 1731
Lines 40841 40842 +1
Branches 4968 4968
===========================================
Hits 33764 33764
Misses 5315 5315
- Partials 1762 1763 +1 ☔ View full report in Codecov by Sentry. |
bmarty
force-pushed
the
feature/bma/improveCoverageMetrics
branch
from
8f9a76c
to
7fdd069
Compare
…ping ClientBuildException and add test for full coverage.
…d to debug the app.
And fix typo.
bmarty
force-pushed
the
feature/bma/improveCoverageMetrics
branch
from
560d1b7
to
e9a898f
Compare
|
| is RustClientException -> { | ||
| when (this) { | ||
| is RustClientException.Generic -> ClientException.Generic(msg) |
There was a problem hiding this comment.
Maybe use is RustClientException.Generic directly in the first branch?
There was a problem hiding this comment.
This is not equivalent, using a dedicated when ensures that all the RustClientException sub-classes are mapped.
| is ClientBuildException -> when (this) { | ||
| is ClientBuildException.Generic -> AuthenticationException.Generic(message) | ||
| is ClientBuildException.InvalidServerName -> AuthenticationException.InvalidServerName(message) | ||
| is ClientBuildException.SlidingSyncVersion -> AuthenticationException.SlidingSyncVersion(message) |
There was a problem hiding this comment.
I hope the Client builder retry wasn't affected by this! I think it uses the raw ClientBuildException.SlidingSyncVersion since it comes directly from the SDK.
There was a problem hiding this comment.
Yes, it should be OK.
But just to understand, there is no code behavior change here, this is just about ensuring that all the ClientBuildException sub classes are mapped, or am I missing something?
There was a problem hiding this comment.
No, it's fine. I double checked and we're using the exception from the SDK instead of the mapped one, so even if anything changed here it wouldn't matter.
|
|
||
| class RoomPreviewMapperTest { | ||
| @Test | ||
| fun `map should map values 1`() { |
There was a problem hiding this comment.
Kind of a generic test name 😅 .
There was a problem hiding this comment.
Yeah, I tried not to spend hours looking for perfect names for tests like this :)
bmarty
added
the
Run-Maestro
github-actions
bot
removed
the
Run-Maestro
This PR is adding more tests on the :matrix:impl module.
Let's merge this PR, it's big enough and add more tests later.