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.

Sign up for GitHub

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

Jump to bottom

Fix #3140 Shifting Instrumentation tests in Unit test directory for AdministratorControlsFragmentTest #4107

Merged
merged 9 commits into from
Jan 27, 2022
Merged

Fix #3140 Shifting Instrumentation tests in Unit test directory for AdministratorControlsFragmentTest #4107

merged 9 commits into from
Jan 27, 2022

Conversation

Rohit1173
Copy link
Contributor

@Rohit1173 Rohit1173 commented Jan 18, 2022 •
edited

Explanation

Fix #3140: Shifting tests which were instrumentation tests but written in the Unit test directory in the AdministratorControlsFragmentTest

Shifted tests Screenshot (Device(Emulator): Pixel-3A , API-28)
Screenshot 2022-01-25 125005

Essential Checklist

  • The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
  • Any changes to scripts/assets files have their rationale included in the PR explanation.
  • The PR follows the style guide.
  • The PR does not contain any unnecessary code changes from Android Studio (reference).
  • The PR is made from a branch that's not called "develop" and is up-to-date with "develop".
  • The PR is assigned to the appropriate reviewers (reference).

For UI-specific PRs only

If your PR includes UI-related changes, then:

  • Add screenshots for portrait/landscape for both a tablet & phone of the before & after UI changes
  • For the screenshots above, include both English and pseudo-localized (RTL) screenshots (see RTL guide)
  • Add a video showing the full UX flow with a screen reader enabled (see accessibility guide)
  • Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing

@Rohit1173
Copy link
Contributor Author

@yash10019coder PTAL

yash10019coder
yash10019coder suggested changes Jan 18, 2022
View reviewed changes
Copy link
Contributor

@yash10019coder yash10019coder left a comment

Choose a reason for hiding this comment

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

Hi @Rohit1173 check for the static checks and also add a newline at the end of the file thanks

...edTest/java/org/oppia/android/app/administratorcontrols/AdministratorControlsFragmentTest.kt Outdated

@Test
fun testAdministratorControlsFragment_clickEditProfile_checkSendingTheCorrectIntent() {
ActivityScenario.launch<AdministratorControlsActivity>(
Copy link
Contributor

Choose a reason for hiding this comment

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

Add resource inputs for these static object references

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

...edTest/java/org/oppia/android/app/administratorcontrols/AdministratorControlsFragmentTest.kt Outdated
)
).use {
testCoroutineDispatchers.runCurrent()
Espresso.onView(ViewMatchers.withId(R.id.edit_profiles_text_view))
Copy link
Contributor

Choose a reason for hiding this comment

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

Same goes here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@oppiabot
Copy link

oppiabot bot commented Jan 18, 2022

Unassigning @yash10019coder since the review is done.

@oppiabot
Copy link

oppiabot bot commented Jan 18, 2022

Hi @Rohit1173, it looks like some changes were requested on this pull request by @yash10019coder. PTAL. Thanks!

@yash10019coder
Copy link
Contributor

@Rohit1173 please fix failing ci checks and also I think that this PR is blocked by #4029 what do you think about this @rt4914 PTAL thanks

yash10019coder
yash10019coder suggested changes Jan 23, 2022
View reviewed changes
Copy link
Contributor

@yash10019coder yash10019coder left a comment

Choose a reason for hiding this comment

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

@Rohit1173 add resource imports and also fix failing test cases thanks

...edTest/java/org/oppia/android/app/administratorcontrols/AdministratorControlsFragmentTest.kt Outdated
)
).use {
testCoroutineDispatchers.runCurrent()
onView(ViewMatchers.withId(R.id.edit_profiles_text_view))
Copy link
Contributor

Choose a reason for hiding this comment

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

Add resource imports for ViewMatchers and click performers in this and the following test

@yash10019coder yash10019coder removed their assignment Jan 23, 2022
yash10019coder
yash10019coder approved these changes Jan 23, 2022
View reviewed changes
Copy link
Contributor

@yash10019coder yash10019coder left a comment

Choose a reason for hiding this comment

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

@Rohit1173 LGTM.

@Rohit1173
Copy link
Contributor Author

@rt4914 @anandwana001 PTAL

@oppiabot
Copy link

oppiabot bot commented Jan 23, 2022

Assigning @rt4914, @anandwana001 for code owner reviews. Thanks!

rt4914
rt4914 suggested changes Jan 25, 2022
View reviewed changes
Copy link
Contributor

@rt4914 rt4914 left a comment

Choose a reason for hiding this comment

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

@Rohit1173 PTAL thanks.

...edTest/java/org/oppia/android/app/administratorcontrols/AdministratorControlsFragmentTest.kt Outdated Show resolved Hide resolved
...edTest/java/org/oppia/android/app/administratorcontrols/AdministratorControlsFragmentTest.kt Outdated Show resolved Hide resolved
...edTest/java/org/oppia/android/app/administratorcontrols/AdministratorControlsFragmentTest.kt Outdated Show resolved Hide resolved
...edTest/java/org/oppia/android/app/administratorcontrols/AdministratorControlsFragmentTest.kt Outdated Show resolved Hide resolved
@rt4914 rt4914 removed their assignment Jan 25, 2022
anandwana001
anandwana001 approved these changes Jan 26, 2022
View reviewed changes
Copy link
Contributor

@anandwana001 anandwana001 left a comment

Choose a reason for hiding this comment

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

LGTM, left a nit suggestion.

...edTest/java/org/oppia/android/app/administratorcontrols/AdministratorControlsFragmentTest.kt
import javax.inject.Inject
import javax.inject.Singleton

@RunWith(AndroidJUnit4::class)
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a comment above this:

/** Tests for [AdministratorControlsFragment]. */

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@oppiabot
Copy link

oppiabot bot commented Jan 26, 2022

Unassigning @anandwana001 since they have already approved the PR.

@Rohit1173
Copy link
Contributor Author

@rt4914 changes done, PTAL.Thanks

@oppiabot oppiabot bot assigned rt4914 and unassigned Rohit1173 Jan 27, 2022
@oppiabot
Copy link

oppiabot bot commented Jan 27, 2022

Unassigning @Rohit1173 since a re-review was requested. @Rohit1173, please make sure you have addressed all review comments. Thanks!

rt4914
rt4914 approved these changes Jan 27, 2022
View reviewed changes
Copy link
Contributor

@rt4914 rt4914 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

@rt4914 rt4914 merged commit 75817ec into oppia:develop Jan 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rt4914 rt4914 rt4914 approved these changes

@anandwana001 anandwana001 anandwana001 approved these changes

@yash10019coder yash10019coder yash10019coder approved these changes

Assignees

@rt4914 rt4914

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Shift Instrumentation test written in Unit Test directory for AdministratorControlsFragmentTest
4 participants
@Rohit1173 @yash10019coder @rt4914 @anandwana001