Fix/alert/UI tests: Fix and complete UI tests for AlertScreen #68
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added `com.linkedin.dexmaker:dexmaker-mockito-inline` dependency to enable Mockito inline for androidTests
Mocked `NavigationActions` using the correct dependencies and passed it as argument to `AlertScreen` instead of placeholder `rememberNavController`. Added a call to `Mockito.verify` to test for the correct navigation on click.
Added tests to check that an invalid alert is not submitted: * createInvalidAlertNoProduct * createInvalidAlertNoUrgencyLevel * createInvalidAlertNoLocation
Checked for a location before submitting the alert. Checked for product and urgency level before submitting: I used states (`productIsSelected` and `urgencyIsSelected`) and their setting function (`setProductIsSelected` and `setUrgencyIsSelected`). Clicking on the submission button with an invalid alert now shows a `Toast` with corresponding error message and does not navigate anywhere.
2 tasks
Had removed it for testing purposed.
francelu
requested changes
Oct 28, 2024
app/src/androidTest/java/com/android/periodpals/ui/alert/AlertScreenTest.kt
Show resolved
Hide resolved
agonzalez-r
approved these changes
Oct 28, 2024
There was a problem hiding this comment.
Summary
Great work! This PR effectively fixes the dependencies issues in order for Mockito to work on Android Tests, and thoroughly tests AlertScreens.
Important
Code Quality
AlertScreenTest: check comment- (
build.gradle.kts: this whole file should be cleaned (lots of commented / repeated code), but not the scope of this PR)
Suggestions
- Future improvements: when data class
Alertis implemented, remove the two variablesproductIsSelectedandurgencyIsSelectedand check if the fields are non-empty instead
app/src/androidTest/java/com/android/periodpals/ui/alert/AlertScreenTest.kt
Show resolved
Hide resolved
|
Thanks for your review @agonzalez-r ! I'll answer to some things that you pointed out quickly:
Can you create an issue about this? We'll get to it when we have time.
I don't believe this is relevant as checking basic properties about field content should be done at the View level and not by the ViewModel or Model because of performance issues (see this Ed post) |
Extracted `composeTestRule.setContent { ... }` to the `@Before setUp` function to avoid code duplication.
This only works if the `setContent` call is the last thing done in `setUp`.
Added checks for the presence of the TopAppBar and BottomNavigationBar, as well as a check to see that the goBack button is not present.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix and complete UI tests for AlertScreen
Description
This PR introduces new tests for the Alert screen and fixes the old ones. It also fixes the problems pointed out by said new tests. It closes issue #64 and fixes part of #63 .
Changes
Added dependencies to make mockito work in androidTests, and properly mocked
NavigationActions. Added verification of correct navigation in the old tests and created new tests to check that nothing happens when trying to submit an invalid Alert.Invalid alerts are ones which don't have a period product, urgency level and location selected.
Files
Added
None
Modified
app/build.gradle.ktsandgradle/libs.versions.toml: add the missing dependencyapp/src/androidTest/java/com/android/periodpals/ui/alert/AlertScreenTest.kt: modified and added testsapp/src/main/java/com/android/periodpals/ui/alert/AlertScreen.kt: fixed problems pointed out by new testsRemoved
None
Dependencies Added
com.linkedin.dexmaker:dexmaker-mockito-inlineto make mockito work with androidTests. Had to removelibs.mockito.androidfor androidTests as a consequenceTesting
All UI tests for
AlertScreenpass. They should be complete enough.