Skip to content

Commit a4f617c

Browse files
committed
feat: Add check runs status before merging PR
This commit introduces a check for the status of check runs before merging a pull request. The PR will only be merged if all required checks have passed. If any checks have failed or are still pending, a comment will be posted on the PR with a table of the failed or pending checks and the merge will be aborted. This change ensures that only PRs with successful checks are merged. Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>
1 parent ebb9516 commit a4f617c

File tree

3 files changed

+124
-1
lines changed

3 files changed

+124
-1
lines changed

boussole/boussole.py

100644100755
Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
from .messages import ( # isort:skip
2020
APPROVED_TEMPLATE,
21+
CHECKS_NOT_PASSED,
2122
COMMENTS_FETCH_ERROR,
2223
HELP_TEXT,
2324
INSUFFICIENT_PERMISSIONS,
@@ -204,6 +205,48 @@ def _get_pr_status(self, number: int) -> RequestResponse:
204205
self._pr_status = self.api.get(endpoint)
205206
return self._pr_status
206207

208+
def _check_runs_status(self) -> Tuple[bool, List[Dict]]:
209+
"""
210+
Checks if all check runs are successful.
211+
212+
Returns a tuple of (all_success, failed_checks).
213+
"""
214+
endpoint = f"commits/{self._get_pr_status(self.pr_num).json()['head']['sha']}/check-runs"
215+
headers = {"Accept": "application/vnd.github.v3+json"}
216+
headers.update(self.api.headers)
217+
218+
response = self.api.get(endpoint)
219+
if response.status_code != 200:
220+
return False, []
221+
222+
check_runs = response.json()["check_runs"]
223+
224+
failed_checks = [
225+
{
226+
"name": check["name"],
227+
"status": check["status"],
228+
"conclusion": check["conclusion"],
229+
"html_url": check["html_url"],
230+
}
231+
for check in check_runs
232+
if check["status"] == "completed"
233+
and check["conclusion"] not in ["success", "skipped"]
234+
]
235+
236+
pending_checks = [
237+
{
238+
"name": check["name"],
239+
"status": check["status"],
240+
"html_url": check.get("html_url", ""),
241+
}
242+
for check in check_runs
243+
if check["status"] != "completed"
244+
and not check["name"].endswith(" / boussole")
245+
]
246+
247+
all_success = not (failed_checks or pending_checks)
248+
return all_success, failed_checks + pending_checks
249+
207250
def check_status(self, num: int, status: str) -> bool:
208251
pr_status = self._get_pr_status(num)
209252
if pr_status.status_code != 200:
@@ -351,7 +394,7 @@ def lgtm(self, send_comment: bool = True) -> int:
351394

352395
def merge_pr(self) -> bool:
353396
"""
354-
Merges the PR if it has enough LGTM approvals and performs cherry-picks.
397+
Merges the PR if it has enough LGTM approvals and all checks are green.
355398
"""
356399
# Check if the user has sufficient permissions to merge
357400
permission, is_valid = self._check_membership(self.comment_sender)
@@ -365,6 +408,27 @@ def merge_pr(self) -> bool:
365408
print(msg, file=sys.stderr)
366409
sys.exit(1)
367410

411+
# Check if all check runs are green
412+
all_checks_passed, failed_checks = self._check_runs_status()
413+
if not all_checks_passed:
414+
status_table = "\n| Check Name | Status |\n|------------|--------|\n"
415+
for check in failed_checks:
416+
status = check.get("conclusion", check["status"])
417+
check_name = check["name"]
418+
# Add the URL link if available in the check data
419+
if "html_url" in check:
420+
check_name = f"[{check['name']}]({check['html_url']})"
421+
status_table += f"| {check_name} | `{status}` |\n"
422+
423+
msg = (
424+
"⚠️ Cannot merge PR: Some checks are not passing.\n\n"
425+
f"{status_table}\n\n"
426+
"Please wait for all checks to pass before merging."
427+
)
428+
self._post_comment(CHECKS_NOT_PASSED.format(status_table=status_table))
429+
print(msg, file=sys.stderr)
430+
sys.exit(1)
431+
368432
# Fetch LGTM votes and check if the threshold is met
369433
valid_votes, lgtm_users = self._fetch_and_validate_lgtm_votes()
370434

boussole/messages.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,3 +214,11 @@
214214
appreciated.
215215
216216
Thank you for your help! 🙌"""
217+
218+
219+
CHECKS_NOT_PASSED = """⚠️ Cannot merge PR: Some required checks haven't completed successfully.
220+
221+
{status_table}
222+
223+
🔍 Please ensure all checks pass before merging.
224+
💡 Tip: Review the failing checks above and address any issues."""

boussole/test_boussole.py

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,41 @@ def test_check_membership_invalid_response(pr_handler, mock_api):
147147
assert is_valid is False
148148

149149

150+
def test_merge_pr_no_all_checks_succeed(capsys, pr_handler, mock_api):
151+
all_checks = MyFakeResponse(
152+
200,
153+
{
154+
"check_runs": [
155+
{
156+
"name": "check-1",
157+
"status": "completed",
158+
"conclusion": "success",
159+
"html_url": "http://test.url",
160+
},
161+
{
162+
"name": "check-2",
163+
"status": "completed",
164+
"conclusion": "failure",
165+
"html_url": "http://test.url",
166+
},
167+
],
168+
},
169+
)
170+
171+
mock_responses = [
172+
MyFakeResponse(200, {"permission": "admin"}),
173+
MyFakeResponse(200, {"head": {"sha": "abc123"}}),
174+
all_checks,
175+
]
176+
177+
mock_api.get.side_effect = mock_responses
178+
179+
with pytest.raises(SystemExit) as exc_info:
180+
pr_handler.merge_pr()
181+
assert "Cannot merge PR" in capsys.err
182+
assert exc_info.value.code == 1
183+
184+
150185
def test_merge_pr_success(pr_handler, mock_api):
151186
all_comments = MyFakeResponse(
152187
200,
@@ -156,8 +191,24 @@ def test_merge_pr_success(pr_handler, mock_api):
156191
],
157192
)
158193

194+
all_checks = MyFakeResponse(
195+
200,
196+
{
197+
"check_runs": [
198+
{
199+
"name": "check-1",
200+
"status": "completed",
201+
"conclusion": "success",
202+
"html_url": "http://test.url",
203+
},
204+
],
205+
},
206+
)
207+
159208
mock_responses = [
160209
MyFakeResponse(200, {"permission": "admin"}),
210+
MyFakeResponse(200, {"head": {"sha": "abc123"}}),
211+
all_checks,
161212
all_comments,
162213
MyFakeResponse(200, {"permission": "write"}), # reviewer1
163214
MyFakeResponse(200, {"permission": "write"}), # reviewer2

0 commit comments

Comments
 (0)