From dee27c04d72730c73c7dacea1d9f63608347a93f Mon Sep 17 00:00:00 2001 From: "Mr. 17" Date: Wed, 26 Jun 2024 17:35:57 +0530 Subject: [PATCH 1/4] Fix part of #5344: Introduce & modify controllers to support multiple classrooms (#5419) ## Explanation Fixes part of #5344 - Introduces `ClassroomController` to surface `getClassroomList` function and adds relevant tests. - Adds `getTopicList` function in the `ClassroomController` and its tests. - Modifies `ProfileManagementController` to store the `last_selected_classroom_id` per profile and adds its tests. ## Essential Checklist - [x] The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".) - [x] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)). - [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)). ## For UI-specific PRs only If your PR includes UI-related changes, then: - Add screenshots for portrait/landscape for both a tablet & phone of the before & after UI changes - For the screenshots above, include both English and pseudo-localized (RTL) screenshots (see [RTL guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines)) - Add a video showing the full UX flow with a screen reader enabled (see [accessibility guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide)) - For PRs introducing new UI elements or color changes, both light and dark mode screenshots must be included - Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing --- .../domain/classroom/ClassroomController.kt | 385 ++++++++++++++++++ .../profile/ProfileManagementController.kt | 45 ++ .../domain/topic/TopicListController.kt | 6 +- .../classroom/ClassroomControllerTest.kt | 354 ++++++++++++++++ .../ProfileManagementControllerTest.kt | 110 +++++ .../domain/topic/TopicListControllerTest.kt | 3 + model/src/main/proto/profile.proto | 3 + model/src/main/proto/thumbnail.proto | 9 + 8 files changed, 910 insertions(+), 5 deletions(-) create mode 100644 domain/src/main/java/org/oppia/android/domain/classroom/ClassroomController.kt create mode 100644 domain/src/test/java/org/oppia/android/domain/classroom/ClassroomControllerTest.kt diff --git a/domain/src/main/java/org/oppia/android/domain/classroom/ClassroomController.kt b/domain/src/main/java/org/oppia/android/domain/classroom/ClassroomController.kt new file mode 100644 index 00000000000..69c2acffa78 --- /dev/null +++ b/domain/src/main/java/org/oppia/android/domain/classroom/ClassroomController.kt @@ -0,0 +1,385 @@ +package org.oppia.android.domain.classroom + +import android.graphics.Color +import org.json.JSONObject +import org.oppia.android.app.model.ClassroomIdList +import org.oppia.android.app.model.ClassroomList +import org.oppia.android.app.model.ClassroomRecord +import org.oppia.android.app.model.ClassroomSummary +import org.oppia.android.app.model.EphemeralClassroomSummary +import org.oppia.android.app.model.EphemeralTopicSummary +import org.oppia.android.app.model.LessonThumbnail +import org.oppia.android.app.model.LessonThumbnailGraphic +import org.oppia.android.app.model.ProfileId +import org.oppia.android.app.model.StoryRecord +import org.oppia.android.app.model.SubtitledHtml +import org.oppia.android.app.model.TopicList +import org.oppia.android.app.model.TopicPlayAvailability +import org.oppia.android.app.model.TopicPlayAvailability.AvailabilityCase.AVAILABLE_TO_PLAY_NOW +import org.oppia.android.app.model.TopicRecord +import org.oppia.android.app.model.TopicSummary +import org.oppia.android.domain.topic.createTopicThumbnailFromJson +import org.oppia.android.domain.translation.TranslationController +import org.oppia.android.domain.util.JsonAssetRetriever +import org.oppia.android.domain.util.getStringFromObject +import org.oppia.android.util.caching.AssetRepository +import org.oppia.android.util.caching.LoadLessonProtosFromAssets +import org.oppia.android.util.data.DataProvider +import org.oppia.android.util.data.DataProviders.Companion.transform +import org.oppia.android.util.locale.OppiaLocale +import java.util.concurrent.TimeUnit +import javax.inject.Inject +import javax.inject.Singleton + +/** ID of test classroom 0. */ +const val TEST_CLASSROOM_ID_0 = "test_classroom_id_0" + +/** ID of test classroom 1. */ +const val TEST_CLASSROOM_ID_1 = "test_classroom_id_1" + +/** ID of test classroom 2. */ +const val TEST_CLASSROOM_ID_2 = "test_classroom_id_2" + +/** Map of classroom ID to its thumbnail. */ +val CLASSROOM_THUMBNAILS = mapOf( + TEST_CLASSROOM_ID_0 to createClassroomThumbnail0(), + TEST_CLASSROOM_ID_1 to createClassroomThumbnail1(), + TEST_CLASSROOM_ID_2 to createClassroomThumbnail2(), +) + +private const val CLASSROOM_BG_COLOR = "#C6DCDA" + +private const val GET_CLASSROOM_LIST_PROVIDER_ID = "get_classroom_list_provider_id" +private const val GET_TOPIC_LIST_PROVIDER_ID = "get_topic_list_provider_id" + +private val EVICTION_TIME_MILLIS = TimeUnit.DAYS.toMillis(1) + +/** Controller for retrieving the list of classrooms & topics available to the learner. */ +@Singleton +class ClassroomController @Inject constructor( + private val jsonAssetRetriever: JsonAssetRetriever, + private val assetRepository: AssetRepository, + private val translationController: TranslationController, + @LoadLessonProtosFromAssets private val loadLessonProtosFromAssets: Boolean, +) { + /** Returns the list of [ClassroomSummary]s currently tracked by the app. */ + fun getClassroomList(profileId: ProfileId): DataProvider { + val translationLocaleProvider = + translationController.getWrittenTranslationContentLocale(profileId) + return translationLocaleProvider.transform( + GET_CLASSROOM_LIST_PROVIDER_ID, + ::createClassroomList + ) + } + + /** + * Returns the list of [TopicSummary]s currently tracked by the app, possibly up to + * [EVICTION_TIME_MILLIS] old. + */ + fun getTopicList(profileId: ProfileId, classroomId: String): DataProvider { + val translationLocaleProvider = + translationController.getWrittenTranslationContentLocale(profileId) + return translationLocaleProvider.transform(GET_TOPIC_LIST_PROVIDER_ID) { contentLocale -> + createTopicList(classroomId, contentLocale) + } + } + + private fun createClassroomList( + contentLocale: OppiaLocale.ContentLocale + ): ClassroomList { + return if (loadLessonProtosFromAssets) + loadClassroomListFromProto(contentLocale) + else + loadClassroomListFromJson(contentLocale) + } + + private fun loadClassroomListFromProto(contentLocale: OppiaLocale.ContentLocale): ClassroomList { + val classroomIdList = assetRepository.loadProtoFromLocalAssets( + assetName = "classrooms", + baseMessage = ClassroomIdList.getDefaultInstance() + ) + return ClassroomList.newBuilder().apply { + addAllClassroomSummary( + classroomIdList.classroomIdsList.map { classroomId -> + createEphemeralClassroomSummary(classroomId, contentLocale) + }.filter { ephemeralClassroomSummary -> + ephemeralClassroomSummary.classroomSummary.topicSummaryList.any { topicSummary -> + topicSummary.topicPlayAvailability.availabilityCase == AVAILABLE_TO_PLAY_NOW + } + } + ) + }.build() + } + + private fun loadClassroomListFromJson(contentLocale: OppiaLocale.ContentLocale): ClassroomList { + val classroomIdJsonArray = jsonAssetRetriever + .loadJsonFromAsset("classrooms.json") + ?.getJSONArray("classroom_id_list") + ?: return ClassroomList.getDefaultInstance() + val classroomListBuilder = ClassroomList.newBuilder() + for (i in 0 until classroomIdJsonArray.length()) { + val classroomId = classroomIdJsonArray.optString(i) + val ephemeralClassroomSummary = createEphemeralClassroomSummary(classroomId, contentLocale) + val hasPublishedTopics = + ephemeralClassroomSummary.classroomSummary.topicSummaryList.any { topicSummary -> + topicSummary.topicPlayAvailability.availabilityCase == AVAILABLE_TO_PLAY_NOW + } + if (hasPublishedTopics) classroomListBuilder.addClassroomSummary(ephemeralClassroomSummary) + } + return classroomListBuilder.build() + } + + private fun createEphemeralClassroomSummary( + classroomId: String, + contentLocale: OppiaLocale.ContentLocale + ): EphemeralClassroomSummary { + return EphemeralClassroomSummary.newBuilder().apply { + classroomSummary = createClassroomSummary(classroomId) + writtenTranslationContext = + translationController.computeWrittenTranslationContext( + classroomSummary.writtenTranslationsMap, contentLocale + ) + }.build() + } + + private fun createClassroomSummary(classroomId: String): ClassroomSummary { + return if (loadLessonProtosFromAssets) { + val classroomRecord = assetRepository.loadProtoFromLocalAssets( + assetName = classroomId, + baseMessage = ClassroomRecord.getDefaultInstance() + ) + return ClassroomSummary.newBuilder().apply { + this.classroomId = classroomId + putAllWrittenTranslations(classroomRecord.writtenTranslationsMap) + classroomTitle = classroomRecord.translatableTitle + classroomThumbnail = createClassroomThumbnailFromProto( + classroomId, + classroomRecord.classroomThumbnail + ) + addAllTopicSummary( + classroomRecord.topicPrerequisitesMap.keys.toList().map { topicId -> + createTopicSummary(topicId, classroomId) + } + ) + }.build() + } else createClassroomSummaryFromJson(classroomId) + } + + private fun createClassroomSummaryFromJson(classroomId: String): ClassroomSummary { + val classroomJsonObject = jsonAssetRetriever + .loadJsonFromAsset("$classroomId.json") + ?: return ClassroomSummary.getDefaultInstance() + return ClassroomSummary.newBuilder().apply { + setClassroomId(classroomJsonObject.getStringFromObject("classroom_id")) + classroomTitle = SubtitledHtml.newBuilder().apply { + val classroomTitleObj = classroomJsonObject.getJSONObject("classroom_title") + contentId = classroomTitleObj.getStringFromObject("content_id") + html = classroomTitleObj.getStringFromObject("html") + }.build() + classroomThumbnail = createClassroomThumbnailFromJson(classroomJsonObject) + val topicIdArray = classroomJsonObject + .getJSONObject("topic_prerequisites").keys().asSequence().toList() + val topicSummaryList = mutableListOf() + topicIdArray.forEach { topicId -> + topicSummaryList.add(createTopicSummary(topicId, classroomId)) + } + addAllTopicSummary(topicSummaryList) + }.build() + } + + private fun createTopicList( + classroomId: String, + contentLocale: OppiaLocale.ContentLocale + ): TopicList { + return TopicList.newBuilder().apply { + addAllTopicSummary( + getTopicIdListFromClassroomRecord(classroomId).topicIdsList.map { topicId -> + createEphemeralTopicSummary(topicId, classroomId, contentLocale) + }.filter { + it.topicSummary.topicPlayAvailability.availabilityCase == AVAILABLE_TO_PLAY_NOW + } + ) + }.build() + } + + private fun getTopicIdListFromClassroomRecord(classroomId: String): ClassroomRecord.TopicIdList { + return if (loadLessonProtosFromAssets) { + val classroomRecord = assetRepository.loadProtoFromLocalAssets( + assetName = classroomId, + baseMessage = ClassroomRecord.getDefaultInstance() + ) + ClassroomRecord.TopicIdList.newBuilder().apply { + addAllTopicIds(classroomRecord.topicPrerequisitesMap.keys.toList()) + }.build() + } else { + val classroomJsonObject = jsonAssetRetriever + .loadJsonFromAsset("$classroomId.json") + ?: return ClassroomRecord.TopicIdList.getDefaultInstance() + val topicIdArray = classroomJsonObject + .getJSONObject("topic_prerequisites").keys().asSequence().toList() + ClassroomRecord.TopicIdList.newBuilder().apply { + topicIdArray.forEach { topicId -> + addTopicIds(topicId) + } + }.build() + } + } + + private fun createEphemeralTopicSummary( + topicId: String, + classroomId: String, + contentLocale: OppiaLocale.ContentLocale + ): EphemeralTopicSummary { + val topicSummary = createTopicSummary(topicId, classroomId) + val classroomSummary = createClassroomSummary(classroomId) + return EphemeralTopicSummary.newBuilder().apply { + this.topicSummary = topicSummary + writtenTranslationContext = + translationController.computeWrittenTranslationContext( + topicSummary.writtenTranslationsMap, contentLocale + ) + classroomWrittenTranslationContext = + translationController.computeWrittenTranslationContext( + classroomSummary.writtenTranslationsMap, contentLocale + ) + classroomTitle = classroomSummary.classroomTitle + }.build() + } + + private fun createTopicSummary(topicId: String, classroomId: String): TopicSummary { + return if (loadLessonProtosFromAssets) { + val topicRecord = + assetRepository.loadProtoFromLocalAssets( + assetName = topicId, + baseMessage = TopicRecord.getDefaultInstance() + ) + val storyRecords = topicRecord.canonicalStoryIdsList.map { + assetRepository.loadProtoFromLocalAssets( + assetName = it, + baseMessage = StoryRecord.getDefaultInstance() + ) + } + TopicSummary.newBuilder().apply { + this.topicId = topicId + putAllWrittenTranslations(topicRecord.writtenTranslationsMap) + title = topicRecord.translatableTitle + this.classroomId = classroomId + totalChapterCount = storyRecords.map { it.chaptersList.size }.sum() + topicThumbnail = topicRecord.topicThumbnail + topicPlayAvailability = if (topicRecord.isPublished) { + TopicPlayAvailability.newBuilder().setAvailableToPlayNow(true).build() + } else { + TopicPlayAvailability.newBuilder().setAvailableToPlayInFuture(true).build() + } + storyRecords.firstOrNull()?.storyId?.let { this.firstStoryId = it } + }.build() + } else { + val topicJsonObject = jsonAssetRetriever + .loadJsonFromAsset("$topicId.json") + ?: return TopicSummary.getDefaultInstance() + createTopicSummaryFromJson(topicId, classroomId, topicJsonObject) + } + } + + private fun createTopicSummaryFromJson( + topicId: String, + classroomId: String, + jsonObject: JSONObject + ): TopicSummary { + var totalChapterCount = 0 + val storyData = jsonObject.getJSONArray("canonical_story_dicts") + for (i in 0 until storyData.length()) { + totalChapterCount += storyData + .getJSONObject(i) + .getJSONArray("node_titles") + .length() + } + val firstStoryId = + if (storyData.length() == 0) "" else storyData.getJSONObject(0).getStringFromObject("id") + + val topicPlayAvailability = if (jsonObject.getBoolean("published")) { + TopicPlayAvailability.newBuilder().setAvailableToPlayNow(true).build() + } else { + TopicPlayAvailability.newBuilder().setAvailableToPlayInFuture(true).build() + } + val topicTitle = SubtitledHtml.newBuilder().apply { + contentId = "title" + html = jsonObject.getStringFromObject("topic_name") + }.build() + // No written translations are included since none are retrieved from JSON. + return TopicSummary.newBuilder() + .setTopicId(topicId) + .setTitle(topicTitle) + .setClassroomId(classroomId) + .setVersion(jsonObject.optInt("version")) + .setTotalChapterCount(totalChapterCount) + .setTopicThumbnail(createTopicThumbnailFromJson(jsonObject)) + .setTopicPlayAvailability(topicPlayAvailability) + .setFirstStoryId(firstStoryId) + .build() + } +} + +/** Creates a [LessonThumbnail] from a classroomJsonObject. */ +internal fun createClassroomThumbnailFromJson(classroomJsonObject: JSONObject): LessonThumbnail { + val classroomId = classroomJsonObject.optString("classroom_id") + val thumbnailBgColor = classroomJsonObject.optString("thumbnail_bg_color") + val thumbnailFilename = classroomJsonObject.optString("thumbnail_filename") + return if (thumbnailFilename.isNotNullOrEmpty() && thumbnailBgColor.isNotNullOrEmpty()) { + LessonThumbnail.newBuilder() + .setThumbnailFilename(thumbnailFilename) + .setBackgroundColorRgb(Color.parseColor(thumbnailBgColor)) + .build() + } else if (CLASSROOM_THUMBNAILS.containsKey(classroomId)) { + CLASSROOM_THUMBNAILS.getValue(classroomId) + } else { + createDefaultClassroomThumbnail() + } +} + +/** Creates a [LessonThumbnail] from a classroom proto. */ +internal fun createClassroomThumbnailFromProto( + classroomId: String, + lessonThumbnail: LessonThumbnail +): LessonThumbnail { + val thumbnailFilename = lessonThumbnail.thumbnailFilename + return when { + thumbnailFilename.isNotNullOrEmpty() -> lessonThumbnail + CLASSROOM_THUMBNAILS.containsKey(classroomId) -> CLASSROOM_THUMBNAILS.getValue(classroomId) + else -> createDefaultClassroomThumbnail() + } +} + +/** Creates a default [LessonThumbnail]. */ +internal fun createDefaultClassroomThumbnail(): LessonThumbnail { + return LessonThumbnail.newBuilder() + .setThumbnailGraphic(LessonThumbnailGraphic.MATHS_CLASSROOM) + .setBackgroundColorRgb(Color.parseColor(CLASSROOM_BG_COLOR)) + .build() +} + +/** Creates a [LessonThumbnail] for [TEST_CLASSROOM_ID_0]. */ +internal fun createClassroomThumbnail0(): LessonThumbnail { + return LessonThumbnail.newBuilder() + .setThumbnailGraphic(LessonThumbnailGraphic.SCIENCE_CLASSROOM) + .setBackgroundColorRgb(Color.parseColor(CLASSROOM_BG_COLOR)) + .build() +} + +/** Creates a [LessonThumbnail] for [TEST_CLASSROOM_ID_1]. */ +internal fun createClassroomThumbnail1(): LessonThumbnail { + return LessonThumbnail.newBuilder() + .setThumbnailGraphic(LessonThumbnailGraphic.MATHS_CLASSROOM) + .setBackgroundColorRgb(Color.parseColor(CLASSROOM_BG_COLOR)) + .build() +} + +/** Creates a [LessonThumbnail] for [TEST_CLASSROOM_ID_2]. */ +internal fun createClassroomThumbnail2(): LessonThumbnail { + return LessonThumbnail.newBuilder() + .setThumbnailGraphic(LessonThumbnailGraphic.ENGLISH_CLASSROOM) + .setBackgroundColorRgb(Color.parseColor(CLASSROOM_BG_COLOR)) + .build() +} + +private fun String?.isNotNullOrEmpty(): Boolean = !this.isNullOrBlank() || this != "null" diff --git a/domain/src/main/java/org/oppia/android/domain/profile/ProfileManagementController.kt b/domain/src/main/java/org/oppia/android/domain/profile/ProfileManagementController.kt index 8e3da7fb459..51db3f941bb 100644 --- a/domain/src/main/java/org/oppia/android/domain/profile/ProfileManagementController.kt +++ b/domain/src/main/java/org/oppia/android/domain/profile/ProfileManagementController.kt @@ -71,6 +71,10 @@ private const val SET_SURVEY_LAST_SHOWN_TIMESTAMP_PROVIDER_ID = "record_survey_last_shown_timestamp_provider_id" private const val RETRIEVE_SURVEY_LAST_SHOWN_TIMESTAMP_PROVIDER_ID = "retrieve_survey_last_shown_timestamp_provider_id" +private const val SET_LAST_SELECTED_CLASSROOM_ID_PROVIDER_ID = + "set_last_selected_classroom_id_provider_id" +private const val RETRIEVE_LAST_SELECTED_CLASSROOM_ID_PROVIDER_ID = + "retrieve_last_selected_classroom_id_provider_id" /** Controller for retrieving, adding, updating, and deleting profiles. */ @Singleton @@ -851,6 +855,47 @@ class ProfileManagementController @Inject constructor( } } + /** + * Sets the last selected [classroomId] for the specified [profileId]. Returns a [DataProvider] + * indicating whether the save was a success. + */ + fun updateLastSelectedClassroomId( + profileId: ProfileId, + classroomId: String + ): DataProvider { + val deferred = profileDataStore.storeDataWithCustomChannelAsync( + updateInMemoryCache = true + ) { profileDatabase -> + val profile = profileDatabase.profilesMap[profileId.internalId] + val updatedProfile = profile?.toBuilder()?.setLastSelectedClassroomId( + classroomId + )?.build() + val profileDatabaseBuilder = profileDatabase.toBuilder().putProfiles( + profileId.internalId, + updatedProfile + ) + Pair(profileDatabaseBuilder.build(), ProfileActionStatus.SUCCESS) + } + return dataProviders.createInMemoryDataProviderAsync( + SET_LAST_SELECTED_CLASSROOM_ID_PROVIDER_ID + ) { + return@createInMemoryDataProviderAsync getDeferredResult(profileId, null, deferred) + } + } + + /** + * Returns a [DataProvider] containing a nullable last selected classroom ID for the specified + * [profileId]. + */ + fun retrieveLastSelectedClassroomId( + profileId: ProfileId + ): DataProvider { + return profileDataStore.transformAsync(RETRIEVE_LAST_SELECTED_CLASSROOM_ID_PROVIDER_ID) { + val lastSelectedClassroomId = it.profilesMap[profileId.internalId]?.lastSelectedClassroomId + AsyncResult.Success(lastSelectedClassroomId) + } + } + private suspend fun getDeferredResult( profileId: ProfileId?, name: String?, diff --git a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt index 067763a9963..4fa2ed9edef 100644 --- a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt +++ b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt @@ -29,6 +29,7 @@ import org.oppia.android.app.model.TopicProgress import org.oppia.android.app.model.TopicRecord import org.oppia.android.app.model.TopicSummary import org.oppia.android.app.model.UpcomingTopic +import org.oppia.android.domain.classroom.TEST_CLASSROOM_ID_0 import org.oppia.android.domain.translation.TranslationController import org.oppia.android.domain.util.JsonAssetRetriever import org.oppia.android.domain.util.getStringFromObject @@ -60,11 +61,6 @@ const val SUBTOPIC_TOPIC_ID = 1 const val SUBTOPIC_TOPIC_ID_2 = 2 const val RATIOS_TOPIC_ID = "omzF4oqgeTXd" -// TODO(#5344): Move these classroom ids to [ClassroomController]. -const val TEST_CLASSROOM_ID_0 = "test_classroom_id_0" -const val TEST_CLASSROOM_ID_1 = "test_classroom_id_1" -const val TEST_CLASSROOM_ID_2 = "test_classroom_id_2" - val TOPIC_THUMBNAILS = mapOf( FRACTIONS_TOPIC_ID to createTopicThumbnail0(), RATIOS_TOPIC_ID to createTopicThumbnail1(), diff --git a/domain/src/test/java/org/oppia/android/domain/classroom/ClassroomControllerTest.kt b/domain/src/test/java/org/oppia/android/domain/classroom/ClassroomControllerTest.kt new file mode 100644 index 00000000000..9ff4b0f29fc --- /dev/null +++ b/domain/src/test/java/org/oppia/android/domain/classroom/ClassroomControllerTest.kt @@ -0,0 +1,354 @@ +package org.oppia.android.domain.classroom + +import android.app.Application +import android.content.Context +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.google.common.truth.Truth.assertThat +import dagger.BindsInstance +import dagger.Component +import dagger.Module +import dagger.Provides +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.oppia.android.app.model.ProfileId +import org.oppia.android.domain.oppialogger.LogStorageModule +import org.oppia.android.domain.oppialogger.LoggingIdentifierModule +import org.oppia.android.domain.oppialogger.analytics.ApplicationLifecycleModule +import org.oppia.android.domain.platformparameter.PlatformParameterSingletonModule +import org.oppia.android.domain.topic.FRACTIONS_TOPIC_ID +import org.oppia.android.domain.topic.RATIOS_TOPIC_ID +import org.oppia.android.domain.topic.TEST_TOPIC_ID_0 +import org.oppia.android.domain.topic.TEST_TOPIC_ID_1 +import org.oppia.android.domain.topic.TEST_TOPIC_ID_2 +import org.oppia.android.testing.OppiaTestRule +import org.oppia.android.testing.TestLogReportingModule +import org.oppia.android.testing.data.DataProviderTestMonitor +import org.oppia.android.testing.environment.TestEnvironmentConfig +import org.oppia.android.testing.platformparameter.TestPlatformParameterModule +import org.oppia.android.testing.robolectric.RobolectricModule +import org.oppia.android.testing.threading.TestDispatcherModule +import org.oppia.android.testing.time.FakeOppiaClockModule +import org.oppia.android.util.caching.AssetRepository +import org.oppia.android.util.caching.LoadLessonProtosFromAssets +import org.oppia.android.util.caching.testing.FakeAssetRepository +import org.oppia.android.util.data.DataProvidersInjector +import org.oppia.android.util.data.DataProvidersInjectorProvider +import org.oppia.android.util.gcsresource.DefaultResourceBucketName +import org.oppia.android.util.locale.LocaleProdModule +import org.oppia.android.util.logging.EnableConsoleLog +import org.oppia.android.util.logging.EnableFileLog +import org.oppia.android.util.logging.GlobalLogLevel +import org.oppia.android.util.logging.LogLevel +import org.oppia.android.util.logging.SyncStatusModule +import org.oppia.android.util.networking.NetworkConnectionUtilDebugModule +import org.oppia.android.util.parser.image.DefaultGcsPrefix +import org.oppia.android.util.parser.image.ImageDownloadUrlTemplate +import org.robolectric.annotation.Config +import org.robolectric.annotation.LooperMode +import javax.inject.Inject +import javax.inject.Singleton + +/** Tests for [ClassroomController]. */ +// FunctionName: test names are conventionally named with underscores. +@Suppress("FunctionName") +@RunWith(AndroidJUnit4::class) +@LooperMode(LooperMode.Mode.PAUSED) +@Config(application = ClassroomControllerTest.TestApplication::class) +class ClassroomControllerTest { + @get:Rule + val oppiaTestRule = OppiaTestRule() + + @Inject + lateinit var classroomController: ClassroomController + + @Inject + lateinit var monitorFactory: DataProviderTestMonitor.Factory + + private lateinit var profileId0: ProfileId + + @Before + fun setUp() { + profileId0 = ProfileId.newBuilder().setInternalId(0).build() + TestPlatformParameterModule.forceEnableMultipleClassrooms(true) + setUpTestApplicationComponent() + } + + @Test + fun testGetClassroomList_isSuccessful() { + val classroomListProvider = classroomController.getClassroomList(profileId0) + + monitorFactory.waitForNextSuccessfulResult(classroomListProvider) + } + + @Test + fun testGetClassroomList_providesListOfMultipleClassrooms() { + val classroomList = getClassroomList(profileId0) + + assertThat(classroomList.classroomSummaryList.size).isGreaterThan(1) + } + + @Test + fun testGetClassroomList_firstClassroom_hasCorrectClassroomInfo() { + val classroomList = getClassroomList(profileId0) + + val firstClassroom = classroomList.classroomSummaryList[0] + assertThat(firstClassroom.classroomSummary.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + assertThat(firstClassroom.classroomSummary.classroomTitle.html).isEqualTo("Science") + } + + @Test + fun testGetClassroomList_firstClassroom_hasCorrectTopicCount() { + val classroomList = getClassroomList(profileId0) + + val firstClassroom = classroomList.classroomSummaryList[0] + assertThat(firstClassroom.classroomSummary.topicSummaryCount).isEqualTo(2) + } + + @Test + fun testGetClassroomList_secondClassroom_hasCorrectClassroomInfo() { + val classroomList = getClassroomList(profileId0) + + val secondClassroom = classroomList.classroomSummaryList[1] + assertThat(secondClassroom.classroomSummary.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) + assertThat(secondClassroom.classroomSummary.classroomTitle.html).isEqualTo("Maths") + } + + @Test + fun testGetClassroomList_secondClassroom_hasCorrectTopicCount() { + val classroomList = getClassroomList(profileId0) + + val secondClassroom = classroomList.classroomSummaryList[1] + assertThat(secondClassroom.classroomSummary.topicSummaryCount).isEqualTo(2) + } + + @Test + fun testGetClassroomList_noPublishedTopicsInThirdClassroom_checkListExcludesThirdClassroom() { + val classroomList = getClassroomList(profileId0) + + assertThat(classroomList.classroomSummaryList.size).isEqualTo(2) + } + + @Test + fun testRetrieveTopicList_isSuccessful() { + val topicListProvider = classroomController.getTopicList(profileId0, TEST_CLASSROOM_ID_0) + + monitorFactory.waitForNextSuccessfulResult(topicListProvider) + } + + @Test + fun testRetrieveTopicList_testTopic0_hasCorrectTopicInfo() { + val topicList = retrieveTopicList(TEST_CLASSROOM_ID_0) + + val firstTopic = topicList.getTopicSummary(0).topicSummary + assertThat(firstTopic.topicId).isEqualTo(TEST_TOPIC_ID_0) + assertThat(firstTopic.title.html).isEqualTo("First Test Topic") + } + + @Test + fun testRetrieveTopicList_testTopic0_hasCorrectClassroomInfo() { + val topicList = retrieveTopicList(TEST_CLASSROOM_ID_0) + + val firstTopic = topicList.getTopicSummary(0) + assertThat(firstTopic.topicSummary.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + assertThat(firstTopic.classroomTitle.html).isEqualTo("Science") + } + + @Test + fun testRetrieveTopicList_testTopic0_hasCorrectLessonCount() { + val topicList = retrieveTopicList(TEST_CLASSROOM_ID_0) + + val firstTopic = topicList.getTopicSummary(0).topicSummary + assertThat(firstTopic.totalChapterCount).isEqualTo(3) + } + + @Test + fun testRetrieveTopicList_testTopic1_hasCorrectTopicInfo() { + val topicList = retrieveTopicList(TEST_CLASSROOM_ID_0) + + val secondTopic = topicList.getTopicSummary(1).topicSummary + assertThat(secondTopic.topicId).isEqualTo(TEST_TOPIC_ID_1) + assertThat(secondTopic.title.html).isEqualTo("Second Test Topic") + } + + @Test + fun testRetrieveTopicList_testTopic1_hasCorrectClassroomInfo() { + val topicList = retrieveTopicList(TEST_CLASSROOM_ID_0) + + val firstTopic = topicList.getTopicSummary(1) + assertThat(firstTopic.topicSummary.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + assertThat(firstTopic.classroomTitle.html).isEqualTo("Science") + } + + @Test + fun testRetrieveTopicList_testTopic1_hasCorrectLessonCount() { + val topicList = retrieveTopicList(TEST_CLASSROOM_ID_0) + + val secondTopic = topicList.getTopicSummary(1).topicSummary + assertThat(secondTopic.totalChapterCount).isEqualTo(1) + } + + @Test + fun testRetrieveTopicList_fractionsTopic_hasCorrectTopicInfo() { + val topicList = retrieveTopicList(TEST_CLASSROOM_ID_1) + + val fractionsTopic = topicList.getTopicSummary(0).topicSummary + assertThat(fractionsTopic.topicId).isEqualTo(FRACTIONS_TOPIC_ID) + assertThat(fractionsTopic.title.html).isEqualTo("Fractions") + } + + @Test + fun testRetrieveTopicList_fractionsTopic_hasCorrectClassroomInfo() { + val topicList = retrieveTopicList(TEST_CLASSROOM_ID_1) + + val firstTopic = topicList.getTopicSummary(0) + assertThat(firstTopic.topicSummary.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) + assertThat(firstTopic.classroomTitle.html).isEqualTo("Maths") + } + + @Test + fun testRetrieveTopicList_fractionsTopic_hasCorrectLessonCount() { + val topicList = retrieveTopicList(TEST_CLASSROOM_ID_1) + + val fractionsTopic = topicList.getTopicSummary(0).topicSummary + assertThat(fractionsTopic.totalChapterCount).isEqualTo(2) + } + + @Test + fun testRetrieveTopicList_ratiosTopic_hasCorrectTopicInfo() { + val topicList = retrieveTopicList(TEST_CLASSROOM_ID_1) + + val ratiosTopic = topicList.getTopicSummary(1).topicSummary + assertThat(ratiosTopic.topicId).isEqualTo(RATIOS_TOPIC_ID) + assertThat(ratiosTopic.title.html).isEqualTo("Ratios and Proportional Reasoning") + } + + @Test + fun testRetrieveTopicList_ratiosTopic_hasCorrectClassroomInfo() { + val topicList = retrieveTopicList(TEST_CLASSROOM_ID_1) + + val firstTopic = topicList.getTopicSummary(1) + assertThat(firstTopic.topicSummary.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) + assertThat(firstTopic.classroomTitle.html).isEqualTo("Maths") + } + + @Test + fun testRetrieveTopicList_ratiosTopic_hasCorrectLessonCount() { + val topicList = retrieveTopicList(TEST_CLASSROOM_ID_1) + + val ratiosTopic = topicList.getTopicSummary(1).topicSummary + assertThat(ratiosTopic.totalChapterCount).isEqualTo(4) + } + + @Test + fun testRetrieveTopicList_doesNotContainUnavailableTopic() { + val topicList = retrieveTopicList(TEST_CLASSROOM_ID_2) + + // Verify that the topic list does not contain a not-yet published topic (since it can't be + // played by the user). + val topicIds = topicList.topicSummaryList.map { it.topicSummary }.map { it.topicId } + assertThat(topicIds).doesNotContain(TEST_TOPIC_ID_2) + } + + private fun getClassroomList(profileId: ProfileId) = + monitorFactory.waitForNextSuccessfulResult(classroomController.getClassroomList(profileId)) + + private fun retrieveTopicList(classroomId: String) = monitorFactory.waitForNextSuccessfulResult( + classroomController.getTopicList(profileId0, classroomId) + ) + + private fun setUpTestApplicationComponent() { + ApplicationProvider.getApplicationContext().inject(this) + } + + // TODO(#89): Move this to a common test application component. + @Module + class TestModule { + @Provides + @Singleton + fun provideContext(application: Application): Context { + return application + } + + @Provides + @DefaultGcsPrefix + @Singleton + fun provideDefaultGcsPrefix(): String { + return "https://storage.googleapis.com/" + } + + @Provides + @DefaultResourceBucketName + @Singleton + fun provideDefaultGcsResource(): String { + return "oppiaserver-resources/" + } + + @Provides + @ImageDownloadUrlTemplate + @Singleton + fun provideImageDownloadUrlTemplate(): String { + return "%s/%s/assets/image/%s" + } + + @EnableConsoleLog + @Provides + fun provideEnableConsoleLog(): Boolean = true + + @EnableFileLog + @Provides + fun provideEnableFileLog(): Boolean = false + + @GlobalLogLevel + @Provides + fun provideGlobalLogLevel(): LogLevel = LogLevel.VERBOSE + + @Provides + @LoadLessonProtosFromAssets + fun provideLoadLessonProtosFromAssets(testEnvironmentConfig: TestEnvironmentConfig): Boolean = + testEnvironmentConfig.isUsingBazel() + + @Provides + fun provideFakeAssetRepository(fakeImpl: FakeAssetRepository): AssetRepository = fakeImpl + } + + // TODO(#89): Move this to a common test application component. + @Singleton + @Component( + modules = [ + TestModule::class, TestLogReportingModule::class, LogStorageModule::class, + TestDispatcherModule::class, RobolectricModule::class, FakeOppiaClockModule::class, + NetworkConnectionUtilDebugModule::class, LocaleProdModule::class, + LoggingIdentifierModule::class, ApplicationLifecycleModule::class, + SyncStatusModule::class, TestPlatformParameterModule::class, + PlatformParameterSingletonModule::class + ] + ) + interface TestApplicationComponent : DataProvidersInjector { + @Component.Builder + interface Builder { + @BindsInstance + fun setApplication(application: Application): Builder + + fun build(): TestApplicationComponent + } + + fun inject(classroomControllerTest: ClassroomControllerTest) + } + + class TestApplication : Application(), DataProvidersInjectorProvider { + private val component: TestApplicationComponent by lazy { + DaggerClassroomControllerTest_TestApplicationComponent.builder() + .setApplication(this) + .build() + } + + fun inject(classroomControllerTest: ClassroomControllerTest) { + component.inject(classroomControllerTest) + } + + override fun getDataProvidersInjector(): DataProvidersInjector = component + } +} diff --git a/domain/src/test/java/org/oppia/android/domain/profile/ProfileManagementControllerTest.kt b/domain/src/test/java/org/oppia/android/domain/profile/ProfileManagementControllerTest.kt index ae98ccd5ae1..ba6082fa1c5 100644 --- a/domain/src/test/java/org/oppia/android/domain/profile/ProfileManagementControllerTest.kt +++ b/domain/src/test/java/org/oppia/android/domain/profile/ProfileManagementControllerTest.kt @@ -25,6 +25,8 @@ import org.oppia.android.app.model.Profile import org.oppia.android.app.model.ProfileDatabase import org.oppia.android.app.model.ProfileId import org.oppia.android.app.model.ReadingTextSize.MEDIUM_TEXT_SIZE +import org.oppia.android.domain.classroom.TEST_CLASSROOM_ID_1 +import org.oppia.android.domain.classroom.TEST_CLASSROOM_ID_2 import org.oppia.android.domain.oppialogger.ApplicationIdSeed import org.oppia.android.domain.oppialogger.LogStorageModule import org.oppia.android.domain.oppialogger.LoggingIdentifierController @@ -132,6 +134,7 @@ class ProfileManagementControllerTest { assertThat(profile.isContinueButtonAnimationSeen).isEqualTo(false) assertThat(File(getAbsoluteDirPath("0")).isDirectory).isTrue() assertThat(profile.surveyLastShownTimestampMs).isEqualTo(0L) + assertThat(profile.lastSelectedClassroomId).isEqualTo("") } @Test @@ -1168,6 +1171,113 @@ class ProfileManagementControllerTest { assertThat(lastShownTimeMs).isEqualTo(DEFAULT_SURVEY_LAST_SHOWN_TIMESTAMP_MILLIS) } + @Test + fun testFetchLastSelectedClassroomId_updateClassroomId_checkUpdateIsSuccessful() { + setUpTestApplicationComponent() + addTestProfiles() + + monitorFactory.ensureDataProviderExecutes( + profileManagementController.loginToProfile(PROFILE_ID_0) + ) + + monitorFactory.waitForNextSuccessfulResult( + profileManagementController.updateLastSelectedClassroomId( + PROFILE_ID_0, + TEST_CLASSROOM_ID_1 + ) + ) + + val lastSelectedClassroomId = monitorFactory.waitForNextSuccessfulResult( + profileManagementController.retrieveLastSelectedClassroomId(PROFILE_ID_0) + ) + + assertThat(lastSelectedClassroomId).isEqualTo(TEST_CLASSROOM_ID_1) + } + + @Test + fun testFetchLastSelectedClassroomId_updateClassroomIdTwice_checkUpdateIsSuccessful() { + setUpTestApplicationComponent() + addTestProfiles() + + monitorFactory.ensureDataProviderExecutes( + profileManagementController.loginToProfile(PROFILE_ID_0) + ) + + monitorFactory.waitForNextSuccessfulResult( + profileManagementController.updateLastSelectedClassroomId( + PROFILE_ID_0, + TEST_CLASSROOM_ID_1 + ) + ) + + monitorFactory.waitForNextSuccessfulResult( + profileManagementController.updateLastSelectedClassroomId( + PROFILE_ID_0, + TEST_CLASSROOM_ID_2 + ) + ) + + val lastSelectedClassroomId = monitorFactory.waitForNextSuccessfulResult( + profileManagementController.retrieveLastSelectedClassroomId(PROFILE_ID_0) + ) + + assertThat(lastSelectedClassroomId).isEqualTo(TEST_CLASSROOM_ID_2) + } + + @Test + fun testFetchLastSelectedClassroomId_updateClassroomIds_checkUpdateIsSuccessfulPerProfile() { + setUpTestApplicationComponent() + addTestProfiles() + + // Login to profile 0 and update the last selected classroom to classroom 1. + monitorFactory.ensureDataProviderExecutes( + profileManagementController.loginToProfile(PROFILE_ID_0) + ) + monitorFactory.waitForNextSuccessfulResult( + profileManagementController.updateLastSelectedClassroomId( + PROFILE_ID_0, + TEST_CLASSROOM_ID_1 + ) + ) + + // Login to profile 1 and update the last selected classroom to classroom 2. + monitorFactory.ensureDataProviderExecutes( + profileManagementController.loginToProfile(PROFILE_ID_1) + ) + monitorFactory.waitForNextSuccessfulResult( + profileManagementController.updateLastSelectedClassroomId( + PROFILE_ID_1, + TEST_CLASSROOM_ID_2 + ) + ) + + // Verify that last selected classroom of profile 0 is classroom 1. + val profile0SelectedClassroomId = monitorFactory.waitForNextSuccessfulResult( + profileManagementController.retrieveLastSelectedClassroomId(PROFILE_ID_0) + ) + assertThat(profile0SelectedClassroomId).isEqualTo(TEST_CLASSROOM_ID_1) + + // Verify that last selected classroom of profile 1 is classroom 2. + val classroomIdProfile1 = monitorFactory.waitForNextSuccessfulResult( + profileManagementController.retrieveLastSelectedClassroomId(PROFILE_ID_1) + ) + assertThat(classroomIdProfile1).isEqualTo(TEST_CLASSROOM_ID_2) + } + + @Test + fun testFetchLastSelectedClassroomId_withoutUpdatingClassroomId_returnEmptyClassroomId() { + setUpTestApplicationComponent() + addTestProfiles() + + monitorFactory.ensureDataProviderExecutes( + profileManagementController.loginToProfile(PROFILE_ID_0) + ) + val lastSelectedClassroomId = monitorFactory.waitForNextSuccessfulResult( + profileManagementController.retrieveLastSelectedClassroomId(PROFILE_ID_0) + ) + assertThat(lastSelectedClassroomId).isEmpty() + } + private fun addTestProfiles() { val profileAdditionProviders = PROFILES_LIST.map { addNonAdminProfile(it.name, pin = it.pin, allowDownloadAccess = it.allowDownloadAccess) diff --git a/domain/src/test/java/org/oppia/android/domain/topic/TopicListControllerTest.kt b/domain/src/test/java/org/oppia/android/domain/topic/TopicListControllerTest.kt index 29006f7e528..052b684c8e6 100644 --- a/domain/src/test/java/org/oppia/android/domain/topic/TopicListControllerTest.kt +++ b/domain/src/test/java/org/oppia/android/domain/topic/TopicListControllerTest.kt @@ -18,6 +18,9 @@ import org.oppia.android.app.model.PromotedActivityList import org.oppia.android.app.model.PromotedStory import org.oppia.android.app.model.TopicRecord import org.oppia.android.app.model.UpcomingTopic +import org.oppia.android.domain.classroom.TEST_CLASSROOM_ID_0 +import org.oppia.android.domain.classroom.TEST_CLASSROOM_ID_1 +import org.oppia.android.domain.classroom.TEST_CLASSROOM_ID_2 import org.oppia.android.domain.oppialogger.LogStorageModule import org.oppia.android.domain.oppialogger.LoggingIdentifierModule import org.oppia.android.domain.oppialogger.analytics.ApplicationLifecycleModule diff --git a/model/src/main/proto/profile.proto b/model/src/main/proto/profile.proto index aadd1f34881..cc395949209 100644 --- a/model/src/main/proto/profile.proto +++ b/model/src/main/proto/profile.proto @@ -87,6 +87,9 @@ message Profile { // Represents the epoch timestamp in milliseconds when the nps survey was previously shown in // this profile. int64 survey_last_shown_timestamp_ms = 18; + + // Represents the ID of the classroom that the user selected during their last login. + string last_selected_classroom_id = 19; } // Represents a profile avatar image. diff --git a/model/src/main/proto/thumbnail.proto b/model/src/main/proto/thumbnail.proto index 607860ba30a..c13fcdbaa95 100644 --- a/model/src/main/proto/thumbnail.proto +++ b/model/src/main/proto/thumbnail.proto @@ -82,4 +82,13 @@ enum LessonThumbnailGraphic { // Corresponds to Number line Fractions subtopic. THE_NUMBER_LINE = 20; + + // Corresponds to Science classroom. + SCIENCE_CLASSROOM = 21; + + // Corresponds to Maths classroom. + MATHS_CLASSROOM = 22; + + // Corresponds to English classroom. + ENGLISH_CLASSROOM = 23; } From 4aa53373d40a0ca6cf728ae3246959c4d9810835 Mon Sep 17 00:00:00 2001 From: RD Rama Devi <122200035+Rd4dev@users.noreply.github.com> Date: Wed, 26 Jun 2024 19:25:52 +0530 Subject: [PATCH 2/4] Fix part of #5343: Implement script to run code coverage for a specific file (#5433) ## Explanation Fixes part of #5343 ### Project [PR 1.3 of Project 4.1] ### Changes Made - Added a new script utility `RunCoverage.kt` that takes a filename as an argument. - Implements handling for exempted files and test target mapping. --- ### Implementation Details - **Exemption Check**: The script first checks if the provided file is exempted from having test file [the implemetation will be changed once PR 1.2 is merged]. If exempted, it exits early with the message "The file is exempted hence no coverage!". - **Test Target Mapping**: - **Script, App, and Default Subpackages**: Constructs mappings for files within these subpackages to their corresponding test targets. - **App Subpackage**: Handles mapping for both `test` and `Localtests`. - **Code Coverage Execution**: - Utilizes the `RunCoverageForTestTarget` script to execute code coverage analysis. - Returns a list of coverage data strings for each run, which will be subsequently saved as protos in PR 1.4. ### Example Usage ```bash bazel run //scripts:run_coverage -- $(pwd) utility/src/main/java/org/oppia/android/util/parser/math/MathModel.kt ``` ## Essential Checklist - [x] The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".) - [x] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)). - [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)). ## For UI-specific PRs only If your PR includes UI-related changes, then: - Add screenshots for portrait/landscape for both a tablet & phone of the before & after UI changes - For the screenshots above, include both English and pseudo-localized (RTL) screenshots (see [RTL guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines)) - Add a video showing the full UX flow with a screen reader enabled (see [accessibility guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide)) - For PRs introducing new UI elements or color changes, both light and dark mode screenshots must be included - Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing --------- Co-authored-by: Ben Henning Co-authored-by: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com> --- scripts/BUILD.bazel | 10 + .../android/scripts/common/BazelClient.kt | 4 +- .../android/scripts/coverage/BUILD.bazel | 14 + .../android/scripts/coverage/RunCoverage.kt | 148 ++++++ .../scripts/testing/TestBazelWorkspace.kt | 55 ++- .../android/scripts/common/BazelClientTest.kt | 5 +- .../android/scripts/coverage/BUILD.bazel | 13 + .../scripts/coverage/CoverageRunnerTest.kt | 11 +- .../scripts/coverage/RunCoverageTest.kt | 459 ++++++++++++++++++ .../scripts/testing/TestBazelWorkspaceTest.kt | 172 ++++++- 10 files changed, 874 insertions(+), 17 deletions(-) create mode 100644 scripts/src/java/org/oppia/android/scripts/coverage/RunCoverage.kt create mode 100644 scripts/src/javatests/org/oppia/android/scripts/coverage/RunCoverageTest.kt diff --git a/scripts/BUILD.bazel b/scripts/BUILD.bazel index 689cf6e53d2..2fba670d34e 100644 --- a/scripts/BUILD.bazel +++ b/scripts/BUILD.bazel @@ -237,6 +237,16 @@ kt_jvm_binary( ], ) +kt_jvm_binary( + name = "run_coverage", + testonly = True, + data = TEST_FILE_EXEMPTION_ASSETS, + main_class = "org.oppia.android.scripts.coverage.RunCoverageKt", + runtime_deps = [ + "//scripts/src/java/org/oppia/android/scripts/coverage:run_coverage_lib", + ], +) + # Note that this is intentionally not test-only since it's used by the app build pipeline. Also, # this apparently needs to be a java_binary to set up runfiles correctly when executed within a # Starlark rule as a tool. diff --git a/scripts/src/java/org/oppia/android/scripts/common/BazelClient.kt b/scripts/src/java/org/oppia/android/scripts/common/BazelClient.kt index 62ce53bfb82..8c3c546f026 100644 --- a/scripts/src/java/org/oppia/android/scripts/common/BazelClient.kt +++ b/scripts/src/java/org/oppia/android/scripts/common/BazelClient.kt @@ -142,9 +142,11 @@ class BazelClient(private val rootDirectory: File, private val commandExecutor: * or null if the coverage data file could not be parsed */ fun runCoverageForTestTarget(bazelTestTarget: String): List? { + val computeInstrumentation = bazelTestTarget.split("/").let { "//${it[2]}/..." } val coverageCommandOutputLines = executeBazelCommand( "coverage", - bazelTestTarget + bazelTestTarget, + "--instrumentation_filter=$computeInstrumentation" ) return parseCoverageDataFilePath(coverageCommandOutputLines)?.let { path -> File(path).readLines() diff --git a/scripts/src/java/org/oppia/android/scripts/coverage/BUILD.bazel b/scripts/src/java/org/oppia/android/scripts/coverage/BUILD.bazel index 4c7ab41a5fa..aa0c56dd7c3 100644 --- a/scripts/src/java/org/oppia/android/scripts/coverage/BUILD.bazel +++ b/scripts/src/java/org/oppia/android/scripts/coverage/BUILD.bazel @@ -4,6 +4,20 @@ Libraries corresponding to developer scripts that obtain coverage data for test load("@io_bazel_rules_kotlin//kotlin:kotlin.bzl", "kt_jvm_library") +kt_jvm_library( + name = "run_coverage_lib", + testonly = True, + srcs = [ + "RunCoverage.kt", + ], + visibility = ["//scripts:oppia_script_binary_visibility"], + deps = [ + ":coverage_runner", + "//scripts/src/java/org/oppia/android/scripts/common:bazel_client", + "//scripts/src/java/org/oppia/android/scripts/proto:script_exemptions_java_proto", + ], +) + kt_jvm_library( name = "coverage_runner", testonly = True, diff --git a/scripts/src/java/org/oppia/android/scripts/coverage/RunCoverage.kt b/scripts/src/java/org/oppia/android/scripts/coverage/RunCoverage.kt new file mode 100644 index 00000000000..35b27a4b95e --- /dev/null +++ b/scripts/src/java/org/oppia/android/scripts/coverage/RunCoverage.kt @@ -0,0 +1,148 @@ +package org.oppia.android.scripts.coverage + +import kotlinx.coroutines.runBlocking +import org.oppia.android.scripts.common.BazelClient +import org.oppia.android.scripts.common.CommandExecutor +import org.oppia.android.scripts.common.CommandExecutorImpl +import org.oppia.android.scripts.common.ScriptBackgroundCoroutineDispatcher +import org.oppia.android.scripts.proto.TestFileExemptions +import java.io.File +import java.util.concurrent.TimeUnit + +/** + * Entry point function for running coverage analysis for a source file. + * + * Usage: + * bazel run //scripts:run_coverage_for_test_target -- + * + * Arguments: + * - path_to_root: directory path to the root of the Oppia Android repository. + * - relative_path_to_file: the relative path to the file to analyse coverage + * + * Example: + * bazel run //scripts:run_coverage -- $(pwd) + * utility/src/main/java/org/oppia/android/util/parser/math/MathModel.kt + * Example with custom process timeout: + * bazel run //scripts:run_coverage -- $(pwd) + * utility/src/main/java/org/oppia/android/util/parser/math/MathModel.kt processTimeout=10 + * + */ +fun main(vararg args: String) { + val repoRoot = args[0] + val filePath = args[1] + + if (!File(repoRoot, filePath).exists()) { + error("File doesn't exist: $filePath.") + } + + ScriptBackgroundCoroutineDispatcher().use { scriptBgDispatcher -> + val processTimeout: Long = args.find { it.startsWith("processTimeout=") } + ?.substringAfter("=") + ?.toLongOrNull() ?: 5 + + val commandExecutor: CommandExecutor = CommandExecutorImpl( + scriptBgDispatcher, processTimeout = processTimeout, processTimeoutUnit = TimeUnit.MINUTES + ) + + RunCoverage(repoRoot, filePath, commandExecutor, scriptBgDispatcher).execute() + } +} + +/** + * Class responsible for executing coverage on a given file. + * + * @param repoRoot the root directory of the repository + * @param filePath the relative path to the file to analyse coverage + * @param commandExecutor executes the specified command in the specified working directory + * @param scriptBgDispatcher the [ScriptBackgroundCoroutineDispatcher] to be used for running the coverage command + */ +class RunCoverage( + private val repoRoot: String, + private val filePath: String, + private val commandExecutor: CommandExecutor, + private val scriptBgDispatcher: ScriptBackgroundCoroutineDispatcher +) { + private val bazelClient by lazy { BazelClient(File(repoRoot), commandExecutor) } + + private val rootDirectory = File(repoRoot).absoluteFile + private val testFileExemptionTextProto = "scripts/assets/test_file_exemptions" + + /** + * Executes coverage analysis for the specified file. + * + * Loads test file exemptions and checks if the specified file is exempted. If exempted, + * prints a message indicating no coverage analysis is performed. Otherwise, initializes + * a Bazel client, finds potential test file paths, retrieves Bazel targets, and initiates + * coverage analysis for each test target found. + * + * @return a list of lists containing coverage data for each requested test target, if + * the file is exempted from having a test file, an empty list is returned + */ + fun execute(): List> { + val testFileExemptionList = loadTestFileExemptionsProto(testFileExemptionTextProto) + .testFileExemptionList + .filter { it.testFileNotRequired } + .map { it.exemptedFilePath } + + if (filePath in testFileExemptionList) { + println("This file is exempted from having a test file; skipping coverage check.") + return emptyList() + } + + val testFilePaths = findTestFile(repoRoot, filePath) + if (testFilePaths.isEmpty()) { + error("No appropriate test file found for $filePath") + } + + val testTargets = bazelClient.retrieveBazelTargets(testFilePaths) + + return testTargets.mapNotNull { testTarget -> + val coverageData = runCoverageForTarget(testTarget) + if (coverageData == null) { + println("Coverage data for $testTarget is null") + } + coverageData + } + } + + private fun runCoverageForTarget(testTarget: String): List? { + return runBlocking { + CoverageRunner(rootDirectory, scriptBgDispatcher, commandExecutor) + .runWithCoverageAsync(testTarget.removeSuffix(".kt")) + .await() + } + } +} + +private fun findTestFile(repoRoot: String, filePath: String): List { + val possibleTestFilePaths = when { + filePath.startsWith("scripts/") -> { + listOf(filePath.replace("/java/", "/javatests/").replace(".kt", "Test.kt")) + } + filePath.startsWith("app/") -> { + listOf( + filePath.replace("/main/", "/sharedTest/").replace(".kt", "Test.kt"), + filePath.replace("/main/", "/test/").replace(".kt", "Test.kt"), + filePath.replace("/main/", "/test/").replace(".kt", "LocalTest.kt") + ) + } + else -> { + listOf(filePath.replace("/main/", "/test/").replace(".kt", "Test.kt")) + } + } + + val repoRootFile = File(repoRoot).absoluteFile + + return possibleTestFilePaths + .map { File(repoRootFile, it) } + .filter(File::exists) + .map { it.relativeTo(repoRootFile).path } +} + +private fun loadTestFileExemptionsProto(testFileExemptiontextProto: String): TestFileExemptions { + return File("$testFileExemptiontextProto.pb").inputStream().use { stream -> + TestFileExemptions.newBuilder().also { builder -> + builder.mergeFrom(stream) + }.build() + } +} diff --git a/scripts/src/java/org/oppia/android/scripts/testing/TestBazelWorkspace.kt b/scripts/src/java/org/oppia/android/scripts/testing/TestBazelWorkspace.kt index 71ee2eb542a..2e4f1b20c0f 100644 --- a/scripts/src/java/org/oppia/android/scripts/testing/TestBazelWorkspace.kt +++ b/scripts/src/java/org/oppia/android/scripts/testing/TestBazelWorkspace.kt @@ -57,22 +57,22 @@ class TestBazelWorkspace(private val temporaryRootFolder: TemporaryFolder) { * @param filename the name of the source file (without the .kt extension) * @param sourceContent the content of the source file * @param testContent the content of the test file - * @param subpackage the subpackage under which the source and test files should be added + * @param sourceSubpackage the subpackage under which the source files should be added + * @param testSubpackage the subpackage under which the test files should be added */ fun addSourceAndTestFileWithContent( filename: String, sourceContent: String, testContent: String, - subpackage: String + sourceSubpackage: String, + testSubpackage: String ) { - val sourceSubpackage = "$subpackage/main/java/com/example" addSourceContentAndBuildFile( filename, sourceContent, sourceSubpackage ) - val testSubpackage = "$subpackage/test/java/com/example" val testFileName = "${filename}Test" addTestContentAndBuildFile( filename, @@ -83,6 +83,51 @@ class TestBazelWorkspace(private val temporaryRootFolder: TemporaryFolder) { ) } + /** + * Adds a source file and 2 test files with the specified name and content, + * and updates the corresponding build configuration. + * + * @param filename the name of the source file (without the .kt extension) + * @param sourceContent the content of the source file + * @param testContentShared the content of the test file for SharedTest Package + * @param testContentLocal the content of the test file for Test Package + * @param subpackage the subpackage under which the source and test files should be added + */ + fun addMultiLevelSourceAndTestFileWithContent( + filename: String, + sourceContent: String, + testContentShared: String, + testContentLocal: String, + subpackage: String + ) { + val sourceSubpackage = "$subpackage/main/java/com/example" + addSourceContentAndBuildFile( + filename, + sourceContent, + sourceSubpackage + ) + + val testSubpackageShared = "$subpackage/sharedTest/java/com/example" + val testFileNameShared = "${filename}Test" + addTestContentAndBuildFile( + filename, + testFileNameShared, + testContentShared, + sourceSubpackage, + testSubpackageShared + ) + + val testSubpackageLocal = "$subpackage/test/java/com/example" + val testFileNameLocal = "${filename}LocalTest" + addTestContentAndBuildFile( + filename, + testFileNameLocal, + testContentLocal, + sourceSubpackage, + testSubpackageLocal + ) + } + /** * Adds a source file with the specified name and content to the specified subpackage, * and updates the corresponding build configuration. @@ -172,7 +217,7 @@ class TestBazelWorkspace(private val temporaryRootFolder: TemporaryFolder) { testBuildFile.appendText( """ kt_jvm_test( - name = "test", + name = "$testName", srcs = ["$testName.kt"], deps = [ "//$sourceSubpackage:${filename.lowercase()}", diff --git a/scripts/src/javatests/org/oppia/android/scripts/common/BazelClientTest.kt b/scripts/src/javatests/org/oppia/android/scripts/common/BazelClientTest.kt index 075feeab3fe..db725861d16 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/common/BazelClientTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/common/BazelClientTest.kt @@ -424,10 +424,11 @@ class BazelClientTest { filename = "TwoSum", sourceContent = sourceContent, testContent = testContent, - subpackage = "coverage" + sourceSubpackage = "coverage/main/java/com/example", + testSubpackage = "coverage/test/java/com/example" ) - val result = bazelClient.runCoverageForTestTarget("//coverage/test/java/com/example:test") + val result = bazelClient.runCoverageForTestTarget("//coverage/test/java/com/example:TwoSumTest") val expectedResult = listOf( "SF:coverage/main/java/com/example/TwoSum.kt", "FN:7,com/example/TwoSum${'$'}Companion::sumNumbers (II)Ljava/lang/Object;", diff --git a/scripts/src/javatests/org/oppia/android/scripts/coverage/BUILD.bazel b/scripts/src/javatests/org/oppia/android/scripts/coverage/BUILD.bazel index f2e5c80564b..6c200e83d8a 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/coverage/BUILD.bazel +++ b/scripts/src/javatests/org/oppia/android/scripts/coverage/BUILD.bazel @@ -15,3 +15,16 @@ kt_jvm_test( "//third_party:org_jetbrains_kotlin_kotlin-test-junit", ], ) + +kt_jvm_test( + name = "RunCoverageTest", + srcs = ["RunCoverageTest.kt"], + deps = [ + "//scripts:test_file_check_assets", + "//scripts/src/java/org/oppia/android/scripts/coverage:run_coverage_lib", + "//scripts/src/java/org/oppia/android/scripts/testing:test_bazel_workspace", + "//testing:assertion_helpers", + "//third_party:com_google_truth_truth", + "//third_party:org_jetbrains_kotlin_kotlin-test-junit", + ], +) diff --git a/scripts/src/javatests/org/oppia/android/scripts/coverage/CoverageRunnerTest.kt b/scripts/src/javatests/org/oppia/android/scripts/coverage/CoverageRunnerTest.kt index bae58d98feb..6a89e9db798 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/coverage/CoverageRunnerTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/coverage/CoverageRunnerTest.kt @@ -37,7 +37,7 @@ class CoverageRunnerTest { } @Test - fun testRunCoverage_emptyDirectory_throwsException() { + fun testRunWithCoverageAsync_emptyDirectory_throwsException() { val exception = assertThrows() { runBlocking { coverageRunner.runWithCoverageAsync(bazelTestTarget).await() @@ -48,7 +48,7 @@ class CoverageRunnerTest { } @Test - fun testRunCoverage_invalidTestTarget_throwsException() { + fun testRunWithCoverageAsync_invalidTestTarget_throwsException() { testBazelWorkspace.initEmptyWorkspace() val exception = assertThrows() { @@ -62,7 +62,7 @@ class CoverageRunnerTest { } @Test - fun testRunCoverage_validSampleTestTarget_returnsCoverageData() { + fun testRunWithCoverageAsync_validSampleTestTarget_returnsCoverageData() { testBazelWorkspace.initEmptyWorkspace() val sourceContent = @@ -105,12 +105,13 @@ class CoverageRunnerTest { filename = "TwoSum", sourceContent = sourceContent, testContent = testContent, - subpackage = "coverage" + sourceSubpackage = "coverage/main/java/com/example", + testSubpackage = "coverage/test/java/com/example" ) val result = runBlocking { coverageRunner.runWithCoverageAsync( - "//coverage/test/java/com/example:test" + "//coverage/test/java/com/example:TwoSumTest" ).await() } val expectedResult = listOf( diff --git a/scripts/src/javatests/org/oppia/android/scripts/coverage/RunCoverageTest.kt b/scripts/src/javatests/org/oppia/android/scripts/coverage/RunCoverageTest.kt new file mode 100644 index 00000000000..fb9db5d14b0 --- /dev/null +++ b/scripts/src/javatests/org/oppia/android/scripts/coverage/RunCoverageTest.kt @@ -0,0 +1,459 @@ +package org.oppia.android.scripts.coverage + +import com.google.common.truth.Truth.assertThat +import org.junit.After +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import org.oppia.android.scripts.common.CommandExecutorImpl +import org.oppia.android.scripts.common.ScriptBackgroundCoroutineDispatcher +import org.oppia.android.scripts.testing.TestBazelWorkspace +import org.oppia.android.testing.assertThrows +import java.io.ByteArrayOutputStream +import java.io.File +import java.io.PrintStream +import java.util.concurrent.TimeUnit + +/** Tests for [RunCoverage]. */ +class RunCoverageTest { + @field:[Rule JvmField] val tempFolder = TemporaryFolder() + + private val outContent: ByteArrayOutputStream = ByteArrayOutputStream() + private val originalOut: PrintStream = System.out + + private val scriptBgDispatcher by lazy { ScriptBackgroundCoroutineDispatcher() } + private val commandExecutor by lazy { CommandExecutorImpl(scriptBgDispatcher) } + private val longCommandExecutor by lazy { initializeCommandExecutorWithLongProcessWaitTime() } + + private lateinit var testBazelWorkspace: TestBazelWorkspace + private lateinit var sampleFilePath: String + + @Before + fun setUp() { + sampleFilePath = "/path/to/Sample.kt" + testBazelWorkspace = TestBazelWorkspace(tempFolder) + System.setOut(PrintStream(outContent)) + } + + @After + fun tearDown() { + System.setOut(originalOut) + scriptBgDispatcher.close() + } + + @Test + fun testRunCoverage_invalidFile_throwsException() { + testBazelWorkspace.initEmptyWorkspace() + val exception = assertThrows() { + main(tempFolder.root.absolutePath, "file.kt") + } + + assertThat(exception).hasMessageThat().contains("File doesn't exist") + } + + @Test + fun testRunCoverage_missingTestFileNotExempted_throwsException() { + testBazelWorkspace.initEmptyWorkspace() + val exception = assertThrows() { + val sampleFile = File(tempFolder.root.absolutePath, "file.kt") + sampleFile.createNewFile() + main(tempFolder.root.absolutePath, "file.kt") + } + + assertThat(exception).hasMessageThat().contains("No appropriate test file found") + } + + @Test + fun testRunCoverage_testFileExempted_noCoverage() { + val exemptedFilePath = "app/src/main/java/org/oppia/android/app/activity/ActivityComponent.kt" + + RunCoverage( + "${tempFolder.root}", + exemptedFilePath, + commandExecutor, + scriptBgDispatcher + ).execute() + + assertThat(outContent.toString()) + .isEqualTo("This file is exempted from having a test file; skipping coverage check.\n") + } + + @Test + fun testRunCoverage_sampleTests_returnsCoverageData() { + testBazelWorkspace.initEmptyWorkspace() + + val sourceContent = + """ + package com.example + + class TwoSum { + + companion object { + fun sumNumbers(a: Int, b: Int): Any { + return if (a ==0 && b == 0) { + "Both numbers are zero" + } else { + a + b + } + } + } + } + """.trimIndent() + + val testContent = + """ + package com.example + + import org.junit.Assert.assertEquals + import org.junit.Test + + class TwoSumTest { + + @Test + fun testSumNumbers() { + assertEquals(TwoSum.sumNumbers(0, 1), 1) + assertEquals(TwoSum.sumNumbers(3, 4), 7) + assertEquals(TwoSum.sumNumbers(0, 0), "Both numbers are zero") + } + } + """.trimIndent() + + testBazelWorkspace.addSourceAndTestFileWithContent( + filename = "TwoSum", + sourceContent = sourceContent, + testContent = testContent, + sourceSubpackage = "coverage/main/java/com/example", + testSubpackage = "coverage/test/java/com/example" + ) + + val result = RunCoverage( + "${tempFolder.root}", + "coverage/main/java/com/example/TwoSum.kt", + longCommandExecutor, + scriptBgDispatcher + ).execute() + + val expectedResultList = listOf( + listOf( + "SF:coverage/main/java/com/example/TwoSum.kt", + "FN:7,com/example/TwoSum${'$'}Companion::sumNumbers (II)Ljava/lang/Object;", + "FN:3,com/example/TwoSum:: ()V", + "FNDA:1,com/example/TwoSum${'$'}Companion::sumNumbers (II)Ljava/lang/Object;", + "FNDA:0,com/example/TwoSum:: ()V", + "FNF:2", + "FNH:1", + "BRDA:7,0,0,1", + "BRDA:7,0,1,1", + "BRDA:7,0,2,1", + "BRDA:7,0,3,1", + "BRF:4", + "BRH:4", + "DA:3,0", + "DA:7,1", + "DA:8,1", + "DA:10,1", + "LH:3", + "LF:4", + "end_of_record" + ) + ) + + assertThat(result).isEqualTo(expectedResultList) + } + + @Test + fun testRunCoverage_scriptTests_returnsCoverageData() { + testBazelWorkspace.initEmptyWorkspace() + + val sourceContent = + """ + package com.example + + class TwoSum { + + companion object { + fun sumNumbers(a: Int, b: Int): Any { + return if (a ==0 && b == 0) { + "Both numbers are zero" + } else { + a + b + } + } + } + } + """.trimIndent() + + val testContent = + """ + package com.example + + import org.junit.Assert.assertEquals + import org.junit.Test + + class TwoSumTest { + + @Test + fun testSumNumbers() { + assertEquals(TwoSum.sumNumbers(0, 1), 1) + assertEquals(TwoSum.sumNumbers(3, 4), 7) + assertEquals(TwoSum.sumNumbers(0, 0), "Both numbers are zero") + } + } + """.trimIndent() + + testBazelWorkspace.addSourceAndTestFileWithContent( + filename = "TwoSum", + sourceContent = sourceContent, + testContent = testContent, + sourceSubpackage = "scripts/java/com/example", + testSubpackage = "scripts/javatests/com/example" + ) + + val result = RunCoverage( + "${tempFolder.root}", + "scripts/java/com/example/TwoSum.kt", + longCommandExecutor, + scriptBgDispatcher + ).execute() + + val expectedResultList = listOf( + listOf( + "SF:scripts/java/com/example/TwoSum.kt", + "FN:7,com/example/TwoSum${'$'}Companion::sumNumbers (II)Ljava/lang/Object;", + "FN:3,com/example/TwoSum:: ()V", + "FNDA:1,com/example/TwoSum${'$'}Companion::sumNumbers (II)Ljava/lang/Object;", + "FNDA:0,com/example/TwoSum:: ()V", + "FNF:2", + "FNH:1", + "BRDA:7,0,0,1", + "BRDA:7,0,1,1", + "BRDA:7,0,2,1", + "BRDA:7,0,3,1", + "BRF:4", + "BRH:4", + "DA:3,0", + "DA:7,1", + "DA:8,1", + "DA:10,1", + "LH:3", + "LF:4", + "end_of_record" + ) + ) + + assertThat(result).isEqualTo(expectedResultList) + } + + @Test + fun testRunCoverage_appTests_returnsCoverageData() { + testBazelWorkspace.initEmptyWorkspace() + + val sourceContent = + """ + package com.example + + class TwoSum { + + companion object { + fun sumNumbers(a: Int, b: Int): Any { + return if (a ==0 && b == 0) { + "Both numbers are zero" + } else { + a + b + } + } + } + } + """.trimIndent() + + val testContent = + """ + package com.example + + import org.junit.Assert.assertEquals + import org.junit.Test + + class TwoSumTest { + + @Test + fun testSumNumbers() { + assertEquals(TwoSum.sumNumbers(0, 1), 1) + assertEquals(TwoSum.sumNumbers(3, 4), 7) + assertEquals(TwoSum.sumNumbers(0, 0), "Both numbers are zero") + } + } + """.trimIndent() + + testBazelWorkspace.addSourceAndTestFileWithContent( + filename = "TwoSum", + sourceContent = sourceContent, + testContent = testContent, + sourceSubpackage = "app/main/java/com/example", + testSubpackage = "app/test/java/com/example" + ) + + val result = RunCoverage( + "${tempFolder.root}", + "app/main/java/com/example/TwoSum.kt", + longCommandExecutor, + scriptBgDispatcher + ).execute() + + val expectedResultList = listOf( + listOf( + "SF:app/main/java/com/example/TwoSum.kt", + "FN:7,com/example/TwoSum${'$'}Companion::sumNumbers (II)Ljava/lang/Object;", + "FN:3,com/example/TwoSum:: ()V", + "FNDA:1,com/example/TwoSum${'$'}Companion::sumNumbers (II)Ljava/lang/Object;", + "FNDA:0,com/example/TwoSum:: ()V", + "FNF:2", + "FNH:1", + "BRDA:7,0,0,1", + "BRDA:7,0,1,1", + "BRDA:7,0,2,1", + "BRDA:7,0,3,1", + "BRF:4", + "BRH:4", + "DA:3,0", + "DA:7,1", + "DA:8,1", + "DA:10,1", + "LH:3", + "LF:4", + "end_of_record" + ) + ) + + assertThat(result).isEqualTo(expectedResultList) + } + + @Test + fun testRunCoverage_sharedAndLocalTests_returnsCoverageData() { + testBazelWorkspace.initEmptyWorkspace() + + val sourceContent = + """ + package com.example + + class TwoSum { + + companion object { + fun sumNumbers(a: Int, b: Int): Any { + return if (a ==0 && b == 0) { + "Both numbers are zero" + } else { + a + b + } + } + } + } + """.trimIndent() + + val testContentShared = + """ + package com.example + + import org.junit.Assert.assertEquals + import org.junit.Test + + class TwoSumTest { + + @Test + fun testSumNumbers() { + assertEquals(TwoSum.sumNumbers(0, 1), 1) + assertEquals(TwoSum.sumNumbers(3, 4), 7) + assertEquals(TwoSum.sumNumbers(0, 0), "Both numbers are zero") + } + } + """.trimIndent() + + val testContentLocal = + """ + package com.example + + import org.junit.Assert.assertEquals + import org.junit.Test + + class TwoSumLocalTest { + + @Test + fun testSumNumbers() { + assertEquals(TwoSum.sumNumbers(0, 1), 1) + assertEquals(TwoSum.sumNumbers(3, 4), 7) + assertEquals(TwoSum.sumNumbers(0, 0), "Both numbers are zero") + } + } + """.trimIndent() + + testBazelWorkspace.addMultiLevelSourceAndTestFileWithContent( + filename = "TwoSum", + sourceContent = sourceContent, + testContentShared = testContentShared, + testContentLocal = testContentLocal, + subpackage = "app" + ) + + val result = RunCoverage( + "${tempFolder.root}", + "app/main/java/com/example/TwoSum.kt", + longCommandExecutor, + scriptBgDispatcher + ).execute() + + val expectedResultList = listOf( + listOf( + "SF:app/main/java/com/example/TwoSum.kt", + "FN:7,com/example/TwoSum${'$'}Companion::sumNumbers (II)Ljava/lang/Object;", + "FN:3,com/example/TwoSum:: ()V", + "FNDA:1,com/example/TwoSum${'$'}Companion::sumNumbers (II)Ljava/lang/Object;", + "FNDA:0,com/example/TwoSum:: ()V", + "FNF:2", + "FNH:1", + "BRDA:7,0,0,1", + "BRDA:7,0,1,1", + "BRDA:7,0,2,1", + "BRDA:7,0,3,1", + "BRF:4", + "BRH:4", + "DA:3,0", + "DA:7,1", + "DA:8,1", + "DA:10,1", + "LH:3", + "LF:4", + "end_of_record" + ), + listOf( + "SF:app/main/java/com/example/TwoSum.kt", + "FN:7,com/example/TwoSum${'$'}Companion::sumNumbers (II)Ljava/lang/Object;", + "FN:3,com/example/TwoSum:: ()V", + "FNDA:1,com/example/TwoSum${'$'}Companion::sumNumbers (II)Ljava/lang/Object;", + "FNDA:0,com/example/TwoSum:: ()V", + "FNF:2", + "FNH:1", + "BRDA:7,0,0,1", + "BRDA:7,0,1,1", + "BRDA:7,0,2,1", + "BRDA:7,0,3,1", + "BRF:4", + "BRH:4", + "DA:3,0", + "DA:7,1", + "DA:8,1", + "DA:10,1", + "LH:3", + "LF:4", + "end_of_record" + ) + ) + + assertThat(result).isEqualTo(expectedResultList) + } + + private fun initializeCommandExecutorWithLongProcessWaitTime(): CommandExecutorImpl { + return CommandExecutorImpl( + scriptBgDispatcher, processTimeout = 5, processTimeoutUnit = TimeUnit.MINUTES + ) + } +} diff --git a/scripts/src/javatests/org/oppia/android/scripts/testing/TestBazelWorkspaceTest.kt b/scripts/src/javatests/org/oppia/android/scripts/testing/TestBazelWorkspaceTest.kt index 9d6a33378d6..b52113e817d 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/testing/TestBazelWorkspaceTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/testing/TestBazelWorkspaceTest.kt @@ -282,7 +282,8 @@ class TestBazelWorkspaceTest { "Main", sourceContent, testContent, - "coverage" + sourceSubpackage = "coverage/main/java/com/example", + testSubpackage = "coverage/test/java/com/example" ) val sourceFile = File(tempFolder.root, "coverage/main/java/com/example/Main.kt") @@ -316,7 +317,8 @@ class TestBazelWorkspaceTest { "Main", sourceContent, testContent, - "coverage" + sourceSubpackage = "coverage/main/java/com/example", + testSubpackage = "coverage/test/java/com/example" ) val sourceBuildFile = File(tempFolder.root, "coverage/main/java/com/example/BUILD.bazel") @@ -338,7 +340,7 @@ class TestBazelWorkspaceTest { """ load("@io_bazel_rules_kotlin//kotlin:jvm.bzl", "kt_jvm_test") kt_jvm_test( - name = "test", + name = "MainTest", srcs = ["MainTest.kt"], deps = [ "//coverage/main/java/com/example:main", @@ -351,6 +353,168 @@ class TestBazelWorkspaceTest { ) } + @Test + fun testAddMultiLevelSourceAndTestFileWithContent_createsSourceAndTestFiles() { + val testBazelWorkspace = TestBazelWorkspace(tempFolder) + val sourceContent = + """ + fun main() { + println("Hello, World!") + } + """ + + val testContentShared = + """ + import org.junit.Test + import kotlin.test.assertEquals + + class MainTest { + + @Test + fun testMain() { + assertEquals(1, 1) + } + } + """ + + val testContentLocal = + """ + import org.junit.Test + import kotlin.test.assertEquals + + class MainTestLocal { + + @Test + fun testMain() { + assertEquals(1, 2) + } + } + """ + + testBazelWorkspace.addMultiLevelSourceAndTestFileWithContent( + filename = "Main", + sourceContent = sourceContent, + testContentShared = testContentShared, + testContentLocal = testContentLocal, + subpackage = "coverage" + ) + + val sourceFile = File(tempFolder.root, "coverage/main/java/com/example/Main.kt") + val testFileShared = File(tempFolder.root, "coverage/sharedTest/java/com/example/MainTest.kt") + val testFileLocal = File(tempFolder.root, "coverage/test/java/com/example/MainLocalTest.kt") + + assertThat(sourceFile.exists()).isTrue() + assertThat(sourceFile.readText()).isEqualTo(sourceContent) + + assertThat(testFileShared.exists()).isTrue() + assertThat(testFileShared.readText()).isEqualTo(testContentShared) + + assertThat(testFileLocal.exists()).isTrue() + assertThat(testFileLocal.readText()).isEqualTo(testContentLocal) + } + + @Test + fun testAddMultiLevelSourceAndTestFileWithContent_updatesBuildFiles() { + val testBazelWorkspace = TestBazelWorkspace(tempFolder) + val sourceContent = + """ + fun main() { + println("Hello, World!") + } + """ + + val testContentShared = + """ + import org.junit.Test + import kotlin.test.assertEquals + + class MainTest { + + @Test + fun testMain() { + assertEquals(1, 1) + } + } + """ + + val testContentLocal = + """ + import org.junit.Test + import kotlin.test.assertEquals + + class MainTestLocal { + + @Test + fun testMain() { + assertEquals(1, 2) + } + } + """ + + testBazelWorkspace.addMultiLevelSourceAndTestFileWithContent( + filename = "Main", + sourceContent = sourceContent, + testContentShared = testContentShared, + testContentLocal = testContentLocal, + subpackage = "coverage" + ) + + val sourceBuildFile = File( + tempFolder.root, "coverage/main/java/com/example/BUILD.bazel" + ) + val testBuildFileShared = File( + tempFolder.root, "coverage/sharedTest/java/com/example/BUILD.bazel" + ) + val testBuildFileLocal = File( + tempFolder.root, "coverage/test/java/com/example/BUILD.bazel" + ) + + assertThat(sourceBuildFile.exists()).isTrue() + assertThat(sourceBuildFile.readText()).contains( + """ + kt_jvm_library( + name = "main", + srcs = ["Main.kt"], + visibility = ["//visibility:public"] + ) + """.trimIndent() + ) + + assertThat(testBuildFileShared.exists()).isTrue() + assertThat(testBuildFileShared.readText()).contains( + """ + load("@io_bazel_rules_kotlin//kotlin:jvm.bzl", "kt_jvm_test") + kt_jvm_test( + name = "MainTest", + srcs = ["MainTest.kt"], + deps = [ + "//coverage/main/java/com/example:main", + "@maven//:junit_junit", + ], + visibility = ["//visibility:public"], + test_class = "com.example.MainTest", + ) + """.trimIndent() + ) + + assertThat(testBuildFileLocal.exists()).isTrue() + assertThat(testBuildFileLocal.readText()).contains( + """ + load("@io_bazel_rules_kotlin//kotlin:jvm.bzl", "kt_jvm_test") + kt_jvm_test( + name = "MainLocalTest", + srcs = ["MainLocalTest.kt"], + deps = [ + "//coverage/main/java/com/example:main", + "@maven//:junit_junit", + ], + visibility = ["//visibility:public"], + test_class = "com.example.MainLocalTest", + ) + """.trimIndent() + ) + } + @Test fun testAddSourceContentAndBuildFile_createsSourceFileAndBuildFile() { val testBazelWorkspace = TestBazelWorkspace(tempFolder) @@ -406,7 +570,7 @@ class TestBazelWorkspaceTest { """ load("@io_bazel_rules_kotlin//kotlin:jvm.bzl", "kt_jvm_test") kt_jvm_test( - name = "test", + name = "MainTest", srcs = ["MainTest.kt"], deps = [ "//coverage/main/java/com/example:main", From 3d044edc0613c26ea5f982f3ca4a7200421ae564 Mon Sep 17 00:00:00 2001 From: Kenneth Murerwa Date: Wed, 26 Jun 2024 20:05:39 +0300 Subject: [PATCH 3/4] Technical Analytics: Milestone 4 - Document Technical Analytics Changes (#5353) ## Explanation When merged, this PR will; - Modify the Platform Parameter and Feature Flags documentation to reflect changes introduced by the Technical Analytics Milestones 1, 2 and 3 PRs. ## Essential Checklist - [x] The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".) - [x] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)). - [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)). ## For UI-specific PRs only If your PR includes UI-related changes, then: - Add screenshots for portrait/landscape for both a tablet & phone of the before & after UI changes - For the screenshots above, include both English and pseudo-localized (RTL) screenshots (see [RTL guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines)) - Add a video showing the full UX flow with a screen reader enabled (see [accessibility guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide)) - For PRs introducing new UI elements or color changes, both light and dark mode screenshots must be included - Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing --------- Co-authored-by: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com> --- wiki/Platform-Parameters-&-Feature-Flags.md | 146 ++++++++++++++++++-- 1 file changed, 132 insertions(+), 14 deletions(-) diff --git a/wiki/Platform-Parameters-&-Feature-Flags.md b/wiki/Platform-Parameters-&-Feature-Flags.md index 2a2b66db027..451baebf93c 100644 --- a/wiki/Platform-Parameters-&-Feature-Flags.md +++ b/wiki/Platform-Parameters-&-Feature-Flags.md @@ -2,7 +2,9 @@ - [Introduction](#introduction) - [How to create a Platform Parameter](#how-to-create-a-platform-parameter) -- [How to consume a Platform Parameter](#how-to-consume-a-platform-parameter) +- [How to create a Feature Flag](#how-to-create-a-feature-flag) +- [How to consume a Platform Parameter or Feature Flag](#how-to-consume-a-platform-parameter-or-feature-flag) +- [Ensuring your Feature Flags are logged on each app session](#ensuring-your-feature-flags-are-logged-on-each-app-session) - [How to write tests related Platform Parameter](#how-to-write-tests-related-platform-parameter) - [1. We actually don't test for platform parameter(s)](#1-we-actually-dont-test-for-platform-parameters) - [2. We test for different values of platform parameter(s)](#2-we-test-for-different-values-of-platform-parameters) @@ -14,13 +16,13 @@ In order to release these types of features in a smooth manner, we need to be ab ## How to create a Platform Parameter 1. Create the Constants - - If the Platform Parameter you intend to create is related to a particular feature, so first check that do there exist a file in the `utility\src\main\java\org\oppia\android\util\platformparameter` which contains other Platform Parameters related to the same feature. If there is no such then create a new Kotlin file along with its name corresponding to the feature. - - After searching/making a "constants" file related to a feature, we need to define three things: + - Platform parameters are typically stored inside `utility\src\main\java\org\oppia\android\util\platformparameter` in the `PlatformParameterConstants.kt`. + - To create a new platform parameter, we need to define three things: 1. Qualifier Annotation which will help us to distinguish our Platform Parameter from others.
- ``` + ```kotlin /** * Qualifier for the platform parameter that defines the time period in hours, after which the * [PlatformParameterSyncUpWorker] will run again. @@ -33,7 +35,7 @@ In order to release these types of features in a smooth manner, we need to be ab
- ``` + ```kotlin /** * Name of the platform parameter that defines the time period in hours, after which the * [PlatformParameterSyncUpWorker] will run again. @@ -45,7 +47,7 @@ In order to release these types of features in a smooth manner, we need to be ab
- ``` + ```kotlin /** * Default value of the platform parameter that defines the time period in hours, after which the * [PlatformParameterSyncUpWorker] will run again. @@ -59,7 +61,7 @@ In order to release these types of features in a smooth manner, we need to be ab
- ``` + ```kotlin /* Dagger module that provides values for individual Platform Parameters. */ @Module class PlatformParameterModule { @@ -83,29 +85,145 @@ Note: If the Platform Parameter that you are creating will only be a Compile Tim - Note that permission will be required before accessing the Feature Gating console in the Oppia backend. -## How to consume a Platform Parameter -To consume a Platform Parameter in any file, we need to inject the specific `PlatformParameterValue\` instance along with the Qualifier Annotation defined for that Parameter. For eg - we are injecting the `SyncUpTimePeriodInHours` platform parameter in `PlatformParameterSyncUpWorkManagerInitializer` +## How to create a Feature Flag +1. Create the Constant + - Feature flags, like platform parameters, are stored inside `utility\src\main\java\org\oppia\android\util\platformparameter` in the `FeatureFlagConstants.kt` file. + - To add new feature flags, we need to define three things: + 1. Qualifier Annotation which will help us to distinguish our Platform Parameter from others. Each feature flag should be prepended with an `Enable` prefix to distinguish it from platform parameters. + +
-``` + ```kotlin + /** + * Qualifier for the [EnableAppAndOsDeprecation] feature flag that controls whether to enable + * app and OS deprecation or not. + */ + @Qualifier + annotation class EnableAppAndOsDeprecation + ``` + + 2. The name of the Feature Flag in String format. This should also be prefixed with `android_enable_` to distinguish it from other feature flags on the Oppia-Web gating console. + +
+ + ```kotlin + /** Name of the feature flag that controls whether to enable app and os deprecation. */ + const val APP_AND_OS_DEPRECATION = "android_enable_app_and_os_deprecation" + ``` + + 3. The default value for the Feature Flag. For eg - here we define a `EnableAppAndOsDeprecation` feature flag and its default value as a constant. + +
+ + ```kotlin + /** + * Default value for the feature flag corresponding to [EnableAppAndOsDeprecation]. + */ + const val ENABLE_APP_AND_OS_DEPRECATION_DEFAULT_VALUE = false + ``` + +2. Provide your Feature Flag + - Feature Flags are still a special type of Platform Parameter and are provided in a similar manner. For providing your Feature Flag in the App, we need to first make a @Provides annotated method in the `PlatformParameterModule(domain\src\main\java\org\oppia\android\domain\platformparameter\PlatformParameterModule.kt)` + - Since feature flags can only be booleans, The return type for this @Provides annotated method will be equal to `PlatformPrameterValue`. Any other type will cause the platform parameter sync to fail. For eg- here we provide `EnableAppAndOsDeprecation` feature flag. + +
+ + ```kotlin + /* Dagger module that provides values for individual Platform Parameters. */ + @Module + class PlatformParameterModule { + ... + @Provides + @EnableAppAndOsDeprecation + fun provideEnableAppAndOsDeprecation( + platformParameterSingleton: PlatformParameterSingleton + ): PlatformParameterValue { + return platformParameterSingleton.getBooleanPlatformParameter(APP_AND_OS_DEPRECATION) + ?: PlatformParameterValue.createDefaultParameter( + ENABLE_APP_AND_OS_DEPRECATION_DEFAULT_VALUE + ) + } + } + ``` + +3. Add Feature Flags to Feature Gating Console + - All feature flags should be added to the feature flags console to allow them to be remotely enabled or disabled. + - Add the name and the value of our Platform Parameter. This change will make our Compile-Time Platform Parameter to be a Run-Time Platform Parameter. This means that we can control its value from backend. + - Note that permission will be required before accessing the Feature Gating console in the Oppia backend. + +## How to consume a Platform Parameter or Feature Flag +To consume a Platform Parameter in any file, we need to inject the specific `PlatformParameterValue` instance along with the Qualifier Annotation defined for that Parameter. For eg - we are injecting the `SyncUpTimePeriodInHours` platform parameter and the `EnableAppAndOSDeprecation` feature flag in `PlatformParameterSyncUpWorkManagerInitializer` + +```kotlin class PlatformParameterSyncUpWorkManagerInitializer @Inject constructor( private val context: Context, - @SyncUpWorkerTimePeriodInHours private val syncUpWorkerTimePeriod : PlatformParameterValue + @SyncUpWorkerTimePeriodInHours private val syncUpWorkerTimePeriod : PlatformParameterValue, + @EnableAppAndOsDeprecation private val enableAppAndOsDeprecation: Provider>, ) : ApplicationStartupListener { ... fun exampleFunction(){ val time: Int = syncUpWorkerTimePeriod.value - // now we can use the value in the "time" variable, which will be an integer. + // Now we can use the value in the "time" variable, which will be an integer. + + val appAndOsDeprecationEnabled = enableAppAndOsDeprecation.value + // This value can then be used to gate some features as desired. } } ``` -## How to write tests related Platform Parameter +## Ensuring your Feature Flags are logged on each app session +As a requirement, all feature flags should be logged at the beginning of each app session. This is done automatically via the `FeatureFlagsLogger.kt` inside `domain/src/main/java/org/oppia/android/domain/oppialogger/analytics/` but some configuration is required. To ensure any newly added feature flags are logged as part of this requirement, follow the steps below; + +### 1. Import the feature flag to the FeatureFlagsLogger +To get the value of the feature flag for logging, we need to consume the created feature flag as shown in the example below to ensure the value is present; + +```kotlin +/** + * Convenience logger for feature flags. + * + * This logger is meant to be used for feature flag-related logging on every app launch. It is + * primarily used within the ApplicationLifecycleObserver to log the status of feature flags in a + * given app session. + */ +@Singleton +class FeatureFlagsLogger @Inject constructor( + ... + @EnableAppAndOsDeprecation + private val enableAppAndOsDeprecation: PlatformParameterValue, +) { + ... +} +``` + +### 2. Add an entry to the list of loggable feature flags +The FeatureFlagsLogger contains a variable `featureFlagItemMap`, which is a map of feature flags to be logged and their names. Any newly added feature flags should also be added here to ensure that they are logged as well. + +```kotlin +/** + * A variable containing a list of all the feature flags in the app. + * + * @return a list of key-value pairs of [String] and [PlatformParameterValue] + */ + private var featureFlagItemMap: Map> = mapOf( + ... + APP_AND_OS_DEPRECATION to enableAppAndOsDeprecation + ) +``` + +### 3. Update the Feature Flags Logger test +Besides the feature-flag logger, the `FeatureFlagLoggerTest` located at `domain/src/test/java/org/oppia/android/domain/oppialogger/analytics/FeatureFlagsLoggerTest.kt` will also need to be updated to reflect the newly added feature flag(s). There are two tests that will need to be changed. + +- The first test that should be updated is the `testLogFeatureFlags_correctNumberOfFeatureFlagsIsLogged` test. For this, only the constant `expectedFeatureFlagCount` will need to be updated. If a new feature flag was added, increment the count and if one was removed, decrement the count. + +- The second test that will need to be updated is the `testLogFeatureFlags_allFeatureFlagNamesAreLogged`. This is a parameterized test that iterates through each currently existing feature flag to ensure each one of them is logged as expected. To update this test and ensure it passes after a feature flag change, modify the `RunParameterized()` section and either add the expected values for the new flag or remove the expected values for a removed feature flag. + +## How to write tests related to Platform Parameters Before writing a test we must understand the purpose of the platform parameter in our class/classes (that needs to be tested). After verifying this we can divide testing procedures into following groups - ### 1. We actually don't test for platform parameter(s) We just need specific platform parameter(s) in the dagger graph because our class needs it, but our test cases are not actually verifying the behaviour of class based on different values of the platform parameter. These are the simplest cases to write tests for. We will only need to create a `TestModule` inside the Test class and then include this into the @Component for the `TestApplicationComponent`. For eg - -``` +```kotlin @Module class TestModule { @Provides From c5de68bdf29f2ac965bc02e266c188a2c3f07962 Mon Sep 17 00:00:00 2001 From: RD Rama Devi <122200035+Rd4dev@users.noreply.github.com> Date: Thu, 27 Jun 2024 14:00:59 +0530 Subject: [PATCH 4/4] Fix part of #5343: Building proto with coverage results for data processing (#5439) ## Explanation Fixes part of #5343 ### Project [PR 1.4 of Project 4.1] ### Changes Made - Introduced `coverage.proto` to store the coverage data generated during coverage analysis. - Parsed coverage data according to the LCOV (geninfo) file format specifications: [LCOV geninfo specifications](https://manpages.debian.org/stretch/lcov/geninfo.1.en.html#FILES) - This maps the Bazel test target to the corresponding SF: source file path and filters the data to generate the Coverage Report proto that will only the coverage of the tests related to the single source file. - With the previous implementation in PR 1.3 to get the related LocalTest and Test file mappings, we utilize that to run coverage for both the test and return them as a list of Coverage Report protos. **To Note:** - Only the app module has the multi test file case. - But as of date, the many app module coverage executions fail. - So to test acquiring multi test coverage execution as list of Coverage Report Proto, I hardcoded and tested the implementation [[hardcoded comment](https://github.com/oppia/oppia-android/pull/5439/files#diff-62c81482e7c4b720da2d4efc73a48d10369eee097e0bf3439def6648ff150c23R89-R93)] (just for testing the actual code wouldn't have that) **Result:** A representation of the list of Coverage Report proto is presented below: This is the output on running 2 hardcoded test cases: `math:MathModelTest` and `math:FloatExtensionsTest`
List of Coverage Report 
Parsed Coverage Data File:
/home/rddev/.../bazel-out/.../util/parser/math/MathModelTest/coverage.dat
Parsed Coverage Data File:
/home/rddev/.../bazel-out/.../util/math/FloatExtensionsTest/coverage.dat
Coverage Data List: [bazel_test_target:
"//utility/src/test/java/org/oppia/android/util/parser/math:MathModelTest"
covered_file {
file_path:
"utility/src/main/java/org/oppia/android/util/parser/math/MathModel.kt"
  file_sha1_hash: "37eb0c6a3a16c300da100e245e7d320c61cc008f"
  covered_line {
    line_number: 14
    coverage: FULL
  }
  covered_line {
    line_number: 15
    coverage: FULL
  }
  covered_line {
    line_number: 16
    coverage: FULL
  }
  covered_line {
    line_number: 17
    coverage: FULL
  }
  covered_line {
    line_number: 21
    coverage: FULL
  }
  covered_line {
    line_number: 33
    coverage: FULL
  }
  covered_line {
    line_number: 34
    coverage: FULL
  }
  covered_line {
    line_number: 35
    coverage: FULL
  }
  covered_line {
    line_number: 36
    coverage: FULL
  }
  covered_line {
    line_number: 41
    coverage: FULL
  }
  covered_line {
    line_number: 42
    coverage: FULL
  }
  covered_line {
    line_number: 43
    coverage: FULL
  }
  covered_line {
    line_number: 44
    coverage: FULL
  }
  covered_line {
    line_number: 45
    coverage: FULL
  }
  covered_line {
    line_number: 46
    coverage: FULL
  }
  covered_line {
    line_number: 47
    coverage: FULL
  }
  covered_line {
    line_number: 49
    coverage: FULL
  }
  covered_line {
    line_number: 58
    coverage: FULL
  }
  covered_line {
    line_number: 59
    coverage: FULL
  }
  lines_found: 19
  lines_hit: 19
  function_coverage {
    line_number: 58
function_name:
"org/oppia/android/util/parser/math/MathModel$MathModelSignature$Companion::createSignature$utility_src_main_java_org_oppia_android_util_parser_math_math_latex_model_kt
(Ljava/lang/String;FZ)Lorg/oppia/android/util/parser/math/MathModel$MathModelSignature;"
    execution_count: 1
    coverage: FULL
  }
  function_coverage {
    line_number: 33
function_name:
"org/oppia/android/util/parser/math/MathModel$MathModelSignature::
(Ljava/lang/String;IZ)V"
    execution_count: 1
    coverage: FULL
  }
  function_coverage {
    line_number: 35
function_name:
"org/oppia/android/util/parser/math/MathModel$MathModelSignature::getLineHeightHundredX
()I"
    execution_count: 1
    coverage: FULL
  }
  function_coverage {
    line_number: 34
function_name:
"org/oppia/android/util/parser/math/MathModel$MathModelSignature::getRawLatex
()Ljava/lang/String;"
    execution_count: 0
    coverage: NONE
  }
  function_coverage {
    line_number: 36
function_name:
"org/oppia/android/util/parser/math/MathModel$MathModelSignature::getUseInlineRendering
()Z"
    execution_count: 1
    coverage: FULL
  }
  function_coverage {
    line_number: 41
function_name:
"org/oppia/android/util/parser/math/MathModel$MathModelSignature::updateDiskCacheKey
(Ljava/security/MessageDigest;)V"
    execution_count: 1
    coverage: FULL
  }
  function_coverage {
    line_number: 14
function_name: "org/oppia/android/util/parser/math/MathModel::
(Ljava/lang/String;FZ)V"
    execution_count: 1
    coverage: FULL
  }
  function_coverage {
    line_number: 16
function_name:
"org/oppia/android/util/parser/math/MathModel::getLineHeight ()F"
    execution_count: 0
    coverage: NONE
  }
  function_coverage {
    line_number: 15
function_name:
"org/oppia/android/util/parser/math/MathModel::getRawLatex
()Ljava/lang/String;"
    execution_count: 0
    coverage: NONE
  }
  function_coverage {
    line_number: 17
function_name:
"org/oppia/android/util/parser/math/MathModel::getUseInlineRendering
()Z"
    execution_count: 0
    coverage: NONE
  }
  function_coverage {
    line_number: 21
function_name:
"org/oppia/android/util/parser/math/MathModel::toKeySignature
()Lorg/oppia/android/util/parser/math/MathModel$MathModelSignature;"
    execution_count: 1
    coverage: FULL
  }
  functions_found: 11
  functions_hit: 7
  branch_coverage {
    line_number: 46
    block_number: 0
    branch_number: 0
    hit_count: 1
    coverage: FULL
  }
  branch_coverage {
    line_number: 46
    block_number: 0
    branch_number: 1
    hit_count: 1
    coverage: FULL
  }
  branches_found: 2
  branches_hit: 2
}
, bazel_test_target:
"//utility/src/test/java/org/oppia/android/util/math:FloatExtensionsTest"
covered_file {
file_path:
"utility/src/main/java/org/oppia/android/util/math/FloatExtensions.kt"
  file_sha1_hash: "178a2d84e9182a11b41adf34abc0a92a7d365f50"
  covered_line {
    line_number: 28
    coverage: FULL
  }
  covered_line {
    line_number: 36
    coverage: FULL
  }
  covered_line {
    line_number: 43
    coverage: FULL
  }
  lines_found: 3
  lines_hit: 3
  function_coverage {
    line_number: 36
function_name:
"org/oppia/android/util/math/FloatExtensionsKt::isApproximatelyEqualTo
(DD)Z"
    execution_count: 1
    coverage: FULL
  }
  function_coverage {
    line_number: 28
function_name:
"org/oppia/android/util/math/FloatExtensionsKt::isApproximatelyEqualTo
(FF)Z"
    execution_count: 1
    coverage: FULL
  }
  function_coverage {
    line_number: 43
function_name:
"org/oppia/android/util/math/FloatExtensionsKt::toPlainString
(D)Ljava/lang/String;"
    execution_count: 1
    coverage: FULL
  }
  functions_found: 3
  functions_hit: 3
  branch_coverage {
    line_number: 28
    block_number: 0
    branch_number: 0
    hit_count: 1
    coverage: FULL
  }
  branch_coverage {
    line_number: 28
    block_number: 0
    branch_number: 1
    hit_count: 1
    coverage: FULL
  }
  branch_coverage {
    line_number: 36
    block_number: 0
    branch_number: 0
    hit_count: 1
    coverage: FULL
  }
  branch_coverage {
    line_number: 36
    block_number: 0
    branch_number: 1
    hit_count: 1
    coverage: FULL
  }
  branches_found: 4
  branches_hit: 4
}
]
**Representation in Protobuf** - Function Coverage: Interpreted execution counts to determine coverage status. Any execution count greater than 0 signifies full coverage, otherwise considered null. - Branch Coverage: Determined coverage based on the recorded hit count for each branch. A hit count greater than 0 denotes full coverage, otherwise marked as null. **Usage:** The stored proto will be used to generate coverage reports in **md** and **HTML** formats [to be handled in PR 1.5] ## Essential Checklist - [x] The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".) - [x] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)). - [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [ ] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)). ## For UI-specific PRs only If your PR includes UI-related changes, then: - Add screenshots for portrait/landscape for both a tablet & phone of the before & after UI changes - For the screenshots above, include both English and pseudo-localized (RTL) screenshots (see [RTL guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines)) - Add a video showing the full UX flow with a screen reader enabled (see [accessibility guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide)) - For PRs introducing new UI elements or color changes, both light and dark mode screenshots must be included - Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing --------- Co-authored-by: Ben Henning Co-authored-by: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com> --- .../android/scripts/coverage/BUILD.bazel | 1 + .../scripts/coverage/CoverageRunner.kt | 76 ++- .../android/scripts/coverage/RunCoverage.kt | 11 +- .../oppia/android/scripts/proto/BUILD.bazel | 11 + .../android/scripts/proto/coverage.proto | 40 ++ .../scripts/testing/TestBazelWorkspace.kt | 4 +- .../android/scripts/common/BazelClientTest.kt | 1 + .../scripts/coverage/CoverageRunnerTest.kt | 58 ++- .../scripts/coverage/RunCoverageTest.kt | 475 +++++++++++++----- .../scripts/testing/TestBazelWorkspaceTest.kt | 2 + 10 files changed, 527 insertions(+), 152 deletions(-) create mode 100644 scripts/src/java/org/oppia/android/scripts/proto/coverage.proto diff --git a/scripts/src/java/org/oppia/android/scripts/coverage/BUILD.bazel b/scripts/src/java/org/oppia/android/scripts/coverage/BUILD.bazel index aa0c56dd7c3..24cc091c3e7 100644 --- a/scripts/src/java/org/oppia/android/scripts/coverage/BUILD.bazel +++ b/scripts/src/java/org/oppia/android/scripts/coverage/BUILD.bazel @@ -27,5 +27,6 @@ kt_jvm_library( visibility = ["//scripts:oppia_script_binary_visibility"], deps = [ "//scripts/src/java/org/oppia/android/scripts/common:bazel_client", + "//scripts/src/java/org/oppia/android/scripts/proto:coverage_java_proto", ], ) diff --git a/scripts/src/java/org/oppia/android/scripts/coverage/CoverageRunner.kt b/scripts/src/java/org/oppia/android/scripts/coverage/CoverageRunner.kt index e4701cd2da3..189f72a4463 100644 --- a/scripts/src/java/org/oppia/android/scripts/coverage/CoverageRunner.kt +++ b/scripts/src/java/org/oppia/android/scripts/coverage/CoverageRunner.kt @@ -6,7 +6,13 @@ import kotlinx.coroutines.async import org.oppia.android.scripts.common.BazelClient import org.oppia.android.scripts.common.CommandExecutor import org.oppia.android.scripts.common.ScriptBackgroundCoroutineDispatcher +import org.oppia.android.scripts.proto.Coverage +import org.oppia.android.scripts.proto.CoverageReport +import org.oppia.android.scripts.proto.CoveredLine import java.io.File +import java.nio.file.Files +import java.nio.file.Paths +import java.security.MessageDigest /** * Class responsible for running coverage analysis asynchronously. @@ -30,9 +36,12 @@ class CoverageRunner( */ fun runWithCoverageAsync( bazelTestTarget: String - ): Deferred?> { + ): Deferred { return CoroutineScope(scriptBgDispatcher).async { - retrieveCoverageResult(bazelTestTarget) + val coverageResult = retrieveCoverageResult(bazelTestTarget) + ?: error("Failed to retrieve coverage result for $bazelTestTarget") + + coverageDataFileLines(coverageResult, bazelTestTarget) } } @@ -41,4 +50,67 @@ class CoverageRunner( ): List? { return bazelClient.runCoverageForTestTarget(bazelTestTarget) } + + private fun coverageDataFileLines( + coverageData: List, + bazelTestTarget: String + ): CoverageReport { + val extractedFileName = "${extractTargetName(bazelTestTarget)}.kt" + + val sfStartIdx = coverageData.indexOfFirst { + it.startsWith("SF:") && it.substringAfter("SF:").substringAfterLast("/") == extractedFileName + } + if (sfStartIdx == -1) throw IllegalArgumentException("File not found") + val eofIdx = coverageData.subList(sfStartIdx, coverageData.size).indexOfFirst { + it.startsWith("end_of_record") + } + if (eofIdx == -1) throw IllegalArgumentException("End of record not found") + + val fileSpecificCovDatLines = coverageData.subList(sfStartIdx, sfStartIdx + eofIdx + 1) + + val coverageDataProps = fileSpecificCovDatLines.groupBy { line -> + line.substringBefore(":") + }.mapValues { (_, lines) -> + lines.map { line -> + line.substringAfter(":").split(",") + } + } + + val filePath = coverageDataProps["SF"]?.firstOrNull()?.get(0) + ?: throw IllegalArgumentException("File path not found") + + val linesFound = coverageDataProps["LF"]?.singleOrNull()?.single()?.toInt() ?: 0 + val linesHit = coverageDataProps["LH"]?.singleOrNull()?.single()?.toInt() ?: 0 + + val coveredLines = coverageDataProps["DA"]?.map { (lineNumStr, hitCountStr) -> + CoveredLine.newBuilder().apply { + this.lineNumber = lineNumStr.toInt() + this.coverage = if (hitCountStr.toInt() > 0) Coverage.FULL else Coverage.NONE + }.build() + }.orEmpty() + + val file = File(repoRoot, filePath) + val fileSha1Hash = calculateSha1(file.absolutePath) + + return CoverageReport.newBuilder() + .setBazelTestTarget(bazelTestTarget) + .setFilePath(filePath) + .setFileSha1Hash(fileSha1Hash) + .addAllCoveredLine(coveredLines) + .setLinesFound(linesFound) + .setLinesHit(linesHit) + .build() + } +} + +private fun extractTargetName(bazelTestTarget: String): String { + val targetName = bazelTestTarget.substringAfterLast(":").trim() + return targetName.removeSuffix("LocalTest").removeSuffix("Test") +} + +private fun calculateSha1(filePath: String): String { + val fileBytes = Files.readAllBytes(Paths.get(filePath)) + val digest = MessageDigest.getInstance("SHA-1") + val hashBytes = digest.digest(fileBytes) + return hashBytes.joinToString("") { "%02x".format(it) } } diff --git a/scripts/src/java/org/oppia/android/scripts/coverage/RunCoverage.kt b/scripts/src/java/org/oppia/android/scripts/coverage/RunCoverage.kt index 35b27a4b95e..448519f947d 100644 --- a/scripts/src/java/org/oppia/android/scripts/coverage/RunCoverage.kt +++ b/scripts/src/java/org/oppia/android/scripts/coverage/RunCoverage.kt @@ -5,6 +5,7 @@ import org.oppia.android.scripts.common.BazelClient import org.oppia.android.scripts.common.CommandExecutor import org.oppia.android.scripts.common.CommandExecutorImpl import org.oppia.android.scripts.common.ScriptBackgroundCoroutineDispatcher +import org.oppia.android.scripts.proto.CoverageReport import org.oppia.android.scripts.proto.TestFileExemptions import java.io.File import java.util.concurrent.TimeUnit @@ -78,7 +79,7 @@ class RunCoverage( * @return a list of lists containing coverage data for each requested test target, if * the file is exempted from having a test file, an empty list is returned */ - fun execute(): List> { + fun execute(): List { val testFileExemptionList = loadTestFileExemptionsProto(testFileExemptionTextProto) .testFileExemptionList .filter { it.testFileNotRequired } @@ -97,15 +98,11 @@ class RunCoverage( val testTargets = bazelClient.retrieveBazelTargets(testFilePaths) return testTargets.mapNotNull { testTarget -> - val coverageData = runCoverageForTarget(testTarget) - if (coverageData == null) { - println("Coverage data for $testTarget is null") - } - coverageData + runCoverageForTarget(testTarget) } } - private fun runCoverageForTarget(testTarget: String): List? { + private fun runCoverageForTarget(testTarget: String): CoverageReport { return runBlocking { CoverageRunner(rootDirectory, scriptBgDispatcher, commandExecutor) .runWithCoverageAsync(testTarget.removeSuffix(".kt")) diff --git a/scripts/src/java/org/oppia/android/scripts/proto/BUILD.bazel b/scripts/src/java/org/oppia/android/scripts/proto/BUILD.bazel index 7265ff0efc3..72956d786a5 100644 --- a/scripts/src/java/org/oppia/android/scripts/proto/BUILD.bazel +++ b/scripts/src/java/org/oppia/android/scripts/proto/BUILD.bazel @@ -21,6 +21,17 @@ java_proto_library( deps = [":affected_tests_proto"], ) +oppia_proto_library( + name = "coverage_proto", + srcs = ["coverage.proto"], +) + +java_proto_library( + name = "coverage_java_proto", + visibility = ["//scripts:oppia_script_library_visibility"], + deps = [":coverage_proto"], +) + oppia_proto_library( name = "filename_pattern_validation_checks_proto", srcs = ["filename_pattern_validation_checks.proto"], diff --git a/scripts/src/java/org/oppia/android/scripts/proto/coverage.proto b/scripts/src/java/org/oppia/android/scripts/proto/coverage.proto new file mode 100644 index 00000000000..3b4af5c070e --- /dev/null +++ b/scripts/src/java/org/oppia/android/scripts/proto/coverage.proto @@ -0,0 +1,40 @@ +syntax = "proto3"; + +package proto; + +option java_package = "org.oppia.android.scripts.proto"; +option java_multiple_files = true; + +// Coverage Report that contains the bazel coverage data retrieved from the +// Bazel coverage execution. +message CoverageReport { + // The test target for which the coverage report is generated. + string bazel_test_target = 1; + // The relative path of the covered file. + string file_path = 2; + // SHA-1 hash of the file content at the time of report (to guard against changes). + string file_sha1_hash = 3; + // The lines of code covered in the report. + repeated CoveredLine covered_line = 4; + // The total number of lines found in the covered file. + int32 lines_found = 5; + // The total number of lines hit in the covered file. + int32 lines_hit = 6; +} + +// Information about a single line that was covered during the tests. +message CoveredLine { + // The line number of the covered line. + int32 line_number = 1; + // The coverage status of the covered line. + Coverage coverage = 2; +} + +enum Coverage { + // Coverage status is unspecified. + UNSPECIFIED = 0; + // The line, branch, or function is fully covered, ie. executed atleast once. + FULL = 1; + // The line, branch, or function is not covered at all. + NONE = 2; +} diff --git a/scripts/src/java/org/oppia/android/scripts/testing/TestBazelWorkspace.kt b/scripts/src/java/org/oppia/android/scripts/testing/TestBazelWorkspace.kt index 2e4f1b20c0f..d9ba7d2fd70 100644 --- a/scripts/src/java/org/oppia/android/scripts/testing/TestBazelWorkspace.kt +++ b/scripts/src/java/org/oppia/android/scripts/testing/TestBazelWorkspace.kt @@ -62,6 +62,7 @@ class TestBazelWorkspace(private val temporaryRootFolder: TemporaryFolder) { */ fun addSourceAndTestFileWithContent( filename: String, + testFilename: String, sourceContent: String, testContent: String, sourceSubpackage: String, @@ -73,10 +74,9 @@ class TestBazelWorkspace(private val temporaryRootFolder: TemporaryFolder) { sourceSubpackage ) - val testFileName = "${filename}Test" addTestContentAndBuildFile( filename, - testFileName, + testFilename, testContent, sourceSubpackage, testSubpackage diff --git a/scripts/src/javatests/org/oppia/android/scripts/common/BazelClientTest.kt b/scripts/src/javatests/org/oppia/android/scripts/common/BazelClientTest.kt index db725861d16..ae4b2deee6f 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/common/BazelClientTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/common/BazelClientTest.kt @@ -422,6 +422,7 @@ class BazelClientTest { testBazelWorkspace.addSourceAndTestFileWithContent( filename = "TwoSum", + testFilename = "TwoSumTest", sourceContent = sourceContent, testContent = testContent, sourceSubpackage = "coverage/main/java/com/example", diff --git a/scripts/src/javatests/org/oppia/android/scripts/coverage/CoverageRunnerTest.kt b/scripts/src/javatests/org/oppia/android/scripts/coverage/CoverageRunnerTest.kt index 6a89e9db798..7d795703912 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/coverage/CoverageRunnerTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/coverage/CoverageRunnerTest.kt @@ -9,6 +9,9 @@ import org.junit.Test import org.junit.rules.TemporaryFolder import org.oppia.android.scripts.common.CommandExecutorImpl import org.oppia.android.scripts.common.ScriptBackgroundCoroutineDispatcher +import org.oppia.android.scripts.proto.Coverage +import org.oppia.android.scripts.proto.CoverageReport +import org.oppia.android.scripts.proto.CoveredLine import org.oppia.android.scripts.testing.TestBazelWorkspace import org.oppia.android.testing.assertThrows import java.util.concurrent.TimeUnit @@ -103,6 +106,7 @@ class CoverageRunnerTest { testBazelWorkspace.addSourceAndTestFileWithContent( filename = "TwoSum", + testFilename = "TwoSumTest", sourceContent = sourceContent, testContent = testContent, sourceSubpackage = "coverage/main/java/com/example", @@ -114,28 +118,38 @@ class CoverageRunnerTest { "//coverage/test/java/com/example:TwoSumTest" ).await() } - val expectedResult = listOf( - "SF:coverage/main/java/com/example/TwoSum.kt", - "FN:7,com/example/TwoSum${'$'}Companion::sumNumbers (II)Ljava/lang/Object;", - "FN:3,com/example/TwoSum:: ()V", - "FNDA:1,com/example/TwoSum${'$'}Companion::sumNumbers (II)Ljava/lang/Object;", - "FNDA:0,com/example/TwoSum:: ()V", - "FNF:2", - "FNH:1", - "BRDA:7,0,0,1", - "BRDA:7,0,1,1", - "BRDA:7,0,2,1", - "BRDA:7,0,3,1", - "BRF:4", - "BRH:4", - "DA:3,0", - "DA:7,1", - "DA:8,1", - "DA:10,1", - "LH:3", - "LF:4", - "end_of_record" - ) + + val expectedResult = CoverageReport.newBuilder() + .setBazelTestTarget("//coverage/test/java/com/example:TwoSumTest") + .setFilePath("coverage/main/java/com/example/TwoSum.kt") + .setFileSha1Hash("f6fb075e115775f6729615a79f0e7e34fe9735b5") + .addCoveredLine( + CoveredLine.newBuilder() + .setLineNumber(3) + .setCoverage(Coverage.NONE) + .build() + ) + .addCoveredLine( + CoveredLine.newBuilder() + .setLineNumber(7) + .setCoverage(Coverage.FULL) + .build() + ) + .addCoveredLine( + CoveredLine.newBuilder() + .setLineNumber(8) + .setCoverage(Coverage.FULL) + .build() + ) + .addCoveredLine( + CoveredLine.newBuilder() + .setLineNumber(10) + .setCoverage(Coverage.FULL) + .build() + ) + .setLinesFound(4) + .setLinesHit(3) + .build() assertThat(result).isEqualTo(expectedResult) } diff --git a/scripts/src/javatests/org/oppia/android/scripts/coverage/RunCoverageTest.kt b/scripts/src/javatests/org/oppia/android/scripts/coverage/RunCoverageTest.kt index fb9db5d14b0..8c03e97a5c4 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/coverage/RunCoverageTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/coverage/RunCoverageTest.kt @@ -8,6 +8,9 @@ import org.junit.Test import org.junit.rules.TemporaryFolder import org.oppia.android.scripts.common.CommandExecutorImpl import org.oppia.android.scripts.common.ScriptBackgroundCoroutineDispatcher +import org.oppia.android.scripts.proto.Coverage +import org.oppia.android.scripts.proto.CoverageReport +import org.oppia.android.scripts.proto.CoveredLine import org.oppia.android.scripts.testing.TestBazelWorkspace import org.oppia.android.testing.assertThrows import java.io.ByteArrayOutputStream @@ -121,6 +124,7 @@ class RunCoverageTest { testBazelWorkspace.addSourceAndTestFileWithContent( filename = "TwoSum", + testFilename = "TwoSumTest", sourceContent = sourceContent, testContent = testContent, sourceSubpackage = "coverage/main/java/com/example", @@ -134,32 +138,41 @@ class RunCoverageTest { scriptBgDispatcher ).execute() - val expectedResultList = listOf( - listOf( - "SF:coverage/main/java/com/example/TwoSum.kt", - "FN:7,com/example/TwoSum${'$'}Companion::sumNumbers (II)Ljava/lang/Object;", - "FN:3,com/example/TwoSum:: ()V", - "FNDA:1,com/example/TwoSum${'$'}Companion::sumNumbers (II)Ljava/lang/Object;", - "FNDA:0,com/example/TwoSum:: ()V", - "FNF:2", - "FNH:1", - "BRDA:7,0,0,1", - "BRDA:7,0,1,1", - "BRDA:7,0,2,1", - "BRDA:7,0,3,1", - "BRF:4", - "BRH:4", - "DA:3,0", - "DA:7,1", - "DA:8,1", - "DA:10,1", - "LH:3", - "LF:4", - "end_of_record" - ) + val expectedResult = listOf( + CoverageReport.newBuilder() + .setBazelTestTarget("//coverage/test/java/com/example:TwoSumTest") + .setFilePath("coverage/main/java/com/example/TwoSum.kt") + .setFileSha1Hash("f6fb075e115775f6729615a79f0e7e34fe9735b5") + .addCoveredLine( + CoveredLine.newBuilder() + .setLineNumber(3) + .setCoverage(Coverage.NONE) + .build() + ) + .addCoveredLine( + CoveredLine.newBuilder() + .setLineNumber(7) + .setCoverage(Coverage.FULL) + .build() + ) + .addCoveredLine( + CoveredLine.newBuilder() + .setLineNumber(8) + .setCoverage(Coverage.FULL) + .build() + ) + .addCoveredLine( + CoveredLine.newBuilder() + .setLineNumber(10) + .setCoverage(Coverage.FULL) + .build() + ) + .setLinesFound(4) + .setLinesHit(3) + .build() ) - assertThat(result).isEqualTo(expectedResultList) + assertThat(result).isEqualTo(expectedResult) } @Test @@ -174,7 +187,7 @@ class RunCoverageTest { companion object { fun sumNumbers(a: Int, b: Int): Any { - return if (a ==0 && b == 0) { + return if (a == 0 && b == 0) { "Both numbers are zero" } else { a + b @@ -204,6 +217,7 @@ class RunCoverageTest { testBazelWorkspace.addSourceAndTestFileWithContent( filename = "TwoSum", + testFilename = "TwoSumTest", sourceContent = sourceContent, testContent = testContent, sourceSubpackage = "scripts/java/com/example", @@ -217,32 +231,41 @@ class RunCoverageTest { scriptBgDispatcher ).execute() - val expectedResultList = listOf( - listOf( - "SF:scripts/java/com/example/TwoSum.kt", - "FN:7,com/example/TwoSum${'$'}Companion::sumNumbers (II)Ljava/lang/Object;", - "FN:3,com/example/TwoSum:: ()V", - "FNDA:1,com/example/TwoSum${'$'}Companion::sumNumbers (II)Ljava/lang/Object;", - "FNDA:0,com/example/TwoSum:: ()V", - "FNF:2", - "FNH:1", - "BRDA:7,0,0,1", - "BRDA:7,0,1,1", - "BRDA:7,0,2,1", - "BRDA:7,0,3,1", - "BRF:4", - "BRH:4", - "DA:3,0", - "DA:7,1", - "DA:8,1", - "DA:10,1", - "LH:3", - "LF:4", - "end_of_record" - ) + val expectedResult = listOf( + CoverageReport.newBuilder() + .setBazelTestTarget("//scripts/javatests/com/example:TwoSumTest") + .setFilePath("scripts/java/com/example/TwoSum.kt") + .setFileSha1Hash("1020b8f405555b3f4537fd07b912d3fb9ffa3354") + .addCoveredLine( + CoveredLine.newBuilder() + .setLineNumber(3) + .setCoverage(Coverage.NONE) + .build() + ) + .addCoveredLine( + CoveredLine.newBuilder() + .setLineNumber(7) + .setCoverage(Coverage.FULL) + .build() + ) + .addCoveredLine( + CoveredLine.newBuilder() + .setLineNumber(8) + .setCoverage(Coverage.FULL) + .build() + ) + .addCoveredLine( + CoveredLine.newBuilder() + .setLineNumber(10) + .setCoverage(Coverage.FULL) + .build() + ) + .setLinesFound(4) + .setLinesHit(3) + .build() ) - assertThat(result).isEqualTo(expectedResultList) + assertThat(result).isEqualTo(expectedResult) } @Test @@ -287,6 +310,100 @@ class RunCoverageTest { testBazelWorkspace.addSourceAndTestFileWithContent( filename = "TwoSum", + testFilename = "TwoSumTest", + sourceContent = sourceContent, + testContent = testContent, + sourceSubpackage = "app/main/java/com/example", + testSubpackage = "app/test/java/com/example" + ) + + val result = RunCoverage( + "${tempFolder.root}", + "app/main/java/com/example/TwoSum.kt", + longCommandExecutor, + scriptBgDispatcher + ).execute() + + val expectedResult = listOf( + CoverageReport.newBuilder() + .setBazelTestTarget("//app/test/java/com/example:TwoSumTest") + .setFilePath("app/main/java/com/example/TwoSum.kt") + .setFileSha1Hash("f6fb075e115775f6729615a79f0e7e34fe9735b5") + .addCoveredLine( + CoveredLine.newBuilder() + .setLineNumber(3) + .setCoverage(Coverage.NONE) + .build() + ) + .addCoveredLine( + CoveredLine.newBuilder() + .setLineNumber(7) + .setCoverage(Coverage.FULL) + .build() + ) + .addCoveredLine( + CoveredLine.newBuilder() + .setLineNumber(8) + .setCoverage(Coverage.FULL) + .build() + ) + .addCoveredLine( + CoveredLine.newBuilder() + .setLineNumber(10) + .setCoverage(Coverage.FULL) + .build() + ) + .setLinesFound(4) + .setLinesHit(3) + .build() + ) + + assertThat(result).isEqualTo(expectedResult) + } + + @Test + fun testRunCoverage_localTests_returnsCoverageData() { + testBazelWorkspace.initEmptyWorkspace() + + val sourceContent = + """ + package com.example + + class TwoSum { + + companion object { + fun sumNumbers(a: Int, b: Int): Any { + return if (a ==0 && b == 0) { + "Both numbers are zero" + } else { + a + b + } + } + } + } + """.trimIndent() + + val testContent = + """ + package com.example + + import org.junit.Assert.assertEquals + import org.junit.Test + + class TwoSumLocalTest { + + @Test + fun testSumNumbers() { + assertEquals(TwoSum.sumNumbers(0, 1), 1) + assertEquals(TwoSum.sumNumbers(3, 4), 7) + assertEquals(TwoSum.sumNumbers(0, 0), "Both numbers are zero") + } + } + """.trimIndent() + + testBazelWorkspace.addSourceAndTestFileWithContent( + filename = "TwoSum", + testFilename = "TwoSumLocalTest", sourceContent = sourceContent, testContent = testContent, sourceSubpackage = "app/main/java/com/example", @@ -300,32 +417,134 @@ class RunCoverageTest { scriptBgDispatcher ).execute() - val expectedResultList = listOf( - listOf( - "SF:app/main/java/com/example/TwoSum.kt", - "FN:7,com/example/TwoSum${'$'}Companion::sumNumbers (II)Ljava/lang/Object;", - "FN:3,com/example/TwoSum:: ()V", - "FNDA:1,com/example/TwoSum${'$'}Companion::sumNumbers (II)Ljava/lang/Object;", - "FNDA:0,com/example/TwoSum:: ()V", - "FNF:2", - "FNH:1", - "BRDA:7,0,0,1", - "BRDA:7,0,1,1", - "BRDA:7,0,2,1", - "BRDA:7,0,3,1", - "BRF:4", - "BRH:4", - "DA:3,0", - "DA:7,1", - "DA:8,1", - "DA:10,1", - "LH:3", - "LF:4", - "end_of_record" - ) + val expectedResult = listOf( + CoverageReport.newBuilder() + .setBazelTestTarget("//app/test/java/com/example:TwoSumLocalTest") + .setFilePath("app/main/java/com/example/TwoSum.kt") + .setFileSha1Hash("f6fb075e115775f6729615a79f0e7e34fe9735b5") + .addCoveredLine( + CoveredLine.newBuilder() + .setLineNumber(3) + .setCoverage(Coverage.NONE) + .build() + ) + .addCoveredLine( + CoveredLine.newBuilder() + .setLineNumber(7) + .setCoverage(Coverage.FULL) + .build() + ) + .addCoveredLine( + CoveredLine.newBuilder() + .setLineNumber(8) + .setCoverage(Coverage.FULL) + .build() + ) + .addCoveredLine( + CoveredLine.newBuilder() + .setLineNumber(10) + .setCoverage(Coverage.FULL) + .build() + ) + .setLinesFound(4) + .setLinesHit(3) + .build() + ) + + assertThat(result).isEqualTo(expectedResult) + } + + @Test + fun testRunCoverage_sharedTests_returnsCoverageData() { + testBazelWorkspace.initEmptyWorkspace() + + val sourceContent = + """ + package com.example + + class TwoSum { + + companion object { + fun sumNumbers(a: Int, b: Int): Any { + return if (a ==0 && b == 0) { + "Both numbers are zero" + } else { + a + b + } + } + } + } + """.trimIndent() + + val testContent = + """ + package com.example + + import org.junit.Assert.assertEquals + import org.junit.Test + + class TwoSumTest { + + @Test + fun testSumNumbers() { + assertEquals(TwoSum.sumNumbers(0, 1), 1) + assertEquals(TwoSum.sumNumbers(3, 4), 7) + assertEquals(TwoSum.sumNumbers(0, 0), "Both numbers are zero") + } + } + """.trimIndent() + + testBazelWorkspace.addSourceAndTestFileWithContent( + filename = "TwoSum", + testFilename = "TwoSumTest", + sourceContent = sourceContent, + testContent = testContent, + sourceSubpackage = "app/main/java/com/example", + testSubpackage = "app/sharedTest/java/com/example" + ) + + val result = RunCoverage( + "${tempFolder.root}", + "app/main/java/com/example/TwoSum.kt", + longCommandExecutor, + scriptBgDispatcher + ).execute() + + val expectedResult = listOf( + CoverageReport.newBuilder() + .setBazelTestTarget("//app/sharedTest/java/com/example:TwoSumTest") + .setFilePath("app/main/java/com/example/TwoSum.kt") + .setFileSha1Hash("f6fb075e115775f6729615a79f0e7e34fe9735b5") + .addCoveredLine( + CoveredLine.newBuilder() + .setLineNumber(3) + .setCoverage(Coverage.NONE) + .build() + ) + .addCoveredLine( + CoveredLine.newBuilder() + .setLineNumber(7) + .setCoverage(Coverage.FULL) + .build() + ) + .addCoveredLine( + CoveredLine.newBuilder() + .setLineNumber(8) + .setCoverage(Coverage.FULL) + .build() + ) + .addCoveredLine( + CoveredLine.newBuilder() + .setLineNumber(10) + .setCoverage(Coverage.FULL) + .build() + ) + .setLinesFound(4) + .setLinesHit(3) + .build() ) - assertThat(result).isEqualTo(expectedResultList) + assertThat(result).isEqualTo(expectedResult) } @Test @@ -401,54 +620,72 @@ class RunCoverageTest { scriptBgDispatcher ).execute() - val expectedResultList = listOf( - listOf( - "SF:app/main/java/com/example/TwoSum.kt", - "FN:7,com/example/TwoSum${'$'}Companion::sumNumbers (II)Ljava/lang/Object;", - "FN:3,com/example/TwoSum:: ()V", - "FNDA:1,com/example/TwoSum${'$'}Companion::sumNumbers (II)Ljava/lang/Object;", - "FNDA:0,com/example/TwoSum:: ()V", - "FNF:2", - "FNH:1", - "BRDA:7,0,0,1", - "BRDA:7,0,1,1", - "BRDA:7,0,2,1", - "BRDA:7,0,3,1", - "BRF:4", - "BRH:4", - "DA:3,0", - "DA:7,1", - "DA:8,1", - "DA:10,1", - "LH:3", - "LF:4", - "end_of_record" - ), - listOf( - "SF:app/main/java/com/example/TwoSum.kt", - "FN:7,com/example/TwoSum${'$'}Companion::sumNumbers (II)Ljava/lang/Object;", - "FN:3,com/example/TwoSum:: ()V", - "FNDA:1,com/example/TwoSum${'$'}Companion::sumNumbers (II)Ljava/lang/Object;", - "FNDA:0,com/example/TwoSum:: ()V", - "FNF:2", - "FNH:1", - "BRDA:7,0,0,1", - "BRDA:7,0,1,1", - "BRDA:7,0,2,1", - "BRDA:7,0,3,1", - "BRF:4", - "BRH:4", - "DA:3,0", - "DA:7,1", - "DA:8,1", - "DA:10,1", - "LH:3", - "LF:4", - "end_of_record" - ) + val expectedResult = listOf( + CoverageReport.newBuilder() + .setBazelTestTarget("//app/sharedTest/java/com/example:TwoSumTest") + .setFilePath("app/main/java/com/example/TwoSum.kt") + .setFileSha1Hash("f6fb075e115775f6729615a79f0e7e34fe9735b5") + .addCoveredLine( + CoveredLine.newBuilder() + .setLineNumber(3) + .setCoverage(Coverage.NONE) + .build() + ) + .addCoveredLine( + CoveredLine.newBuilder() + .setLineNumber(7) + .setCoverage(Coverage.FULL) + .build() + ) + .addCoveredLine( + CoveredLine.newBuilder() + .setLineNumber(8) + .setCoverage(Coverage.FULL) + .build() + ) + .addCoveredLine( + CoveredLine.newBuilder() + .setLineNumber(10) + .setCoverage(Coverage.FULL) + .build() + ) + .setLinesFound(4) + .setLinesHit(3) + .build(), + CoverageReport.newBuilder() + .setBazelTestTarget("//app/test/java/com/example:TwoSumLocalTest") + .setFilePath("app/main/java/com/example/TwoSum.kt") + .setFileSha1Hash("f6fb075e115775f6729615a79f0e7e34fe9735b5") + .addCoveredLine( + CoveredLine.newBuilder() + .setLineNumber(3) + .setCoverage(Coverage.NONE) + .build() + ) + .addCoveredLine( + CoveredLine.newBuilder() + .setLineNumber(7) + .setCoverage(Coverage.FULL) + .build() + ) + .addCoveredLine( + CoveredLine.newBuilder() + .setLineNumber(8) + .setCoverage(Coverage.FULL) + .build() + ) + .addCoveredLine( + CoveredLine.newBuilder() + .setLineNumber(10) + .setCoverage(Coverage.FULL) + .build() + ) + .setLinesFound(4) + .setLinesHit(3) + .build() ) - assertThat(result).isEqualTo(expectedResultList) + assertThat(result).isEqualTo(expectedResult) } private fun initializeCommandExecutorWithLongProcessWaitTime(): CommandExecutorImpl { diff --git a/scripts/src/javatests/org/oppia/android/scripts/testing/TestBazelWorkspaceTest.kt b/scripts/src/javatests/org/oppia/android/scripts/testing/TestBazelWorkspaceTest.kt index b52113e817d..aeab27fe8d3 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/testing/TestBazelWorkspaceTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/testing/TestBazelWorkspaceTest.kt @@ -280,6 +280,7 @@ class TestBazelWorkspaceTest { testBazelWorkspace.addSourceAndTestFileWithContent( "Main", + "MainTest", sourceContent, testContent, sourceSubpackage = "coverage/main/java/com/example", @@ -315,6 +316,7 @@ class TestBazelWorkspaceTest { testBazelWorkspace.addSourceAndTestFileWithContent( "Main", + "MainTest", sourceContent, testContent, sourceSubpackage = "coverage/main/java/com/example",