Skip to content

Commit

Permalink
[ISV-4528] Automatically merge approved PRs (#746)
Browse files Browse the repository at this point in the history
* [ISV-4528] Automatically merge approved PRs

---------

Signed-off-by: Maurizio Porrato <mporrato@redhat.com>
  • Loading branch information
mporrato authored Nov 6, 2024
1 parent ca83aa3 commit 82742c3
Show file tree
Hide file tree
Showing 2 changed files with 135 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,34 @@ def github_repo_org(self) -> str:
"""
return urllib.parse.urlparse(self.pull_request_url).path.split("/")[1]

@property
def github_repo_name(self) -> str:
"""
The name of the github repo the pull request belongs to
Returns:
str: the github repo name in the form "organization_name/repo_name"
"""
return "/".join(
urllib.parse.urlparse(self.pull_request_url).path.split("/")[1:3]
)

@property
def pr_labels(self) -> set[str]:
"""
The set of labels applied to the pull request
Returns:
set[str]: the labels applied to the pull request
"""
github_auth = Auth.Token(os.environ.get("GITHUB_TOKEN") or "")
github = Github(auth=github_auth)
pr_no = int(urllib.parse.urlparse(self.pull_request_url).path.split("/")[-1])
return {
x.name
for x in github.get_repo(self.github_repo_name).get_pull(pr_no).get_labels()
}

def check_permissions(self) -> bool:
"""
Check if the pull request owner has permissions to submit a PR for the operator
Expand Down Expand Up @@ -271,20 +299,27 @@ def check_permission_for_community(self) -> bool:
f"{self.operator} does not have any reviewers in the base repository "
"or is brand new."
)
if self.pr_owner not in self.reviewers:

if self.pr_owner in self.reviewers:
LOGGER.info(
"Pull request owner %s is not in the list of reviewers %s",
"Pull request owner %s can submit PR for operator %s",
self.pr_owner,
self.reviewers,
self.operator.operator_name,
)
self.request_review_from_owners()
return False
return True

LOGGER.info("Checking if the pull request is approved by a reviewer")
if "approved" in self.pr_labels:
LOGGER.info("Pull request is approved by a reviewer")
return True

LOGGER.info(
"Pull request owner %s can submit PR for operator %s",
"Pull request owner %s is not in the list of reviewers %s",
self.pr_owner,
self.operator.operator_name,
self.reviewers,
)
return True
self.request_review_from_owners()
return False

def request_review_from_maintainers(self) -> None:
"""
Expand Down Expand Up @@ -312,11 +347,12 @@ def request_review_from_owners(self) -> None:
"""
reviewers_with_at = ", ".join(map(lambda x: f"@{x}", self.reviewers))
comment_text = (
"Author of the PR is not listed as one of the reviewers in ci.yaml.\n"
"Please review the PR and approve it with \`/lgtm\` comment.\n" # pylint: disable=anomalous-backslash-in-string
f"{reviewers_with_at} \n\n"
"Consider adding author of the PR to the ci.yaml file if you want automated "
"approval for a followup submissions."
"The author of the PR is not listed as one of the reviewers in ci.yaml.\n"
f"{reviewers_with_at}: please review the PR and approve it with an "
"`/approve` comment.\n\n"
"Consider adding the author of the PR to the list of reviewers in "
"the ci.yaml file if you want automated merge without explicit "
"approval."
)

run_command(
Expand Down
102 changes: 86 additions & 16 deletions operator-pipeline-images/tests/entrypoints/test_check_permissions.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from pathlib import Path
from typing import Any
from typing import Any, Optional
from unittest import mock
from unittest.mock import MagicMock, call, patch

Expand Down Expand Up @@ -105,6 +105,33 @@ def test_OperatorReview_github_repo_org(
assert review_community.github_repo_org == "my-org"


def test_OperatorReview_github_repo_name(
review_community: check_permissions.OperatorReview,
) -> None:
assert review_community.github_repo_name == "my-org/repo-123"


@patch("operatorcert.entrypoints.check_permissions.Github")
@patch("operatorcert.entrypoints.check_permissions.Auth.Token")
def test_OperatorReview_pr_labels(
mock_token: MagicMock,
mock_github: MagicMock,
review_community: check_permissions.OperatorReview,
) -> None:
repo = MagicMock()
pull = MagicMock()

def _mock_label(name: str) -> MagicMock:
m = MagicMock()
m.name = name
return m

pull.get_labels.return_value = [_mock_label("foo"), _mock_label("bar")]
repo.get_pull.return_value = pull
mock_github.return_value.get_repo.return_value = repo
assert review_community.pr_labels == {"foo", "bar"}


@pytest.mark.parametrize(
"is_org_member, is_partner, permission_partner, permission_community, permission_partner_called, permission_community_called, expected_result",
[
Expand Down Expand Up @@ -225,31 +252,73 @@ def test_OperatorReview_check_permission_for_partner(
review_partner.check_permission_for_partner()


@pytest.mark.parametrize(
["reviewers", "labels", "owners_review", "result"],
[
pytest.param(
[],
set(),
False,
check_permissions.MaintainersReviewNeeded,
id="No reviewers",
),
pytest.param(
["owner", "foo"],
set(),
False,
True,
id="author is reviewer",
),
pytest.param(
["foo", "bar"],
set(),
True,
False,
id="author is not reviewer",
),
pytest.param(
["bar", "baz"],
{"approved"},
False,
True,
id="author is not reviewer, PR approved",
),
],
)
@patch(
"operatorcert.entrypoints.check_permissions.OperatorReview.request_review_from_owners"
)
@patch(
"operatorcert.entrypoints.check_permissions.OperatorReview.reviewers",
new_callable=mock.PropertyMock,
)
@patch(
"operatorcert.entrypoints.check_permissions.OperatorReview.pr_labels",
new_callable=mock.PropertyMock,
)
def test_OperatorReview_check_permission_for_community(
mock_labels: MagicMock,
mock_reviewers: MagicMock,
mock_review_from_owners: MagicMock,
review_community: check_permissions.OperatorReview,
reviewers: list[str],
labels: set[str],
owners_review: bool,
result: bool | type,
) -> None:
mock_reviewers.return_value = []
with pytest.raises(check_permissions.MaintainersReviewNeeded):
review_community.check_permission_for_community()

mock_reviewers.return_value = ["user1", "user2"]
mock_reviewers.return_value = reviewers
mock_labels.return_value = labels

assert review_community.check_permission_for_community() == False
mock_review_from_owners.assert_called_once()
if isinstance(result, bool):
assert review_community.check_permission_for_community() == result
else:
with pytest.raises(result):
review_community.check_permission_for_community()

mock_review_from_owners.reset_mock()
mock_reviewers.return_value = ["owner"]
assert review_community.check_permission_for_community() == True
mock_review_from_owners.assert_not_called()
if owners_review:
mock_review_from_owners.assert_called_once()
else:
mock_review_from_owners.assert_not_called()


@patch("operatorcert.entrypoints.check_permissions.run_command")
Expand Down Expand Up @@ -283,10 +352,11 @@ def test_OperatorReview_request_review_from_owners(
"comment",
review_community.pull_request_url,
"--body",
"Author of the PR is not listed as one of the reviewers in ci.yaml.\n"
"Please review the PR and approve it with \\`/lgtm\\` comment.\n"
"@user1, @user2 \n\nConsider adding author of the PR to the ci.yaml "
"file if you want automated approval for a followup submissions.",
"The author of the PR is not listed as one of the reviewers in ci.yaml.\n"
"@user1, @user2: please review the PR and approve it with an `/approve` comment.\n\n"
"Consider adding the author of the PR to the list of reviewers in "
"the ci.yaml file if you want automated merge without explicit "
"approval.",
]
)

Expand Down

0 comments on commit 82742c3

Please sign in to comment.