diff --git a/tests/python/unittest/test_ci.py b/tests/python/unittest/test_ci.py index f5183d79b768..4e0690cf0933 100644 --- a/tests/python/unittest/test_ci.py +++ b/tests/python/unittest/test_ci.py @@ -206,11 +206,20 @@ 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( @@ -218,7 +227,7 @@ def run(pr, check): "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"}, ] }, }, diff --git a/tests/scripts/ping_reviewers.py b/tests/scripts/ping_reviewers.py index 50e67824888c..5c881594cf30 100755 --- a/tests/scripts/ping_reviewers.py +++ b/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,8 +95,25 @@ 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 @@ -101,17 +121,16 @@ def check_pr(pr, wait_time, now): 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: