-
Notifications
You must be signed in to change notification settings - Fork 2
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
fix/authentication/signup-ui-tests : Fix UI tests for M1: SignUp #96
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary
These tests are very complete, good job! Small changes here and there, check the comments!
Important
Code Quality
Very well formatted code, very consistent as well. I like that you extracted email and password variations in a companion object, it helps with consistency and error checking.
Functionality
I think the tests check for the proper things, and correctly. Be careful of edge cases though, see comments.
Testing
The tests pass and are very complete! Great job:)
Documentation
Code is self-documenting thanks to the function and variable clear nomenclature!
Suggestions
See comments
Steps before PR approved
See comments. Any comment that is not annotated with "nitpick" has to be resolved before I approve this PR.
app/src/androidTest/java/com/android/periodpals/ui/authentication/SignUpTest.kt
Outdated
Show resolved
Hide resolved
app/src/androidTest/java/com/android/periodpals/ui/authentication/SignUpTest.kt
Outdated
Show resolved
Hide resolved
app/src/androidTest/java/com/android/periodpals/ui/authentication/SignUpTest.kt
Show resolved
Hide resolved
app/src/androidTest/java/com/android/periodpals/ui/authentication/SignUpTest.kt
Outdated
Show resolved
Hide resolved
Replaced ``when`(navigationActions.currentRoute()).thenReturn(Route.ALERT_LIST) ` with correct return screen (`Screen.SIGN_UP`). Replaced "User is logged up" by "signed up". Completed empty email tests and others with the missing "signUpConfirmPassword". Added a test for the edge case where only the confirm-password and only the password field is empty. Suppress the useless TODO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary
LGTM.
Important
Code Quality
- The separation of tests for "doesNotCallVM" seems redundant but maintains consistency with
SignInScreen
.
Functionality
- The tests correctly validate expected behaviors.
Steps before PR approved
- None, keep up the good work!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for applying the changes! All tests pass and seem correct:)
LGTM
Fix UI tests for M1: SignUp
Description
This PR introduces UI tests for the SignUpScreen component in the authentication module. It validates various user input scenarios to ensure proper error handling and navigation flow. It closes issue #76.
Changes
I added more tests, including tests for the calls to the ViewModel. Now the navigation tests also work.
Files
Modified
SignUpScreenTest.kt