Skip to content

Commit 38c2e7e

Browse files
ytauschytausch
committed
detect duplicate pull requests
1 parent f481f43 commit 38c2e7e

File tree

4 files changed

+185
-46
lines changed

4 files changed

+185
-46
lines changed

‎conda_forge_tick/git_utils.py‎

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,14 @@ class GitPlatformError(Exception):
146146
pass
147147

148148

149+
class DuplicatePullRequestError(GitPlatformError):
150+
"""
151+
Raised if a pull request already exists.
152+
"""
153+
154+
pass
155+
156+
149157
class RepositoryNotFoundError(Exception):
150158
"""
151159
Raised when a repository is not found.
@@ -685,6 +693,7 @@ def create_pull_request(
685693
:returns: The data of the created pull request.
686694
687695
:raises GitPlatformError: If the pull request could not be created.
696+
:raises DuplicatePullRequestError: If a pull request already exists and the backend checks for it.
688697
"""
689698
pass
690699

@@ -882,12 +891,19 @@ def create_pull_request(
882891
target_owner, target_repo
883892
)
884893

885-
response: github3.pulls.ShortPullRequest | None = repo.create_pull(
886-
title=title,
887-
base=base_branch,
888-
head=f"{self.user}:{head_branch}",
889-
body=body,
890-
)
894+
try:
895+
response: github3.pulls.ShortPullRequest | None = repo.create_pull(
896+
title=title,
897+
base=base_branch,
898+
head=f"{self.user}:{head_branch}",
899+
body=body,
900+
)
901+
except github3.exceptions.UnprocessableEntity as e:
902+
if any("already exists" in error.get("message", "") for error in e.errors):
903+
raise DuplicatePullRequestError(
904+
f"Pull request from {self.user}:{head_branch} to {target_owner}:{base_branch} already exists."
905+
) from e
906+
raise
891907

892908
if response is None:
893909
raise GitPlatformError("Could not create pull request.")

‎tests/github_api/create_pull_duplicate.json‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
{
2+
"message": "Validation Failed",
3+
"errors": [
4+
{
5+
"resource": "PullRequest",
6+
"code": "custom",
7+
"message": "A pull request already exists for OWNER:BRANCH."
8+
}
9+
],
10+
"documentation_url": "https://docs.github.com/rest/pulls/pulls#create-a-pull-request",
11+
"status": "422"
12+
}

‎tests/github_api/create_pull_validation_error.json‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
{
2+
"message": "Validation Failed",
3+
"errors": [
4+
{
5+
"resource": "PullRequest",
6+
"field": "head",
7+
"code": "invalid"
8+
}
9+
],
10+
"documentation_url": "https://docs.github.com/rest/pulls/pulls#create-a-pull-request",
11+
"status": "422"
12+
}

‎tests/test_git_utils.py‎

Lines changed: 139 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
from conda_forge_tick.git_utils import (
1717
Bound,
1818
DryRunBackend,
19+
DuplicatePullRequestError,
1920
GitCli,
2021
GitCliError,
2122
GitConnectionMode,
@@ -1044,6 +1045,41 @@ def test_git_cli_clone_fork_and_branch_non_existing_remote_existing_target_dir(c
10441045
assert "trying to reset hard" in caplog.text
10451046

10461047

1048+
def _github_api_json_fixture(name: str) -> dict:
1049+
with Path(__file__).parent.joinpath(f"github_api/{name}.json").open() as f:
1050+
return json.load(f)
1051+
1052+
1053+
@pytest.fixture()
1054+
def github_response_create_issue_comment() -> dict:
1055+
return _github_api_json_fixture("create_issue_comment_pytest")
1056+
1057+
1058+
@pytest.fixture()
1059+
def github_response_create_pull_duplicate() -> dict:
1060+
return _github_api_json_fixture("create_pull_duplicate")
1061+
1062+
1063+
@pytest.fixture()
1064+
def github_response_create_pull_validation_error() -> dict:
1065+
return _github_api_json_fixture("create_pull_validation_error")
1066+
1067+
1068+
@pytest.fixture()
1069+
def github_response_get_pull() -> dict:
1070+
return _github_api_json_fixture("get_pull_pytest")
1071+
1072+
1073+
@pytest.fixture()
1074+
def github_response_get_repo() -> dict:
1075+
return _github_api_json_fixture("get_repo_pytest")
1076+
1077+
1078+
@pytest.fixture()
1079+
def github_response_headers() -> dict:
1080+
return _github_api_json_fixture("github_response_headers")
1081+
1082+
10471083
def test_github_backend_from_token():
10481084
token = "TOKEN"
10491085

@@ -1305,28 +1341,23 @@ def test_github_backend_get_api_requests_left_zero_valid_reset_time(caplog):
13051341

13061342

13071343
@mock.patch("requests.Session.request")
1308-
def test_github_backend_create_pull_request_mock(request_mock: MagicMock):
1309-
with open(Path(__file__).parent / "github_api" / "get_repo_pytest.json") as f:
1310-
get_repo_response = json.load(f)
1311-
1312-
with open(
1313-
Path(__file__).parent / "github_api" / "github_response_headers.json"
1314-
) as f:
1315-
response_headers = json.load(f)
1316-
1317-
with open(Path(__file__).parent / "github_api" / "get_pull_pytest.json") as f:
1318-
create_pull_response = json.load(f)
1319-
1344+
def test_github_backend_create_pull_request_mock(
1345+
request_mock: MagicMock,
1346+
github_response_get_repo: dict,
1347+
github_response_headers: dict,
1348+
github_response_get_pull: dict,
1349+
):
13201350
def request_side_effect(method, _url, **_kwargs):
13211351
response = requests.Response()
13221352
if method == "GET":
13231353
response.status_code = 200
1324-
response.json = lambda: get_repo_response
1354+
response.json = lambda: github_response_get_repo
13251355
return response
13261356
if method == "POST":
13271357
response.status_code = 201
1328-
response.json = lambda: create_pull_response
1329-
response.headers = CaseInsensitiveDict(response_headers)
1358+
# note that the "create pull" response body is identical to the "get pull" response body
1359+
response.json = lambda: github_response_get_pull
1360+
response.headers = CaseInsensitiveDict(github_response_headers)
13301361
return response
13311362
assert False, f"Unexpected method: {method}"
13321363

@@ -1380,42 +1411,117 @@ def request_side_effect(method, _url, **_kwargs):
13801411

13811412

13821413
@mock.patch("requests.Session.request")
1383-
def test_github_backend_comment_on_pull_request_success(request_mock: MagicMock):
1384-
with open(Path(__file__).parent / "github_api" / "get_repo_pytest.json") as f:
1385-
get_repo_response = json.load(f)
1414+
def test_github_backend_create_pull_request_duplicate(
1415+
request_mock: MagicMock,
1416+
github_response_get_repo: dict,
1417+
github_response_create_pull_duplicate: dict,
1418+
):
1419+
def request_side_effect(method, _url, **_kwargs):
1420+
response = requests.Response()
1421+
if method == "GET":
1422+
response.status_code = 200
1423+
response.json = lambda: github_response_get_repo
1424+
return response
1425+
if method == "POST":
1426+
response.status_code = 422
1427+
# note that the "create pull" response body is identical to the "get pull" response body
1428+
response.json = lambda: github_response_create_pull_duplicate
1429+
return response
1430+
assert False, f"Unexpected method: {method}"
1431+
1432+
request_mock.side_effect = request_side_effect
1433+
1434+
pygithub_mock = MagicMock()
1435+
pygithub_mock.get_user.return_value.login = "CURRENT_USER"
1436+
1437+
backend = GitHubBackend(github3.login(token="TOKEN"), pygithub_mock, "")
1438+
1439+
with pytest.raises(
1440+
DuplicatePullRequestError,
1441+
match="Pull request from CURRENT_USER:HEAD_BRANCH to conda-forge:BASE_BRANCH already exists",
1442+
):
1443+
backend.create_pull_request(
1444+
"conda-forge",
1445+
"pytest-feedstock",
1446+
"BASE_BRANCH",
1447+
"HEAD_BRANCH",
1448+
"TITLE",
1449+
"BODY",
1450+
)
1451+
1452+
1453+
@mock.patch("requests.Session.request")
1454+
def test_github_backend_create_pull_request_validation_error(
1455+
request_mock: MagicMock,
1456+
github_response_get_repo: dict,
1457+
github_response_create_pull_validation_error: dict,
1458+
):
1459+
"""
1460+
Test that other GitHub API 422 validation errors are not caught as DuplicatePullRequestError.
1461+
"""
13861462

1387-
with open(Path(__file__).parent / "github_api" / "get_pull_pytest.json") as f:
1388-
get_pull_response = json.load(f)
1463+
def request_side_effect(method, _url, **_kwargs):
1464+
response = requests.Response()
1465+
if method == "GET":
1466+
response.status_code = 200
1467+
response.json = lambda: github_response_get_repo
1468+
return response
1469+
if method == "POST":
1470+
response.status_code = 422
1471+
# note that the "create pull" response body is identical to the "get pull" response body
1472+
response.json = lambda: github_response_create_pull_validation_error
1473+
return response
1474+
assert False, f"Unexpected method: {method}"
1475+
1476+
request_mock.side_effect = request_side_effect
1477+
1478+
pygithub_mock = MagicMock()
1479+
pygithub_mock.get_user.return_value.login = "CURRENT_USER"
1480+
1481+
backend = GitHubBackend(github3.login(token="TOKEN"), pygithub_mock, "")
1482+
1483+
with pytest.raises(github3.exceptions.UnprocessableEntity):
1484+
backend.create_pull_request(
1485+
"conda-forge",
1486+
"pytest-feedstock",
1487+
"BASE_BRANCH",
1488+
"HEAD_BRANCH",
1489+
"TITLE",
1490+
"BODY",
1491+
)
13891492

1390-
with open(
1391-
Path(__file__).parent / "github_api" / "create_issue_comment_pytest.json"
1392-
) as f:
1393-
create_comment_response = json.load(f)
13941493

1494+
@mock.patch("requests.Session.request")
1495+
def test_github_backend_comment_on_pull_request_success(
1496+
request_mock: MagicMock,
1497+
github_response_get_repo: dict,
1498+
github_response_get_pull: dict,
1499+
github_response_create_issue_comment: dict,
1500+
):
13951501
def request_side_effect(method, url, **_kwargs):
13961502
response = requests.Response()
13971503
if (
13981504
method == "GET"
13991505
and url == "https://api.github.com/repos/conda-forge/pytest-feedstock"
14001506
):
14011507
response.status_code = 200
1402-
response.json = lambda: get_repo_response
1508+
response.json = lambda: github_response_get_repo
14031509
return response
14041510
if (
14051511
method == "GET"
14061512
and url
14071513
== "https://api.github.com/repos/conda-forge/pytest-feedstock/pulls/1337"
14081514
):
14091515
response.status_code = 200
1410-
response.json = lambda: get_pull_response
1516+
response.json = lambda: github_response_get_pull
14111517
return response
14121518
if (
14131519
method == "POST"
14141520
and url
14151521
== "https://api.github.com/repos/conda-forge/pytest-feedstock/issues/1337/comments"
14161522
):
14171523
response.status_code = 201
1418-
response.json = lambda: create_comment_response
1524+
response.json = lambda: github_response_create_issue_comment
14191525
return response
14201526
assert False, f"Unexpected endpoint: {method} {url}"
14211527

@@ -1467,18 +1573,16 @@ def request_side_effect(method, url, **_kwargs):
14671573
@mock.patch("requests.Session.request")
14681574
def test_github_backend_comment_on_pull_request_pull_request_not_found(
14691575
request_mock: MagicMock,
1576+
github_response_get_repo: dict,
14701577
):
1471-
with open(Path(__file__).parent / "github_api" / "get_repo_pytest.json") as f:
1472-
get_repo_response = json.load(f)
1473-
14741578
def request_side_effect(method, url, **_kwargs):
14751579
response = requests.Response()
14761580
if (
14771581
method == "GET"
14781582
and url == "https://api.github.com/repos/conda-forge/pytest-feedstock"
14791583
):
14801584
response.status_code = 200
1481-
response.json = lambda: get_repo_response
1585+
response.json = lambda: github_response_get_repo
14821586
return response
14831587
if (
14841588
method == "GET"
@@ -1507,30 +1611,25 @@ def request_side_effect(method, url, **_kwargs):
15071611
@mock.patch("requests.Session.request")
15081612
def test_github_backend_comment_on_pull_request_unexpected_response(
15091613
request_mock: MagicMock,
1614+
github_response_get_repo: dict,
1615+
github_response_get_pull: dict,
15101616
):
1511-
with open(Path(__file__).parent / "github_api" / "get_repo_pytest.json") as f:
1512-
get_repo_response = json.load(f)
1513-
1514-
with open(Path(__file__).parent / "github_api" / "get_pull_pytest.json") as f:
1515-
get_pull_response = json.load(f)
1516-
15171617
def request_side_effect(method, url, **_kwargs):
1518-
# noinspection DuplicatedCode
15191618
response = requests.Response()
15201619
if (
15211620
method == "GET"
15221621
and url == "https://api.github.com/repos/conda-forge/pytest-feedstock"
15231622
):
15241623
response.status_code = 200
1525-
response.json = lambda: get_repo_response
1624+
response.json = lambda: github_response_get_repo
15261625
return response
15271626
if (
15281627
method == "GET"
15291628
and url
15301629
== "https://api.github.com/repos/conda-forge/pytest-feedstock/pulls/1337"
15311630
):
15321631
response.status_code = 200
1533-
response.json = lambda: get_pull_response
1632+
response.json = lambda: github_response_get_pull
15341633
return response
15351634
if (
15361635
method == "POST"

0 commit comments

Comments
 (0)