-
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 #3420: Separate MarkChaptersCompletedActivityTest into activity and fragment test files #3469
Fix #3420: Separate MarkChaptersCompletedActivityTest into activity and fragment test files #3469
Conversation
…ity and fragment test files
Hi! @yashraj-01 Welcome to Oppia! Could you please follow the instructions here and sign the CLA Sheet to get started? You'll need to do this before we can accept your PR. 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.
Ditto for this PR--same comments as #3471 apply here. PTAL.
Please add screenshot showing that Espresso Tests are passing for the files modified in this PR. Also once @BenHenning 's comments are resolved and fixed just ping me so that I can check it accordingly. |
I have updated the PR as per comments on #3471. PTAL. Thanks. |
Deferring to @Sarthak2601 for the main reviews per #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.
Thanks @yashraj-01. Left some thoughts regarding configChanges
.
app/src/sharedTest/java/org/oppia/android/app/devoptions/MarkChaptersCompletedFragmentTest.kt
Outdated
Show resolved
Hide resolved
app/src/sharedTest/java/org/oppia/android/app/devoptions/MarkChaptersCompletedFragmentTest.kt
Outdated
Show resolved
Hide resolved
app/src/sharedTest/java/org/oppia/android/app/devoptions/MarkChaptersCompletedFragmentTest.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.
@yashraj-01 PTAL.
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!
Assigning @rt4914 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.
LGTM
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.
Explanation
Fixes #3420. Separated MarkChaptersCompletedActivityTest into activity and fragment test files.
Screenshots
Expresso test results for
MarkChaptersCompletedActivityTest
Expresso test results for
MarkChaptersCompletedFrgmentTest
Checklist