Skip to content

Commit

Permalink
[RunAllTests] Fix part of oppia#2787: Add support for proto lesson lo…
Browse files Browse the repository at this point in the history 
…ading from local assets (oppia#2978)

* Add support for loading proto versions of lessons.

* Upgrade Espresso versions to work around build issues.

Note that I can't actually verify Espresso tests are working with these
versions since the Bazel project doesn't yet have Espresso tests set up.

* Lint fix post-copying from oppia#2927.

* Remove image loading annotation.

This will be added in a later branch in the chain.

* Add caching module to needed test suites.

* Add support for importing text protos as assets.

This actually provides Bazel-only support for converting text protos to
binary and including those as assets so that we can avoid checking in
binary protos to the codebase.

* Add gitignore for Android Studio Bazel plugin output.

* Import textproto versions of existing dev assets.

This imports local conversions of all existing dev assets as text proto
leveraging the Bazel-only conversion system to convert these to binary
protos and include them as assets.

This is a necessary step to both eventually replace JSON assets with
text proto, and to test that the new proto loading pipeline is working
as designed.

* Undo Espresso version change since it breaks Bazel tests.

* Fix tests.

Various attempts to fix existing tests + move some of the domain tests
over to actually using proto when running with Bazel.

App module tests are hanging currently for unknown reasons (only in
Bazel).

* Use correct Espresso core.

3.1.0 causes tests to hang indefinitely when run with Bazel.

* Move coroutine dispatchers.

This moves the coroutine dispatchers to a new threading subpackage. It
also simplifies the TestCoroutineDispatcher interface & implementations
(which required moving runUntilIdle to CoroutineExecutorServiceTest).

The test coroutine annotations were also moved to their own files.

* Refactor testing utilities build graph.

This introduces new Bazel libraries for:
- Test dispatchers
- Fake system/Oppia clocks
- Robolectric dependencies

* Introduce dedicated test for TestCoroutineDispatcher.

This also fixes some threading issues in the dispatcher, and clarifies
some of its API that was previously unclear (and can lead to subtle race
conditions).

* Add tests for TestCoroutineDispatcher.

This required a bunch of refactoring, adding the first examples of
dedicated Bazel test build files, and introducing a new test pattern for
sharing code between different implementations. Further, it refined the
API for TesCoroutineDispatcher & fixed some issues in the
implementations (especially Robolectric).

Tests verified as non-flaky in Robolectric across 1000 runs, and 0.2%
flaky across 1000 runs for Espresso (though that may be fixed; waiting
on a follow-up run to confirm stability).

* Workaround ktlint semicolon issue.

* Lint fixes.

* Fix test post-merge.

* Move CoroutineDispatcher to be part of threading.

* Lint fixes.

* Undo version upgrade.

* Fix broken tests.

* Lint fixes.

* Add a bit more proto loading test coverage.

* Comment fix.

* Lint fixes.

* Address earlier TODO.

* Fix broken test build.
  • Loading branch information
@BenHenning
BenHenning authored Apr 15, 2021
1 parent 2becfa9 commit cb1ddc8
Show file tree
Hide file tree
Showing 66 changed files with 39,043 additions and 740 deletions.
6 changes: 6 additions & 0 deletions WORKSPACE
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,12 @@ bind(
actual = "//tools/android:compiler_annotation_processor",
)

http_archive(
name = "protobuf_tools",
strip_prefix = "protobuf-%s" % HTTP_DEPENDENCY_VERSIONS["protobuf_tools"]["version"],
urls = ["https://github.com/protocolbuffers/protobuf/releases/download/v{0}/protobuf-all-{0}.zip".format(HTTP_DEPENDENCY_VERSIONS["protobuf_tools"]["version"])],
)

load("@rules_jvm_external//:defs.bzl", "maven_install")

# Note to developers: new dependencies should be added to //third_party:versions.bzl, not here.
Expand Down
4 changes: 2 additions & 2 deletions app/src/main/java/org/oppia/android/app/testing/TopicTestActivityForStory.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import org.oppia.android.app.topic.TopicActivityPresenter
import org.oppia.android.app.topic.TopicFragment
import org.oppia.android.app.topic.questionplayer.QuestionPlayerActivity
import org.oppia.android.app.topic.revisioncard.RevisionCardActivity
import org.oppia.android.domain.topic.TEST_STORY_ID_1
import org.oppia.android.domain.topic.TEST_STORY_ID_0
import org.oppia.android.domain.topic.TEST_TOPIC_ID_0
import javax.inject.Inject

Expand All @@ -33,7 +33,7 @@ class TopicTestActivityForStory :
topicActivityPresenter.handleOnCreate(
internalProfileId = 0,
topicId = TEST_TOPIC_ID_0,
storyId = TEST_STORY_ID_1
storyId = TEST_STORY_ID_0
)
}

Expand Down
16 changes: 8 additions & 8 deletions app/src/sharedTest/java/org/oppia/android/app/home/HomeActivityTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -499,8 +499,8 @@ class HomeActivityTest {
* # Logic for recommendation system
*
* We always recommend the next topic that all dependencies are completed for. If a topic with
* prerequisites is completed out-of-order (e.g. test topic 1 above) then we assume fractions is already done.
* In the same way, finishing test topic 2 means there's nothing else to recommend.
* prerequisites is completed out-of-order (e.g. test topic 1 above) then we assume fractions is
* already done. In the same way, finishing test topic 2 means there's nothing else to recommend.
*/
@Test
fun testHomeActivity_markStory0DonePlayStory1FirstTestTopic_playFractionsTopic_orderIsCorrect() {
Expand All @@ -513,7 +513,7 @@ class HomeActivityTest {
profileId = profileId1,
timestampOlderThanOneWeek = false
)
storyProgressTestHelper.markRecentlyPlayedTestTopic0Story1Exp1(
storyProgressTestHelper.markRecentlyPlayedTestTopic1Story0(
profileId = profileId1,
timestampOlderThanOneWeek = false
)
Expand All @@ -525,23 +525,23 @@ class HomeActivityTest {
targetViewId = R.id.recently_played_stories_text_view,
stringToMatch = context.getString(R.string.stories_for_you)
)
scrollToPositionOfPromotedList(position = 1)
scrollToPositionOfPromotedList(position = 0)
verifyTextOnPromotedListItemAtPosition(
itemPosition = 0,
targetViewId = R.id.topic_name_text_view,
stringToMatch = "Fractions"
stringToMatch = "Second Test Topic"
)
scrollToPositionOfPromotedList(position = 1)
verifyTextOnPromotedListItemAtPosition(
itemPosition = 1,
targetViewId = R.id.topic_name_text_view,
stringToMatch = "Ratios and Proportional Reasoning"
stringToMatch = "Fractions"
)
scrollToPositionOfPromotedList(position = 2)
verifyTextOnPromotedListItemAtPosition(
itemPosition = 2,
targetViewId = R.id.topic_name_text_view,
stringToMatch = "First Test Topic"
stringToMatch = "Ratios and Proportional Reasoning"
)
}
}
Expand Down Expand Up @@ -825,7 +825,7 @@ class HomeActivityTest {
verifyTextOnHomeListItemAtPosition(
itemPosition = 3,
targetViewId = R.id.lesson_count_text_view,
stringToMatch = "5 Lessons"
stringToMatch = "2 Lessons"
)
}
}
Expand Down
10 changes: 5 additions & 5 deletions app/src/sharedTest/java/org/oppia/android/app/story/StoryActivityTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ import org.oppia.android.domain.oppialogger.loguploader.LogUploadWorkerModule
import org.oppia.android.domain.oppialogger.loguploader.WorkManagerConfigurationModule
import org.oppia.android.domain.question.QuestionModule
import org.oppia.android.domain.topic.PrimeTopicAssetsControllerModule
import org.oppia.android.domain.topic.TEST_EXPLORATION_ID_1
import org.oppia.android.domain.topic.TEST_STORY_ID_1
import org.oppia.android.domain.topic.TEST_EXPLORATION_ID_2
import org.oppia.android.domain.topic.TEST_STORY_ID_0
import org.oppia.android.domain.topic.TEST_TOPIC_ID_0
import org.oppia.android.testing.TestImageLoaderModule
import org.oppia.android.testing.TestLogReportingModule
Expand Down Expand Up @@ -115,7 +115,7 @@ class StoryActivityTest {
createStoryActivityIntent(
internalProfileId = internalProfileId,
topicId = TEST_TOPIC_ID_0,
storyId = TEST_STORY_ID_1
storyId = TEST_STORY_ID_0
)
)
val title = activityTestRule.activity.title
Expand All @@ -131,7 +131,7 @@ class StoryActivityTest {
createStoryActivityIntent(
internalProfileId = internalProfileId,
topicId = TEST_TOPIC_ID_0,
storyId = TEST_STORY_ID_1
storyId = TEST_STORY_ID_0
)
).use {
testCoroutineDispatchers.runCurrent()
Expand All @@ -153,7 +153,7 @@ class StoryActivityTest {
allOf(
hasExtra(
ExplorationActivity.EXPLORATION_ACTIVITY_EXPLORATION_ID_ARGUMENT_KEY,
TEST_EXPLORATION_ID_1
TEST_EXPLORATION_ID_2
),
hasComponent(ExplorationActivity::class.java.name)
)
Expand Down
20 changes: 10 additions & 10 deletions app/src/sharedTest/java/org/oppia/android/app/story/StoryFragmentTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ import org.oppia.android.domain.question.QuestionModule
import org.oppia.android.domain.topic.FRACTIONS_STORY_ID_0
import org.oppia.android.domain.topic.FRACTIONS_TOPIC_ID
import org.oppia.android.domain.topic.PrimeTopicAssetsControllerModule
import org.oppia.android.domain.topic.TEST_STORY_ID_1
import org.oppia.android.domain.topic.TEST_TOPIC_ID_1
import org.oppia.android.domain.topic.TEST_STORY_ID_0
import org.oppia.android.domain.topic.TEST_TOPIC_ID_0
import org.oppia.android.testing.TestLogReportingModule
import org.oppia.android.testing.profile.ProfileTestHelper
import org.oppia.android.testing.robolectric.RobolectricModule
Expand Down Expand Up @@ -303,9 +303,9 @@ class StoryFragmentTest {
).check(
matches(
withText(
"This is outline/summary for Second Exploration. It is very long but " +
"it has to be fully visible. You wil be learning about oppia app in Second Story. " +
"Learn about oppia app via testing in second exploration."
"This is the outline/summary for the first exploration of the story. It is very long " +
"but it has to be fully visible. You wil be learning about Oppia interactions. " +
"There is no second story to follow-up, but there is a second chapter."
)
)
)
Expand All @@ -331,9 +331,9 @@ class StoryFragmentTest {
).check(
matches(
withText(
"This is outline/summary for Second Exploration. It is very long but " +
"it has to be fully visible. You wil be learning about oppia app in Second Story. " +
"Learn about oppia app via testing in second exploration."
"This is the outline/summary for the first exploration of the story. It is very long " +
"but it has to be fully visible. You wil be learning about Oppia interactions. " +
"There is no second story to follow-up, but there is a second chapter."
)
)
)
Expand Down Expand Up @@ -600,8 +600,8 @@ class StoryFragmentTest {
return StoryActivity.createStoryActivityIntent(
ApplicationProvider.getApplicationContext(),
internalProfileId,
TEST_TOPIC_ID_1,
TEST_STORY_ID_1
TEST_TOPIC_ID_0,
TEST_STORY_ID_0
)
}

Expand Down
3 changes: 2 additions & 1 deletion app/src/sharedTest/java/org/oppia/android/app/testing/TopicTestActivityForStoryTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,11 @@ class TopicTestActivityForStoryTest {
fun testTopicTestActivityForStory_playTopicTab_storyItemIsExpanded() {
launch(TopicTestActivityForStory::class.java).use {
testCoroutineDispatchers.runCurrent()
// Story 0 of the topic should be expanded.
onView(
atPositionOnView(
recyclerViewId = R.id.story_summary_recycler_view,
position = 2,
position = 1,
targetViewId = R.id.chapter_recycler_view
)
).check(matches(isDisplayed()))
Expand Down
48 changes: 47 additions & 1 deletion domain/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,61 @@ This library contains the frontend controller and business service logic for the

load("@dagger//:workspace_defs.bzl", "dagger_rules")
load("@io_bazel_rules_kotlin//kotlin:kotlin.bzl", "kt_android_library")
load("//domain:domain_assets.bzl", "generate_assets_list_from_text_protos")
load("//domain:domain_test.bzl", "domain_test")

DOMAIN_ASSETS = generate_assets_list_from_text_protos(
name = "domain_assets",
exploration_file_names = [
"test_exp_id_2",
"13",
"test_exp_id_4",
"umPkwp0L1M0-",
"MjZzEVOG47_1",
"2mzzFVDLuAj8",
"5NWuolNcwH6e",
"k2bQ7z5XHNbK",
"tIoSb3HZFN6e",
],
skills_file_names = [
"skills",
],
story_file_names = [
"test_story_id_0",
"test_story_id_2",
"wANbh4oOClga",
"wAMdg4oOClga",
"xBSdg4oOClga",
],
subtopic_file_names = [
"GJ2rLXRKD5hw_1",
"GJ2rLXRKD5hw_2",
"GJ2rLXRKD5hw_3",
"GJ2rLXRKD5hw_4",
"omzF4oqgeTXd_1",
],
topic_file_names = [
"test_topic_id_0",
"test_topic_id_1",
"test_topic_id_2",
"GJ2rLXRKD5hw",
"omzF4oqgeTXd",
],
topic_list_file_names = [
"topics",
],
)

kt_android_library(
name = "domain",
srcs = glob(
["src/main/java/org/oppia/android/domain/**/*.kt"],
exclude = ["src/main/java/org/oppia/android/domain/testing/**/*.kt"],
),
assets = glob(["src/main/assets/**"]),
assets = glob(
["src/main/assets/**"],
exclude = ["src/main/assets/**/*.textproto"],
) + DOMAIN_ASSETS,
assets_dir = "src/main/assets/",
custom_package = "org.oppia.android.domain",
manifest = "src/main/AndroidManifest.xml",
Expand Down
Loading

0 comments on commit cb1ddc8

Please sign in to comment.