Fix #3607: ExplorationPlayer End to End tests #3608

FareesHussain · 2021-07-29T00:31:52Z

Explanation

Fixes #3607

This pr introduces e2e tests in ExplorationPlayerTests using Ui Automator

How to run UI Automator tests

Prerequisites:

To be able to build oppia using bazel
```
bazel build oppia
```
Add adb to the environment (platform-tools) i.e, add the following line to the .bashrc or the file path according to the Operating System.
```
export PATH=/home/<username>/Android/Sdk/platform-tools:$PATH
```
download and install test-services-1.1.0.apk and orchestrator-1.1.0 in the emulator. (run command in the directory where both apk are downloaded)
```
adb install -r test-services-1.1.0.apk && adb install -r orchestrator-1.1.0.apk
```
java version 8 (Optional, only for uiautomatorviewer)
```
java -version
```
output:
openjdk version "1.8.0_292"
OpenJDK Runtime Environment (build 1.8.0_292-8u292-b10-0ubuntu1~18.04-b10)
OpenJDK 64-Bit Server VM (build 25.292-b10, mixed mode)

Steps to run the tests:

Build the BaseTest android_binary from the instrumentation module

bazel build :oppia_test && bazel build //instrumentation:ExplorationPlayerTest

install the oppia_test.apk and the ExplorationPlayerTest.apk

adb install -r bazel-bin/oppia_test.apk && adb install -r bazel-bin/instrumentation/ExplorationPlayerTest.apk

Run the instrumentation tests using am instrument command

adb shell 'CLASSPATH=$(pm path androidx.test.services) app_process / \
androidx.test.services.shellexecutor.ShellMain am instrument -w -e clearPackageData true \
-e targetInstrumentation org.oppia.android.app.instrumentation/androidx.test.runner.AndroidJUnitRunner \
androidx.test.orchestrator/.AndroidTestOrchestrator'

Using.android.test.orchestrator.mp4

Ran using single testcase for both with argument and without argument

Failing.test.case.included.mp4

Ran using duplicate testcase for both without argument and with argument

Using.duplicate.test.case.2.2.pr.mp4

Note: There are few flacky tests which is obvious for E2E tests but each test passes on individual run.

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.

… setup-uiautomator � Conflicts: � third_party/maven_install.json

…oppia-android; branch 'develop' of https://github.com/oppia/oppia-android into setup-uiautomator

anandwana001 · 2021-07-29T04:42:01Z

Assigning to @FareesHussain as well, to keep track of adding screenshots.

FareesHussain · 2021-07-29T07:10:52Z

Assigning to @FareesHussain as well, to keep track of adding screenshots.

Adding screenshots might take a time here as the whole test suite has 188 testcases, it might almost take more than 2 hrs

BenHenning

Thanks @FareesHussain. Left one comment, but in general I think that:

We need to leverage helpers to simplify the tests (per a comment I added in one of the base PRs)
The tests we add for UiAutomator should be higher-level and cover general user flows rather than testing at the same level of specificity of Robolectric. Please follow up with me & Akshay if you're unsure how to proceed, but I did add one example to my comment. PTAL.

instrumentation/src/javatest/org/oppia/android/instrumentation/ExplorationPlayerTest.kt

… setup-uiautomator � Conflicts: � third_party/maven_install.json

…oppia-android into exploration-e2e-tests � Conflicts: � instrumentation/src/javatest/org/oppia/android/instrumentation/ExplorationPlayerTest.kt

FareesHussain · 2021-08-19T09:34:17Z

instrumentation/src/javatests/org/oppia/android/instrumentation/player/ExplorationPlayerTest.kt

+    val imageSelectionView = device.findObjectByRes("interaction_container_frame_layout")
+    device.waitForRes("image_click_interaction_image_view")
+    imageSelectionView!!.children!!.get(2)!!.click()
+    device.findObjectByText("SUBMIT")!!.click()


Here I'm using !!. instead of ?.
So that in case the view doesn't exist the test fails immediately, else the test runs as normal till it reaches the assertion and then gives a failure which might be confusing

I suggest returning only non-nullable types from your helper class and utilize checkNotNull with clearer error messages if an object isn't found. That'll give you the quick-failure you're looking for with clearer context, and doesn't even give the choice to test authors to potentially avoid the null-check (since we do want things to be really crisp).

FareesHussain · 2021-08-19T22:01:08Z

app/src/main/java/org/oppia/android/app/utility/ClickableAreasImage.kt

@@ -116,6 +116,7 @@ class ClickableAreasImage(
        newView.isFocusable = true
        newView.isFocusableInTouchMode = true
        newView.tag = clickableArea.label
+        newView.contentDescription = clickableArea.label


@aggarwalpulkit596 PTAL

This enables content description for each image region in ImageRegionSelectionInteraction

This isn't correct. The label will give away the answer, so we introduced another property for content descriptions in image region.

However, looking at https://github.com/oppia/oppia/blob/develop/extensions/interactions/ImageClickInput/ImageClickInput.py it seems we never did add this. Per oppia/oppia#9924 I guess we didn't finish this which introduces a problem. We unfortunately won't be able to use this method.

Can you file an issue to add support for oppia/oppia#9924 once it's ready & add that as a TODO here to fix the reference? We probably aren't going to be able to do better than the direct indexing without content description support.

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

rt4914

Approved for code owner file.

oppiabot · 2021-08-20T15:38:03Z

Unassigning @rt4914 since they have already approved the PR.

aggarwalpulkit596

LGTM

oppiabot · 2021-08-20T20:21:47Z

Unassigning @aggarwalpulkit596 since they have already approved the PR.

BenHenning

Thanks @FareesHussain. Couple more comments, but this looks pretty close to LGTM. Once you address these I'll take on more full pass (check open conversation threads; I didn't submit the review correctly this time).

BenHenning · 2021-08-20T22:22:27Z

Also @rt4914 PTAL at the comment I replied to above re: the content description.

BenHenning

Thanks @FareesHussain. This LGTM!

app/src/main/res/layout-land/story_chapter_view.xml

BenHenning · 2021-08-21T09:01:39Z

Since no reviewer threads are open & everyone's approved, merging this.

FareesHussain added 13 commits July 15, 2021 13:08

Introducing UiAutomator

b720222

Using 23 api level

f90cbfc

some local tweaks

930abe0

Updated implementation based on adb commands

bb6f5bb

Merge branch 'develop' of https://github.com/oppia/oppia-android into…

46a1b56

… setup-uiautomator � Conflicts: � third_party/maven_install.json

Nit fixes

44f4003

Nit fixes

72620a2

Merge branch 'setup-uiautomator' of https://github.com/FareesHussain/…

7a384c3

…oppia-android; branch 'develop' of https://github.com/oppia/oppia-android into setup-uiautomator

Requested changes

8a641d7

fix class name

c58b6f6

update dependencies

09c419e

Nit fixes

004e3a1

Initial e2e tests

7124642

FareesHussain requested review from BenHenning and anandwana001 July 29, 2021 00:31

FareesHussain assigned BenHenning and anandwana001 Jul 29, 2021

anandwana001 assigned FareesHussain Jul 29, 2021

Sparsh1212 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 Jul 30, 2021

BenHenning requested changes Jul 30, 2021

View reviewed changes

instrumentation/src/javatest/org/oppia/android/instrumentation/ExplorationPlayerTest.kt Outdated Show resolved Hide resolved

BenHenning removed their assignment Jul 30, 2021

FareesHussain added 7 commits July 30, 2021 23:34

Requested changes

7a1221d

Merge branch 'develop' of https://github.com/oppia/oppia-android into…

0aec4a5

… setup-uiautomator � Conflicts: � third_party/maven_install.json

updated maven_install

6e09966

Merge branch 'setup-uiautomator' of https://github.com/FareesHussain/…

8ca2bb9

…oppia-android into exploration-e2e-tests � Conflicts: � instrumentation/src/javatest/org/oppia/android/instrumentation/ExplorationPlayerTest.kt

Nit fix

4486882

Add CODEOWNERS

f64d32c

Added KDoc

dc66ba2

FareesHussain commented Aug 19, 2021

View reviewed changes

FareesHussain assigned BenHenning and unassigned FareesHussain Aug 19, 2021

FareesHussain added 3 commits August 19, 2021 15:23

fixed tests

c7bd1dd

reverted chapter_completed_tick content desc

f01a655

Updated content description for ImageRegionSelectionInteraction

4811a07

FareesHussain assigned aggarwalpulkit596 Aug 19, 2021

FareesHussain commented Aug 19, 2021

View reviewed changes

testing ImageRegionSelectionInteractionView

b9e9fbe

FareesHussain commented Aug 19, 2021

View reviewed changes

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

FareesHussain added 2 commits August 20, 2021 14:22

updated test

196f8db

remoced ignore

bf1e0eb

FareesHussain commented Aug 20, 2021

View reviewed changes

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

rt4914 approved these changes Aug 20, 2021

View reviewed changes

oppiabot bot unassigned rt4914 Aug 20, 2021

aggarwalpulkit596 approved these changes Aug 20, 2021

View reviewed changes

oppiabot bot unassigned aggarwalpulkit596 Aug 20, 2021

BenHenning requested changes Aug 20, 2021

View reviewed changes

BenHenning assigned rt4914 and FareesHussain and unassigned BenHenning Aug 20, 2021

Removed content description and Added checkNotNull

63ed680

FareesHussain assigned BenHenning and unassigned FareesHussain Aug 20, 2021

BenHenning approved these changes Aug 21, 2021

View reviewed changes

app/src/main/res/layout-land/story_chapter_view.xml Outdated Show resolved Hide resolved

BenHenning merged commit 206f92f into oppia:develop Aug 21, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #3607: ExplorationPlayer End to End tests #3608

Fix #3607: ExplorationPlayer End to End tests #3608

FareesHussain commented Jul 29, 2021 •

edited

Loading

anandwana001 commented Jul 29, 2021

FareesHussain commented Jul 29, 2021

BenHenning left a comment

FareesHussain Aug 19, 2021

BenHenning Aug 20, 2021

FareesHussain Aug 20, 2021

FareesHussain Aug 19, 2021

BenHenning Aug 20, 2021

FareesHussain Aug 20, 2021

rt4914 left a comment

oppiabot bot commented Aug 20, 2021

aggarwalpulkit596 left a comment

oppiabot bot commented Aug 20, 2021

BenHenning left a comment •

edited

Loading

BenHenning commented Aug 20, 2021

BenHenning left a comment

BenHenning commented Aug 21, 2021

Fix #3607: ExplorationPlayer End to End tests #3608

Fix #3607: ExplorationPlayer End to End tests #3608

Conversation

FareesHussain commented Jul 29, 2021 • edited Loading

Explanation

How to run UI Automator tests

Checklist

anandwana001 commented Jul 29, 2021

FareesHussain commented Jul 29, 2021

BenHenning left a comment

Choose a reason for hiding this comment

FareesHussain Aug 19, 2021

Choose a reason for hiding this comment

BenHenning Aug 20, 2021

Choose a reason for hiding this comment

FareesHussain Aug 20, 2021

Choose a reason for hiding this comment

FareesHussain Aug 19, 2021

Choose a reason for hiding this comment

BenHenning Aug 20, 2021

Choose a reason for hiding this comment

FareesHussain Aug 20, 2021

Choose a reason for hiding this comment

rt4914 left a comment

Choose a reason for hiding this comment

oppiabot bot commented Aug 20, 2021

aggarwalpulkit596 left a comment

Choose a reason for hiding this comment

oppiabot bot commented Aug 20, 2021

BenHenning left a comment • edited Loading

Choose a reason for hiding this comment

BenHenning commented Aug 20, 2021

BenHenning left a comment

Choose a reason for hiding this comment

BenHenning commented Aug 21, 2021

FareesHussain commented Jul 29, 2021 •

edited

Loading

BenHenning left a comment •

edited

Loading