From 41f4fd5ec51c2359de11f4d9868fd85b73bb3aa0 Mon Sep 17 00:00:00 2001 From: Simon Steinbeiss Date: Thu, 21 Dec 2023 01:38:21 +0100 Subject: [PATCH 1/2] pr-review-queue: Improve CI status matching Previously, we only looked at check_runs status when the combined status was empty. Now we look at the native GitHub actions check_runs in any case, and even avoid an API call if check runs have failed. --- pr_review_queue.py | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/pr_review_queue.py b/pr_review_queue.py index 0c903d0..e6aaa0a 100755 --- a/pr_review_queue.py +++ b/pr_review_queue.py @@ -76,22 +76,37 @@ def get_commit_status(github_api, repo, pull_request_details): Check whether the HEAD commit has passed the CI tests """ head = pull_request_details["head"] + combined_status = "failure" # failure by default + # Check GitHub run status + check_run_status, state = get_check_runs(github_api, repo, head["sha"]) + # Exit early if there are failed check runs + if check_run_status == "failure": + return combined_status, state + + # Check external CI status status = github_api.repos.get_combined_status_for_ref(repo=repo,ref=head["sha"]) - if status.state == "success": + + if (status.state == "success" and + check_run_status == "success"): state = "🟢" + combined_status = "success" elif status.state == "failure": state = "🔴" + # For simplicity, we consider "pending" as "failure" unless there are check runs elif status.state == "pending": state = "🟠" - # Check if the state is not really 'pending' but if there is none + # Check if the state is not really 'pending' but if there is actually none single_status = github_api.repos.list_commit_statuses_for_ref(repo=repo,ref=head["sha"]) if single_status == []: - status.state, state = get_check_runs(github_api, repo, head["sha"]) + # The combined_status should still be a success if all check runs have passed + if check_run_status == "success": + state = "🟢" + combined_status = "success" else: state = status.state - return status.state, state + return combined_status, state def get_archived_repos(github_api, org): From d68b7727f39568b31234a47b0f13d57dbfa957b5 Mon Sep 17 00:00:00 2001 From: Simon Steinbeiss Date: Thu, 21 Dec 2023 01:40:19 +0100 Subject: [PATCH 2/2] pr-review-queue: Improve check_runs code The API actually returns the expected amount of checks, so rely on that vs. on len(runs). Also, only return the short status so matching becomes more elegant (plus we don't use that information anywhere). --- pr_review_queue.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/pr_review_queue.py b/pr_review_queue.py index e6aaa0a..e38da41 100755 --- a/pr_review_queue.py +++ b/pr_review_queue.py @@ -52,20 +52,21 @@ def get_check_runs(github_api, repo, head): """ check_runs = github_api.checks.list_for_ref(repo=repo,ref=head, per_page=100) runs = check_runs["check_runs"] + total_count = check_runs["total_count"] successful_runs = 0 for run in runs: if run['status'] == "completed" and run['conclusion'] == "success": successful_runs += 1 - if successful_runs == len(runs): - status = f"success ({successful_runs}/{len(runs)})" + if successful_runs == total_count: + status = "success" state = "🟢" - elif successful_runs < len(runs): - status = f"failure ({successful_runs}/{len(runs)})" + elif successful_runs < total_count: + status = "failure" state = "🔴" else: - print(f"Warning: something is terribly wrong: successful runs ({successful_runs}) should never be more than total runs ({len(runs)}).") + print(f"Warning: something is terribly wrong: successful runs ({successful_runs}) should never be more than total runs ({total_count}).") sys.exit(1) return status, state