From f9106d91297abd09b24da25d5485deb8473b0125 Mon Sep 17 00:00:00 2001 From: RD Rama Devi <122200035+Rd4dev@users.noreply.github.com> Date: Wed, 14 Aug 2024 00:03:54 +0530 Subject: [PATCH] [RunAllTests] Fix part of #5343: Upload generated code coverage report as comments (#5469) ## Explanation Fixes part of #5343 ### Project [PR 2.4 of Project 4.1] ### Changes Made With the protos collected and stored with their respective file paths through #5465, this PR will focus on collecting the stored proto files and passing it to the script to generate a coverage report along with a status check to decide the success failure case of the CI Coverage Check run. **Collection of protos** - The stored protos are now being uploaded as an artifact. - The name of the artifact is set to a dynamic value corresponding to its shard_name. - The shard_name is extracted from the first portion of the `CHANGED_FILES_BUCKET_BASE64_ENCODED_SHARD` value. - eg: The matrix job - [app-shard0;H4sIAAAAAAAAAONiTiwoEHIFEvrFRcn6uYmZefpZi....] will save the artifact with name **coverage-report-app-shard0** - The dynamic or unique name is important to prevent the following error: ``` Error: Failed to CreateArtifact: Received non-retryable error: Failed request: (409) Conflict: an artifact with this name already exists on the workflow run ``` - The artifacts that match the pattern of coverage-report-* are downloaded. ``` Found 4 artifact(s) Filtering artifacts by pattern 'coverage-report-*' Preparing to download the following artifacts: - coverage-report-domain-shard0 (ID: 1799350592, Size: 489) - coverage-report-scripts-shard2 (ID: 1799348996, Size: 2075) - coverage-report-app-shard3 (ID: 1799348891, Size: 770) - coverage-report-generic-shard1 (ID: 1799348719, Size: 551) ``` - All the stored coverage_report.pb files are found and their paths are stored as list. - The list of coverage_report.pb files are then passed to the CoverageReporter.kt script to handle the coverage report protos. - The script combines them as one single CoverageReportContainer to generate a Markdown report with it. - Finally, a .md report is stored and based on the status of the coverage check, the job either succeeds or fails. - The stored md report is again uploaded and downloaded as an artifact to pass it for publication of the report. - The CoverageReport.md file is then uploaded as a comment to the corresponding PR using - https://github.com/peter-evans/create-or-update-comment action >The comment will be published regardless of whether the coverage passes or fails. The only time comments will not be published is if the unit tests themselves fail, making the coverage checks non-functional (also less cluttered). # ### CI Run Data This check was run with 4 different shards on all possible cases capable of being produced. Check : https://github.com/oppia/oppia-android/actions/runs/10340867060?pr=5469 Comment Published with the above run: https://github.com/oppia/oppia-android/pull/5469#issuecomment-2282804068 The above cases are run with custom temporary changes to the test exemption file (testing purposes). ``` test_file_exemption { exempted_file_path: "scripts/src/java/org/oppia/android/scripts/testfile/TestFileCheck.kt" override_min_coverage_percent_required: 101 } test_file_exemption { exempted_file_path: "utility/src/main/java/org/oppia/android/util/logging/ConsoleLogger.kt" override_min_coverage_percent_required: 30 } ``` ## 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)). --- .github/workflows/code_coverage.yml | 98 +++++- scripts/BUILD.bazel | 10 + scripts/assets/test_file_exemptions.textproto | 96 +++++- .../oppia/android/scripts/common/GitClient.kt | 11 +- .../android/scripts/coverage/BUILD.bazel | 16 +- .../android/scripts/coverage/RunCoverage.kt | 19 +- .../scripts/coverage/reporter/BUILD.bazel | 19 ++ .../{ => reporter}/CoverageReporter.kt | 104 +++++- .../scripts/ci/ComputeChangedFilesTest.kt | 1 + .../android/scripts/common/GitClientTest.kt | 44 +++ .../android/scripts/coverage/BUILD.bazel | 13 - .../scripts/coverage/RunCoverageTest.kt | 150 ++++++--- .../scripts/coverage/reporter/BUILD.bazel | 19 ++ .../{ => reporter}/CoverageReporterTest.kt | 317 ++++++++++++++++-- 14 files changed, 796 insertions(+), 121 deletions(-) create mode 100644 scripts/src/java/org/oppia/android/scripts/coverage/reporter/BUILD.bazel rename scripts/src/java/org/oppia/android/scripts/coverage/{ => reporter}/CoverageReporter.kt (86%) create mode 100644 scripts/src/javatests/org/oppia/android/scripts/coverage/reporter/BUILD.bazel rename scripts/src/javatests/org/oppia/android/scripts/coverage/{ => reporter}/CoverageReporterTest.kt (70%) diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml index 711151743b2..07824dfb179 100644 --- a/.github/workflows/code_coverage.yml +++ b/.github/workflows/code_coverage.yml @@ -11,6 +11,10 @@ on: # Push events on develop branch - develop +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + jobs: check_unit_tests_completed: name: Check unit test completed @@ -149,11 +153,12 @@ jobs: - name: Extract caching bucket env: - CHANGED_FILESS_BUCKET_BASE64_ENCODED_SHARD: ${{ matrix.changed-files-bucket-base64-encoded-shard }} + CHANGED_FILES_BUCKET_BASE64_ENCODED_SHARD: ${{ matrix.changed-files-bucket-base64-encoded-shard }} run: | # See https://stackoverflow.com/a/29903172 for cut logic. This is needed to remove the # user-friendly shard prefix from the matrix value. - CHANGED_FILES_BUCKET_BASE64=$(echo "$CHANGED_FILESS_BUCKET_BASE64_ENCODED_SHARD" | cut -d ";" -f 2) + CHANGED_FILES_BUCKET_BASE64=$(echo "$CHANGED_FILES_BUCKET_BASE64_ENCODED_SHARD" | cut -d ";" -f 2) + SHARD_NAME=$(echo "$CHANGED_FILES_BUCKET_BASE64_ENCODED_SHARD" | cut -d ";" -f 1) bazel run //scripts:retrieve_changed_files -- $(pwd) $CHANGED_FILES_BUCKET_BASE64 $(pwd)/file_bucket_name $(pwd)/changed_files $(pwd)/bazel_test_targets FILE_CATEGORY=$(cat ./file_bucket_name) CHANGED_FILES=$(cat ./changed_files) @@ -161,6 +166,8 @@ jobs: echo "File category: $FILE_CATEGORY" echo "Changed Files: $CHANGED_FILES" echo "Bazel test targets: $BAZEL_TEST_TARGETS" + echo "Shard name: $SHARD_NAME" + echo "SHARD_NAME=$SHARD_NAME" >> $GITHUB_ENV echo "FILE_CACHING_BUCKET=$FILE_CATEGORY" >> $GITHUB_ENV echo "CHANGED_FILES=$CHANGED_FILES" >> $GITHUB_ENV echo "BAZEL_TEST_TARGETS=$BAZEL_TEST_TARGETS" >> $GITHUB_ENV @@ -239,12 +246,91 @@ jobs: env: CHANGED_FILES: ${{ env.CHANGED_FILES }} run: | - bazel run //scripts:run_coverage -- $(pwd) $CHANGED_FILES --format=MARKDOWN --processTimeout=15 + echo "CHANGED FILES: $CHANGED_FILES" + bazel run //scripts:run_coverage -- $(pwd) $CHANGED_FILES --format=PROTO --processTimeout=15 + + - name: Upload Coverage Report Artifact + uses: actions/upload-artifact@v4 + with: + name: coverage-report-${{ env.SHARD_NAME }} # Saving with unique names to avoid conflict + path: coverage_reports + + evaluate-code-coverage-reports: + name: Evaluate Code Coverage Reports + runs-on: ubuntu-20.04 + needs: code_coverage_run + env: + CACHE_DIRECTORY: ~/.bazel_cache + steps: + - uses: actions/checkout@v2 + + - name: Download Coverage Report Artifacts + uses: actions/download-artifact@v4 + with: + path: coverage-report-artifact + pattern: coverage-report-* + merge-multiple: true + + - name: Filter Coverage Reports + run: | + # Find all coverage_report.pb files in the current directory and subdirectories + PB_FILES_LIST=($(find . -name "coverage_report.pb" -type f -print0 | xargs -0 -n 1 echo)) + echo "${PB_FILES_LIST[@]}" > pb_files.txt + + - name: Set up Bazel + uses: abhinavsingh/setup-bazel@v3 + with: + version: 6.5.0 + + - uses: actions/cache@v2 + id: scripts_cache + with: + path: ${{ env.CACHE_DIRECTORY }} + key: ${{ runner.os }}-${{ env.CACHE_DIRECTORY }}-bazel-scripts-${{ github.sha }} + restore-keys: | + ${{ runner.os }}-${{ env.CACHE_DIRECTORY }}-bazel-scripts- + ${{ runner.os }}-${{ env.CACHE_DIRECTORY }}-bazel- + + - name: Set up build environment + uses: ./.github/actions/set-up-android-bazel-build-environment + + - name: Generate Markdown Coverage Report + run: | + bazel run //scripts:coverage_reporter -- $(pwd) pb_files.txt + + - name: Upload Generated Markdown Report + uses: actions/upload-artifact@v4 + if: ${{ !cancelled() }} # IMPORTANT: Upload reports regardless of success or failure status + with: + name: final-coverage-report + path: coverage_reports/CoverageReport.md + + publish_coverage_report: + name: Publish Code Coverage Report + needs: evaluate-code-coverage-reports + permissions: + pull-requests: write + + # 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-latest + steps: + - name: Download Generated Markdown Report + uses: actions/download-artifact@v4 + with: + name: final-coverage-report + + - name: Upload Coverage Report as PR Comment + uses: peter-evans/create-or-update-comment@v4 + with: + issue-number: ${{ github.event.pull_request.number }} + body-path: 'CoverageReport.md' # Reference: https://github.community/t/127354/7. check_coverage_results: name: Check Code Coverage Results - needs: [ compute_changed_files, code_coverage_run ] + needs: [ 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() }} @@ -253,3 +339,7 @@ jobs: - name: Check coverages passed if: ${{ needs.compute_changed_files.outputs.can_skip_files != 'true' && needs.code_coverage_run.result != 'success' }} run: exit 1 + + - name: Check that coverage status is passed + if: ${{ needs.evaluate-code-coverage-reports.result != 'success' }} + run: exit 1 diff --git a/scripts/BUILD.bazel b/scripts/BUILD.bazel index d92196994e5..6ef9a5d6739 100644 --- a/scripts/BUILD.bazel +++ b/scripts/BUILD.bazel @@ -247,6 +247,16 @@ kt_jvm_binary( ], ) +kt_jvm_binary( + name = "coverage_reporter", + testonly = True, + data = TEST_FILE_EXEMPTION_ASSETS, + main_class = "org.oppia.android.scripts.coverage.reporter.CoverageReporterKt", + runtime_deps = [ + "//scripts/src/java/org/oppia/android/scripts/coverage/reporter:coverage_reporter_lib", + ], +) + kt_jvm_binary( name = "retrieve_changed_files", testonly = True, diff --git a/scripts/assets/test_file_exemptions.textproto b/scripts/assets/test_file_exemptions.textproto index e6e4866267b..bbc8a2fd872 100644 --- a/scripts/assets/test_file_exemptions.textproto +++ b/scripts/assets/test_file_exemptions.textproto @@ -10,6 +10,10 @@ test_file_exemption { exempted_file_path: "app/src/main/java/org/oppia/android/app/activity/ActivityComponentImpl.kt" test_file_not_required: true } +test_file_exemption { + exempted_file_path: "app/src/main/java/org/oppia/android/app/activity/ActivityIntentFactories.kt" + override_min_coverage_percent_required: 0 +} test_file_exemption { exempted_file_path: "app/src/main/java/org/oppia/android/app/activity/ActivityIntentFactoriesModule.kt" test_file_not_required: true @@ -1015,11 +1019,11 @@ test_file_exemption { source_file_is_incompatible_with_code_coverage: true } test_file_exemption { - exempted_file_path: "app/src/main/java/org/oppia/android/app/home/classroomlist/ClassroomSummaryClickListener.kt" + exempted_file_path: "app/src/main/java/org/oppia/android/app/home/classroomlist/AllClassroomsViewModel.kt" test_file_not_required: true } test_file_exemption { - exempted_file_path: "app/src/main/java/org/oppia/android/app/home/classroomlist/AllClassroomsViewModel.kt" + exempted_file_path: "app/src/main/java/org/oppia/android/app/home/classroomlist/ClassroomSummaryClickListener.kt" test_file_not_required: true } test_file_exemption { @@ -3070,6 +3074,10 @@ test_file_exemption { exempted_file_path: "data/src/main/java/org/oppia/android/data/backends/gae/Constants.kt" test_file_not_required: true } +test_file_exemption { + exempted_file_path: "data/src/main/java/org/oppia/android/data/backends/gae/JsonPrefixNetworkInterceptor.kt" + override_min_coverage_percent_required: 28 +} test_file_exemption { exempted_file_path: "data/src/main/java/org/oppia/android/data/backends/gae/NetworkApiKey.kt" test_file_not_required: true @@ -3082,6 +3090,10 @@ test_file_exemption { exempted_file_path: "data/src/main/java/org/oppia/android/data/backends/gae/XssiPrefix.kt" test_file_not_required: true } +test_file_exemption { + exempted_file_path: "data/src/main/java/org/oppia/android/data/backends/gae/api/FeedbackReportingService.kt" + override_min_coverage_percent_required: 0 +} test_file_exemption { exempted_file_path: "data/src/main/java/org/oppia/android/data/backends/gae/model/GaeFeedbackReport.kt" test_file_not_required: true @@ -3114,6 +3126,10 @@ test_file_exemption { exempted_file_path: "domain/src/main/java/org/oppia/android/domain/audio/CellularAudioDialogController.kt" source_file_is_incompatible_with_code_coverage: true } +test_file_exemption { + exempted_file_path: "domain/src/main/java/org/oppia/android/domain/auth/AuthenticationController.kt" + override_min_coverage_percent_required: 21 +} test_file_exemption { exempted_file_path: "domain/src/main/java/org/oppia/android/domain/auth/FirebaseAuthInstance.kt" test_file_not_required: true @@ -3130,6 +3146,10 @@ test_file_exemption { exempted_file_path: "domain/src/main/java/org/oppia/android/domain/auth/FirebaseAuthWrapper.kt" test_file_not_required: true } +test_file_exemption { + exempted_file_path: "domain/src/main/java/org/oppia/android/domain/auth/FirebaseAuthWrapperImpl.kt" + override_min_coverage_percent_required: 31 +} test_file_exemption { exempted_file_path: "domain/src/main/java/org/oppia/android/domain/auth/FirebaseUserWrapper.kt" test_file_not_required: true @@ -3582,6 +3602,10 @@ test_file_exemption { exempted_file_path: "domain/src/main/java/org/oppia/android/domain/oppialogger/OppiaLogger.kt" source_file_is_incompatible_with_code_coverage: true } +test_file_exemption { + exempted_file_path: "domain/src/main/java/org/oppia/android/domain/oppialogger/analytics/AnalyticsController.kt" + override_min_coverage_percent_required: 68 +} test_file_exemption { exempted_file_path: "domain/src/main/java/org/oppia/android/domain/oppialogger/analytics/AnalyticsStartupListener.kt" test_file_not_required: true @@ -3594,6 +3618,10 @@ test_file_exemption { exempted_file_path: "domain/src/main/java/org/oppia/android/domain/oppialogger/analytics/CpuPerformanceLoggingTimePeriodMillis.kt" test_file_not_required: true } +test_file_exemption { + exempted_file_path: "domain/src/main/java/org/oppia/android/domain/oppialogger/analytics/CpuPerformanceSnapshotterModule.kt" + override_min_coverage_percent_required: 26 +} test_file_exemption { exempted_file_path: "domain/src/main/java/org/oppia/android/domain/oppialogger/analytics/LearnerAnalyticsInactivityLimitMillis.kt" test_file_not_required: true @@ -3838,6 +3866,18 @@ test_file_exemption { exempted_file_path: "instrumentation/src/java/org/oppia/android/instrumentation/testing/EndToEndTestHelper.kt" test_file_not_required: true } +test_file_exemption { + exempted_file_path: "scripts/src/java/org/oppia/android/scripts/apkstats/ApkAnalyzerClient.kt" + override_min_coverage_percent_required: 6 +} +test_file_exemption { + exempted_file_path: "scripts/src/java/org/oppia/android/scripts/apkstats/BundleToolClient.kt" + override_min_coverage_percent_required: 59 +} +test_file_exemption { + exempted_file_path: "scripts/src/java/org/oppia/android/scripts/apkstats/ComputeAabDifferences.kt" + override_min_coverage_percent_required: 6 +} test_file_exemption { exempted_file_path: "scripts/src/java/org/oppia/android/scripts/common/CommandExecutor.kt" test_file_not_required: true @@ -3990,10 +4030,18 @@ test_file_exemption { exempted_file_path: "testing/src/main/java/org/oppia/android/testing/espresso/KonfettiViewMatcher.kt" test_file_not_required: true } +test_file_exemption { + exempted_file_path: "testing/src/main/java/org/oppia/android/testing/espresso/TextInputAction.kt" + override_min_coverage_percent_required: 60 +} test_file_exemption { exempted_file_path: "testing/src/main/java/org/oppia/android/testing/junit/DefineAppLanguageLocaleContext.kt" test_file_not_required: true } +test_file_exemption { + exempted_file_path: "testing/src/main/java/org/oppia/android/testing/junit/InitializeDefaultLocaleRule.kt" + override_min_coverage_percent_required: 38 +} test_file_exemption { exempted_file_path: "testing/src/main/java/org/oppia/android/testing/junit/OppiaParameterizedBaseRunner.kt" test_file_not_required: true @@ -4042,6 +4090,10 @@ test_file_exemption { exempted_file_path: "testing/src/main/java/org/oppia/android/testing/logging/EventLogSubject.kt" test_file_not_required: true } +test_file_exemption { + exempted_file_path: "testing/src/main/java/org/oppia/android/testing/logging/SyncStatusTestModule.kt" + override_min_coverage_percent_required: 0 +} test_file_exemption { exempted_file_path: "testing/src/main/java/org/oppia/android/testing/math/ComparableOperationSubject.kt" test_file_not_required: true @@ -4098,6 +4150,10 @@ test_file_exemption { exempted_file_path: "testing/src/main/java/org/oppia/android/testing/network/MockFeedbackReportingService.kt" test_file_not_required: true } +test_file_exemption { + exempted_file_path: "testing/src/main/java/org/oppia/android/testing/network/MockPlatformParameterService.kt" + override_min_coverage_percent_required: 64 +} test_file_exemption { exempted_file_path: "testing/src/main/java/org/oppia/android/testing/network/MockQuestionPlayerService.kt" test_file_not_required: true @@ -4274,6 +4330,10 @@ test_file_exemption { exempted_file_path: "utility/src/main/java/org/oppia/android/util/gcsresource/GcsResourceModule.kt" test_file_not_required: true } +test_file_exemption { + exempted_file_path: "utility/src/main/java/org/oppia/android/util/locale/LocaleProdModule.kt" + override_min_coverage_percent_required: 0 +} test_file_exemption { exempted_file_path: "utility/src/main/java/org/oppia/android/util/locale/OppiaBidiFormatter.kt" test_file_not_required: true @@ -4282,10 +4342,18 @@ test_file_exemption { exempted_file_path: "utility/src/main/java/org/oppia/android/util/locale/OppiaLocale.kt" test_file_not_required: true } +test_file_exemption { + exempted_file_path: "utility/src/main/java/org/oppia/android/util/locale/testing/LocaleTestModule.kt" + override_min_coverage_percent_required: 0 +} test_file_exemption { exempted_file_path: "utility/src/main/java/org/oppia/android/util/logging/AnalyticsEventLogger.kt" test_file_not_required: true } +test_file_exemption { + exempted_file_path: "utility/src/main/java/org/oppia/android/util/logging/ConsoleLogger.kt" + override_min_coverage_percent_required: 54 +} test_file_exemption { exempted_file_path: "utility/src/main/java/org/oppia/android/util/logging/ConsoleLoggerInjector.kt" test_file_not_required: true @@ -4294,6 +4362,10 @@ test_file_exemption { exempted_file_path: "utility/src/main/java/org/oppia/android/util/logging/ConsoleLoggerInjectorProvider.kt" test_file_not_required: true } +test_file_exemption { + exempted_file_path: "utility/src/main/java/org/oppia/android/util/logging/EventLoggingConfigurationModule.kt" + override_min_coverage_percent_required: 0 +} test_file_exemption { exempted_file_path: "utility/src/main/java/org/oppia/android/util/logging/EventTypeToHumanReadableNameConverter.kt" test_file_not_required: true @@ -4302,6 +4374,10 @@ test_file_exemption { exempted_file_path: "utility/src/main/java/org/oppia/android/util/logging/ExceptionLogger.kt" test_file_not_required: true } +test_file_exemption { + exempted_file_path: "utility/src/main/java/org/oppia/android/util/logging/KenyaAlphaEventLoggingConfigurationModule.kt" + override_min_coverage_percent_required: 0 +} test_file_exemption { exempted_file_path: "utility/src/main/java/org/oppia/android/util/logging/KenyaAlphaEventTypeToHumanReadableNameConverterImpl.kt" test_file_not_required: true @@ -4398,6 +4474,10 @@ test_file_exemption { exempted_file_path: "utility/src/main/java/org/oppia/android/util/math/FractionExtensions.kt" test_file_not_required: true } +test_file_exemption { + exempted_file_path: "utility/src/main/java/org/oppia/android/util/math/MathExpressionExtensions.kt" + override_min_coverage_percent_required: 42 +} test_file_exemption { exempted_file_path: "utility/src/main/java/org/oppia/android/util/math/MathParsingError.kt" test_file_not_required: true @@ -4418,6 +4498,14 @@ test_file_exemption { exempted_file_path: "utility/src/main/java/org/oppia/android/util/networking/NetworkConnectionUtil.kt" test_file_not_required: true } +test_file_exemption { + exempted_file_path: "utility/src/main/java/org/oppia/android/util/networking/NetworkConnectionUtilDebugModule.kt" + override_min_coverage_percent_required: 0 +} +test_file_exemption { + exempted_file_path: "utility/src/main/java/org/oppia/android/util/networking/NetworkConnectionUtilProdModule.kt" + override_min_coverage_percent_required: 0 +} test_file_exemption { exempted_file_path: "utility/src/main/java/org/oppia/android/util/parser/html/ExplorationHtmlParserEntityType.kt" test_file_not_required: true @@ -4494,6 +4582,10 @@ test_file_exemption { exempted_file_path: "utility/src/main/java/org/oppia/android/util/parser/image/TextPictureDrawable.kt" test_file_not_required: true } +test_file_exemption { + exempted_file_path: "utility/src/main/java/org/oppia/android/util/parser/image/UrlImageParser.kt" + override_min_coverage_percent_required: 57 +} test_file_exemption { exempted_file_path: "utility/src/main/java/org/oppia/android/util/parser/math/MathBitmapModelLoader.kt" test_file_not_required: true diff --git a/scripts/src/java/org/oppia/android/scripts/common/GitClient.kt b/scripts/src/java/org/oppia/android/scripts/common/GitClient.kt index 54cc56e2dda..2f00133c3ed 100644 --- a/scripts/src/java/org/oppia/android/scripts/common/GitClient.kt +++ b/scripts/src/java/org/oppia/android/scripts/common/GitClient.kt @@ -28,7 +28,10 @@ class GitClient( val changedFiles: Set by lazy { retrieveChangedFilesWithPotentialDuplicates().toSet() } /** The list of files that have been committed in the local branch. */ - val committedFiles: List by lazy { retrieveChangedCommittedFiles() } + val committedFiles: List by lazy { + retrieveChangedCommittedFiles() + + retrieveRenamedFiles() + } private fun retrieveCurrentCommit(): String { return executeGitCommandWithOneLineOutput("rev-parse HEAD") @@ -68,6 +71,12 @@ class GitClient( return executeGitCommand("ls-files --others --exclude-standard") } + private fun retrieveRenamedFiles(): List { + val renamedFilesCommand = executeGitCommand("diff -M --name-status ${computeCommitRange()}") + return renamedFilesCommand.filter { it.startsWith("R") } + .map { it.split("\t")[1] } + } + private fun computeCommitRange(): String = "HEAD..$branchMergeBase" private fun executeGitCommandWithOneLineOutput(argumentsLine: String): String { 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 775357ab218..dc8f18d9e31 100644 --- a/scripts/src/java/org/oppia/android/scripts/coverage/BUILD.bazel +++ b/scripts/src/java/org/oppia/android/scripts/coverage/BUILD.bazel @@ -12,10 +12,10 @@ kt_jvm_library( ], visibility = ["//scripts:oppia_script_binary_visibility"], deps = [ - ":coverage_reporter", ":coverage_runner", "//scripts/src/java/org/oppia/android/scripts/common:bazel_client", "//scripts/src/java/org/oppia/android/scripts/common:proto_string_encoder", + "//scripts/src/java/org/oppia/android/scripts/coverage/reporter:coverage_reporter_lib", "//scripts/src/java/org/oppia/android/scripts/proto:script_exemptions_java_proto", ], ) @@ -32,17 +32,3 @@ kt_jvm_library( "//scripts/src/java/org/oppia/android/scripts/proto:coverage_java_proto", ], ) - -kt_jvm_library( - name = "coverage_reporter", - testonly = True, - srcs = [ - "CoverageReporter.kt", - ], - 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", - "//scripts/src/java/org/oppia/android/scripts/proto:script_exemptions_java_proto", - ], -) 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 81f230ebf6b..513a7e2329a 100644 --- a/scripts/src/java/org/oppia/android/scripts/coverage/RunCoverage.kt +++ b/scripts/src/java/org/oppia/android/scripts/coverage/RunCoverage.kt @@ -4,6 +4,13 @@ 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.coverage.reporter.BOLD +import org.oppia.android.scripts.coverage.reporter.CoverageCheck +import org.oppia.android.scripts.coverage.reporter.CoverageReporter +import org.oppia.android.scripts.coverage.reporter.GREEN +import org.oppia.android.scripts.coverage.reporter.RED +import org.oppia.android.scripts.coverage.reporter.RESET +import org.oppia.android.scripts.coverage.reporter.ReportFormat import org.oppia.android.scripts.proto.Coverage import org.oppia.android.scripts.proto.CoverageDetails import org.oppia.android.scripts.proto.CoverageExemption @@ -15,21 +22,11 @@ import org.oppia.android.scripts.proto.TestFileExemptions import java.io.File import java.util.concurrent.TimeUnit -/** ANSI escape codes for colors. */ -/** Green text. */ -const val GREEN = "\u001B[32m" -/** Red text. */ -const val RED = "\u001B[31m" -/** Default text. */ -const val RESET = "\u001B[0m" -/** Bold text. */ -const val BOLD = "\u001B[1m" - /** * Entry point function for running coverage analysis for a source file. * * Usage: - * bazel run //scripts:run_coverage_for_test_target -- + * bazel run //scripts:run_coverage -- * * Arguments: * - path_to_root: directory path to the root of the Oppia Android repository. 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 new file mode 100644 index 00000000000..85b59087cc2 --- /dev/null +++ b/scripts/src/java/org/oppia/android/scripts/coverage/reporter/BUILD.bazel @@ -0,0 +1,19 @@ +""" +Library corresponding to developer scripts that generates coverage reports with the coverage data. +""" + +load("@io_bazel_rules_kotlin//kotlin:kotlin.bzl", "kt_jvm_library") + +kt_jvm_library( + name = "coverage_reporter_lib", + testonly = True, + srcs = [ + "CoverageReporter.kt", + ], + visibility = ["//visibility:public"], + deps = [ + "//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", + ], +) diff --git a/scripts/src/java/org/oppia/android/scripts/coverage/CoverageReporter.kt b/scripts/src/java/org/oppia/android/scripts/coverage/reporter/CoverageReporter.kt similarity index 86% rename from scripts/src/java/org/oppia/android/scripts/coverage/CoverageReporter.kt rename to scripts/src/java/org/oppia/android/scripts/coverage/reporter/CoverageReporter.kt index 60e2be8f580..b27ec9283ce 100644 --- a/scripts/src/java/org/oppia/android/scripts/coverage/CoverageReporter.kt +++ b/scripts/src/java/org/oppia/android/scripts/coverage/reporter/CoverageReporter.kt @@ -1,4 +1,4 @@ -package org.oppia.android.scripts.coverage +package org.oppia.android.scripts.coverage.reporter import org.oppia.android.scripts.proto.Coverage import org.oppia.android.scripts.proto.CoverageReport @@ -9,6 +9,79 @@ import java.io.File /** Minimum coverage percentage required. */ const val MIN_THRESHOLD = 70 +/* ANSI escape codes for colors. */ + +/** Green text. */ +const val GREEN = "\u001B[32m" +/** Red text. */ +const val RED = "\u001B[31m" +/** Default text. */ +const val RESET = "\u001B[0m" +/** Bold text. */ +const val BOLD = "\u001B[1m" + +/** + * Function for generating coverage report for a list of proto files. + * + * Usage: + * bazel run //scripts:coverage_runner -- + * + * + * Arguments: + * - path_to_root: directory path to the root of the Oppia Android repository. + * - text_file_with_list_of_coverage_data_proto_paths: the text file that contains + * the list of relative path to the proto files containing coverage report data + * separated by spaces to analyse coverage. + * Sample `coverage_proto_list.txt` content: + * ``` + * coverage_reports/coverage_report1.pb coverage_reports/coverage_report2.pb + * ``` + * + * Example: + * bazel run //scripts:coverage_reporter -- $(pwd) coverage_proto_list.txt + */ +fun main(vararg args: String) { + val repoRoot = args[0] + val pbTxtFile = File(repoRoot, args[1]) + + pbTxtFile.takeIf { it.exists() }?.let { + val pbList = pbTxtFile.readText() + val filePathList = pbList.split(" ") + .filter { it.isNotBlank() } + .map { it.trim() } + + val coverageResultList = filePathList.mapNotNull { filePath -> + try { + println("Filepath: $filePath") + File(repoRoot, filePath).inputStream().use { stream -> + CoverageReport.newBuilder().also { builder -> + builder.mergeFrom(stream) + }.build() + } + } catch (e: Exception) { + error("Error processing file $filePath: ${e.message}") + } + } + + val coverageReportContainer = CoverageReportContainer.newBuilder().apply { + addAllCoverageReport(coverageResultList) + }.build() + + val coverageStatus = CoverageReporter( + repoRoot, + coverageReportContainer, + ReportFormat.MARKDOWN + ).generateRichTextReport() + + when (coverageStatus) { + CoverageCheck.PASS -> println("Coverage Analysis$BOLD$GREEN PASSED$RESET") + CoverageCheck.FAIL -> error("Coverage Analysis$BOLD$RED FAILED$RESET") + } + } ?: run { + error("File not found: ${pbTxtFile.absolutePath}") + } +} + /** * Class responsible for generating rich text coverage report. * @@ -261,7 +334,7 @@ class CoverageReporter( ?.takeIf { it.isNotEmpty() } ?.let { getFilenameAsDetailsSummary(it) } ?: failure.bazelTestTarget - "| $failurePath | ${failure.failureMessage} |" + "| $failurePath | ${failure.failureMessage} | :x: |" } }.joinToString(separator = "\n") @@ -322,7 +395,7 @@ class CoverageReporter( .map { exemption -> val filePath = exemption.exemption.filePath val exemptionReason = exemption.exemption.exemptionReason - "${getFilenameAsDetailsSummary(filePath, exemptionReason)}" + "| ${getFilenameAsDetailsSummary(filePath)} | $exemptionReason |" }.joinToString(separator = "\n") { "$it" } val tableHeader = buildString { @@ -334,8 +407,8 @@ class CoverageReporter( if (failureTableRows.isNotEmpty()) { append("\n\n") append("### Failure Cases\n\n") - append("| File | Failure Reason |\n") - append("|------|----------------|\n") + append("| File | Failure Reason | Status |\n") + append("|------|----------------|--------|\n") append(failureTableRows) } } @@ -346,7 +419,10 @@ class CoverageReporter( append("### Failing coverage") append("\n\n") append(tableHeader) - append(failureBelowThresholdTableRows) + if (failureBelowThresholdTableRows.isNotEmpty()) { + append(failureBelowThresholdTableRows) + append('\n') + } if (exemptedFailureTableRows.isNotEmpty()) { append(exemptedFailureTableRows) append( @@ -377,7 +453,10 @@ class CoverageReporter( append("Files with passing code coverage
\n\n") if (successTableRows.isNotEmpty()) { append(tableHeader) - append(successTableRows) + if (successTableRows.isNotEmpty()) { + append(successTableRows) + append('\n') + } if (exemptedSuccessTableRows.isNotEmpty()) { append(exemptedSuccessTableRows) append( @@ -399,11 +478,20 @@ class CoverageReporter( } else "" val testFileExemptedSection = buildString { + val exemptionsReferenceNote = ">Refer [test_file_exemptions.textproto]" + + "(https://github.com/oppia/oppia-android/blob/develop/" + + "scripts/assets/test_file_exemptions.textproto) for the comprehensive " + + "list of file exemptions and their required coverage percentages." if (testFileExemptedCasesList.isNotEmpty()) { append("\n\n") append("### Exempted coverage\n") - append("
Files exempted from coverage
") + append("
Files exempted from coverage
") + append("\n\n") + append("| File | Exemption Reason |\n") + append("|------|------------------|\n") append(testFileExemptedCasesList) + append("\n\n") + append(exemptionsReferenceNote) append("
") } } diff --git a/scripts/src/javatests/org/oppia/android/scripts/ci/ComputeChangedFilesTest.kt b/scripts/src/javatests/org/oppia/android/scripts/ci/ComputeChangedFilesTest.kt index b66667901ee..4b324b84004 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/ci/ComputeChangedFilesTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/ci/ComputeChangedFilesTest.kt @@ -1029,6 +1029,7 @@ class ComputeChangedFilesTest { oldFilePath.copyTo(newFilePath) oldFilePath.delete() + testGitRepository.stageFileForCommit(oldFilePath) testGitRepository.stageFileForCommit(newFilePath) testGitRepository.commit(message = "Move file from $oldFilePath to $newFilePath") } diff --git a/scripts/src/javatests/org/oppia/android/scripts/common/GitClientTest.kt b/scripts/src/javatests/org/oppia/android/scripts/common/GitClientTest.kt index 2741900296b..6e76f31e28a 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/common/GitClientTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/common/GitClientTest.kt @@ -253,6 +253,45 @@ class GitClientTest { ) } + @Test + fun testCommittedFiles_featureBranch_committedFile_includesFile() { + initializeRepoWithDevelopBranch() + testGitRepository.checkoutNewBranch("introduce-feature") + commitNewFile("example_file") + + val gitClient = GitClient(tempFolder.root, "develop", commandExecutor) + val committedFiles = gitClient.committedFiles + + assertThat(committedFiles).containsExactly("example_file") + } + + @Test + fun testCommittedFiles_featureBranch_movedFile_includesFile() { + initializeRepoWithDevelopBranch() + testGitRepository.checkoutNewBranch("introduce-feature") + commitNewFile("example_file") + moveFile(File(tempFolder.root, "example_file"), File(tempFolder.root, "moved_file")) + + val gitClient = GitClient(tempFolder.root, "develop", commandExecutor) + val committedFiles = gitClient.committedFiles + + assertThat(committedFiles).containsExactly("moved_file") + } + + @Test + fun testCommittedFiles_committedAndMovedFiles_includeAllFiles() { + initializeRepoWithDevelopBranch() + testGitRepository.checkoutNewBranch("introduce-feature") + commitNewFile("committed_file") + commitNewFile("to_be_moved_file") + moveFile(File(tempFolder.root, "to_be_moved_file"), File(tempFolder.root, "moved_file")) + + val gitClient = GitClient(tempFolder.root, "develop", commandExecutor) + val committedAndMovedFiles = gitClient.committedFiles + + assertThat(committedAndMovedFiles).containsExactly("committed_file", "moved_file") + } + private fun initializeRepoWithDevelopBranch() { testGitRepository.init() testGitRepository.setUser(email = "test@oppia.org", name = "Test User") @@ -283,6 +322,11 @@ class GitClientTest { testGitRepository.commit(message = "Add file $name.") } + private fun moveFile(oldFile: File, newFile: File) { + testGitRepository.moveFileForCommit(oldFile, newFile) + testGitRepository.commit(message = "Move from $oldFile to $newFile") + } + private fun modifyFile(name: String) { File(tempFolder.root, name).appendText("More text") } 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 754febac19b..1b791beb372 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/coverage/BUILD.bazel +++ b/scripts/src/javatests/org/oppia/android/scripts/coverage/BUILD.bazel @@ -31,16 +31,3 @@ kt_jvm_test( "//third_party:org_jetbrains_kotlin_kotlin-test-junit", ], ) - -kt_jvm_test( - name = "CoverageReporterTest", - srcs = ["CoverageReporterTest.kt"], - deps = [ - "//scripts:test_file_check_assets", - "//scripts/src/java/org/oppia/android/scripts/coverage:coverage_reporter", - "//scripts/src/java/org/oppia/android/scripts/proto:script_exemptions_java_proto", - "//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/RunCoverageTest.kt b/scripts/src/javatests/org/oppia/android/scripts/coverage/RunCoverageTest.kt index 3400b363c97..864ff9beb51 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,11 @@ 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.coverage.reporter.BOLD +import org.oppia.android.scripts.coverage.reporter.MIN_THRESHOLD +import org.oppia.android.scripts.coverage.reporter.RED +import org.oppia.android.scripts.coverage.reporter.RESET +import org.oppia.android.scripts.coverage.reporter.ReportFormat import org.oppia.android.scripts.proto.Coverage import org.oppia.android.scripts.proto.CoverageReport import org.oppia.android.scripts.proto.CoveredLine @@ -82,9 +87,9 @@ class RunCoverageTest { append("Coverage Analysis: **FAIL** :x:\n") append("##\n\n") append("### Failure Cases\n\n") - append("| File | Failure Reason |\n") - append("|------|----------------|\n") - append("| ${getFilenameAsDetailsSummary(sampleFile)} | $failureMessage |") + append("| File | Failure Reason | Status |\n") + append("|------|----------------|--------|\n") + append("| ${getFilenameAsDetailsSummary(sampleFile)} | $failureMessage | :x: |") } assertThat(readFinalMdReport()).isEqualTo(expectedMarkdown) @@ -185,7 +190,11 @@ class RunCoverageTest { fun testRunCoverage_testFileExempted_exemptedFromCoverageAnalysis() { val exemptedFile = "TestExempted.kt" val exemptedFilePathList = listOf(exemptedFile) - val additionalData = "This file is exempted from having a test file; skipping coverage check." + val exemptionReason = "This file is exempted from having a test file; skipping coverage check." + val exemptionsReferenceNote = ">Refer [test_file_exemptions.textproto]" + + "(https://github.com/oppia/oppia-android/blob/develop/" + + "scripts/assets/test_file_exemptions.textproto) for the comprehensive " + + "list of file exemptions and their required coverage percentages." val testFileExemption = TestFileExemptions.TestFileExemption.newBuilder().apply { this.exemptedFilePath = exemptedFile @@ -212,8 +221,13 @@ class RunCoverageTest { append("Coverage Analysis: **PASS** :white_check_mark:\n") append("##\n\n") append("### Exempted coverage\n") - append("
Files exempted from coverage
") - append("${getFilenameAsDetailsSummary(exemptedFile, additionalData)}") + append("
Files exempted from coverage
") + append("\n\n") + append("| File | Exemption Reason |\n") + append("|------|------------------|\n") + append("| ${getFilenameAsDetailsSummary(exemptedFile)} | $exemptionReason |") + append("\n\n") + append(exemptionsReferenceNote) append("
") } @@ -224,8 +238,12 @@ class RunCoverageTest { fun testRunCoverage_sourceFileIncompatibleWithCodeCoverage_exemptedFromCoverageAnalysis() { val exemptedFile = "SourceIncompatibleWithCoverage.kt" val exemptedFilePathList = listOf(exemptedFile) - val additionalData = "This file is incompatible with code coverage tooling; " + + val exemptionReason = "This file is incompatible with code coverage tooling; " + "skipping coverage check." + val exemptionsReferenceNote = ">Refer [test_file_exemptions.textproto]" + + "(https://github.com/oppia/oppia-android/blob/develop/" + + "scripts/assets/test_file_exemptions.textproto) for the comprehensive " + + "list of file exemptions and their required coverage percentages." val testFileExemption = TestFileExemptions.TestFileExemption.newBuilder().apply { this.exemptedFilePath = exemptedFile @@ -252,8 +270,13 @@ class RunCoverageTest { append("Coverage Analysis: **PASS** :white_check_mark:\n") append("##\n\n") append("### Exempted coverage\n") - append("
Files exempted from coverage
") - append("${getFilenameAsDetailsSummary(exemptedFile, additionalData)}") + append("
Files exempted from coverage
") + append("\n\n") + append("| File | Exemption Reason |\n") + append("|------|------------------|\n") + append("| ${getFilenameAsDetailsSummary(exemptedFile)} | $exemptionReason |") + append("\n\n") + append(exemptionsReferenceNote) append("
") } @@ -359,9 +382,9 @@ class RunCoverageTest { append("Coverage Analysis: **FAIL** :x:\n") append("##\n\n") append("### Failure Cases\n\n") - append("| File | Failure Reason |\n") - append("|------|----------------|\n") - append("| //coverage/example:AddNumsTest | $failureMessage |") + append("| File | Failure Reason | Status |\n") + append("|------|----------------|--------|\n") + append("| //coverage/example:AddNumsTest | $failureMessage | :x: |") } assertThat(readFinalMdReport()).isEqualTo(expectedMarkdown) @@ -443,9 +466,9 @@ class RunCoverageTest { append("Coverage Analysis: **FAIL** :x:\n") append("##\n\n") append("### Failure Cases\n\n") - append("| File | Failure Reason |\n") - append("|------|----------------|\n") - append("| //coverage/test/java/com/example:SubNumsTest | $failureMessage |") + append("| File | Failure Reason | Status |\n") + append("|------|----------------|--------|\n") + append("| //coverage/test/java/com/example:SubNumsTest | $failureMessage | :x: |") } assertThat(readFinalMdReport()).isEqualTo(expectedMarkdown) @@ -629,7 +652,7 @@ class RunCoverageTest { ) append( "| ${getFilenameAsDetailsSummary(filePathList.get(1))} | 75.00% | 3 / 4 | " + - ":white_check_mark: | $MIN_THRESHOLD% |\n" + ":white_check_mark: | $MIN_THRESHOLD% |\n\n" ) append("
") } @@ -818,7 +841,7 @@ class RunCoverageTest { append("|------|:--------:|----------:|:------:|:------------:|\n") append( "| ${getFilenameAsDetailsSummary(filePathList.get(0))} | 0.00% | 0 / 4 | " + - ":x: | $MIN_THRESHOLD% |" + ":x: | $MIN_THRESHOLD% |\n" ) } @@ -1055,7 +1078,7 @@ class RunCoverageTest { append("|------|:--------:|----------:|:------:|:------------:|\n") append( "| ${getFilenameAsDetailsSummary(filePathList.get(1))} | 0.00% | 0 / 4 | " + - ":x: | $MIN_THRESHOLD% |\n" + ":x: | $MIN_THRESHOLD% |\n\n" ) append("### Passing coverage\n\n") append("
\n") @@ -1064,7 +1087,7 @@ class RunCoverageTest { append("|------|:--------:|----------:|:------:|:------------:|\n") append( "| ${getFilenameAsDetailsSummary(filePathList.get(0))} | 75.00% | 3 / 4 | " + - ":white_check_mark: | $MIN_THRESHOLD% |\n" + ":white_check_mark: | $MIN_THRESHOLD% |\n\n" ) append("
") } @@ -1075,7 +1098,12 @@ class RunCoverageTest { @Test fun testRunCoverage_withSuccessAndExemptedFiles_generatesFinalCoverageReport() { val exemptedFile = "TestExempted.kt" - val additionalData = "This file is exempted from having a test file; skipping coverage check." + val exemptionReason = "This file is exempted from having a test file; skipping coverage check." + val exemptionsReferenceNote = ">Refer [test_file_exemptions.textproto]" + + "(https://github.com/oppia/oppia-android/blob/develop/" + + "scripts/assets/test_file_exemptions.textproto) for the comprehensive " + + "list of file exemptions and their required coverage percentages." + val filePathList = listOf( "coverage/main/java/com/example/AddNums.kt", exemptedFile @@ -1122,14 +1150,17 @@ class RunCoverageTest { append("|------|:--------:|----------:|:------:|:------------:|\n") append( "| ${getFilenameAsDetailsSummary(filePathList.get(0))} | 75.00% | 3 / 4 | " + - ":white_check_mark: | $MIN_THRESHOLD% |\n" + ":white_check_mark: | $MIN_THRESHOLD% |\n\n" ) append("
\n\n") append("### Exempted coverage\n") - append("
Files exempted from coverage
") - append( - "${getFilenameAsDetailsSummary(filePathList.get(1), additionalData)}" - ) + append("
Files exempted from coverage
") + append("\n\n") + append("| File | Exemption Reason |\n") + append("|------|------------------|\n") + append("| ${getFilenameAsDetailsSummary(filePathList.get(1))} | $exemptionReason |") + append("\n\n") + append(exemptionsReferenceNote) append("
") } @@ -1139,7 +1170,12 @@ class RunCoverageTest { @Test fun testRunCoverage_withFailureAndExemptedFiles_generatesFinalCoverageReport() { val exemptedFile = "TestExempted.kt" - val additionalData = "This file is exempted from having a test file; skipping coverage check." + val exemptionReason = "This file is exempted from having a test file; skipping coverage check." + val exemptionsReferenceNote = ">Refer [test_file_exemptions.textproto]" + + "(https://github.com/oppia/oppia-android/blob/develop/" + + "scripts/assets/test_file_exemptions.textproto) for the comprehensive " + + "list of file exemptions and their required coverage percentages." + val filePathList = listOf( "coverage/main/java/com/example/LowTestNums.kt", exemptedFile @@ -1192,9 +1228,15 @@ class RunCoverageTest { "| ${getFilenameAsDetailsSummary(filePathList.get(0))} | 0.00% | 0 / 4 | " + ":x: | $MIN_THRESHOLD% |\n\n" ) + append("\n") append("### Exempted coverage\n") - append("
Files exempted from coverage
") - append("${getFilenameAsDetailsSummary(filePathList.get(1), additionalData)}") + append("
Files exempted from coverage
") + append("\n\n") + append("| File | Exemption Reason |\n") + append("|------|------------------|\n") + append("| ${getFilenameAsDetailsSummary(filePathList.get(1))} | $exemptionReason |") + append("\n\n") + append(exemptionsReferenceNote) append("
") } @@ -1204,7 +1246,12 @@ class RunCoverageTest { @Test fun testRunCoverage_withSuccessFailureAndExemptedFiles_generatesFinalCoverageReport() { val exemptedFile = "TestExempted.kt" - val additionalData = "This file is exempted from having a test file; skipping coverage check." + val exemptionReason = "This file is exempted from having a test file; skipping coverage check." + val exemptionsReferenceNote = ">Refer [test_file_exemptions.textproto]" + + "(https://github.com/oppia/oppia-android/blob/develop/" + + "scripts/assets/test_file_exemptions.textproto) for the comprehensive " + + "list of file exemptions and their required coverage percentages." + val filePathList = listOf( "coverage/main/java/com/example/AddNums.kt", "coverage/main/java/com/example/LowTestNums.kt", @@ -1265,7 +1312,7 @@ class RunCoverageTest { append("|------|:--------:|----------:|:------:|:------------:|\n") append( "| ${getFilenameAsDetailsSummary(filePathList.get(1))} | 0.00% | 0 / 4 | " + - ":x: | $MIN_THRESHOLD% |\n" + ":x: | $MIN_THRESHOLD% |\n\n" ) append("### Passing coverage\n\n") append("
\n") @@ -1274,12 +1321,17 @@ class RunCoverageTest { append("|------|:--------:|----------:|:------:|:------------:|\n") append( "| ${getFilenameAsDetailsSummary(filePathList.get(0))} | 75.00% | 3 / 4 | " + - ":white_check_mark: | $MIN_THRESHOLD% |\n" + ":white_check_mark: | $MIN_THRESHOLD% |\n\n" ) append("
\n\n") append("### Exempted coverage\n") - append("
Files exempted from coverage
") - append("${getFilenameAsDetailsSummary(filePathList.get(2), additionalData)}") + append("
Files exempted from coverage
") + append("\n\n") + append("| File | Exemption Reason |\n") + append("|------|------------------|\n") + append("| ${getFilenameAsDetailsSummary(filePathList.get(2))} | $exemptionReason |") + append("\n\n") + append(exemptionsReferenceNote) append("
") } @@ -1289,7 +1341,12 @@ class RunCoverageTest { @Test fun testRunCoverage_withSuccessFailureMissingTestAndExemptedFiles_generatesFinalReport() { val exemptedFile = "TestExempted.kt" - val additionalData = "This file is exempted from having a test file; skipping coverage check." + val exemptionReason = "This file is exempted from having a test file; skipping coverage check." + val exemptionsReferenceNote = ">Refer [test_file_exemptions.textproto]" + + "(https://github.com/oppia/oppia-android/blob/develop/" + + "scripts/assets/test_file_exemptions.textproto) for the comprehensive " + + "list of file exemptions and their required coverage percentages." + val filePathList = listOf( "coverage/main/java/com/example/AddNums.kt", "coverage/main/java/com/example/LowTestNums.kt", @@ -1352,15 +1409,15 @@ class RunCoverageTest { append("Coverage Analysis: **FAIL** :x:\n") append("##\n\n") append("### Failure Cases\n\n") - append("| File | Failure Reason |\n") - append("|------|----------------|\n") - append("| ${getFilenameAsDetailsSummary("file.kt")} | $failureMessage |\n\n") + append("| File | Failure Reason | Status |\n") + append("|------|----------------|--------|\n") + append("| ${getFilenameAsDetailsSummary("file.kt")} | $failureMessage | :x: |\n\n") append("### Failing coverage\n\n") append("| File | Coverage | Lines Hit | Status | Min Required |\n") append("|------|:--------:|----------:|:------:|:------------:|\n") append( "| ${getFilenameAsDetailsSummary(filePathList.get(1))} | 0.00% | 0 / 4 | " + - ":x: | $MIN_THRESHOLD% |\n" + ":x: | $MIN_THRESHOLD% |\n\n" ) append("### Passing coverage\n\n") append("
\n") @@ -1369,12 +1426,17 @@ class RunCoverageTest { append("|------|:--------:|----------:|:------:|:------------:|\n") append( "| ${getFilenameAsDetailsSummary(filePathList.get(0))} | 75.00% | 3 / 4 | " + - ":white_check_mark: | $MIN_THRESHOLD% |\n" + ":white_check_mark: | $MIN_THRESHOLD% |\n\n" ) append("
\n\n") append("### Exempted coverage\n") - append("
Files exempted from coverage
") - append("${getFilenameAsDetailsSummary(filePathList.get(2), additionalData)}") + append("
Files exempted from coverage
") + append("\n\n") + append("| File | Exemption Reason |\n") + append("|------|------------------|\n") + append("| ${getFilenameAsDetailsSummary(filePathList.get(2))} | $exemptionReason |") + append("\n\n") + append(exemptionsReferenceNote) append("
") } @@ -1577,7 +1639,7 @@ class RunCoverageTest { append("|------|:--------:|----------:|:------:|:------------:|\n") append( "| ${getFilenameAsDetailsSummary(filePathList.get(0))} | 75.00% | 3 / 4 | " + - ":white_check_mark: | $MIN_THRESHOLD% |\n" + ":white_check_mark: | $MIN_THRESHOLD% |\n\n" ) append("
") } @@ -1657,7 +1719,7 @@ class RunCoverageTest { append("|------|:--------:|----------:|:------:|:------------:|\n") append( "| ${getFilenameAsDetailsSummary(filePathList.get(0))} | 75.00% | 3 / 4 | " + - ":white_check_mark: | $MIN_THRESHOLD% |\n" + ":white_check_mark: | $MIN_THRESHOLD% |\n\n" ) append("
") } @@ -2217,7 +2279,7 @@ class RunCoverageTest { append("|------|:--------:|----------:|:------:|:------------:|\n") append( "| ${getFilenameAsDetailsSummary(filePath)} | 75.00% | " + - "3 / 4 | :white_check_mark: | $MIN_THRESHOLD% |\n" + "3 / 4 | :white_check_mark: | $MIN_THRESHOLD% |\n\n" ) append("
") } 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 new file mode 100644 index 00000000000..6571e76119e --- /dev/null +++ b/scripts/src/javatests/org/oppia/android/scripts/coverage/reporter/BUILD.bazel @@ -0,0 +1,19 @@ +""" +Tests corresponding to developer scripts that help with obtaining coverage data for test targets. +""" + +load("@io_bazel_rules_kotlin//kotlin:jvm.bzl", "kt_jvm_test") + +kt_jvm_test( + name = "CoverageReporterTest", + srcs = ["CoverageReporterTest.kt"], + test_class = "org.oppia.android.scripts.coverage.reporter.CoverageReporterTest", + deps = [ + "//scripts:test_file_check_assets", + "//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_truth_truth", + "//third_party:org_jetbrains_kotlin_kotlin-test-junit", + ], +) diff --git a/scripts/src/javatests/org/oppia/android/scripts/coverage/CoverageReporterTest.kt b/scripts/src/javatests/org/oppia/android/scripts/coverage/reporter/CoverageReporterTest.kt similarity index 70% rename from scripts/src/javatests/org/oppia/android/scripts/coverage/CoverageReporterTest.kt rename to scripts/src/javatests/org/oppia/android/scripts/coverage/reporter/CoverageReporterTest.kt index d508543f83e..cfbfb5d45b2 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/coverage/CoverageReporterTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/coverage/reporter/CoverageReporterTest.kt @@ -1,4 +1,4 @@ -package org.oppia.android.scripts.coverage +package org.oppia.android.scripts.coverage.reporter import com.google.common.truth.Truth.assertThat import org.junit.After @@ -13,6 +13,7 @@ import org.oppia.android.scripts.proto.CoverageReport import org.oppia.android.scripts.proto.CoverageReportContainer import org.oppia.android.scripts.proto.TestFileExemptions import org.oppia.android.scripts.proto.TestFileExemptions.TestFileExemption +import org.oppia.android.testing.assertThrows import java.io.ByteArrayOutputStream import java.io.File import java.io.PrintStream @@ -71,7 +72,7 @@ class CoverageReporterTest { append("|------|:--------:|----------:|:------:|:------------:|\n") append( "| ${getFilenameAsDetailsSummary(filename)} " + - "| 100.00% | 10 / 10 | :white_check_mark: | $MIN_THRESHOLD% |\n" + "| 100.00% | 10 / 10 | :white_check_mark: | $MIN_THRESHOLD% |\n\n" ) append("
") } @@ -113,7 +114,7 @@ class CoverageReporterTest { append("|------|:--------:|----------:|:------:|:------------:|\n") append( "| ${getFilenameAsDetailsSummary(filename)} | " + - "0.00% | 0 / 10 | :x: | $MIN_THRESHOLD% |" + "0.00% | 0 / 10 | :x: | $MIN_THRESHOLD% |\n" ) } @@ -148,9 +149,9 @@ class CoverageReporterTest { append("Coverage Analysis: **FAIL** :x:\n") append("##\n\n") append("### Failure Cases\n\n") - append("| File | Failure Reason |\n") - append("|------|----------------|\n") - append("| ://bazelTestTarget | Failure Message |") + append("| File | Failure Reason | Status |\n") + append("|------|----------------|--------|\n") + append("| ://bazelTestTarget | Failure Message | :x: |") } assertThat(readFinalMdReport()).isEqualTo(expectedMarkdown) @@ -159,7 +160,12 @@ class CoverageReporterTest { @Test fun testGenerateMarkDownReport_withTestFileExemptionCoverageReport_generatesMarkdownTable() { val testExemptedFilePath = "TestExempted.kt" - val additionalData = "This file is exempted from having a test file; skipping coverage check." + val exemptionReason = "This file is exempted from having a test file; skipping coverage check." + val exemptionsReferenceNote = ">Refer [test_file_exemptions.textproto]" + + "(https://github.com/oppia/oppia-android/blob/develop/" + + "scripts/assets/test_file_exemptions.textproto) for the comprehensive " + + "list of file exemptions and their required coverage percentages." + val testFileExemption = TestFileExemptions.TestFileExemption.newBuilder().apply { this.exemptedFilePath = testExemptedFilePath this.testFileNotRequired = true @@ -172,7 +178,7 @@ class CoverageReporterTest { .setExemption( CoverageExemption.newBuilder() .setFilePath(testExemptedFilePath) - .setExemptionReason(additionalData) + .setExemptionReason(exemptionReason) .build() ).build() @@ -195,8 +201,13 @@ class CoverageReporterTest { append("Coverage Analysis: **PASS** :white_check_mark:\n") append("##\n\n") append("### Exempted coverage\n") - append("
Files exempted from coverage
") - append("${getFilenameAsDetailsSummary(testExemptedFilePath, additionalData)}") + append("
Files exempted from coverage
") + append("\n\n") + append("| File | Exemption Reason |\n") + append("|------|------------------|\n") + append("| ${getFilenameAsDetailsSummary(testExemptedFilePath)} | $exemptionReason |") + append("\n\n") + append(exemptionsReferenceNote) append("
") } @@ -206,8 +217,13 @@ class CoverageReporterTest { @Test fun testGenerateMarkDownReport_withSourceIncompatibilityExemption_generatesMarkdownTable() { val testExemptedFilePath = "TestExempted.kt" - val additionalData = "This file is incompatible with code coverage tooling; " + + val exemptionReason = "This file is incompatible with code coverage tooling; " + "skipping coverage check." + val exemptionsReferenceNote = ">Refer [test_file_exemptions.textproto]" + + "(https://github.com/oppia/oppia-android/blob/develop/" + + "scripts/assets/test_file_exemptions.textproto) for the comprehensive " + + "list of file exemptions and their required coverage percentages." + val testFileExemption = TestFileExemptions.TestFileExemption.newBuilder().apply { this.exemptedFilePath = testExemptedFilePath this.testFileNotRequired = true @@ -220,7 +236,7 @@ class CoverageReporterTest { .setExemption( CoverageExemption.newBuilder() .setFilePath(testExemptedFilePath) - .setExemptionReason(additionalData) + .setExemptionReason(exemptionReason) .build() ).build() @@ -243,8 +259,13 @@ class CoverageReporterTest { append("Coverage Analysis: **PASS** :white_check_mark:\n") append("##\n\n") append("### Exempted coverage\n") - append("
Files exempted from coverage
") - append("${getFilenameAsDetailsSummary(testExemptedFilePath, additionalData)}") + append("
Files exempted from coverage
") + append("\n\n") + append("| File | Exemption Reason |\n") + append("|------|------------------|\n") + append("| ${getFilenameAsDetailsSummary(testExemptedFilePath)} | $exemptionReason |") + append("\n\n") + append(exemptionsReferenceNote) append("
") } @@ -363,7 +384,11 @@ class CoverageReporterTest { val successFileName = "SampleSuccessFile.kt" val failureFileName = "SampleFailureFile.kt" val testExemptedFilePath = "TestExempted.kt" - val additionalData = "This file is exempted from having a test file; skipping coverage check." + val exemptionReason = "This file is exempted from having a test file; skipping coverage check." + val exemptionsReferenceNote = ">Refer [test_file_exemptions.textproto]" + + "(https://github.com/oppia/oppia-android/blob/develop/" + + "scripts/assets/test_file_exemptions.textproto) for the comprehensive " + + "list of file exemptions and their required coverage percentages." val testFileExemption = TestFileExemptions.TestFileExemption.newBuilder().apply { this.exemptedFilePath = testExemptedFilePath @@ -377,7 +402,7 @@ class CoverageReporterTest { .setExemption( CoverageExemption.newBuilder() .setFilePath(testExemptedFilePath) - .setExemptionReason(additionalData) + .setExemptionReason(exemptionReason) .build() ).build() @@ -429,15 +454,15 @@ class CoverageReporterTest { append("Coverage Analysis: **FAIL** :x:\n") append("##\n\n") append("### Failure Cases\n\n") - append("| File | Failure Reason |\n") - append("|------|----------------|\n") - append("| ://bazelTestTarget | Failure Message |\n\n") + append("| File | Failure Reason | Status |\n") + append("|------|----------------|--------|\n") + append("| ://bazelTestTarget | Failure Message | :x: |\n\n") append("### Failing coverage\n\n") append("| File | Coverage | Lines Hit | Status | Min Required |\n") append("|------|:--------:|----------:|:------:|:------------:|\n") append( "| ${getFilenameAsDetailsSummary(failureFileName)} | " + - "0.00% | 0 / 10 | :x: | $MIN_THRESHOLD% |\n" + "0.00% | 0 / 10 | :x: | $MIN_THRESHOLD% |\n\n" ) append("### Passing coverage\n\n") append("
\n") @@ -446,12 +471,17 @@ class CoverageReporterTest { append("|------|:--------:|----------:|:------:|:------------:|\n") append( "| ${getFilenameAsDetailsSummary(successFileName)} | " + - "100.00% | 10 / 10 | :white_check_mark: | $MIN_THRESHOLD% |\n" + "100.00% | 10 / 10 | :white_check_mark: | $MIN_THRESHOLD% |\n\n" ) append("
\n\n") append("### Exempted coverage\n") - append("
Files exempted from coverage
") - append("${getFilenameAsDetailsSummary(testExemptedFilePath, additionalData)}") + append("
Files exempted from coverage
") + append("\n\n") + append("| File | Exemption Reason |\n") + append("|------|------------------|\n") + append("| ${getFilenameAsDetailsSummary(testExemptedFilePath)} | $exemptionReason |") + append("\n\n") + append(exemptionsReferenceNote) append("
") } @@ -723,6 +753,237 @@ class CoverageReporterTest { assertThat(outContent.toString().trim()).isEqualTo("-> $testExemptedFilePath - $additionalData") } + @Test + fun testCoverageReporter_passingInvalidProtoListTextPath_throwsException() { + val invalidProtoListTextPath = "invalid.txt" + + val exception = assertThrows() { + main( + "${tempFolder.root}", + invalidProtoListTextPath + ) + } + + assertThat(exception).hasMessageThat() + .contains("File not found") + } + + @Test + fun testCoverageReporter_passingInvalidProtoPath_throwsException() { + val protoListTextPath = "protoList.txt" + val protoListTextFile = tempFolder.newFile(protoListTextPath) + val invalidProtoPath = "invalid.pb" + protoListTextFile.writeText(invalidProtoPath) + + val exception = assertThrows() { + main( + "${tempFolder.root}", + protoListTextPath + ) + } + + assertThat(exception).hasMessageThat() + .contains("Error processing file $invalidProtoPath") + } + + @Test + fun testCoverageReporter_successCoverageProtoPath_checksCoverageStatus() { + System.setOut(PrintStream(outContent)) + val validProtoPath = "coverageReport.pb" + val protoFile = tempFolder.newFile(validProtoPath) + + val coverageReport = CoverageReport.newBuilder() + .setDetails( + CoverageDetails.newBuilder() + .setFilePath("file.kt") + .setLinesFound(10) + .setLinesHit(10) + .build() + ).build() + + protoFile.outputStream().use { outputStream -> + coverageReport.writeTo(outputStream) + } + + val protoListTextPath = "protoList.txt" + val protoListTextFile = tempFolder.newFile(protoListTextPath) + protoListTextFile.writeText(validProtoPath) + + main( + "${tempFolder.root}", + protoListTextPath + ) + + assertThat(outContent.toString().trim()) + .contains("Coverage Analysis$BOLD$GREEN PASSED$RESET") + } + + @Test + fun testCoverageReporter_failureCoverageProtoPath_checksCoverageStatus() { + val validProtoPath = "coverageReport.pb" + val protoFile = tempFolder.newFile(validProtoPath) + + val coverageReport = CoverageReport.newBuilder() + .setFailure( + CoverageFailure.newBuilder() + .setBazelTestTarget("//:coverageReport") + .setFailureMessage( + "Coverage retrieval failed for the test target: " + + "//:coverageReport" + ) + .build() + ) + .build() + + protoFile.outputStream().use { outputStream -> + coverageReport.writeTo(outputStream) + } + + val protoListTextPath = "protoList.txt" + val protoListTextFile = tempFolder.newFile(protoListTextPath) + protoListTextFile.writeText(validProtoPath) + + val exception = assertThrows() { + main( + "${tempFolder.root}", + protoListTextPath + ) + } + + assertThat(exception).hasMessageThat() + .contains("Coverage Analysis$BOLD$RED FAILED$RESET") + } + + @Test + fun testCoverageReporter_listOfCoverageProtoPath_checksCoverageStatus() { + val successProtoPath = "successCoverageReport.pb" + val successProtoFile = tempFolder.newFile(successProtoPath) + + val successCoverageReport = CoverageReport.newBuilder() + .setDetails( + CoverageDetails.newBuilder() + .setFilePath("file.kt") + .setLinesFound(10) + .setLinesHit(10) + .build() + ).build() + + successProtoFile.outputStream().use { outputStream -> + successCoverageReport.writeTo(outputStream) + } + + val failureProtoPath = "failureCoverageReport.pb" + val failureProtoFile = tempFolder.newFile(failureProtoPath) + + val failureCoverageReport = CoverageReport.newBuilder() + .setFailure( + CoverageFailure.newBuilder() + .setBazelTestTarget("//:coverageReport") + .setFailureMessage( + "Coverage retrieval failed for the test target: " + + "//:coverageReport" + ) + .build() + ) + .build() + + failureProtoFile.outputStream().use { outputStream -> + failureCoverageReport.writeTo(outputStream) + } + + val protoListTextPath = "protoList.txt" + val protoListTextFile = tempFolder.newFile(protoListTextPath) + protoListTextFile.appendText(successProtoPath) + protoListTextFile.appendText(" ") + protoListTextFile.appendText(failureProtoPath) + + val exception = assertThrows() { + main( + "${tempFolder.root}", + protoListTextPath + ) + } + + assertThat(exception).hasMessageThat() + .contains("Coverage Analysis$BOLD$RED FAILED$RESET") + } + + @Test + fun testCoverageReporter_listOfCoverageProtoPath_generatesMarkdownReport() { + val successProtoPath = "successCoverageReport.pb" + val successProtoFile = tempFolder.newFile(successProtoPath) + + val successCoverageReport = CoverageReport.newBuilder() + .setDetails( + CoverageDetails.newBuilder() + .setFilePath("file.kt") + .setLinesFound(10) + .setLinesHit(10) + .build() + ).build() + + successProtoFile.outputStream().use { outputStream -> + successCoverageReport.writeTo(outputStream) + } + + val failureProtoPath = "failureCoverageReport.pb" + val failureProtoFile = tempFolder.newFile(failureProtoPath) + + val failureCoverageReport = CoverageReport.newBuilder() + .setFailure( + CoverageFailure.newBuilder() + .setBazelTestTarget("//:coverageReport") + .setFailureMessage("Failure Message") + .build() + ) + .build() + + failureProtoFile.outputStream().use { outputStream -> + failureCoverageReport.writeTo(outputStream) + } + + val protoListTextPath = "protoList.txt" + val protoListTextFile = tempFolder.newFile(protoListTextPath) + protoListTextFile.appendText(successProtoPath) + protoListTextFile.appendText(" ") + protoListTextFile.appendText(failureProtoPath) + + val exception = assertThrows() { + main( + "${tempFolder.root}", + protoListTextPath + ) + } + + assertThat(exception).hasMessageThat() + .contains("Coverage Analysis$BOLD$RED FAILED$RESET") + + val expectedMarkdown = buildString { + append("## Coverage Report\n\n") + append("### Results\n") + append("Number of files assessed: 2\n") + append("Overall Coverage: **100.00%**\n") + append("Coverage Analysis: **FAIL** :x:\n") + append("##\n\n") + append("### Failure Cases\n\n") + append("| File | Failure Reason | Status |\n") + append("|------|----------------|--------|\n") + append("| //:coverageReport | Failure Message | :x: |\n") + append("### Passing coverage\n\n") + append("
\n") + append("Files with passing code coverage
\n\n") + append("| File | Coverage | Lines Hit | Status | Min Required |\n") + append("|------|:--------:|----------:|:------:|:------------:|\n") + append( + "| ${getFilenameAsDetailsSummary("file.kt")} " + + "| 100.00% | 10 / 10 | :white_check_mark: | $MIN_THRESHOLD% |\n\n" + ) + append("
") + } + + assertThat(readFinalMdReport()).isEqualTo(expectedMarkdown) + } + private fun readFinalMdReport(): String { return File( "${tempFolder.root}" + @@ -745,4 +1006,14 @@ class CoverageReporterTest { it.outputStream().use(testFileExemptions::writeTo) }.path } + + private fun loadCoverageReportProto( + coverageReportProtoPath: String + ): CoverageReport { + return File("$coverageReportProtoPath").inputStream().use { stream -> + CoverageReport.newBuilder().also { builder -> + builder.mergeFrom(stream) + }.build() + } + } }