-
Notifications
You must be signed in to change notification settings - Fork 499
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 #3418: Separate DeveloperOptionsActivityTest into activity and fragment test files #3516
Conversation
…agment test files
Deferring to @Sarthak2601 to perform a first pass based on #3471. |
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.
Looks good overall, left some additional thoughts.
app/src/sharedTest/java/org/oppia/android/app/devoptions/DeveloperOptionsFragmentTest.kt
Show resolved
Hide resolved
app/src/sharedTest/java/org/oppia/android/app/devoptions/DeveloperOptionsFragmentTest.kt
Outdated
Show resolved
Hide resolved
Unassigning @Sarthak2601 since the review is done. |
Hi @yashraj-01, it looks like some changes were requested on this pull request by @Sarthak2601. PTAL. Thanks! |
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.
LGTM. Add tests as per @Sarthak2601 comments.
Assigning @BenHenning for code owner reviews. Thanks! |
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.
Left thoughts on existing review comments.
app/src/sharedTest/java/org/oppia/android/app/devoptions/DeveloperOptionsFragmentTest.kt
Outdated
Show resolved
Hide resolved
Will need to look at this tomorrow--I've got to load balance my current PR load. |
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.
LGTM, thanks!
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 @yashraj-01! LGTM.
exempted_activity: "app/src/main/java/org/oppia/android/app/devoptions/markchapterscompleted/testing/MarkChaptersCompletedTestActivity" | ||
exempted_activity: "app/src/main/java/org/oppia/android/app/devoptions/markstoriescompleted/testing/MarkStoriesCompletedTestActivity" |
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.
Shouldn't these be added in earlier PRs, or were they missed?
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.
They were missed actually.
Explanation
Fixes #3418. Separated DeveloperOptionsActivityTest into activity and fragment test files. This PR is based on the comments on the PR #3471.
Screenshots
Expresso test results for
DeveloperOptionsActivityTest
Expresso test results for
DeveloperOptionsFrgmentTest
Checklist