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

Add first tests on compose click interaction. #2175

Merged
merged 6 commits into from
Jan 8, 2024

Conversation

bmarty
Copy link
Member

@bmarty bmarty commented Jan 5, 2024

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other : more test

Content

Add compose test to check eventSink and callback behavior. Should increase code coverage.
This PR introduce some utility classes, but there is lots of work to do to add tests on the project. This first PR add test on LogoutView as a POC.

Motivation and context

#813

Screenshots / GIFs

Tests

  • No change on the app to test

Tested devices

  • NA

Checklist

@bmarty bmarty requested a review from a team as a code owner January 5, 2024 17:03
@bmarty bmarty requested review from jmartinesp and removed request for a team January 5, 2024 17:03
)
}
rule.onNode(hasContentDescription("Back")).performClick()
callback.assertSuccess()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to find a way to avoid having to write this line. Because if the developer forget it, the test will always pass.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe create a wrapper, like roboTest { ... } that internally always calls this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added fun run in 2c667a0, maybe not the best solution, but works for now I guess. But I'll think about it again.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Improved a bit in 2a6a3de

Copy link
Contributor

github-actions bot commented Jan 5, 2024

📱 Scan the QR code below to install the build (arm64 only) for this PR.
QR code
If you can't scan the QR code you can install the build via this link: https://i.diawi.com/wz9RJC

@bmarty
Copy link
Member Author

bmarty commented Jan 8, 2024

I do not see any test coverage report in this PR :/

Comment on lines +21 to +23
class EventsRecorder<T>(
private val expectEvents: Boolean = true
) : (T) -> Unit {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clever!

onSuccessLogout = EnsureNeverCalledWithParam(),
)
}
rule.clickOn("Sign out")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to use resources instead? It seems safer.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I forgot this part. Thanks!

@jmartinesp
Copy link
Member

I do not see any test coverage report in this PR :/

I re-run the test job just in case, but if it doesn't report any coverage we'll have to investigate this a bit further.

Other than that, great job! I wasn't sure if this was actually feasible, and it's great to see it's not that much work and you found an elegant way to handle the success/failure problem in these special tests.

Copy link

codecov bot commented Jan 8, 2024

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (3a2191e) 66.98% compared to head (2a6a3de) 67.02%.
Report is 25 commits behind head on develop.

Files Patch % Lines
...ndroid/features/logout/impl/LogoutStateProvider.kt 50.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2175      +/-   ##
===========================================
+ Coverage    66.98%   67.02%   +0.03%     
===========================================
  Files         1376     1376              
  Lines        34201    34202       +1     
  Branches      7445     7445              
===========================================
+ Hits         22911    22923      +12     
+ Misses        7672     7661      -11     
  Partials      3618     3618              

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

Copy link
Member

@jmartinesp jmartinesp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The coverage was reported now, so I'm approving it, thanks! Could you take a look at my previous comments, in case we can do those improvements?

@bmarty
Copy link
Member Author

bmarty commented Jan 8, 2024

Some details about the code coverage change:

image

Copy link

sonarcloud bot commented Jan 8, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@bmarty bmarty merged commit b2cff05 into develop Jan 8, 2024
14 of 15 checks passed
@bmarty bmarty deleted the feature/bma/composeTest branch January 8, 2024 10:43
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