Check if the dist-git branch exists before creating it
The scm_request_processor didn't checked if the branch already exists in dist-git before processing the ticket further. Let's fix this by adding the check. Signed-off-by: Michal Konečný <mkonecny@redhat.com>
This commit is contained in:
Michal Konečný
parent
247f93f9bc
commit
d580f4b498
4 changed files with 197 additions and 1 deletions
|
|
@ -1530,6 +1530,7 @@ class TestCreateNewBranch:
|
|||
}
|
||||
|
||||
mock_valid_epel_package.return_value = False
|
||||
self.toddler.dist_git.get_branches.return_value = []
|
||||
|
||||
self.toddler.create_new_branch(issue, json)
|
||||
|
||||
|
|
@ -1538,6 +1539,8 @@ class TestCreateNewBranch:
|
|||
namespace, repo
|
||||
)
|
||||
|
||||
self.toddler.dist_git.get_branches.assert_called_with(namespace, repo)
|
||||
|
||||
mock_valid_epel_package.assert_called_with(repo, branch)
|
||||
|
||||
self.toddler.pagure_io.close_issue.assert_called_with(
|
||||
|
|
@ -1547,6 +1550,42 @@ class TestCreateNewBranch:
|
|||
reason="Invalid",
|
||||
)
|
||||
|
||||
def test_create_new_branch_branch_exist_in_distgit(self):
|
||||
"""
|
||||
Assert that ticket will be closed if requester is not a maintainer of package.
|
||||
"""
|
||||
issue = {"id": 100, "user": {"name": "zlopez"}}
|
||||
|
||||
repo = "repo"
|
||||
branch = "rawhide"
|
||||
namespace = "rpms"
|
||||
action = "new_branch"
|
||||
sls = {"rawhide": "2050-06-01"}
|
||||
json = {
|
||||
"repo": repo,
|
||||
"branch": branch,
|
||||
"namespace": namespace,
|
||||
"action": action,
|
||||
"sls": sls,
|
||||
}
|
||||
self.toddler.dist_git.get_branches.return_value = [branch]
|
||||
|
||||
self.toddler.create_new_branch(issue, json)
|
||||
|
||||
# Asserts
|
||||
self.toddler.dist_git.get_project_contributors.assert_called_with(
|
||||
namespace, repo
|
||||
)
|
||||
|
||||
self.toddler.dist_git.get_branches.assert_called_with(namespace, repo)
|
||||
|
||||
self.toddler.pagure_io.close_issue.assert_called_with(
|
||||
100,
|
||||
namespace=scm_request_processor.PROJECT_NAMESPACE,
|
||||
message="Branch '{}' already exists in dist-git".format(branch),
|
||||
reason="Invalid",
|
||||
)
|
||||
|
||||
@patch("toddlers.plugins.scm_request_processor.fedora_account")
|
||||
def test_create_new_branch_requester_is_not_maintainer(self, mock_fedora_account):
|
||||
"""
|
||||
|
|
@ -1571,6 +1610,7 @@ class TestCreateNewBranch:
|
|||
"groups": {"admin": ["group"], "commit": [], "collaborators": []},
|
||||
}
|
||||
mock_fedora_account.user_member_of.return_value = False
|
||||
self.toddler.dist_git.get_branches.return_value = []
|
||||
|
||||
self.toddler.create_new_branch(issue, json)
|
||||
|
||||
|
|
@ -1579,6 +1619,8 @@ class TestCreateNewBranch:
|
|||
namespace, repo
|
||||
)
|
||||
|
||||
self.toddler.dist_git.get_branches.assert_called_with(namespace, repo)
|
||||
|
||||
mock_fedora_account.user_member_of.assert_called_with(
|
||||
mock_fedora_account.get_user_by_username(), "group"
|
||||
)
|
||||
|
|
@ -1619,6 +1661,7 @@ class TestCreateNewBranch:
|
|||
"users": {"admin": [], "commit": [], "collaborators": []},
|
||||
"groups": {"admin": ["group"], "commit": [], "collaborators": []},
|
||||
}
|
||||
self.toddler.dist_git.get_branches.return_value = []
|
||||
self.toddler.dist_git.get_default_branch.return_value = None
|
||||
self.toddler.dist_git._pagure_url = "https://fp.o"
|
||||
mock_fedora_account.user_member_of.return_value = True
|
||||
|
|
@ -1635,6 +1678,8 @@ class TestCreateNewBranch:
|
|||
namespace, repo
|
||||
)
|
||||
|
||||
self.toddler.dist_git.get_branches.assert_called_with(namespace, repo)
|
||||
|
||||
mock_fedora_account.user_member_of.assert_called_with(
|
||||
mock_fedora_account.get_user_by_username(), "group"
|
||||
)
|
||||
|
|
@ -1697,6 +1742,7 @@ class TestCreateNewBranch:
|
|||
"users": {"admin": [], "commit": [], "collaborators": []},
|
||||
"groups": {"admin": ["group"], "commit": [], "collaborators": []},
|
||||
}
|
||||
self.toddler.dist_git.get_branches.return_value = []
|
||||
self.toddler.dist_git.get_default_branch.return_value = default_branch
|
||||
self.toddler.dist_git._pagure_url = "https://fp.o"
|
||||
mock_fedora_account.user_member_of.return_value = True
|
||||
|
|
@ -1717,6 +1763,8 @@ class TestCreateNewBranch:
|
|||
namespace, repo
|
||||
)
|
||||
|
||||
self.toddler.dist_git.get_branches.assert_called_with(namespace, repo)
|
||||
|
||||
mock_fedora_account.user_member_of.assert_called_with(
|
||||
mock_fedora_account.get_user_by_username(), "group"
|
||||
)
|
||||
|
|
@ -1785,6 +1833,7 @@ class TestCreateNewBranch:
|
|||
"users": {"admin": [], "commit": [], "collaborators": []},
|
||||
"groups": {"admin": ["group"], "commit": [], "collaborators": []},
|
||||
}
|
||||
self.toddler.dist_git.get_branches.return_value = []
|
||||
self.toddler.dist_git._pagure_url = "https://fp.o"
|
||||
mock_fedora_account.user_member_of.return_value = True
|
||||
|
||||
|
|
@ -1795,6 +1844,8 @@ class TestCreateNewBranch:
|
|||
namespace, repo
|
||||
)
|
||||
|
||||
self.toddler.dist_git.get_branches.assert_called_with(namespace, repo)
|
||||
|
||||
mock_fedora_account.user_member_of.assert_called_with(
|
||||
mock_fedora_account.get_user_by_username(), "group"
|
||||
)
|
||||
|
|
@ -1855,6 +1906,7 @@ class TestCreateNewBranch:
|
|||
"users": {"admin": [], "commit": [], "collaborators": []},
|
||||
"groups": {"admin": ["group"], "commit": [], "collaborators": []},
|
||||
}
|
||||
self.toddler.dist_git.get_branches.return_value = []
|
||||
self.toddler.dist_git._pagure_url = "https://fp.o"
|
||||
mock_fedora_account.user_member_of.return_value = True
|
||||
|
||||
|
|
@ -1865,6 +1917,8 @@ class TestCreateNewBranch:
|
|||
namespace, repo
|
||||
)
|
||||
|
||||
self.toddler.dist_git.get_branches.assert_called_with(namespace, repo)
|
||||
|
||||
mock_fedora_account.user_member_of.assert_called_with(
|
||||
mock_fedora_account.get_user_by_username(), "group"
|
||||
)
|
||||
|
|
|
|||
|
|
@ -994,6 +994,98 @@ class TestPagureGetProjectContributors:
|
|||
)
|
||||
|
||||
|
||||
class TestPagureGetBranches:
|
||||
"""
|
||||
Test class for `toddlers.pagure.Pagure.get_branches` method.
|
||||
"""
|
||||
|
||||
def setup(self):
|
||||
"""
|
||||
Setup method for the test class.
|
||||
"""
|
||||
config = {
|
||||
"pagure_url": "https://pagure.io",
|
||||
"pagure_api_key": "Very secret key",
|
||||
}
|
||||
self.pagure = pagure.set_pagure(config)
|
||||
self.pagure._requests_session = Mock()
|
||||
|
||||
def test_get_branches(self):
|
||||
"""
|
||||
Assert that getting branches works correctly.
|
||||
"""
|
||||
response_mock = Mock()
|
||||
response_mock.status_code = 200
|
||||
|
||||
branch = "branch"
|
||||
data = {"branches": [branch]}
|
||||
|
||||
response_mock.json.return_value = data
|
||||
|
||||
self.pagure._requests_session.get.return_value = response_mock
|
||||
|
||||
namespace = "namespace"
|
||||
repo = "repo"
|
||||
|
||||
result = self.pagure.get_branches(namespace, repo)
|
||||
|
||||
self.pagure._requests_session.get.assert_called_with(
|
||||
"https://pagure.io/api/0/{0}/{1}/git/branches".format(namespace, repo),
|
||||
headers=self.pagure.get_auth_header(),
|
||||
)
|
||||
|
||||
assert result == [branch]
|
||||
|
||||
def test_get_branches_no_branches(self):
|
||||
"""
|
||||
Assert that getting branches returns [] if the project doesn't have any branch.
|
||||
"""
|
||||
response_mock = Mock()
|
||||
response_mock.status_code = 200
|
||||
|
||||
data = {}
|
||||
|
||||
response_mock.json.return_value = data
|
||||
|
||||
self.pagure._requests_session.get.return_value = response_mock
|
||||
|
||||
namespace = "namespace"
|
||||
repo = "repo"
|
||||
|
||||
result = self.pagure.get_branches(namespace, repo)
|
||||
|
||||
self.pagure._requests_session.get.assert_called_with(
|
||||
"https://pagure.io/api/0/{0}/{1}/git/branches".format(namespace, repo),
|
||||
headers=self.pagure.get_auth_header(),
|
||||
)
|
||||
|
||||
assert result == []
|
||||
|
||||
def test_get_branches_failure(self):
|
||||
"""
|
||||
Assert that failing to get branches is handled correctly.
|
||||
"""
|
||||
response_mock = Mock()
|
||||
response_mock.status_code = 500
|
||||
|
||||
self.pagure._requests_session.get.side_effect = response_mock
|
||||
|
||||
namespace = "namespace"
|
||||
repo = "repo"
|
||||
|
||||
expected_error = "Couldn't get list of branches for project '{0}/{1}'".format(
|
||||
namespace, repo
|
||||
)
|
||||
|
||||
with pytest.raises(PagureError, match=expected_error):
|
||||
self.pagure.get_branches(namespace, repo)
|
||||
|
||||
self.pagure._requests_session.get.assert_called_with(
|
||||
"https://pagure.io/api/0/{0}/{1}/git/branches".format(namespace, repo),
|
||||
headers=self.pagure.get_auth_header(),
|
||||
)
|
||||
|
||||
|
||||
class TestPagureGetDefaultBranch:
|
||||
"""
|
||||
Test class for `toddlers.pagure.Pagure.get_default_branch` method.
|
||||
|
|
@ -1036,7 +1128,7 @@ class TestPagureGetDefaultBranch:
|
|||
|
||||
assert result == branch
|
||||
|
||||
def test_get_default_branch_not_ser(self):
|
||||
def test_get_default_branch_not_set(self):
|
||||
"""
|
||||
Assert that getting default branch returns None if not set.
|
||||
"""
|
||||
|
|
|
|||
|
|
@ -694,6 +694,17 @@ class SCMRequestProcessor(ToddlerBase):
|
|||
return
|
||||
|
||||
branch_name = issue_body_json["branch"].strip()
|
||||
# Check if the branch doesn't exist in dist git
|
||||
branches = self.dist_git.get_branches(namespace, repo)
|
||||
if branch_name in branches:
|
||||
self.pagure_io.close_issue(
|
||||
issue["id"],
|
||||
namespace=PROJECT_NAMESPACE,
|
||||
message="Branch '{}' already exists in dist-git".format(branch_name),
|
||||
reason="Invalid",
|
||||
)
|
||||
return
|
||||
|
||||
if re.match(EPEL_REGEX, branch_name) and not self.valid_epel_package(
|
||||
repo, branch_name
|
||||
):
|
||||
|
|
|
|||
|
|
@ -494,6 +494,45 @@ class Pagure:
|
|||
|
||||
return result
|
||||
|
||||
def get_branches(self, namespace: str, repo: str) -> Any:
|
||||
"""
|
||||
Return all branches for the specified repository.
|
||||
|
||||
Params:
|
||||
namespace: Namespace of the project
|
||||
repo: Name of the project
|
||||
|
||||
Returns:
|
||||
List of the branches in repository.
|
||||
|
||||
Raises:
|
||||
`toddlers.utils.exceptions.PagureError``: When getting branches fails.
|
||||
"""
|
||||
branches_api_url = "{0}/api/0/{1}/{2}/git/branches".format(
|
||||
self._pagure_url, namespace, repo
|
||||
)
|
||||
headers = self.get_auth_header()
|
||||
|
||||
log.debug(
|
||||
"Getting list of branches for project '{0}/{1}'".format(namespace, repo)
|
||||
)
|
||||
response = self._requests_session.get(branches_api_url, headers=headers)
|
||||
|
||||
if response.status_code != 200:
|
||||
log.error(
|
||||
"Error when getting list of branches for project '{0}/{1}'. "
|
||||
"Got status_code '{2}'.".format(namespace, repo, response.status_code)
|
||||
)
|
||||
raise PagureError(
|
||||
"Couldn't get list of branches for project '{0}/{1}'".format(
|
||||
namespace, repo
|
||||
)
|
||||
)
|
||||
|
||||
result = response.json().get("branches", [])
|
||||
|
||||
return result
|
||||
|
||||
def get_default_branch(self, namespace: str, repo: str) -> Any:
|
||||
"""
|
||||
Return the default branch for the specified repository.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue