diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index 0d2097c41f..e874642a74 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -1136,14 +1136,18 @@ def not_eligible(msg): if not test_report_found: res = not_eligible(msg_tmpl % "(no test reports found)") - # check for requested changes and approved review + # check for approved review approved_review_by = [] - changes_requested_by = [] for review in pr_data['reviews']: if review['state'] == 'APPROVED': approved_review_by.append(review['user']['login']) + + # check for requested changes + changes_requested_by = [] + for review in pr_data['reviews']: if review['state'] == 'CHANGES_REQUESTED': - changes_requested_by.append(review['user']['login']) + if review['user']['login'] not in approved_review_by: + changes_requested_by.append(review['user']['login']) msg_tmpl = "* no pending change requests: %s" if changes_requested_by: diff --git a/test/framework/github.py b/test/framework/github.py index a1b9b348ab..9dadbfb25f 100644 --- a/test/framework/github.py +++ b/test/framework/github.py @@ -721,9 +721,17 @@ def run_check(expected_result=False): expected_warning += " => not eligible for merging!" run_check() - pr_data['reviews'] = [{'state': 'APPROVED', 'user': {'login': 'boegel'}}] - expected_stdout += "* no pending change requests: OK\n" - expected_stdout += "* approved review: OK (by boegel)\n" + # if PR is approved by a different user that requested changes and that request has not been dismissed, + # the PR is still not mergeable + pr_data['reviews'].append({'state': 'APPROVED', 'user': {'login': 'not_boegel'}}) + expected_stdout_saved = expected_stdout + expected_stdout += "* approved review: OK (by not_boegel)\n" + run_check() + + # if the user that requested changes approves the PR, it's mergeable + pr_data['reviews'].append({'state': 'APPROVED', 'user': {'login': 'boegel'}}) + expected_stdout = expected_stdout_saved + "* no pending change requests: OK\n" + expected_stdout += "* approved review: OK (by not_boegel, boegel)\n" expected_warning = '' run_check()