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).
oppia · BenHenning · Apr 15, 2021 · Mar 23, 2021 · Mar 23, 2021 · Mar 23, 2021
commit be5b78558ac468244542d77b03494be6712d2d7a
@@ -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
 
@@ -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
     )
   }
 

@@ -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() {
@@ -513,7 +513,7 @@ class HomeActivityTest {
       profileId = profileId1,
       timestampOlderThanOneWeek = false
     )
-    storyProgressTestHelper.markRecentlyPlayedTestTopic0Story1Exp1(
+    storyProgressTestHelper.markRecentlyPlayedTestTopic1Story0(
       profileId = profileId1,
       timestampOlderThanOneWeek = false
     )
@@ -541,7 +541,7 @@ class HomeActivityTest {
       verifyTextOnPromotedListItemAtPosition(
         itemPosition = 2,
         targetViewId = R.id.topic_name_text_view,
-        stringToMatch = "First Test Topic"
+        stringToMatch = "Second Test Topic"
       )
     }
   }

@@ -51,7 +51,7 @@ import org.oppia.android.domain.oppialogger.loguploader.WorkManagerConfiguration
 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_STORY_ID_0
 import org.oppia.android.domain.topic.TEST_TOPIC_ID_0
 import org.oppia.android.testing.RobolectricModule
 import org.oppia.android.testing.TestCoroutineDispatchers
@@ -101,7 +101,7 @@ class StoryActivityTest {
       createStoryActivityIntent(
         internalProfileId,
         TEST_TOPIC_ID_0,
-        TEST_STORY_ID_1
+        TEST_STORY_ID_0
       )
     ).use {
       testCoroutineDispatchers.runCurrent()

@@ -76,8 +76,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.RobolectricModule
 import org.oppia.android.testing.TestCoroutineDispatchers
 import org.oppia.android.testing.TestDispatcherModule
@@ -502,8 +502,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
     )
   }
 

@@ -23,7 +23,7 @@ def _gen_binary_proto_from_text_impl(ctx):
     input_proto_files = _extract_proto_sources(ctx.attr.proto_deps)
 
     # See 'protoc --help' for specifics on the arguments passed to the tool for converting text
-    # proto to binary, and epxected stdin/stdout configurations. Note that the actual proto files
+    # proto to binary, and expected stdin/stdout configurations. Note that the actual proto files
     # are passed to the compiler since it requires them in order to transcode the text proto file.
     command_path = ctx.executable._protoc_tool.path
     arguments = [command_path] + [
@@ -68,7 +68,7 @@ _gen_binary_proto_from_text = rule(
         "_protoc_tool": attr.label(
             # This was partly inspired by https://stackoverflow.com/a/39138074.
             executable = True,
-            cfg = "exec",
+            cfg = "host",
             default = "@protobuf_tools//:protoc",
         ),
     },
@@ -113,7 +113,7 @@ def _generate_single_asset_proto_binary(name, proto_dep_name, proto_type_name):
         proto_dep_name: str. The name of the proto library under //model that contains the proto
             definition being converted to binary.
         proto_type_name: str. The name of the proto type being converted in the text proto. This is
-            assumed to be part of the shared 'model' packa
+            assumed to be part of the shared 'model' package.
 
     Returns:
         str. The path to the newly generated binary file.

@@ -3,6 +3,7 @@ Macros for domain module tests.
 """
 
 load("//:oppia_android_test.bzl", "oppia_android_module_level_test")
+load("//domain:domain_assets.bzl", "generate_assets_list_from_text_protos")
 
 # TODO(#1620): Remove module-specific test macros once Gradle is removed
 def domain_test(name, filtered_tests, deps):
@@ -14,12 +15,11 @@ def domain_test(name, filtered_tests, deps):
         filtered_tests: list of str. The test files that should not have tests defined for them.
         deps: list of str. The list of dependencies needed to build and run this test.
     """
+
     oppia_android_module_level_test(
         name = name,
         filtered_tests = filtered_tests,
         deps = deps,
         custom_package = "org.oppia.android.domain",
         test_manifest = "src/test/AndroidManifest.xml",
-        assets = native.glob(["src/main/assets/**"]),
-        assets_dir = "src/main/assets/",
     )
@@ -1,5 +1,6 @@
 concept_cards {
   skill_id: "test_skill_id_0"
+  skill_description: "An important skill"
   explanation {
     html: "Hello. Welcome to Oppia."
     content_id: "explanation"
@@ -57,6 +58,7 @@ concept_cards {
 }
 concept_cards {
   skill_id: "test_skill_id_1"
+  skill_description: "Another important skill"
   explanation {
     html: "Explanation with <b>rich text</b>."
     content_id: "explanation"
@@ -88,6 +90,7 @@ concept_cards {
 }
 concept_cards {
   skill_id: "test_skill_id_2"
+  skill_description: "A different skill in a different topic"
   explanation {
     html: "Explanation without rich text."
     content_id: "explanation"
@@ -123,6 +126,7 @@ concept_cards {
 }
 concept_cards {
   skill_id: "test_skill_id_3"
+  skill_description: "Another important skill 4"
   explanation {
     html: "The content for test skill id 3. This is a dummy skill content."
     content_id: "explanation"
@@ -150,6 +154,7 @@ concept_cards {
 }
 concept_cards {
   skill_id: "5RM9KPfQxobH"
+  skill_description: "Given a picture divided into unequal parts, write the fraction."
   explanation {
     html: "<p>First, divide the picture into equal parts, so that you can figure out what the denominator (the total number of parts) is.<br>\n<br>\nThen, count the number of equal parts that are shaded/selected. That is the numerator of the fraction.</p>"
     content_id: "explanation"
@@ -181,6 +186,7 @@ concept_cards {
 }
 concept_cards {
   skill_id: "UxTGIJqaHMLa"
+  skill_description: "Identify the numerator and denominator of a fraction."
   explanation {
     html: "<p>The numerator is the number at the \"top\" of the fraction. It counts (\"enumerates\") how many pieces the fraction represents.<br>\n<br>\nThe denominator is the number at the \"bottom\" of the fraction. It names (\"denominates\") the size of each piece.<br>\n<br>\nFor example, in 2/3, the numerator is 2 and the denominator is 3.</p>"
     content_id: "explanation"
@@ -198,6 +204,7 @@ concept_cards {
 }
 concept_cards {
   skill_id: "B39yK4cbHZYI"
+  skill_description: "Explain the conceptual meaning of the terms \"numerator\" and \"denominator\"."
   explanation {
     html: "<p>The numerator is the number at the \"top\" of the fraction. It counts (\"enumerates\") how many pieces the fraction represents.<br>\n<br>\nThe denominator is the number at the \"bottom\" of the fraction. It names (\"denominates\") what the size of each piece is, based on how many equal parts the \"whole\" is divided into.</p>"
     content_id: "explanation"
@@ -229,6 +236,7 @@ concept_cards {
 }
 concept_cards {
   skill_id: "NGZ89uMw0IGV"
+  skill_description: "Derive a ratio from a description or a picture"
   explanation {
     html: "<p>A ratio represents a relative relationship between two or more amounts. To write a ratio, start by writing these amounts, separating them by a colon. Remember that the order matters!</p><p>For example, let\'s say you have 9 apples and 6 pears. You can write the ratio of apples to pears as <b> 9 : 6.</b></p>If, instead, you have 3 pears, 5 apples and 9 bananas, you can write the ratio of apples to bananas to pears as <b>5 : 9 : 3.</b> Notice that the order of items in the ratio follows the order that was asked for."
     content_id: "explanation"

@@ -11,19 +11,6 @@
       "thumbnail_filename": "",
       "published": true,
       "id": "test_story_id_0"
-    },
-    {
-      "thumbnail_bg_color": "#78B3F1",
-      "description": "",
-      "title": "Second Story",
-      "node_titles": [
-        "Second Exploration",
-        "Third Exploration",
-        "Fourth Exploration"
-      ],
-      "thumbnail_filename": "",
-      "published": true,
-      "id": "test_story_id_1"
     }
   ],
   "uncategorized_skill_ids": [],

@@ -4,4 +4,5 @@ description: "A topic considering the various implications of having especially
 canonical_story_ids: "test_story_id_2"
 is_published: true
 topic_thumbnail {
+  background_color_rgb: 16741119
 }
@@ -1,6 +1,7 @@
 package org.oppia.android.domain.topic
 
 import org.json.JSONArray
+import org.json.JSONException
 import org.json.JSONObject
 import org.oppia.android.app.model.ConceptCard
 import org.oppia.android.app.model.ConceptCardList
@@ -25,81 +26,85 @@ class ConceptCardRetriever @Inject constructor(
    * Returns a [ConceptCard] corresponding to the specified skill ID, loaded from the filesystem.
    */
   fun loadConceptCard(skillId: String): ConceptCard {
-    return if (loadLessonProtosFromAssets) {
+    val conceptCard = if (loadLessonProtosFromAssets) {
       val conceptCardList =
         assetRepository.loadProtoFromLocalAssets(
           assetName = "skills",
           baseMessage = ConceptCardList.getDefaultInstance()
         )
-      return conceptCardList.conceptCardsList.find { it.skillId == skillId }
-        ?: error("Failed to load concept card for skill: $skillId")
+      conceptCardList.conceptCardsList.find { it.skillId == skillId }
     } else loadConceptCardFromJson(skillId)
+    return conceptCard ?: error("Failed to load concept card for skill: $skillId")
   }
 
-  private fun loadConceptCardFromJson(skillId: String): ConceptCard {
-    val skillData = getSkillJsonObject(skillId)
-    if (skillData.length() <= 0) {
-      return ConceptCard.getDefaultInstance()
-    }
-    val skillContents = skillData.getJSONObject("skill_contents")
-    val workedExamplesList = createWorkedExamplesFromJson(
-      skillContents.getJSONArray(
-        "worked_examples"
+  private fun loadConceptCardFromJson(skillId: String): ConceptCard? {
+    try {
+      val skillData = getSkillJsonObject(skillId)
+      if (skillData.length() <= 0) {
+        return ConceptCard.getDefaultInstance()
+      }
+      val skillContents = skillData.getJSONObject("skill_contents")
+      val workedExamplesList = createWorkedExamplesFromJson(
+        skillContents.getJSONArray(
+          "worked_examples"
+        )
       )
-    )
 
-    val recordedVoiceoverMapping = hashMapOf<String, VoiceoverMapping>()
-    recordedVoiceoverMapping["explanation"] = createRecordedVoiceoversFromJson(
-      skillContents
-        .optJSONObject("recorded_voiceovers")
-        .optJSONObject("voiceovers_mapping")
-        .optJSONObject(
-          skillContents.optJSONObject("explanation").optString("content_id")
-        )!!
-    )
-    for (workedExample in workedExamplesList) {
-      recordedVoiceoverMapping[workedExample.contentId] = createRecordedVoiceoversFromJson(
+      val recordedVoiceoverMapping = hashMapOf<String, VoiceoverMapping>()
+      recordedVoiceoverMapping["explanation"] = createRecordedVoiceoversFromJson(
         skillContents
           .optJSONObject("recorded_voiceovers")
           .optJSONObject("voiceovers_mapping")
-          .optJSONObject(workedExample.contentId)
+          .optJSONObject(
+            skillContents.optJSONObject("explanation").optString("content_id")
+          )!!
       )
-    }
+      for (workedExample in workedExamplesList) {
+        recordedVoiceoverMapping[workedExample.contentId] = createRecordedVoiceoversFromJson(
+          skillContents
+            .optJSONObject("recorded_voiceovers")
+            .optJSONObject("voiceovers_mapping")
+            .optJSONObject(workedExample.contentId)
+        )
+      }
 
-    val writtenTranslationMapping = hashMapOf<String, TranslationMapping>()
-    writtenTranslationMapping["explanation"] = createWrittenTranslationFromJson(
-      skillContents
-        .optJSONObject("written_translations")
-        .optJSONObject("translations_mapping")
-        .optJSONObject(
-          skillContents.optJSONObject("explanation").optString("content_id")
-        )!!
-    )
-    for (workedExample in workedExamplesList) {
-      writtenTranslationMapping[workedExample.contentId] = createWrittenTranslationFromJson(
+      val writtenTranslationMapping = hashMapOf<String, TranslationMapping>()
+      writtenTranslationMapping["explanation"] = createWrittenTranslationFromJson(
         skillContents
           .optJSONObject("written_translations")
           .optJSONObject("translations_mapping")
-          .optJSONObject(workedExample.contentId)
+          .optJSONObject(
+            skillContents.optJSONObject("explanation").optString("content_id")
+          )!!
       )
-    }
+      for (workedExample in workedExamplesList) {
+        writtenTranslationMapping[workedExample.contentId] = createWrittenTranslationFromJson(
+          skillContents
+            .optJSONObject("written_translations")
+            .optJSONObject("translations_mapping")
+            .optJSONObject(workedExample.contentId)
+        )
+      }
 
-    return ConceptCard.newBuilder()
-      .setSkillId(skillData.getString("id"))
-      .setSkillDescription(skillData.getString("description"))
-      .setExplanation(
-        SubtitledHtml.newBuilder()
-          .setHtml(skillContents.getJSONObject("explanation").getString("html"))
-          .setContentId(
-            skillContents.getJSONObject("explanation").getString(
-              "content_id"
-            )
-          ).build()
-      )
-      .addAllWorkedExample(workedExamplesList)
-      .putAllWrittenTranslation(writtenTranslationMapping)
-      .putAllRecordedVoiceover(recordedVoiceoverMapping)
-      .build()
+      return ConceptCard.newBuilder()
+        .setSkillId(skillData.getString("id"))
+        .setSkillDescription(skillData.getString("description"))
+        .setExplanation(
+          SubtitledHtml.newBuilder()
+            .setHtml(skillContents.getJSONObject("explanation").getString("html"))
+            .setContentId(
+              skillContents.getJSONObject("explanation").getString(
+                "content_id"
+              )
+            ).build()
+        )
+        .addAllWorkedExample(workedExamplesList)
+        .putAllWrittenTranslation(writtenTranslationMapping)
+        .putAllRecordedVoiceover(recordedVoiceoverMapping)
+        .build()
+    } catch (e: JSONException) {
+      return null
+    }
   }
 
   private fun getSkillJsonObject(skillId: String): JSONObject {

@@ -17,7 +17,6 @@ import javax.inject.Inject
 import javax.inject.Singleton
 
 const val TEST_STORY_ID_0 = "test_story_id_0"
-const val TEST_STORY_ID_1 = "test_story_id_1"
 const val TEST_STORY_ID_2 = "test_story_id_2"
 const val FRACTIONS_STORY_ID_0 = "wANbh4oOClga"
 const val RATIOS_STORY_ID_0 = "wAMdg4oOClga"

@@ -64,7 +64,6 @@ val STORY_THUMBNAILS = mapOf(
   RATIOS_STORY_ID_0 to createStoryThumbnail1(),
   RATIOS_STORY_ID_1 to createStoryThumbnail2(),
   TEST_STORY_ID_0 to createStoryThumbnail3(),
-  TEST_STORY_ID_1 to createStoryThumbnail4(),
   TEST_STORY_ID_2 to createStoryThumbnail5()
 )
 val EXPLORATION_THUMBNAILS = mapOf(