Fix #2536: Shift RecyclerViewMatcher to Central testing utility #3030

Sparsh1212 · 2021-03-30T21:47:24Z

Explanation

Fixes #2536: Shift RecyclerViewMatcher to Central testing utility

Checklist

The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
The PR explanation includes the words "Fixes #bugnum: ..." (or "Fixes part of #bugnum" if the PR only partially fixes an issue).
The PR follows the style guide.
The PR does not contain any unnecessary auto-generated code from Android Studio.
The PR is made from a branch that's not called "develop".
The PR is made from a branch that is up-to-date with "develop".
The PR's branch is based on "develop" and not on any other branch.
The PR is assigned to an appropriate reviewer in both the Assignees and the Reviewers sections.

Sparsh1212 · 2021-03-30T21:47:46Z

anandwana001

Seems like we have few failure, I think we face the similar failure in the last closed PR. Could you take a look and fix it.

anandwana001 · 2021-04-06T09:51:17Z

@Sparsh1212 any update on this?

Sparsh1212 · 2021-04-06T09:55:09Z

Will resolve the conflicts and update it by today itself.

Sparsh1212 · 2021-04-06T19:58:46Z

@anandwana001 I have resolved the merge conflicts, and also updated the PR. But some checks still fail, can you please take a look why they are still failing.

anandwana001 · 2021-04-07T03:30:01Z

Trying to re-run the actions.

anandwana001

@Sparsh1212 So far looks good to me.
Left a nit update.

Also, mark the checklist which are completed

testing/src/main/java/org/oppia/android/testing/RecyclerViewMatcher.kt

anandwana001

LGTM

rt4914

LGTM. Thanks @Sparsh1212

BenHenning · 2021-04-09T02:42:20Z

Apologies--will need to look at this tomorrow.

BenHenning

Thanks @Sparsh1212! Leaving some initial thoughts.

testing/src/main/java/org/oppia/android/testing/RecyclerViewMatcher.kt

BenHenning · 2021-04-09T20:59:21Z

testing/src/main/java/org/oppia/android/testing/RecyclerViewMatcher.kt

+     * within the item inside RecyclerView from a specified position.
+     */
+    fun verifyItemDisplayedOnRecyclerView(
+      recyclerView: Int,


This & targetView should be annotated with ViewRes.

I couldn't import this annotation. Can you please guide me here.

Ah, I should have instead said IdRes. See reference:

oppia-android/app/src/sharedTest/java/org/oppia/android/app/parser/HtmlParserTest.kt

Line 515 in 1e4c24c

@IdRes viewResId: Int

Could targetView also be annotated with IdRes?

testing/src/main/java/org/oppia/android/testing/RecyclerViewMatcher.kt

BenHenning · 2021-04-09T21:00:29Z

testing/src/main/java/org/oppia/android/testing/RecyclerViewMatcher.kt

@@ -1,12 +1,15 @@
-package org.oppia.android.app.recyclerview
+package org.oppia.android.testing


We ought to have a subpackage for this.

@fsharpasharp or @anandwana001 do you have any thoughts on how we should organize where Espresso helpers go in the testing package?

in one of the PR we are doing org.oppia.android.testing.robolectric so we can go with org.oppia.android.testing.espresso

That sounds good to me. WDYT @Sparsh1212?

BenHenning · 2021-04-09T21:02:04Z

testing/src/main/java/org/oppia/android/testing/RecyclerViewMatcher.kt

@@ -65,6 +68,24 @@ class RecyclerViewMatcher {
      }
    }

+    /**
+     * This function verifies item displayed for a specific view


While here, could you also add tests for these view matchers? It's important to make sure our test utilities work correctly, so having a RecyclerViewMatcherTest would be really helpful.

Can you please give me a reference or anything like that for writing a test for the ViewMatchers. That would be really helpful for me.

I don't think we have any tests for matchers yet (@anandwana001 can you confirm?), so this might be a new thing.

To start, I suggest copying an existing test suite & clearing out all of the modules & tests to start from a fresh state. In order to test these methods you'll need to create a test activity and test layout file (search for 'TestActivity' for examples of this in app module tests) where the layout file sets up test views to specifically test different conditions for these matchers.

If you're unsure how to write the tests, I suggest starting with setting up the test environment (e.g. the layout & activity) and then push those changes. We can provide further guidance on writing specific tests.

Yes, we don't have any tests for matchers yet.

Sparsh1212 · 2021-04-15T14:16:49Z

Apologies for the delay, I have my semester exams going on currently.
@anandwana001 Will it be okay if I finish this by the end of this week.

BenHenning · 2021-04-23T03:12:34Z

Apologies--will need to look at this tomorrow.

BenHenning

Thanks @Sparsh1212! Left a few comments. PTAL when you get a chance.

BenHenning · 2021-04-24T06:05:51Z

testing/src/main/java/org/oppia/android/testing/RecyclerViewMatcher.kt

@@ -1,12 +1,15 @@
-package org.oppia.android.app.recyclerview
+package org.oppia.android.testing


That sounds good to me. WDYT @Sparsh1212?

BenHenning · 2021-04-24T06:06:19Z

testing/src/main/java/org/oppia/android/testing/RecyclerViewMatcher.kt

+     * Verifies that the item is displayed for a specific view
+     * within the item inside RecyclerView from a specified position.


Here & elsewhere: please make sure KDocs fit the entire line (100 character cut-off) before being wrapped.

BenHenning · 2021-04-24T06:07:00Z

testing/src/main/java/org/oppia/android/testing/RecyclerViewMatcher.kt

+     * Verifies that the item is displayed for a specific view
+     * within the item inside RecyclerView from a specified position.
+     */
+    fun verifyItemDisplayedOnRecyclerView(


Perhaps instead: verifyItemDisplayedInRecyclerView? 'On' doesn't seem like the correct preposition to use here.

BenHenning · 2021-04-24T06:08:55Z

testing/src/main/java/org/oppia/android/testing/RecyclerViewMatcher.kt

+     * within the item inside RecyclerView from a specified position.
+     */
+    fun verifyItemDisplayedOnRecyclerView(
+      recyclerView: Int,


Ah, I should have instead said IdRes. See reference:

oppia-android/app/src/sharedTest/java/org/oppia/android/app/parser/HtmlParserTest.kt

Line 515 in 1e4c24c

@IdRes viewResId: Int

Could targetView also be annotated with IdRes?

BenHenning · 2021-04-24T06:12:08Z

testing/src/main/java/org/oppia/android/testing/RecyclerViewMatcher.kt

@@ -65,6 +68,24 @@ class RecyclerViewMatcher {
      }
    }

+    /**
+     * This function verifies item displayed for a specific view


I don't think we have any tests for matchers yet (@anandwana001 can you confirm?), so this might be a new thing.

To start, I suggest copying an existing test suite & clearing out all of the modules & tests to start from a fresh state. In order to test these methods you'll need to create a test activity and test layout file (search for 'TestActivity' for examples of this in app module tests) where the layout file sets up test views to specifically test different conditions for these matchers.

If you're unsure how to write the tests, I suggest starting with setting up the test environment (e.g. the layout & activity) and then push those changes. We can provide further guidance on writing specific tests.

Sparsh1212 added 2 commits March 31, 2021 03:12

Fix #2536: Shift RecylerViewMatcher

1893bbb

nit fixes

c9b6632

Sparsh1212 requested review from anandwana001, BenHenning and rt4914 as code owners March 30, 2021 21:47

Sparsh1212 assigned anandwana001 Mar 30, 2021

anandwana001 suggested changes Mar 31, 2021

View reviewed changes

anandwana001 assigned Sparsh1212 and unassigned anandwana001 Mar 31, 2021

Sparsh1212 added 2 commits April 7, 2021 00:51

merge develop

40d45dc

Add recycler view dependency to testing/build.gradle

b21d2d5

Sparsh1212 assigned anandwana001 and unassigned Sparsh1212 Apr 6, 2021

anandwana001 suggested changes Apr 7, 2021

View reviewed changes

testing/src/main/java/org/oppia/android/testing/RecyclerViewMatcher.kt Show resolved Hide resolved

anandwana001 assigned Sparsh1212 and unassigned anandwana001 Apr 7, 2021

Add KDoc

7b1c1e6

Sparsh1212 assigned anandwana001 and unassigned Sparsh1212 Apr 7, 2021

anandwana001 approved these changes Apr 7, 2021

View reviewed changes

anandwana001 assigned rt4914 and unassigned anandwana001 Apr 7, 2021

rt4914 approved these changes Apr 8, 2021

View reviewed changes

rt4914 assigned BenHenning and unassigned rt4914 Apr 8, 2021

BenHenning requested changes Apr 9, 2021

View reviewed changes

BenHenning assigned fsharpasharp, anandwana001 and Sparsh1212 and unassigned BenHenning Apr 9, 2021

anandwana001 removed their assignment Apr 12, 2021

Sparsh1212 added 4 commits April 15, 2021 20:33

Merge develop and resolve merge conflicts

64cf2ed

Refactor imports

e14315e

Refactor imports

3a62520

work on suggestions

094270c

Sparsh1212 assigned BenHenning and Sparsh1212 and unassigned Sparsh1212 Apr 22, 2021

BenHenning requested changes Apr 24, 2021

View reviewed changes

BenHenning assigned Sparsh1212 and anandwana001 and unassigned BenHenning Apr 24, 2021

anandwana001 removed their assignment Apr 28, 2021

anandwana001 added the PR don't merge - NEEDS UPDATE Corresponds to PRs that need to be updated with the latest develop changes before they can be merged label May 7, 2021

Sparsh1212 closed this Jun 9, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #2536: Shift RecyclerViewMatcher to Central testing utility #3030

Fix #2536: Shift RecyclerViewMatcher to Central testing utility #3030

Sparsh1212 commented Mar 30, 2021 •

edited

Loading

Sparsh1212 commented Mar 30, 2021

anandwana001 left a comment

anandwana001 commented Apr 6, 2021

Sparsh1212 commented Apr 6, 2021

Sparsh1212 commented Apr 6, 2021

anandwana001 commented Apr 7, 2021

anandwana001 left a comment •

edited

Loading

anandwana001 left a comment

rt4914 left a comment

BenHenning commented Apr 9, 2021

BenHenning left a comment

BenHenning Apr 9, 2021

Sparsh1212 Apr 15, 2021

BenHenning Apr 24, 2021

BenHenning Apr 9, 2021

anandwana001 Apr 12, 2021

BenHenning Apr 24, 2021

BenHenning Apr 9, 2021

Sparsh1212 Apr 15, 2021

BenHenning Apr 24, 2021

anandwana001 Apr 28, 2021

Sparsh1212 commented Apr 15, 2021

BenHenning commented Apr 23, 2021 •

edited

Loading

BenHenning left a comment

BenHenning Apr 24, 2021

BenHenning Apr 24, 2021

BenHenning Apr 24, 2021

BenHenning Apr 24, 2021

BenHenning Apr 24, 2021

		@@ -1,12 +1,15 @@
		package org.oppia.android.app.recyclerview
		package org.oppia.android.testing

		* Verifies that the item is displayed for a specific view
		* within the item inside RecyclerView from a specified position.

Fix #2536: Shift RecyclerViewMatcher to Central testing utility #3030

Fix #2536: Shift RecyclerViewMatcher to Central testing utility #3030

Conversation

Sparsh1212 commented Mar 30, 2021 • edited Loading

Explanation

Checklist

Sparsh1212 commented Mar 30, 2021

anandwana001 left a comment

Choose a reason for hiding this comment

anandwana001 commented Apr 6, 2021

Sparsh1212 commented Apr 6, 2021

Sparsh1212 commented Apr 6, 2021

anandwana001 commented Apr 7, 2021

anandwana001 left a comment • edited Loading

Choose a reason for hiding this comment

anandwana001 left a comment

Choose a reason for hiding this comment

rt4914 left a comment

Choose a reason for hiding this comment

BenHenning commented Apr 9, 2021

BenHenning left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sparsh1212 commented Apr 15, 2021

BenHenning commented Apr 23, 2021 • edited Loading

BenHenning left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sparsh1212 commented Mar 30, 2021 •

edited

Loading

anandwana001 left a comment •

edited

Loading

BenHenning commented Apr 23, 2021 •

edited

Loading