[RunAllTests] Fix part of #5343: Upload generated code coverage repor…

…t 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: #5469 (comment) 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)).
oppia · Aug 13, 2024 · f9106d9 · f9106d9
1 parent 3d85ce0
commit f9106d9
Show file tree

Hide file tree

Showing 14 changed files with 796 additions and 121 deletions.
diff --git 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,18 +153,21 @@ 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)
           BAZEL_TEST_TARGETS=$(cat ./bazel_test_targets)
           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
@@ -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
@@ -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