Fix #5508: Skipping redundant code coverage and APK/AAB report commen…

…ts (#5580)  ## Explanation  Fix #5508 TODO: - [Done] - ~~Update Indents and latest upstream changes~~ ### This PR includes **1. Code Coverage Comment:** - In previous implementations, redundant code coverage reports could accumulate, even when they provided no additional information, leading to cluttered PR threads. - To address this, a comparison step has been added to **check the newly generated code coverage report against the latest posted code coverage comment**. - If the current report is identical to the latest comment, the script will skip posting a new comment. This ensures that the last coverage comment in the PR thread accurately reflects the latest report, preventing unnecessary repetitions. **2. Stats Comment:** - The Stat reports were being generated even when no new changes were made to the PR, causing repetitive APK/AAB report comments and hindering the stale comment checks. - A new step is added to track the presence of new commits. Now, the **stats analysis only triggers if there has been a new commit since the latest APK/AAB report comment**. - This approach reduces redundant analysis, ensuring that builds are only processed when relevant *PR changes are made. ***Limitation:** - These changes aim to help the stale comment checks. However, the trade-off is: merge commits to the `develop` branch will still generate new build reports. Allowing these reports would negate the benefits of stale comment checks, as weekly or bi-weekly merges can cause build variations. - Consequently, the [older method](#5532) of comparing previous and new reports has been removed due to flakiness in the stat.yml. While it is technically possible to use the currently generated report for comparison with the latest comment, it would include variations from merge changes, thus failing to prevent stale comments. Including the comparison step source for reference: ```sh - name: Compare Generated APK & AAB Analysis with the Previous Report if: ${{ steps.track_commits.outputs.new_commits == 'true' }} run: | if [ -f latest_aab_comment_body.log ]; then sed -i -e '$a\' ./develop/brief_build_summary.log sed -i -e '$a\' latest_aab_comment_body.log if diff -B ./develop/brief_build_summary.log latest_aab_comment_body.log > /dev/null; then echo "No significant changes detected; skipping apk aab analysis comment." echo "skip_apk_aab_comment=true" >> $GITHUB_ENV else echo "Changes detected; proceeding with the apk aab analysis comment." diff ./develop/brief_build_summary.log latest_aab_comment_body.log || true echo "skip_apk_aab_comment=false" >> $GITHUB_ENV fi else echo "No previous APK & AAB report posted; Commenting analysed APK & AAB report." echo "skip_apk_aab_comment=false" >> $GITHUB_ENV fi ``` # ### Demonstration >Demonstrated PR: Rd4dev/Oppia-Android-Fork-from-Fork#44 Tested with souce code - [comment_code_coverage.yml](https://github.com/Rd4dev/Oppia-Android-Fork-from-Fork/blob/4492ab36dc1efbcbec9cf0b7c164d24f9a5d4511/.github/workflows/comment_coverage_report.yml#L3) and [stats.yml](https://github.com/Rd4dev/Oppia-Android-Fork-from-Fork/blob/4492ab36dc1efbcbec9cf0b7c164d24f9a5d4511/.github/workflows/stats.yml#L3) - Initial Code Coverage Comment | [Comment](Rd4dev/Oppia-Android-Fork-from-Fork#44 (comment)) | [Stack Trace](https://github.com/Rd4dev/Oppia-Android-Fork-from-Fork/actions/runs/11957616687/job/33335730714#step:5:20) - Initial APK & AAB Analysis Comment | [Comment](Rd4dev/Oppia-Android-Fork-from-Fork#44 (comment)) | [Stack Trace](https://github.com/Rd4dev/Oppia-Android-Fork-from-Fork/actions/runs/11957621776/job/33337727450#step:20:348) - Redundant Code Coverage Comment Skipped | [Stack Trace](https://github.com/Rd4dev/Oppia-Android-Fork-from-Fork/actions/runs/11958563394/job/33338783923?pr=44#step:5:20) - Varying Code Coverage Comment Posted | [Comment](Rd4dev/Oppia-Android-Fork-from-Fork#44 (comment)) | [Stack Trace](https://github.com/Rd4dev/Oppia-Android-Fork-from-Fork/actions/runs/11959332332/job/33341699858#step:5:20) - APK & AAB Analysis Posted with follow up commits | [Comment](Rd4dev/Oppia-Android-Fork-from-Fork#44 (comment)) | [Stack Trace](https://github.com/Rd4dev/Oppia-Android-Fork-from-Fork/actions/runs/11959338918/job/33340923675#step:20:348) - APK & AAB Analysis Skipped with no follow up commits | [Stack Trace](https://github.com/Rd4dev/Oppia-Android-Fork-from-Fork/actions/runs/11959461908/job/33341312780#step:3:33) ## 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)). --------- Co-authored-by: Ben Henning <[email protected]>
oppia · Dec 12, 2024 · d321478 · d321478
1 parent f4a4a47
commit d321478
Show file tree

Hide file tree

Showing 2 changed files with 106 additions and 58 deletions.
diff --git a/.github/workflows/comment_coverage_report.yml b/.github/workflows/comment_coverage_report.yml
@@ -48,6 +48,24 @@ jobs:
     if: ${{ !cancelled() }}
     runs-on: ubuntu-latest
     steps:
+      - name: Find the latest Code Coverage Report Comment 
+        uses: actions/github-script@v6
+        with:
+           script: |
+             const comments = await github.paginate(github.rest.issues.listComments, {
+               owner: context.repo.owner,
+               repo: context.repo.repo,
+               issue_number: ${{ github.event.pull_request.number }},
+             });            
+
+             for (let i = comments.length - 1; i >= 0; i--) {
+               if (comments[i].body.includes("## Coverage Report")) {
+                 const latestCodeCoverageComment = comments[i].body;
+                 require('fs').writeFileSync('latest_code_coverage_comment.md', latestCodeCoverageComment, 'utf8');
+                 return
+               }
+             }
+    
       - name: Find CI workflow run for PR
         id: find-workflow-run
         uses: actions/github-script@v7
@@ -82,8 +100,28 @@ jobs:
           name: final-coverage-report
           github-token: ${{ secrets.GITHUB_TOKEN }}
           run-id: ${{ steps.find-workflow-run.outputs.run-id }}
+
+      - name: Compare Current Coverage Report with the Latest Coverage Report
+        run: |        
+          if [ -f latest_code_coverage_comment.md ]; then
+            sed -i -e '$a\' CoverageReport.md
+            sed -i -e '$a\' latest_code_coverage_comment.md
+          
+            if diff -B CoverageReport.md latest_code_coverage_comment.md > /dev/null; then
+              echo "No changes detected; skipping coverage comment."
+              echo "skip_coverage_comment=true" >> $GITHUB_ENV
+            else
+              echo "Changes detected; proceeding with the coverage comment."
+              diff CoverageReport.md latest_code_coverage_comment.md || true
+              echo "skip_coverage_comment=false" >> $GITHUB_ENV
+            fi
+          else
+            echo "No previous coverage comment found to compare; posting evaluated coverage comment."
+            echo "skip_coverage_comment=false" >> $GITHUB_ENV
+          fi
 
       - name: Upload Coverage Report as PR Comment
+        if: ${{ env.skip_coverage_comment == 'false' }}
         uses: peter-evans/create-or-update-comment@v4
         with:
           issue-number: ${{ github.event.pull_request.number }}

diff --git a/.github/workflows/stats.yml b/.github/workflows/stats.yml
@@ -42,13 +42,65 @@ jobs:
       ENABLE_CACHING: false
       CACHE_DIRECTORY: ~/.bazel_cache
     steps:
+      - name: Find the latest APK & AAB Analysis Comment 
+        uses: actions/github-script@v6
+        id: find_latest_apk_aab_comment
+        with:
+           script: |
+             const comments = await github.paginate(github.rest.issues.listComments, {
+               owner: context.repo.owner,
+               repo: context.repo.repo,
+               issue_number: ${{ matrix.prInfo.number }},
+             });            
+
+             for (let i = comments.length - 1; i >= 0; i--) {
+               if (comments[i].body.includes("# APK & AAB differences analysis")) {
+                 const latestComment = comments[i];
+                 const commentBody = latestComment.body;
+                 const commentDate = latestComment.created_at;
+
+                 require('fs').writeFileSync('latest_aab_comment_body.log', commentBody, 'utf8');
+                 core.setOutput("latest_aab_comment_created_at", commentDate);
+                
+                 return
+               }
+             }
+
+      - name: Track Commits following the latest APK & AAB Analysis Comment
+        uses: actions/github-script@v6
+        id: track_commits
+        with:
+           script: |
+             const commits = await github.paginate(github.rest.pulls.listCommits, {
+               owner: context.repo.owner,
+               repo: context.repo.repo,
+               pull_number: ${{ matrix.prInfo.number }},
+             });
+            
+             const latestCommentDate = "${{ steps.find_latest_apk_aab_comment.outputs.latest_aab_comment_created_at }}";
+             const recentCommits = commits.filter(commit => {
+               const commitDate = new Date(commit.commit.committer.date);
+               return commitDate > new Date("${{ steps.find_latest_apk_aab_comment.outputs.latest_aab_comment_created_at }}");
+             });
+            
+             const recentCommitsCount = recentCommits.length;
+             if(recentCommitsCount > 0 || !latestCommentDate) {
+               core.setOutput("new_commits", "true");
+             } else {
+               console.log("No new commits since the last APK & AAB report; Skipping redundant analysis.");
+               core.setOutput("new_commits", "false");
+             }
+          
       - name: Compute PR head owner/repo reference
+        if: ${{ steps.track_commits.outputs.new_commits == 'true' }}
         env:
           PR_HEAD_REPO: ${{ matrix.prInfo.headRepository.name }}
           PR_HEAD_REPO_OWNER: ${{ matrix.prInfo.headRepositoryOwner.login }}
         run: |
           echo "PR_HEAD=$PR_HEAD_REPO_OWNER/$PR_HEAD_REPO" >> "$GITHUB_ENV"
+         
       - name: Print PR information for this run
+        if: ${{ steps.track_commits.outputs.new_commits == 'true' }}
         env:
           PR_BASE_REF_NAME: ${{ matrix.prInfo.baseRefName }}
           PR_HEAD_REF_NAME: ${{ matrix.prInfo.headRefName }}
@@ -57,11 +109,13 @@ jobs:
           echo "PR $PR_NUMBER is merging into $PR_BASE_REF_NAME from https://github.com/$PR_HEAD branch $PR_HEAD_REF_NAME."
 
       - name: Set up JDK 11
+        if: ${{ steps.track_commits.outputs.new_commits == 'true' }}
         uses: actions/setup-java@v1
         with:
           java-version: 11
 
       - name: Set up Bazel
+        if: ${{ steps.track_commits.outputs.new_commits == 'true' }}
         uses: abhinavsingh/setup-bazel@v3
         with:
           version: 6.5.0
@@ -73,6 +127,7 @@ jobs:
       # benefit from incremental build performance (assuming that actions/cache aggressively removes
       # older caches due to the 5GB cache limit size & Bazel's large cache size).
       - uses: actions/cache@v2
+        if: ${{ steps.track_commits.outputs.new_commits == 'true' }}
         id: cache
         with:
           path: ${{ env.CACHE_DIRECTORY }}
@@ -87,6 +142,7 @@ jobs:
       # few slower CI actions around the time cache is detected to be too large, but it should
       # incrementally improve thereafter.
       - name: Ensure cache size
+        if: ${{ steps.track_commits.outputs.new_commits == 'true' }}
         env:
           BAZEL_CACHE_DIR: ${{ env.CACHE_DIRECTORY }}
         run: |
@@ -105,6 +161,7 @@ jobs:
           fi
 
       - name: Configure Bazel to use a local cache
+        if: ${{ steps.track_commits.outputs.new_commits == 'true' }}
         env:
           BAZEL_CACHE_DIR: ${{ env.CACHE_DIRECTORY }}
         run: |
@@ -117,19 +174,23 @@ jobs:
       # run from the latest develop rather than the base branch (which might be different for
       # chained PRs).
       - name: Check out develop repository
+        if: ${{ steps.track_commits.outputs.new_commits == 'true' }}
         uses: actions/checkout@v4
         with:
           path: develop
 
       - name: Set up build environment
+        if: ${{ steps.track_commits.outputs.new_commits == 'true' }}
         uses: ./develop/.github/actions/set-up-android-bazel-build-environment
 
       - name: Check Bazel environment
+        if: ${{ steps.track_commits.outputs.new_commits == 'true' }}
         run: |
           cd develop
           bazel info
 
       - name: Check out base repository and branch
+        if: ${{ steps.track_commits.outputs.new_commits == 'true' }}
         env:
           PR_BASE_REF_NAME: ${{ matrix.prInfo.baseRefName }}
         uses: actions/checkout@v4
@@ -139,6 +200,7 @@ jobs:
           path: base
 
       - name: Check out head repository and branch
+        if: ${{ steps.track_commits.outputs.new_commits == 'true' }}
         env:
           PR_HEAD_REF_NAME: ${{ matrix.prInfo.headRefName }}
         uses: actions/checkout@v4
@@ -152,6 +214,7 @@ jobs:
       # up being active (due to multiple repositories being used) and this can quickly overwhelm CI
       # worker resources.
       - name: Build Oppia dev, alpha, beta, and GA (feature branch)
+        if: ${{ steps.track_commits.outputs.new_commits == 'true' }}
         run: |
           cd head
           git log -n 1
@@ -163,6 +226,7 @@ jobs:
           bazel shutdown
 
       - name: Build Oppia dev, alpha, beta, and GA (base branch)
+        if: ${{ steps.track_commits.outputs.new_commits == 'true' }}
         run: |
           cd base
           git log -n 1
@@ -174,6 +238,7 @@ jobs:
           bazel shutdown
 
       - name: Run stats analysis tool (develop branch)
+        if: ${{ steps.track_commits.outputs.new_commits == 'true' }}
         run: |
           cd develop
           git log -n 1
@@ -184,66 +249,10 @@ jobs:
             beta $(pwd)/oppia_beta_without_changes.aab $(pwd)/oppia_beta_with_changes.aab \
             ga $(pwd)/oppia_ga_without_changes.aab $(pwd)/oppia_ga_with_changes.aab
 
-      - name: Find CI workflow run for PR
-        id: find-workflow-run
-        uses: actions/github-script@v7
-        continue-on-error: true
-        with:
-          script: |
-            const { owner, repo } = context.repo;
-            const runsResponse = await github.rest.actions.listWorkflowRuns({
-              owner,
-              repo,
-              workflow_id: 'stats.yml',
-            });
-
-            const runs = runsResponse.data.workflow_runs;
-            runs.sort((a, b) => new Date(b.created_at).getTime() - new Date(a.created_at).getTime());
-
-            const run = runs[1];
-            if(!run) {
-              core.setFailed('Could not find a succesful workflow run');
-              return;
-            }
-            console.log(run.id);
-
-            core.setOutput('run-id', run.id);
-
-      - name: Download previous build summary
-        uses: actions/download-artifact@v4
-        with:
-          name: brief_build_summary_${{ matrix.prInfo.number }}
-          path: ./previous_build_logs
-          github-token: ${{ secrets.GITHUB_TOKEN }}
-          run-id: ${{ steps.find-workflow-run.outputs.run-id }}
-        continue-on-error: true # Ignore errors if the file doesn't exist (first run)
-
-      - name: Compare current build summary with the previous one
-        id: build-comparison
-        run: |
-          if [ -f ./develop/brief_build_summary.log ]; then
-            echo "Comparing current and previous build summaries..."
-            if diff ./develop/brief_build_summary.log ./previous_build_logs/brief_build_summary.log > /dev/null; then
-              echo "No changes detected; skipping comment."
-              echo "skip_comment=true" >> $GITHUB_ENV
-            else
-              echo "Changes detected; proceeding with the comment."
-              echo "skip_comment=false" >> $GITHUB_ENV
-            fi
-          else
-            echo "No previous summary found; proceeding with the comment."
-            echo "skip_comment=false" >> $GITHUB_ENV
-          fi
-
-      - name: Upload current build summary for future comparison
-        uses: actions/upload-artifact@v4
-        with:
-          name: brief_build_summary_${{ matrix.prInfo.number }}
-          path: ./develop/brief_build_summary.log
-
       # Reference: https://github.com/peter-evans/create-or-update-comment#setting-the-comment-body-from-a-file.
       # Also, for multi-line env values, see: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#multiline-strings.
       - name: Extract reports for uploading & commenting
+        if: ${{ steps.track_commits.outputs.new_commits == 'true' }}
         env:
           PR_NUMBER: ${{ matrix.prInfo.number }}
         id: compute-comment-body
@@ -260,7 +269,7 @@ jobs:
           cp "$GITHUB_WORKSPACE/develop/full_build_summary.log" "$FULL_BUILD_SUMMARY_FILE_PATH"
 
       - name: Add build stats summary comment
-        if: ${{ env.skip_comment == 'false' }}
+        if: ${{ steps.track_commits.outputs.new_commits == 'true' }}
         env:
           PR_NUMBER: ${{ matrix.prInfo.number }}
         uses: peter-evans/create-or-update-comment@v1
@@ -269,6 +278,7 @@ jobs:
           body: ${{ steps.compute-comment-body.outputs.comment_body }}
 
       - uses: actions/upload-artifact@v4
+        if: ${{ steps.track_commits.outputs.new_commits == 'true' }}
         with:
           name: ${{ env.FULL_BUILD_SUMMARY_FILE_NAME }}
           path: ${{ env.FULL_BUILD_SUMMARY_FILE_PATH }}