[ci] Check more events before pinging reviewers

tests/python/unittest/test_ci.py

@@ -206,19 +206,28 @@ def run(pr, check):
 
         assert check in proc.stdout
 
+    def all_time_keys(time):
+        return {
+            "updatedAt": time,
+            "lastEditedAt": time,
+            "createdAt": time,
+            "publishedAt": time,
+        }
+
     run(
         {
             "isDraft": True,
+            "number": 2,
         },
-        "Checking 0 PRs",
+        "Checking 0 of 1 fetched",
     )
 
     run(
         {
             "isDraft": False,
             "number": 2,
         },
-        "Checking 0 PRs",
+        "Checking 0 of 1 fetched",
     )
 
     run(
@@ -229,7 +238,7 @@ def run(pr, check):
             "isDraft": False,
             "author": {"login": "user"},
             "reviews": {"nodes": []},
-            "publishedAt": "2022-01-18T17:54:19Z",
+            **all_time_keys("2022-01-18T17:54:19Z"),
             "comments": {"nodes": []},
         },
         "Pinging reviewers ['someone'] on https://github.com/apache/tvm/pull/123",
@@ -244,14 +253,14 @@ def run(pr, check):
             "isDraft": False,
             "author": {"login": "user2"},
             "reviews": {"nodes": []},
-            "publishedAt": "2022-01-18T17:54:19Z",
+            **all_time_keys("2022-01-18T17:54:19Z"),
             "comments": {
                 "nodes": [
-                    {"updatedAt": "2022-01-19T17:54:19Z", "bodyText": "abc"},
+                    {**all_time_keys("2022-01-19T17:54:19Z"), "bodyText": "abc"},
                 ]
             },
         },
-        "Checking 0 PRs",
+        "Checking 0 of 1 fetched",
     )
 
     # Old comment, ping
@@ -263,10 +272,13 @@ def run(pr, check):
             "isDraft": False,
             "author": {"login": "user"},
             "reviews": {"nodes": []},
-            "publishedAt": "2022-01-18T17:54:19Z",
+            **all_time_keys("2022-01-18T17:54:19Z"),
             "comments": {
                 "nodes": [
-                    {"updatedAt": "2022-01-19T17:54:19Z", "bodyText": "abc"},
+                    {
+                        **all_time_keys("2022-01-18T17:54:19Z"),
+                        "bodyText": "abc",
+                    },
                 ]
             },
         },
@@ -282,10 +294,10 @@ def run(pr, check):
             "isDraft": False,
             "author": {"login": "user"},
             "reviews": {"nodes": []},
-            "publishedAt": "2022-01-18T17:54:19Z",
+            **all_time_keys("2022-01-18T17:54:19Z"),
             "comments": {
                 "nodes": [
-                    {"updatedAt": "2022-01-27T17:54:19Z", "bodyText": "abc"},
+                    {**all_time_keys("2022-01-27T17:54:19Z"), "bodyText": "abc"},
                 ]
             },
         },

tests/scripts/ping_reviewers.py

@@ -32,6 +32,7 @@ def prs_query(user: str, repo: str, cursor: str = None):
     after = ""
     if cursor is not None:
         after = f', before:"{cursor}"'
+    time_keys = "createdAt\nupdatedAt\nlastEditedAt\npublishedAt"
     return f"""
         {{
     repository(name: "{repo}", owner: "{user}") {{
@@ -43,27 +44,29 @@ def prs_query(user: str, repo: str, cursor: str = None):
             number
             url
             body
+            {time_keys}
             isDraft
             author {{
                 login
             }}
             reviews(last:100) {{
                 nodes {{
+                    {time_keys}
+                    bodyText
                     author {{ login }}
                     comments(last:100) {{
                         nodes {{
-                            updatedAt
+                            {time_keys}
                             bodyText
                         }}
                     }}
                 }}
             }}
-            publishedAt
             comments(last:100) {{
                 nodes {{
                     authorAssociation
                     bodyText
-                    updatedAt
+                    {time_keys}
                     author {{
                         login
                     }}
@@ -92,26 +95,42 @@ def find_reviewers(body: str) -> List[str]:
 
 
 def check_pr(pr, wait_time, now):
-    published_at = datetime.datetime.strptime(pr["publishedAt"], GIT_DATE_FORMAT)
-    last_action = published_at
+    last_action = None
+
+    def update_last(new_time, description):
+        if isinstance(new_time, str):
+            new_time = datetime.datetime.strptime(new_time, GIT_DATE_FORMAT)
+        if new_time is None:
+            print(f"  time not found: {description}")
+            return
+        nonlocal last_action
+        if last_action is None or new_time > last_action[0]:
+            last_action = (new_time, description)
+
+    def check_obj(obj, name):
+        update_last(obj["publishedAt"], f"{name} publishedAt: {obj}")
+        update_last(obj["updatedAt"], f"{name} updatedAt: {obj}")
+        update_last(obj["lastEditedAt"], f"{name} lastEditedAt: {obj}")
+        update_last(obj["createdAt"], f"{name} lastEditedAt: {obj}")
+
+    check_obj(pr, "pr")
 
     # GitHub counts comments left as part of a review separately than standalone
     # comments
     reviews = pr["reviews"]["nodes"]
     review_comments = []
     for review in reviews:
         review_comments += review["comments"]["nodes"]
+        check_obj(review, "review")
 
     # Collate all comments
     comments = pr["comments"]["nodes"] + review_comments
 
     # Find the last date of any comment
     for comment in comments:
-        commented_at = datetime.datetime.strptime(comment["updatedAt"], GIT_DATE_FORMAT)
-        if commented_at > last_action:
-            last_action = commented_at
+        check_obj(comment, "comment")
 
-    time_since_last_action = now - last_action
+    time_since_last_action = now - last_action[0]
 
     # Find reviewers in the PR's body
     pr_body_reviewers = find_reviewers(pr["body"])
@@ -128,17 +147,19 @@ def check_pr(pr, wait_time, now):
 
     if time_since_last_action > wait_time:
         print(
-            "    Pinging reviewers",
+            "  Pinging reviewers",
             reviewers,
             "on",
             pr["url"],
             "since it has been",
             time_since_last_action,
-            "since anything happened on that PR",
+            f"since anything happened on that PR (last action: {last_action[1]})",
         )
         return reviewers
     else:
-        print(f"Not pinging PR {pr['number']}")
+        print(
+            f"  Not pinging PR {pr['number']} since it has been only {time_since_last_action} since the last action: {last_action[1]}"
+        )
 
     return None
 
@@ -206,18 +227,27 @@ def ping_reviewers(pr, reviewers):
         prs = r["data"]["repository"]["pullRequests"]["nodes"]
 
         # Don't look at draft PRs at all
-        prs = [pr for pr in prs if not pr["isDraft"]]
-
-        # Don't look at super old PRs
-        prs = [pr for pr in prs if pr["number"] > cutoff_pr_number]
-
-        # [slow rollout]
-        prs = [pr for pr in prs if pr["author"]["login"] in author_allowlist]
-
-        print(f"Checking {len(prs)} PRs: {[pr['number'] for pr in prs]}")
+        prs_to_check = []
+        for pr in prs:
+            if pr["isDraft"]:
+                print(f"Skipping #{pr['number']} since it's a draft")
+            elif pr["number"] <= cutoff_pr_number:
+                print(
+                    f"Skipping #{pr['number']} since it's too old ({pr['number']} <= {cutoff_pr_number})"
+                )
+            elif pr["author"]["login"] not in author_allowlist:
+                # [slow rollout]
+                print(
+                    f"Skipping #{pr['number']} since author {pr['author']['login']} is not in allowlist: {author_allowlist}"
+                )
+            else:
+                print(f"Checking #{pr['number']}, {author_allowlist}")
+                prs_to_check.append(pr)
+
+        print(f"Summary: Checking {len(prs_to_check)} of {len(prs)} fetched")
 
         # Ping reviewers on each PR in the response if necessary
-        for pr in prs:
+        for pr in prs_to_check:
             print("Checking", pr["url"])
             reviewers = check_pr(pr, wait_time, now)
             if reviewers is not None and not args.dry_run: