-
Notifications
You must be signed in to change notification settings - Fork 527
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.
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
Fix #3528: Setup Infrastructure to test using local dev server #3529
Fix #3528: Setup Infrastructure to test using local dev server #3529
Conversation
… setup-uiautomator � Conflicts: � third_party/maven_install.json
…oppia-android; branch 'develop' of https://github.com/oppia/oppia-android into setup-uiautomator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had a quick look here, will look into more detail.
instrumentation/src/java/org/oppia/android/instrumentation/OppiaTestApplication.kt
Outdated
Show resolved
Hide resolved
Unassigning @anandwana001 since the review is done. |
Hi @FareesHussain, it looks like some changes were requested on this pull request by @anandwana001. PTAL. Thanks! |
@anandwana001 @BenHenning PTAL Here the AndroidManifest only as two changes compared to the app module.
and
There are some pending kdoc need to be updated, I will update them once the approach is accepted. |
… checking-test-application � Conflicts: � app/src/sharedTest/java/org/oppia/android/app/help/HelpActivityTest.kt � instrumentation/BUILD.bazel � instrumentation/src/java/org/oppia/android/instrumentation/testing/BUILD.bazel � scripts/assets/test_file_exemptions.textproto � scripts/src/javatests/org/oppia/android/scripts/ci/ComputeAffectedTestsTest.kt
@BenHenning can you check how to add a TestFile to get the target if I use
I get the following target |
It is fixed now, not sure if it is the right way to do that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @FareesHussain. Just had one comment, otherwise the PR LGTM.
scripts/src/java/org/oppia/android/scripts/testing/TestBazelWorkspace.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @FareesHussain. Given that the past CI run was green & all other comments are resolved, this LGTM. However, please make sure at least https://github.com/oppia/oppia-android/pull/3529/checks?check_run_id=3368705606 is passing before merging since we don't have a required check (yet) for these script tests (which means breakages can be checked in & merged).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just waiting for all checks to pass and nothing was broken, before merging.
Unassigning @anandwana001 since they have already approved the PR. |
… checking-test-application � Conflicts: � app/src/main/java/org/oppia/android/app/application/ApplicationComponent.kt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Latest changes LGTM.
Explanation
Fixes #3528
This pr introduces a Testing infrastructure that enables instrumentation tests for downloading explorations and loading images from the local dev server.
This introduces a TestNetworkModule, TestImageParsingModule, and a TestGcsResourceModule.
These modules are used by the OppiaTestApplication and added to a manifest used by the oppia_test android_binary.
oppia_test is similar to the oppia target but it uses the above three modules and a different AndroidManifest
Here I've used two different AndroidManifest in the instrumentation module
one under java directory used for the OppiaTestApplication
and under javatest directory used for InstrumentationTests
Build the test app using:
Screenshot
Checklist