Fix #3362 Use OppiaTestRule in all Espresso Tests #4179

vrajdesai78 · 2022-02-14T04:32:24Z

Explanation

Fix #3362 by fixing all to-dos which were mentioned by Github-Actions. I have removed AccessibilityChecks according to mentioned To-dos.

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).

Screenshots of passing testcases

Issues Filed for failing test cases

…ajdesai78/oppia-android into OppiaTestRule-in-all-Espresso

rt4914

@vrajdesai78 Three suggestions:

If you have removed DisableAccessibilityChecks then // TODO(#3362): Enable AccessibilityChecks is not required.
If you have added DisableAccessibilityChecks please add a comment+TODO mentioning why this is needed, maybe explain the type of error.
For all test files which are changed in this PR please add screenshot of them running on Espresso.

app/src/sharedTest/java/org/oppia/android/app/story/StoryActivityTest.kt

vrajdesai78 · 2022-02-14T11:35:48Z

What issue number should I add in TODO can I add #3245 as it is related to enabling Accessibility Checks or current issue number if I add DisableAccessbilityCheckes

@rt4914 PTAL.

vrajdesai78 · 2022-02-14T11:37:32Z

Passing Testcases after DisablingAccessbility Checks

oppiabot · 2022-02-14T11:38:52Z

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

rt4914

@vrajdesai78 I think the above suggestions are not fully applied yet.

There are many new files which uses OppiaTestRule in this PR and I am unable to see screenshots of espresso-test for all of them.
In PinPasswordActivityTest uses DisableAccessibilityChecks and there are no comments for it.
If there is any ...TestActivityTest like InputInteractionViewTestActivityTest . In this along with DisableAccessibilityChecks please add one comment (not TODO) mentioning Disabled as this is a test file and will not be used by user. (We might update this comment later but for now having a comment is important)

app/src/sharedTest/java/org/oppia/android/app/story/StoryFragmentTest.kt

rt4914 · 2022-02-14T13:24:55Z

app/src/sharedTest/java/org/oppia/android/app/databinding/ImageViewBindingAdaptersTest.kt

@@ -98,6 +99,9 @@ class ImageViewBindingAdaptersTest {
  @get:Rule
  val initializeDefaultLocaleRule = InitializeDefaultLocaleRule()

+  @get:Rule
+  val oppiaTestRule = OppiaTestRule()


Please add screenshot of espresso tests passing for this file too.

Actually, there is an issue with this Test. I have already created an issue for same #4149

vrajdesai78 · 2022-02-14T14:34:01Z

@vrajdesai78 I think the above suggestions are not fully applied yet.

There are many new files which uses OppiaTestRule in this PR and I am unable to see screenshots of espresso-test for all of them.

In PinPasswordActivityTest uses DisableAccessibilityChecks and there are no comments for it.

If there is any ...TestActivityTest like InputInteractionViewTestActivityTest . In this along with DisableAccessibilityChecks please add one comment (not TODO) mentioning Disabled as this is a test file and will not be used by user. (We might update this comment later but for now having a comment is important)

Actually, I have added all the screenshots in #4133.

vrajdesai78 · 2022-02-14T18:18:07Z

@rt4914 PTAL.

…ajdesai78/oppia-android into OppiaTestRule-in-all-Espresso

rt4914

@vrajdesai78 Suggested changes.

app/src/sharedTest/java/org/oppia/android/app/story/StoryFragmentTest.kt

rt4914 · 2022-03-01T19:10:34Z

app/src/sharedTest/java/org/oppia/android/app/story/StoryFragmentTest.kt

@@ -613,7 +616,9 @@ class StoryFragmentTest {
  }

  @Test
-  @DisableAccessibilityChecks // TODO(#3362): Enable AccessibilityChecks
+  // TODO(#3245): Error -> View falls below the minimum recommended size for touch targets
+  // TODO(#3245): Error -> URLSpan should be used in place of ClickableSpan


Please mention it as a single TODO with both the errors.

// TODO(#3245): Use URLSpan and minimum touch target size should be 48x48 dp

rt4914 · 2022-03-01T19:11:00Z

app/src/sharedTest/java/org/oppia/android/app/story/StoryFragmentTest.kt

@@ -668,6 +673,8 @@ class StoryFragmentTest {
  }

  @Config(qualifiers = "+sw600dp")
+  // TODO(#4212): Error -> No views in hierarchy found matching: with
+  //  id: org.oppia.android:id/story_chapter_list


Reduce to single line and don't mention ids. Just the error info is enough.

rt4914 · 2022-03-01T19:11:06Z

app/src/sharedTest/java/org/oppia/android/app/story/StoryFragmentTest.kt

@@ -684,6 +691,8 @@ class StoryFragmentTest {
  }

  @Config(qualifiers = "+sw600dp")
+  // TODO(#4212): Error -> No views in hierarchy found matching: with
+  //  id: org.oppia.android:id/story_chapter_list


rt4914 · 2022-03-01T19:11:11Z

app/src/sharedTest/java/org/oppia/android/app/story/StoryFragmentTest.kt

@@ -699,6 +708,8 @@ class StoryFragmentTest {
  }

  @Config(qualifiers = "+sw600dp")
+  // TODO(#4212): Error -> No views in hierarchy found matching: with
+  //  id: org.oppia.android:id/story_chapter_list


rt4914

@vrajdesai78 LGTM. Suggested one minor change. When writing such code please focuse on consistency.

app/src/sharedTest/java/org/oppia/android/app/story/StoryFragmentTest.kt

oppiabot · 2022-03-02T06:28:28Z

Assigning @BenHenning for code owner reviews. Thanks!

anandwana001

LGTM,

left a nit suggestion.

app/src/sharedTest/java/org/oppia/android/app/testing/InputInteractionViewTestActivityTest.kt

BenHenning · 2022-03-04T05:22:10Z

It appears my codeowners is no longer needed. I think @rt4914 just needs to verify the remaining changes before this can be merged.

vrajdesai78 added 7 commits January 25, 2022 15:56

OppiatestRule added in all espresso tests

c39dcfe

DisableAccessibilityChecks Addedi in PinPasswordActivityTest

15f8178

DisableAccessibilityChecks added

701f77e

Klint issue fixed

7d86c61

Imports optimized

42235a7

Merge branch 'OppiaTestRule-in-all-Espresso' of https://github.com/vr…

6a8c636

…ajdesai78/oppia-android into OppiaTestRule-in-all-Espresso

Fixed todos to remove AccessibilityChecks

53b4408

vrajdesai78 requested review from anandwana001, BenHenning and rt4914 as code owners February 14, 2022 04:32

klint issue fixed

ad58a1c

rt4914 suggested changes Feb 14, 2022

View reviewed changes

app/src/sharedTest/java/org/oppia/android/app/story/StoryActivityTest.kt Outdated Show resolved Hide resolved

rt4914 assigned vrajdesai78 Feb 14, 2022

Disabled AccessibilityChecks with TODOs

3909431

oppiabot bot assigned rt4914 and unassigned vrajdesai78 Feb 14, 2022

klint issue fixed

4028f21

rt4914 suggested changes Feb 14, 2022

View reviewed changes

rt4914 assigned vrajdesai78 and unassigned rt4914 Feb 14, 2022

Write Error in proper way

c5df528

vrajdesai78 and others added 3 commits February 14, 2022 20:15

Added comments on DisableAccessbilityChecks

35999e0

Merge branch 'develop' into OppiaTestRule-in-all-Espresso

18cf2cb

Klint issue fixed and more comments added

5539606

oppiabot bot assigned rt4914 Feb 14, 2022

Added TODOs

1279488

vrajdesai78 assigned rt4914 Feb 21, 2022

vrajdesai78 requested a review from rt4914 February 21, 2022 18:36

vrajdesai78 and others added 3 commits February 24, 2022 19:13

Merge branch 'develop' into OppiaTestRule-in-all-Espresso

7eba2f1

Resolved conflicts

804db15

Merge branch 'OppiaTestRule-in-all-Espresso' of https://github.com/vr…

16c582f

…ajdesai78/oppia-android into OppiaTestRule-in-all-Espresso

vrajdesai78 assigned anandwana001 and unassigned rt4914 Feb 24, 2022

rt4914 assigned rt4914 and unassigned vrajdesai78 Mar 1, 2022

rt4914 suggested changes Mar 1, 2022

View reviewed changes

rt4914 assigned vrajdesai78 and unassigned rt4914 Mar 1, 2022

vrajdesai78 and others added 2 commits March 2, 2022 01:38

Merge branch 'develop' into OppiaTestRule-in-all-Espresso

aac1a7c

Updated TODOs

eda6d8d

vrajdesai78 assigned rt4914 Mar 1, 2022

vrajdesai78 requested a review from rt4914 March 1, 2022 20:21

rt4914 approved these changes Mar 2, 2022

View reviewed changes

app/src/sharedTest/java/org/oppia/android/app/story/StoryFragmentTest.kt Outdated Show resolved Hide resolved

rt4914 removed their assignment Mar 2, 2022

oppiabot bot assigned BenHenning Mar 2, 2022

anandwana001 approved these changes Mar 2, 2022

View reviewed changes

app/src/sharedTest/java/org/oppia/android/app/testing/InputInteractionViewTestActivityTest.kt Outdated Show resolved Hide resolved

anandwana001 removed their assignment Mar 2, 2022

vrajdesai78 added 3 commits March 2, 2022 14:54

nit changes

ba53d60

klint issue fixed

085c3a0

Removed unnecessary change

83c18b7

BenHenning removed their assignment Mar 4, 2022

BenHenning assigned rt4914 Mar 4, 2022

rt4914 merged commit 0d61dfc into oppia:develop Mar 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #3362 Use OppiaTestRule in all Espresso Tests #4179

Fix #3362 Use OppiaTestRule in all Espresso Tests #4179

vrajdesai78 commented Feb 14, 2022 •

edited

rt4914 left a comment

vrajdesai78 commented Feb 14, 2022

vrajdesai78 commented Feb 14, 2022

oppiabot bot commented Feb 14, 2022

rt4914 left a comment

rt4914 Feb 14, 2022

vrajdesai78 Feb 14, 2022

vrajdesai78 commented Feb 14, 2022 •

edited

vrajdesai78 commented Feb 14, 2022

rt4914 left a comment

rt4914 Mar 1, 2022

rt4914 Mar 1, 2022

rt4914 Mar 1, 2022

rt4914 Mar 1, 2022

rt4914 left a comment

oppiabot bot commented Mar 2, 2022

anandwana001 left a comment

BenHenning commented Mar 4, 2022

Fix #3362 Use OppiaTestRule in all Espresso Tests #4179

Fix #3362 Use OppiaTestRule in all Espresso Tests #4179

Conversation

vrajdesai78 commented Feb 14, 2022 • edited

Explanation

Essential Checklist

Screenshots of passing testcases

Issues Filed for failing test cases

rt4914 left a comment

Choose a reason for hiding this comment

vrajdesai78 commented Feb 14, 2022

vrajdesai78 commented Feb 14, 2022

oppiabot bot commented Feb 14, 2022

rt4914 left a comment

Choose a reason for hiding this comment

rt4914 Feb 14, 2022

Choose a reason for hiding this comment

vrajdesai78 Feb 14, 2022

Choose a reason for hiding this comment

vrajdesai78 commented Feb 14, 2022 • edited

vrajdesai78 commented Feb 14, 2022

rt4914 left a comment

Choose a reason for hiding this comment

rt4914 Mar 1, 2022

Choose a reason for hiding this comment

rt4914 Mar 1, 2022

Choose a reason for hiding this comment

rt4914 Mar 1, 2022

Choose a reason for hiding this comment

rt4914 Mar 1, 2022

Choose a reason for hiding this comment

rt4914 left a comment

Choose a reason for hiding this comment

oppiabot bot commented Mar 2, 2022

anandwana001 left a comment

Choose a reason for hiding this comment

BenHenning commented Mar 4, 2022

vrajdesai78 commented Feb 14, 2022 •

edited

vrajdesai78 commented Feb 14, 2022 •

edited