Fix #3315: Set up UIAutomator with Bazel #3505

FareesHussain · 2021-07-20T16:23:20Z

Explanation

Fixes #3315

This pr introduces UI Automator with a BaseTest generated using Bazel

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
```
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:ExplorationPlayerTestBinary

install the oppia_test.apk and the ExplorationPlayerTestBinary.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 am instrument -w org.oppia.android.app.instrumentation/androidx.test.runner.AndroidJUnitRunner

2.1.Exploration.Player.Test.take.1.mp4

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

FareesHussain · 2021-07-20T20:29:04Z

BUILD.bazel

@@ -59,6 +59,7 @@ android_binary(
        "versionName": "0.1-alpha",
    },
    multidex = "native",  # TODO(#1875): Re-enable legacy for optimized release builds.
+    visibility = ["//visibility:public"],


I'm not sure what package_group I need to use here

We ought to create another binary specifically for tests, set that visibility to oppia_testing_visibility & set is as testonly.

instrumentation/BUILD.bazel

FareesHussain · 2021-07-20T20:33:33Z

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

+import org.junit.Test
+
+/** Tests to load Oppia using UI Automator. */
+class BaseTest {


This test suite is basically to check if the UI Automator is working fine or not
and to check if the app is loading as expected.

I'm navigating to the ProfileDashboard especially to verify if the we don't face similar errors we faced when using android_instrumentation_test target

Not using other test cases as the only purpose was to be able to run the end to end tests.
Other test cases may be used in future work.

Test suites should be specific to the things they are testing. In this case, I'd expect something like ExplorationPlayerTest. Please split this up/update it as needed to be specific to the sets of tests it will contain, and then update the packages & Bazel structure accordingly.

I think I also have feedback on how to change the Bazel structure, but it's hard for me to see the difference between which pieces a test-specific vs. generic. After this is addressed I should be able to give more specific feedback.

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

FareesHussain · 2021-07-20T20:37:32Z

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

+  @Test
+  fun baseTest_openProfileDashboard_titleExists() {
+    val skip_button = device.findObject(By.res("$OPPIA_PACKAGE:id/skip_text_view"))
+    skip_button?.let {


Null check, incase the app OnBoarding was already used once.

FareesHussain · 2021-07-21T05:30:56Z

third_party/versions.bzl

+    "androidx.test.ext:junit": "1.1.3",
+    "androidx.test:core": "1.4.0",
+    "androidx.test:runner": "1.4.0",


@BenHenning I've updated these version when we were trying the previous implementation.
Shall I revert them?

I lean toward not upgrading versions unless specifically needed.

FareesHussain · 2021-07-21T06:08:22Z

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

+/** Tests to load Oppia using UI Automator. */
+class BaseTest {
+  private val OPPIA_PACKAGE = "org.oppia.android"
+  private val LAUNCH_TIMEOUT = 5000


I choose 5 seconds as per the Ui Automator examples I've referred to,
Tried using 1 sec and 2 secs both fail, 3 sec seems to be working.

We could also use 30 seconds. Generally, we see 10, 30, and 60 second timeouts, or 5 minutes as the longest possible (since Bazel will fail the test at that point) depending on the context. 30 seems more fitting here since there are a lot of things we may need to wait for on the emulator or device side for the test to actually start.

Done, I've also added a seperate timeout (TRANSITION_TIMEOUT) of 5 seconds to wait for the navigation between the activities, I used 5 seconds as minimum expecting that for the failure case the 30sec timeout would be very large for the activites.

BenHenning · 2021-07-21T07:03:58Z

@FareesHussain given your comments, is this PR in a state ready to be in full review or should it be in draft?

Also, I probably won't be able to look at this for another day--sorry about that.

FareesHussain · 2021-07-21T07:05:09Z

@FareesHussain given your comments, is this PR in a state ready to be in full review or should it be in draft?

Also, I probably won't be able to look at this for another day--sorry about that.

It is ready for review, I've added the comments just to explain some parts of the code.

BenHenning

Thanks @FareesHussain. Took a very high-level pass & left some comments--PTAL.

BenHenning · 2021-07-22T06:50:19Z

BUILD.bazel

@@ -59,6 +59,7 @@ android_binary(
        "versionName": "0.1-alpha",
    },
    multidex = "native",  # TODO(#1875): Re-enable legacy for optimized release builds.
+    visibility = ["//visibility:public"],


We ought to create another binary specifically for tests, set that visibility to oppia_testing_visibility & set is as testonly.

BenHenning · 2021-07-22T06:52:17Z

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

+import org.junit.Test
+
+/** Tests to load Oppia using UI Automator. */
+class BaseTest {


Test suites should be specific to the things they are testing. In this case, I'd expect something like ExplorationPlayerTest. Please split this up/update it as needed to be specific to the sets of tests it will contain, and then update the packages & Bazel structure accordingly.

I think I also have feedback on how to change the Bazel structure, but it's hard for me to see the difference between which pieces a test-specific vs. generic. After this is addressed I should be able to give more specific feedback.

BenHenning · 2021-07-22T06:54:34Z

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

+/** Tests to load Oppia using UI Automator. */
+class BaseTest {
+  private val OPPIA_PACKAGE = "org.oppia.android"
+  private val LAUNCH_TIMEOUT = 5000


We could also use 30 seconds. Generally, we see 10, 30, and 60 second timeouts, or 5 minutes as the longest possible (since Bazel will fail the test at that point) depending on the context. 30 seems more fitting here since there are a lot of things we may need to wait for on the emulator or device side for the test to actually start.

BenHenning · 2021-07-22T06:55:08Z

third_party/versions.bzl

+    "androidx.test.ext:junit": "1.1.3",
+    "androidx.test:core": "1.4.0",
+    "androidx.test:runner": "1.4.0",


I lean toward not upgrading versions unless specifically needed.

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

anandwana001 · 2021-07-22T13:30:04Z

@FareesHussain Let's review it tomorrow while our meeting.

FareesHussain · 2021-08-11T13:50:17Z

Tried

using executeShellCommand
manually clear storage within the instrumentation
activityManager.clearApplicationUserData()
deleting the cacheDir

None of them seems to work

BenHenning

@FareesHussain what specifically didn't work in each case?

scripts/src/javatests/org/oppia/android/scripts/ci/ComputeAffectedTestsTest.kt

FareesHussain · 2021-08-13T06:20:24Z

@FareesHussain what specifically didn't work in each case?

I did mention this in the mail.

executeShellCommand("pm clear $OPPIA_PACKAGE")
and manually clearing the storage had the same error

https://gist.github.com/FareesHussain/b7ea9d5a8fdafe41f0dcc7536cd0111b

clearApplicationData() from the application context also had the same error
https://gist.github.com/FareesHussain/ec88b857f1d1cb1feef51f4c7580a47e

deleting the cacheDir doesn't have any change (I don't think it would reset the app as clearing application storage and deleting cache is a bit different), This doesn't crash the tests but also doesn't reset the app

BenHenning

Thanks @FareesHussain. One follow-up. I think the main thing left is ensuring data is properly reset between tests before this PR can be considered ready to merge (from my perspective).

scripts/src/javatests/org/oppia/android/scripts/ci/ComputeAffectedTestsTest.kt

FareesHussain · 2021-08-15T17:53:16Z

Thanks @FareesHussain. One follow-up. I think the main thing left is ensuring data is properly reset between tests before this PR can be considered ready to merge (from my perspective).

running tests on fresh install / after clearing the application data, doesn't save the application data.

This was the test for 2.2 pr where the app starts from the onboarding screen for each test case.

anandwana001

everything LGTM,

Need to follow up with Ben's comment about clearing app data in every test case.
comment - #3505 (comment)

oppiabot · 2021-08-16T10:45:44Z

Unassigning @anandwana001 since they have already approved the PR.

… setup-uiautomator

BenHenning

Thanks @FareesHussain. Left one follow-up comment with a suggestion to unblock this PR so that we can get it merged. PTAL.

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

scripts/src/javatests/org/oppia/android/scripts/ci/ComputeAffectedTestsTest.kt

BenHenning

Since everything else looks good here, approving. Thanks @FareesHussain!

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

BenHenning · 2021-08-18T07:37:14Z

Since there are no other open reviewer threads, merging this.

FareesHussain added 4 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

FareesHussain requested review from anandwana001 and BenHenning as code owners July 20, 2021 16:23

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

46a1b56

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

FareesHussain self-assigned this Jul 20, 2021

FareesHussain added 2 commits July 20, 2021 22:40

Nit fixes

44f4003

Nit fixes

72620a2

FareesHussain assigned BenHenning and anandwana001 and unassigned FareesHussain Jul 20, 2021

FareesHussain commented Jul 20, 2021

View reviewed changes

instrumentation/BUILD.bazel Outdated Show resolved Hide resolved

FareesHussain commented Jul 20, 2021

View reviewed changes

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

FareesHussain commented Jul 20, 2021

View reviewed changes

FareesHussain commented Jul 21, 2021

View reviewed changes

BenHenning reviewed Jul 22, 2021

View reviewed changes

BenHenning assigned FareesHussain and unassigned BenHenning Jul 22, 2021

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

7a384c3

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

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 Jul 22, 2021

FareesHussain added 2 commits July 23, 2021 01:30

Requested changes

8a641d7

fix class name

c58b6f6

FareesHussain assigned BenHenning Aug 11, 2021

BenHenning reviewed Aug 13, 2021

View reviewed changes

scripts/src/javatests/org/oppia/android/scripts/ci/ComputeAffectedTestsTest.kt Outdated Show resolved Hide resolved

BenHenning assigned FareesHussain and unassigned BenHenning Aug 13, 2021

updated the ComputedAffectedTests

0d50914

FareesHussain assigned BenHenning and unassigned FareesHussain Aug 13, 2021

ignoring player directory

e2db957

BenHenning reviewed Aug 13, 2021

View reviewed changes

scripts/src/javatests/org/oppia/android/scripts/ci/ComputeAffectedTestsTest.kt Outdated Show resolved Hide resolved

BenHenning assigned FareesHussain and unassigned BenHenning Aug 13, 2021

FareesHussain assigned BenHenning and unassigned FareesHussain Aug 14, 2021

anandwana001 approved these changes Aug 16, 2021

View reviewed changes

oppiabot bot unassigned anandwana001 Aug 16, 2021

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

9ffd04e

… setup-uiautomator

BenHenning reviewed Aug 17, 2021

View reviewed changes

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

scripts/src/javatests/org/oppia/android/scripts/ci/ComputeAffectedTestsTest.kt Outdated Show resolved Hide resolved

BenHenning assigned FareesHussain and unassigned BenHenning Aug 17, 2021

FareesHussain assigned BenHenning and unassigned FareesHussain Aug 17, 2021

BenHenning changed the title ~~Fix #3315: Setup UIAutomator with Bazel~~ Fix #3315: Set up UIAutomator with Bazel Aug 18, 2021

BenHenning approved these changes Aug 18, 2021

View reviewed changes

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

BenHenning merged commit a55f808 into oppia:develop Aug 18, 2021

BenHenning mentioned this pull request Aug 18, 2021

Fix #3607: ExplorationPlayer End to End tests #3608

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #3315: Set up UIAutomator with Bazel #3505

Fix #3315: Set up UIAutomator with Bazel #3505

FareesHussain commented Jul 20, 2021 •

edited

Loading

FareesHussain Jul 20, 2021

BenHenning Jul 22, 2021

FareesHussain Jul 22, 2021

FareesHussain Jul 20, 2021

FareesHussain Jul 20, 2021

BenHenning Jul 22, 2021

FareesHussain Jul 22, 2021

FareesHussain Jul 20, 2021

FareesHussain Jul 21, 2021 •

edited

Loading

BenHenning Jul 22, 2021

FareesHussain Jul 22, 2021

FareesHussain Jul 21, 2021

BenHenning Jul 22, 2021

FareesHussain Jul 22, 2021

BenHenning commented Jul 21, 2021

FareesHussain commented Jul 21, 2021 •

edited

Loading

BenHenning left a comment •

edited

Loading

BenHenning Jul 22, 2021

BenHenning Jul 22, 2021

BenHenning Jul 22, 2021

BenHenning Jul 22, 2021

anandwana001 commented Jul 22, 2021

FareesHussain commented Aug 11, 2021

BenHenning left a comment

FareesHussain commented Aug 13, 2021

BenHenning left a comment

FareesHussain commented Aug 15, 2021 •

edited

Loading

anandwana001 left a comment

oppiabot bot commented Aug 16, 2021

BenHenning left a comment

BenHenning left a comment

BenHenning commented Aug 18, 2021

Fix #3315: Set up UIAutomator with Bazel #3505

Fix #3315: Set up UIAutomator with Bazel #3505

Conversation

FareesHussain commented Jul 20, 2021 • edited Loading

Explanation

How to run UI Automator tests

Checklist

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

FareesHussain Jul 21, 2021 • edited Loading

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

BenHenning commented Jul 21, 2021

FareesHussain commented Jul 21, 2021 • edited Loading

BenHenning left a comment • edited Loading

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

anandwana001 commented Jul 22, 2021

FareesHussain commented Aug 11, 2021

BenHenning left a comment

Choose a reason for hiding this comment

FareesHussain commented Aug 13, 2021

BenHenning left a comment

Choose a reason for hiding this comment

FareesHussain commented Aug 15, 2021 • edited Loading

anandwana001 left a comment

Choose a reason for hiding this comment

oppiabot bot commented Aug 16, 2021

BenHenning left a comment

Choose a reason for hiding this comment

BenHenning left a comment

Choose a reason for hiding this comment

BenHenning commented Aug 18, 2021

FareesHussain commented Jul 20, 2021 •

edited

Loading

FareesHussain Jul 21, 2021 •

edited

Loading

FareesHussain commented Jul 21, 2021 •

edited

Loading

BenHenning left a comment •

edited

Loading

FareesHussain commented Aug 15, 2021 •

edited

Loading