diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml index 992a7e86cb1..276f2841312 100644 --- a/.github/workflows/code_coverage.yml +++ b/.github/workflows/code_coverage.yml @@ -258,10 +258,10 @@ jobs: evaluate_code_coverage_reports: name: Evaluate Code Coverage Reports runs-on: ubuntu-20.04 - needs: code_coverage_run + needs: [ check_unit_tests_completed, code_coverage_run ] # The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations, # serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows. - if: ${{ !cancelled() }} + if: ${{ !cancelled() && needs.check_unit_tests_completed.result == 'success'}} env: CACHE_DIRECTORY: ~/.bazel_cache steps: @@ -311,12 +311,16 @@ jobs: # Reference: https://github.community/t/127354/7. check_coverage_results: name: Check Code Coverage Results - needs: [ compute_changed_files, code_coverage_run, evaluate_code_coverage_reports ] + needs: [ check_unit_tests_completed, compute_changed_files, code_coverage_run, evaluate_code_coverage_reports ] # The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations, # serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows. if: ${{ !cancelled() }} runs-on: ubuntu-20.04 steps: + - name: Check unit tests passed + if: ${{ needs.check_unit_tests_completed.result != 'success' }} + run: exit 1 + - name: Check coverages passed if: ${{ needs.compute_changed_files.outputs.can_skip_files != 'true' && needs.code_coverage_run.result != 'success' }} run: exit 1 diff --git a/.github/workflows/comment_coverage_report.yml b/.github/workflows/comment_coverage_report.yml index 9ecb4e2e95c..16c1ae0da63 100644 --- a/.github/workflows/comment_coverage_report.yml +++ b/.github/workflows/comment_coverage_report.yml @@ -3,11 +3,11 @@ name: Comment Coverage Report # Controls when the action will run. Triggers the workflow on pull request events -# (assigned, opened, synchronize, reopened) +# (opened, synchronize, reopened) on: pull_request_target: - types: [assigned, opened, synchronize, reopened] + types: [opened, synchronize, reopened] concurrency: group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }} diff --git a/app/src/main/java/org/oppia/android/app/hintsandsolution/HintsAndSolutionDialogFragment.kt b/app/src/main/java/org/oppia/android/app/hintsandsolution/HintsAndSolutionDialogFragment.kt index c36cafe9ecc..e81aca55b3f 100644 --- a/app/src/main/java/org/oppia/android/app/hintsandsolution/HintsAndSolutionDialogFragment.kt +++ b/app/src/main/java/org/oppia/android/app/hintsandsolution/HintsAndSolutionDialogFragment.kt @@ -14,6 +14,7 @@ import org.oppia.android.app.model.HintsAndSolutionDialogFragmentStateBundle import org.oppia.android.app.model.ProfileId import org.oppia.android.app.model.State import org.oppia.android.app.model.WrittenTranslationContext +import org.oppia.android.app.topic.conceptcard.ConceptCardFragment import org.oppia.android.util.extensions.getProto import org.oppia.android.util.extensions.putProto import javax.inject.Inject @@ -192,4 +193,12 @@ class HintsAndSolutionDialogFragment : isSolutionRevealed ) } + + /** + * Delegates the removal of all [ConceptCardFragment] instances + * to the [hintsAndSolutionDialogFragmentPresenter]. + */ + fun dismissConceptCard() { + hintsAndSolutionDialogFragmentPresenter.dismissConceptCard() + } } diff --git a/app/src/main/java/org/oppia/android/app/hintsandsolution/HintsAndSolutionDialogFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/hintsandsolution/HintsAndSolutionDialogFragmentPresenter.kt index 16f9dec7b19..824c777ca78 100644 --- a/app/src/main/java/org/oppia/android/app/hintsandsolution/HintsAndSolutionDialogFragmentPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/hintsandsolution/HintsAndSolutionDialogFragmentPresenter.kt @@ -331,4 +331,9 @@ class HintsAndSolutionDialogFragmentPresenter @Inject constructor( override fun onConceptCardLinkClicked(view: View, skillId: String) { ConceptCardFragment.bringToFrontOrCreateIfNew(skillId, profileId, fragment.childFragmentManager) } + + /** Removes all [ConceptCardFragment] in the given FragmentManager. */ + fun dismissConceptCard() { + ConceptCardFragment.dismissAll(fragment.childFragmentManager) + } } diff --git a/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationActivity.kt b/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationActivity.kt index b95785bf2d0..6912d639ed9 100755 --- a/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationActivity.kt +++ b/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationActivity.kt @@ -190,7 +190,9 @@ class ExplorationActivity : this.writtenTranslationContext = writtenTranslationContext } - override fun dismissConceptCard() = explorationActivityPresenter.dismissConceptCard() + override fun dismissConceptCard() { + getHintsAndSolution()?.dismissConceptCard() + } override fun requestVoiceOverIconSpotlight(numberOfLogins: Int) { explorationActivityPresenter.requestVoiceOverIconSpotlight(numberOfLogins) diff --git a/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationActivityPresenter.kt b/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationActivityPresenter.kt index 9d1c50ec2ea..1c493c19bfa 100644 --- a/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationActivityPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationActivityPresenter.kt @@ -334,10 +334,6 @@ class ExplorationActivityPresenter @Inject constructor( showDialogFragmentBasedOnCurrentCheckpointState() } - fun dismissConceptCard() { - getExplorationFragment()?.dismissConceptCard() - } - private fun updateToolbarTitle(explorationId: String) { subscribeToExploration( explorationDataController.getExplorationById(profileId, explorationId).toLiveData() diff --git a/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationFragment.kt b/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationFragment.kt index fdffb73b32d..6d4cb2a6330 100755 --- a/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationFragment.kt +++ b/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationFragment.kt @@ -84,7 +84,5 @@ class ExplorationFragment : InjectableFragment() { explorationFragmentPresenter.viewSolution() } - fun dismissConceptCard() = explorationFragmentPresenter.dismissConceptCard() - fun getExplorationCheckpointState() = explorationFragmentPresenter.getExplorationCheckpointState() } diff --git a/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationFragmentPresenter.kt index 151f2456f53..207f5bf9e7f 100755 --- a/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationFragmentPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationFragmentPresenter.kt @@ -141,8 +141,6 @@ class ExplorationFragmentPresenter @Inject constructor( getStateFragment()?.viewSolution() } - fun dismissConceptCard() = getStateFragment()?.dismissConceptCard() - fun getExplorationCheckpointState() = getStateFragment()?.getExplorationCheckpointState() private fun getStateFragment(): StateFragment? { diff --git a/app/src/main/java/org/oppia/android/app/player/state/StateFragment.kt b/app/src/main/java/org/oppia/android/app/player/state/StateFragment.kt index 50f05d60082..df5e8a07dde 100755 --- a/app/src/main/java/org/oppia/android/app/player/state/StateFragment.kt +++ b/app/src/main/java/org/oppia/android/app/player/state/StateFragment.kt @@ -162,8 +162,6 @@ class StateFragment : stateFragmentPresenter.viewSolution() } - fun dismissConceptCard() = stateFragmentPresenter.dismissConceptCard() - fun getExplorationCheckpointState() = stateFragmentPresenter.getExplorationCheckpointState() override fun onSaveInstanceState(outState: Bundle) { diff --git a/app/src/main/java/org/oppia/android/app/player/state/StateFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/player/state/StateFragmentPresenter.kt index 672595d81ef..0147fb7e82c 100755 --- a/app/src/main/java/org/oppia/android/app/player/state/StateFragmentPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/player/state/StateFragmentPresenter.kt @@ -38,7 +38,6 @@ import org.oppia.android.app.player.state.listener.RouteToHintsAndSolutionListen import org.oppia.android.app.player.stopplaying.StopStatePlayingSessionWithSavedProgressListener import org.oppia.android.app.survey.SurveyWelcomeDialogFragment import org.oppia.android.app.survey.TAG_SURVEY_WELCOME_DIALOG -import org.oppia.android.app.topic.conceptcard.ConceptCardFragment import org.oppia.android.app.translation.AppLanguageResourceHandler import org.oppia.android.app.utility.SplitScreenManager import org.oppia.android.app.utility.lifecycle.LifecycleSafeTimerFactory @@ -428,10 +427,6 @@ class StateFragmentPresenter @Inject constructor( subscribeToAnswerOutcome(explorationProgressController.submitAnswer(answer).toLiveData()) } - fun dismissConceptCard() { - ConceptCardFragment.dismissAll(fragment.childFragmentManager) - } - private fun moveToNextState() { stateViewModel.setCanSubmitAnswer(canSubmitAnswer = false) explorationProgressController.moveToNextState().toLiveData().observe( diff --git a/app/src/main/java/org/oppia/android/app/testing/ConceptCardFragmentTestActivity.kt b/app/src/main/java/org/oppia/android/app/testing/ConceptCardFragmentTestActivity.kt index b87c9c8a431..c428093696f 100644 --- a/app/src/main/java/org/oppia/android/app/testing/ConceptCardFragmentTestActivity.kt +++ b/app/src/main/java/org/oppia/android/app/testing/ConceptCardFragmentTestActivity.kt @@ -29,7 +29,7 @@ class ConceptCardFragmentTestActivity : } override fun dismissConceptCard() { - getConceptCardFragment()?.dismiss() + ConceptCardFragment.dismissAll(supportFragmentManager) } private fun getConceptCardFragment(): ConceptCardFragment? { diff --git a/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerActivityPresenter.kt b/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerActivityPresenter.kt index 5c1f7484766..523ebf2cc4d 100644 --- a/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerActivityPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerActivityPresenter.kt @@ -274,7 +274,9 @@ class QuestionPlayerActivityPresenter @Inject constructor( getHintsAndSolutionDialogFragment()?.dismiss() } - fun dismissConceptCard() = getQuestionPlayerFragment()?.dismissConceptCard() + fun dismissConceptCard() { + getHintsAndSolutionDialogFragment()?.dismissConceptCard() + } private fun getHintsAndSolutionDialogFragment(): HintsAndSolutionDialogFragment? { return activity.supportFragmentManager.findFragmentByTag( diff --git a/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerFragment.kt b/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerFragment.kt index b5e67ec1318..c825f53e5cf 100644 --- a/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerFragment.kt +++ b/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerFragment.kt @@ -106,8 +106,6 @@ class QuestionPlayerFragment : questionPlayerFragmentPresenter.revealSolution() } - fun dismissConceptCard() = questionPlayerFragmentPresenter.dismissConceptCard() - companion object { /** Arguments key for [QuestionPlayerFragment]. */ diff --git a/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerFragmentPresenter.kt index 7b4861580ab..6319e930125 100644 --- a/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerFragmentPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerFragmentPresenter.kt @@ -27,7 +27,6 @@ import org.oppia.android.app.player.state.StatePlayerRecyclerViewAssembler import org.oppia.android.app.player.state.listener.RouteToHintsAndSolutionListener import org.oppia.android.app.player.stopplaying.RestartPlayingSessionListener import org.oppia.android.app.player.stopplaying.StopStatePlayingSessionListener -import org.oppia.android.app.topic.conceptcard.ConceptCardFragment import org.oppia.android.app.utility.FontScaleConfigurationUtil import org.oppia.android.app.utility.SplitScreenManager import org.oppia.android.databinding.QuestionPlayerFragmentBinding @@ -124,10 +123,6 @@ class QuestionPlayerFragmentPresenter @Inject constructor( subscribeToHintSolution(questionAssessmentProgressController.submitSolutionIsRevealed()) } - fun dismissConceptCard() { - ConceptCardFragment.dismissAll(fragment.childFragmentManager) - } - private fun retrieveArguments(): QuestionPlayerFragmentArguments { return fragment.requireArguments().getProto( QuestionPlayerFragment.ARGUMENTS_KEY, QuestionPlayerFragmentArguments.getDefaultInstance() diff --git a/app/src/sharedTest/java/org/oppia/android/app/player/exploration/ExplorationActivityTest.kt b/app/src/sharedTest/java/org/oppia/android/app/player/exploration/ExplorationActivityTest.kt index 134272b12de..d5adce598af 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/player/exploration/ExplorationActivityTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/player/exploration/ExplorationActivityTest.kt @@ -3,7 +3,9 @@ package org.oppia.android.app.player.exploration import android.app.Application import android.content.Context import android.content.Intent +import android.text.Spannable import android.text.TextUtils +import android.text.style.ClickableSpan import android.view.View import android.widget.TextView import androidx.appcompat.app.AppCompatActivity @@ -48,6 +50,7 @@ import org.hamcrest.CoreMatchers.containsString import org.hamcrest.Description import org.hamcrest.Matcher import org.hamcrest.Matchers.not +import org.hamcrest.TypeSafeMatcher import org.junit.After import org.junit.Before import org.junit.Ignore @@ -2558,6 +2561,92 @@ class ExplorationActivityTest { } } + @Test + @RunOn(TestPlatform.ROBOLECTRIC) // TODO(#3858): Enable for Espresso. + fun testExpActivity_openConceptCard_selectNavigationUp_conceptCardCloses() { + markAllSpotlightsSeen() + launch( + createExplorationActivityIntent( + internalProfileId, + TEST_CLASSROOM_ID_0, + TEST_TOPIC_ID_0, + TEST_STORY_ID_0, + TEST_EXPLORATION_ID_2, + shouldSavePartialProgress = false + ) + ).use { + explorationDataController.startPlayingNewExploration( + internalProfileId, + TEST_CLASSROOM_ID_0, + TEST_TOPIC_ID_0, + TEST_STORY_ID_0, + TEST_EXPLORATION_ID_2 + ) + testCoroutineDispatchers.runCurrent() + clickContinueButton() + // Submit two incorrect answers. + submitFractionAnswer(answerText = "1/3") + submitFractionAnswer(answerText = "1/4") + + // Reveal the hint. + openHintsAndSolutionsDialog() + pressRevealHintButton(hintPosition = 0) + + onView(withId(R.id.hints_and_solution_summary)) + .inRoot(isDialog()) + .perform(openClickableSpan("test_skill_id_1 concept card")) + + testCoroutineDispatchers.runCurrent() + + onView(withText("Concept Card")).inRoot(isDialog()).check(matches(isDisplayed())) + onView(withText("Another important skill")).inRoot(isDialog()).check(matches(isDisplayed())) + onView(withId(R.id.concept_card_toolbar)).check(matches(isDisplayed())) + + onView(withContentDescription(R.string.navigate_up)).perform(click()) + + testCoroutineDispatchers.runCurrent() + onView(withId(R.id.concept_card_toolbar)).check(doesNotExist()) + } + explorationDataController.stopPlayingExploration(isCompletion = false) + } + + private fun openClickableSpan(text: String): ViewAction { + return object : ViewAction { + override fun getDescription(): String = "openClickableSpan" + + override fun getConstraints(): Matcher = hasClickableSpanWithText(text) + + override fun perform(uiController: UiController?, view: View?) { + // The view shouldn't be null if the constraints are being met. + (view as? TextView)?.getClickableSpans()?.findMatchingTextOrNull(text)?.onClick(view) + } + } + } + + private fun List>.findMatchingTextOrNull(text: String) = + find { text in it.first }?.second + + private fun TextView.getClickableSpans(): List> { + val viewText = text + return (viewText as Spannable).getSpans( + /* start= */ 0, /* end= */ text.length, ClickableSpan::class.java + ).map { + viewText.subSequence(viewText.getSpanStart(it), viewText.getSpanEnd(it)).toString() to it + } + } + + private fun hasClickableSpanWithText(text: String): Matcher { + return object : TypeSafeMatcher(TextView::class.java) { + override fun describeTo(description: Description?) { + description?.appendText("has ClickableSpan with text")?.appendValue(text) + } + + override fun matchesSafely(item: View?): Boolean { + return (item as? TextView)?.getClickableSpans()?.findMatchingTextOrNull(text) != null + } + } + } + private fun markSpotlightSeen(feature: Spotlight.FeatureCase) { val profileId = ProfileId.newBuilder().setInternalId(internalProfileId).build() spotlightStateController.markSpotlightViewed(profileId, feature) diff --git a/scripts/src/java/org/oppia/android/scripts/coverage/reporter/BUILD.bazel b/scripts/src/java/org/oppia/android/scripts/coverage/reporter/BUILD.bazel index 85b59087cc2..217bc961aa1 100644 --- a/scripts/src/java/org/oppia/android/scripts/coverage/reporter/BUILD.bazel +++ b/scripts/src/java/org/oppia/android/scripts/coverage/reporter/BUILD.bazel @@ -15,5 +15,6 @@ kt_jvm_library( "//scripts/src/java/org/oppia/android/scripts/common:bazel_client", "//scripts/src/java/org/oppia/android/scripts/proto:coverage_java_proto", "//scripts/src/java/org/oppia/android/scripts/proto:script_exemptions_java_proto", + "//third_party:com_google_guava_guava", ], ) diff --git a/scripts/src/java/org/oppia/android/scripts/coverage/reporter/CoverageReporter.kt b/scripts/src/java/org/oppia/android/scripts/coverage/reporter/CoverageReporter.kt index 31763392945..f447a58c256 100644 --- a/scripts/src/java/org/oppia/android/scripts/coverage/reporter/CoverageReporter.kt +++ b/scripts/src/java/org/oppia/android/scripts/coverage/reporter/CoverageReporter.kt @@ -1,5 +1,6 @@ package org.oppia.android.scripts.coverage.reporter +import com.google.common.html.HtmlEscapers import org.oppia.android.scripts.proto.Coverage import org.oppia.android.scripts.proto.CoverageReport import org.oppia.android.scripts.proto.CoverageReportContainer @@ -277,7 +278,7 @@ class CoverageReporter( """ ${lineNumber.toString().padStart(4, ' ')} - $line + ${HtmlEscapers.htmlEscaper().escape(line)} """.trimIndent() ) 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 7ef0183f00d..3d28a816c7d 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/coverage/RunCoverageTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/coverage/RunCoverageTest.kt @@ -317,7 +317,13 @@ class RunCoverageTest { val expectedResult = getExpectedHtmlText(kotlinFilePath) - assertThat(readHtmlReport(kotlinFilePath)).isEqualTo(expectedResult) + val unescapedHtmlReport = readHtmlReport(kotlinFilePath) + .replace(""", "\"") + .replace("&", "&") + .replace("<", "<") + .replace(">", ">") + + assertThat(unescapedHtmlReport).isEqualTo(expectedResult) } @Test @@ -342,7 +348,13 @@ class RunCoverageTest { val expectedResult = getExpectedHtmlText(sourceFilePath) - assertThat(readHtmlReport(sourceFilePath)).isEqualTo(expectedResult) + val unescapedHtmlReport = readHtmlReport(sourceFilePath) + .replace(""", "\"") + .replace("&", "&") + .replace("<", "<") + .replace(">", ">") + + assertThat(unescapedHtmlReport).isEqualTo(expectedResult) } @Test @@ -541,7 +553,13 @@ class RunCoverageTest { val expectedResult = getExpectedHtmlText(filePath) - assertThat(readHtmlReport(filePath)).isEqualTo(expectedResult) + val unescapedHtmlReport = readHtmlReport(filePath) + .replace(""", "\"") + .replace("&", "&") + .replace("<", "<") + .replace(">", ">") + + assertThat(unescapedHtmlReport).isEqualTo(expectedResult) } @Test @@ -1880,10 +1898,20 @@ class RunCoverageTest { ).execute() val expectedResult1 = getExpectedHtmlText(filePathList.get(0)) - assertThat(readHtmlReport(filePathList.get(0))).isEqualTo(expectedResult1) + val unescapedHtmlReport1 = readHtmlReport(filePathList.get(0)) + .replace(""", "\"") + .replace("&", "&") + .replace("<", "<") + .replace(">", ">") + assertThat(unescapedHtmlReport1).isEqualTo(expectedResult1) val expectedResult2 = getExpectedHtmlText(filePathList.get(1)) - assertThat(readHtmlReport(filePathList.get(1))).isEqualTo(expectedResult2) + val unescapedHtmlReport2 = readHtmlReport(filePathList.get(1)) + .replace(""", "\"") + .replace("&", "&") + .replace("<", "<") + .replace(">", ">") + assertThat(unescapedHtmlReport2).isEqualTo(expectedResult2) } @Test @@ -1910,7 +1938,13 @@ class RunCoverageTest { val expectedResult = getExpectedHtmlText(filePathList.get(0)) - assertThat(readHtmlReport(filePathList.get(0))).isEqualTo(expectedResult) + val unescapedHtmlReport = readHtmlReport(filePathList.get(0)) + .replace(""", "\"") + .replace("&", "&") + .replace("<", "<") + .replace(">", ">") + + assertThat(unescapedHtmlReport).isEqualTo(expectedResult) } @Test @@ -1937,7 +1971,13 @@ class RunCoverageTest { val expectedResult = getExpectedHtmlText(filePathList.get(0)) - assertThat(readHtmlReport(filePathList.get(0))).isEqualTo(expectedResult) + val unescapedHtmlReport = readHtmlReport(filePathList.get(0)) + .replace(""", "\"") + .replace("&", "&") + .replace("<", "<") + .replace(">", ">") + + assertThat(unescapedHtmlReport).isEqualTo(expectedResult) } @Test @@ -1964,7 +2004,13 @@ class RunCoverageTest { val expectedResult = getExpectedHtmlText(filePathList.get(0)) - assertThat(readHtmlReport(filePathList.get(0))).isEqualTo(expectedResult) + val unescapedHtmlReport = readHtmlReport(filePathList.get(0)) + .replace(""", "\"") + .replace("&", "&") + .replace("<", "<") + .replace(">", ">") + + assertThat(unescapedHtmlReport).isEqualTo(expectedResult) } @Test @@ -2009,7 +2055,13 @@ class RunCoverageTest { val expectedResult = getExpectedHtmlText(filePathList.get(0)) - assertThat(readHtmlReport(filePathList.get(0))).isEqualTo(expectedResult) + val unescapedHtmlReport = readHtmlReport(filePathList.get(0)) + .replace(""", "\"") + .replace("&", "&") + .replace("<", "<") + .replace(">", ">") + + assertThat(unescapedHtmlReport).isEqualTo(expectedResult) } @Test @@ -2036,7 +2088,13 @@ class RunCoverageTest { val expectedResult = getExpectedHtmlText(filePathList.get(0)) - assertThat(readHtmlReport(filePathList.get(0))).isEqualTo(expectedResult) + val unescapedHtmlReport = readHtmlReport(filePathList.get(0)) + .replace(""", "\"") + .replace("&", "&") + .replace("<", "<") + .replace(">", ">") + + assertThat(unescapedHtmlReport).isEqualTo(expectedResult) } @Test @@ -2262,7 +2320,13 @@ class RunCoverageTest { """.trimIndent() - assertThat(readHtmlReport(filePathList.get(0))).isEqualTo(expectedResult) + val unescapedHtmlReport = readHtmlReport(filePathList.get(0)) + .replace(""", "\"") + .replace("&", "&") + .replace("<", "<") + .replace(">", ">") + + assertThat(unescapedHtmlReport).isEqualTo(expectedResult) } @Test diff --git a/scripts/src/javatests/org/oppia/android/scripts/coverage/reporter/BUILD.bazel b/scripts/src/javatests/org/oppia/android/scripts/coverage/reporter/BUILD.bazel index 6571e76119e..0747162663f 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/coverage/reporter/BUILD.bazel +++ b/scripts/src/javatests/org/oppia/android/scripts/coverage/reporter/BUILD.bazel @@ -13,6 +13,7 @@ kt_jvm_test( "//scripts/src/java/org/oppia/android/scripts/coverage/reporter:coverage_reporter_lib", "//scripts/src/java/org/oppia/android/scripts/proto:script_exemptions_java_proto", "//testing:assertion_helpers", + "//third_party:com_google_guava_guava", "//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/reporter/CoverageReporterTest.kt b/scripts/src/javatests/org/oppia/android/scripts/coverage/reporter/CoverageReporterTest.kt index b0af56a1115..ae06e6a693e 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/coverage/reporter/CoverageReporterTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/coverage/reporter/CoverageReporterTest.kt @@ -697,7 +697,13 @@ class CoverageReporterTest { """.trimIndent() - assertThat(outputReportText).isEqualTo(expectedHtml) + val unescapedOutputReportText = outputReportText + .replace(""", "\"") + .replace("&", "&") + .replace("<", "<") + .replace(">", ">") + + assertThat(unescapedOutputReportText).isEqualTo(expectedHtml) } @Test diff --git a/wiki/Oppia-Android-Code-Coverage.md b/wiki/Oppia-Android-Code-Coverage.md index e5ed0479619..499b3591642 100644 --- a/wiki/Oppia-Android-Code-Coverage.md +++ b/wiki/Oppia-Android-Code-Coverage.md @@ -139,7 +139,7 @@ Coverage Analysis: **FAIL** :x:
| File | Coverage | Lines Hit | Status | Min Required | |------|:--------:|----------:|:------:|:------------:| -|
MathTokenizer.ktutility/src/main/java/org/oppia/android/util/math/MathTokenizer.kt
| 94.26% | 197 / 209 | :white_check_mark: | 70% | +|
Pass.ktutility/src/main/java/org/oppia/android/util/math/Pass.kt
| 94.26% | 197 / 209 | :white_check_mark: | 70% | ### Exempted coverage @@ -282,7 +282,7 @@ Certain files are exempt from coverage checks. These exemptions include: 1. **Test File Exemptions:** Files that are exempted from having corresponding test files are also exempted from coverage checks. Since no test files are available for these sources, coverage analysis cannot be performed, and these files are therefore skipped. -2. **Source File Incompatibility Exemptions:** Some files are currently incompatible with Bazel coverage execution ([see tracking issue #5481](https://github.com/oppia/oppia-android/issues/5481)) and are temporarily excluded from coverage checks. +2. **Source File Incompatibility Exemptions:** Some files are currently incompatible with Bazel coverage execution (see tracking issue [#5481](https://github.com/oppia/oppia-android/issues/5481)) and are temporarily excluded from coverage checks. You can find the complete list of exemptions in this file: [test_file_exemptions.textproto](https://github.com/oppia/oppia-android/blob/develop/scripts/assets/test_file_exemptions.textproto) @@ -311,7 +311,19 @@ bazel run //scripts:run_coverage -- : Your root directory. - : Files you want to generate coverage reports for. -For example, to analyze coverage for the file MathTokenizer.kt, use the relative path: +To get the relative path of a file: + +1. Navigate to the Project view on the left-hand side in Android Studio. +2. Locate the file to analyze Code Coverage for. +3. Right click the file and select Copy Path. To get the path relative to the root. + +Alternatively, the coverage report itself provides the relative paths. You can reveal this information by clicking on the drop-down that precedes the file name in the report. + +| File | Coverage | Lines Hit | Status | Min Required | +|------|:--------:|----------:|:------:|:------------:| +|
MathTokenizer.ktutility/src/main/java/org/oppia/android/util/math/MathTokenizer.kt
| 94.26% | 197 / 209 | :white_check_mark: | 70% | + +To analyze coverage for the file MathTokenizer.kt, use the relative path: ```sh bazel run //scripts:run_coverage -- $(pwd) utility/src/main/java/org/oppia/android/util/math/MathTokenizer.kt @@ -456,6 +468,14 @@ It’s essential to ensure that each source file is directly tested by its own c ## Limitations of the code coverage tool -1. **Incompatibility with Code Coverage Analysis:** Certain test targets in the Oppia-Android codebase fail to execute and collect coverage using the Bazel coverage command. The underlying issues are still being investigated ([see tracking issue #5481](https://github.com/oppia/oppia-android/issues/5481)), and these files are currently exempt from coverage checks. However, it's expected that all new test files should work without needing this exemption. +1. **Incompatibility with Code Coverage Analysis:** Certain test targets in the Oppia-Android codebase fail to execute and collect coverage using the Bazel coverage command. The underlying issues are still being investigated (see tracking issue [#5481](https://github.com/oppia/oppia-android/issues/5481)), and these files are currently exempt from coverage checks. However, it's expected that all new test files should work without needing this exemption. 2. **Function and Branch Coverage:** The Oppia-Android code coverage tool currently provides only line coverage data. It does not include information on function or branch coverage. + +3. **Kotlin inline functions:** With JaCoCo coverage gaps, Kotlin inline functions may be inaccurately reported as uncovered in coverage reports. (See tracking issue [#5501](https://github.com/oppia/oppia-android/issues/5501)) + +4. **Line and Partial Coverages:** The current line coverage analysis in Oppia Android is limited and may not accurately reflect the execution of complex or multi-branch code within a single line, reporting lines as fully covered even if only part of the logic within those lines is executed, leading to potentially misleading coverage data. (See tracking issue [#5503](https://github.com/oppia/oppia-android/issues/5503)) + +5. **Flow Interrupting Statements:** The coverage reports may inaccurately reflect the coverage of flow-interrupting statements (e.g., exitProcess(1), assertion failures, break). These lines may be marked as uncovered even when executed, due to JaCoCo's limitations in tracking code execution after abrupt control flow interruptions. (See tracking issue [#5506](https://github.com/oppia/oppia-android/issues/5506)) + +6. **Uncovered Last Curly Brace in Kotlin:** The last curly brace of some Kotlin functions may be reported as uncovered, even when the function is fully executed during tests. This issue requires further investigation to determine if it's due to incomplete test execution or dead code generated by the Kotlin compiler. (See tracking issue [#5523](https://github.com/oppia/oppia-android/issues/5523)) \ No newline at end of file diff --git a/wiki/Writing-tests-with-good-behavioral-coverage.md b/wiki/Writing-tests-with-good-behavioral-coverage.md index 7a0aa5caebc..db50cfa83a6 100644 --- a/wiki/Writing-tests-with-good-behavioral-coverage.md +++ b/wiki/Writing-tests-with-good-behavioral-coverage.md @@ -1017,7 +1017,7 @@ Note: For more information on how to utilize the code coverage analysis tool, pl ## Testing a Single Outcome in Multiple Ways -When testing a single outcome like a successful withdrawal, you can use multiple approaches to verify the if the balance is updated correctly. Here are different ways to ensure the single outcome of withdrawal was processed correctly, each following a distinct approach. +When testing a single outcome, such as a successful withdrawal, you can use multiple approaches to verify if the balance is updated correctly. Here are different ways to ensure the single outcome of withdrawal was processed correctly, each following a distinct approach. **a. To verify correctness of output:** diff --git a/wiki/_Sidebar.md b/wiki/_Sidebar.md index ea5a254f2c6..c1f9834bc49 100644 --- a/wiki/_Sidebar.md +++ b/wiki/_Sidebar.md @@ -25,7 +25,7 @@ * Testing * [Oppia Android Testing](https://github.com/oppia/oppia-android/wiki/Oppia-Android-Testing) * [End to End Testing Guide](https://github.com/oppia/oppia-android/wiki/End-to-End-Testing-Guide) - * [Oppia Android Code Coverage](https://github.com/oppia/oppia-android-workflow/wiki/Oppia-Android-Code-Coverage) + * [Oppia Android Code Coverage](https://github.com/oppia/oppia-android/wiki/Oppia-Android-Code-Coverage) * [Writing Tests with Good Behavioral Coverage](https://github.com/oppia/oppia-android/wiki/Writing-Tests-With-Good-Behavioral-Coverage) * [Developing Skills](https://github.com/oppia/oppia-android/wiki/Developing-skills) * [Frequent Errors and Solutions](https://github.com/oppia/oppia-android/wiki/Frequent-Errors-and-Solutions)