From fdbac55fc50c9fce4b429c9accf974fc8da4295b Mon Sep 17 00:00:00 2001
From: Kevin Schoedel <67607049+kpschoedel@users.noreply.github.com>
Date: Thu, 21 Oct 2021 09:14:50 -0400
Subject: [PATCH] Size reports: Don't comment on outdated builds (#10565)

#### Problem

`gh_report.py` may send comments on builds that are already outdated,
and may do so out of chronological order, which clutters PRs and may
eclipse the actually relevant report.

#### Change overview

Before commenting, check that the report is for the latest commit.

#### Testing

Offline run identified and correctly handled such a case.

    scripts/tools/memory/gh_report.py --db=/tmp/test.db --log-level=debug \
        --pr --github-repo project-chip/connectedhomeip \
        --github-limit-artifacts=500 --github-limit-artifact-pages=100 \
        --github-keep --github-comment --github-dryrun-comment
---
 scripts/tools/memory/gh_report.py | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/scripts/tools/memory/gh_report.py b/scripts/tools/memory/gh_report.py
index b2b23d8f7b8476..e4ea4eabb3d3b6 100755
--- a/scripts/tools/memory/gh_report.py
+++ b/scripts/tools/memory/gh_report.py
@@ -367,6 +367,11 @@ def gh_get_comments_for_pr(gh: ghapi.core.GhApi, pr: int):
         ghapi.all.paged(gh.issues.list_comments, pr))
 
 
+def gh_get_commits_for_pr(gh: ghapi.core.GhApi, pr: int):
+    return itertools.chain.from_iterable(
+        ghapi.all.paged(gh.pulls.list_commits, pr))
+
+
 def percent_change(a: int, b: int) -> float:
     if a == 0:
         return 0.0 if b == 0 else float('inf')
@@ -455,6 +460,20 @@ def gh_send_change_report(db: SizeDatabase, df: pd.DataFrame,
     if not db.gh:
         return False
     pr = df.attrs['pr']
+
+    # Check the most recent commit on the PR, so that we don't comment on
+    # builds that are already outdated.
+    commit = df.attrs['commit']
+    commits = sorted(gh_get_commits_for_pr(db.gh, pr),
+                     key=lambda c: c.commit.committer.date,
+                     reverse=True)
+    if commits and commit != commits[0].sha:
+        logging.debug('SCS: PR #%s: not commenting for stale %s; newest is %s',
+                      pr, commit, commits[0].sha)
+        return False
+
+    # Check for an existing size report comment. If one exists, we'll add
+    # the new report to it.
     title = df.attrs['title']
     existing_comment_id = 0
     for comment in gh_get_comments_for_pr(db.gh, pr):
@@ -546,7 +565,6 @@ def report_matching_commits(db: SizeDatabase) -> Dict[str, pd.DataFrame]:
 
         if (pr and comment_enabled
                 and (comment_limit == 0 or comment_limit > comment_count)):
-            comment_count += 1
             if gh_send_change_report(db, df, tdf):
                 # Mark the originating builds, and remove the originating
                 # artifacts, so that they don't generate duplicate report
@@ -556,6 +574,7 @@ def report_matching_commits(db: SizeDatabase) -> Dict[str, pd.DataFrame]:
                     for artifact_id in df.attrs['artifacts']:
                         logging.info('RMC: deleting artifact %d', artifact_id)
                         db.delete_artifact(artifact_id)
+                comment_count += 1
     return dfs