diff --git a/bedevere/gh_issue.py b/bedevere/gh_issue.py index 9370fbfe..dd09a8ca 100644 --- a/bedevere/gh_issue.py +++ b/bedevere/gh_issue.py @@ -24,6 +24,10 @@ "gh": "https://github.com/python/cpython/issues/{issue_number}", "bpo": "https://bugs.python.org/issue?@action=redirect&bpo={issue_number}" } +ISSUE_CHECK_URL: Dict[IssueKind, str] = { + "gh": "https://api.github.com/repos/python/cpython/issues/{issue_number}", + "bpo": "https://bugs.python.org/issue{issue_number}" +} @router.register("pull_request", action="opened") @@ -33,12 +37,15 @@ async def set_status( event, gh: GitHubAPI, *args, session: ClientSession, **kwargs ): """Set the issue number status on the pull request.""" - issue = await util.issue_for_PR(gh, event.data["pull_request"]) + pull_request = event.data["pull_request"] + issue = await util.issue_for_PR(gh, pull_request) + if util.skip("issue", issue): await util.post_status(gh, event, SKIP_ISSUE_STATUS) return - issue_number_found = ISSUE_RE.search(event.data["pull_request"]["title"]) + issue_number_found = ISSUE_RE.search(pull_request["title"]) + if not issue_number_found: status = create_failure_status_no_issue() else: @@ -50,7 +57,16 @@ async def set_status( if issue_found: status = create_success_status(issue_number, kind=issue_kind) if issue_kind == "gh": - await util.patch_body(gh, event.data["pull_request"], issue_number) + # Add the issue number to the pull request's body + await util.patch_body(gh, pull_request, issue_number, "Issue") + # Get GitHub Issue data + issue_data = await gh.getitem( + ISSUE_CHECK_URL[issue_kind].format(issue_number=issue_number) + ) + # Add the pull request number to the issue's body + await util.patch_body( + gh, issue_data, pull_request["number"], "PR" + ) else: status = create_failure_status_issue_not_present( issue_number, kind=issue_kind @@ -123,14 +139,14 @@ async def _validate_issue_number( ) -> bool: """Ensure the GitHub Issue number is valid.""" if kind == "bpo": - url = f"https://bugs.python.org/issue{issue_number}" + url = ISSUE_CHECK_URL[kind].format(issue_number=issue_number) async with session.head(url) as res: return res.status != 404 if kind != "gh": raise ValueError(f"Unknown issue kind {kind}") - url = f"/repos/python/cpython/issues/{issue_number}" + url = ISSUE_CHECK_URL[kind].format(issue_number=issue_number) try: response = await gh.getitem(url) except gidgethub.BadRequest: diff --git a/bedevere/util.py b/bedevere/util.py index 88b6f567..4f273ea8 100644 --- a/bedevere/util.py +++ b/bedevere/util.py @@ -1,19 +1,20 @@ import enum import re import sys +from typing import Any, Dict, Union import gidgethub - +from gidgethub.abc import GitHubAPI DEFAULT_BODY = "" -TAG_NAME = "gh-issue-number" +TAG_NAME = f"gh-{{tag_type}}-number" NEWS_NEXT_DIR = "Misc/NEWS.d/next/" CLOSING_TAG = f"" BODY = f"""\ {{body}} - -* Issue: gh-{{issue_number}} + +* {{key}}: gh-{{pr_or_issue_number}} {CLOSING_TAG} """ @@ -93,23 +94,32 @@ async def issue_for_PR(gh, pull_request): return await gh.getitem(pull_request["issue_url"]) -async def patch_body(gh, pull_request, issue_number): - """Updates the description of a PR with the gh issue number if it exists. +async def patch_body( + gh: GitHubAPI, + pr_or_issue: Dict[str, Any], + pr_or_issue_number: int, + key: str +) -> Union[bytes, None]: + """Updates the description of a PR/Issue with the gh issue/pr number if it exists. - returns if body exists with issue_number + returns if body exists with issue/pr number """ - if "body" not in pull_request or pull_request["body"] is None: - return await gh.patch( - pull_request["url"], - data={"body": BODY.format(body=DEFAULT_BODY, issue_number=issue_number)}, - ) + body = pr_or_issue.get("body", None) or DEFAULT_BODY + body_search_pattern = rf"(^|\b)(GH-|gh-|#){pr_or_issue_number}\b" - if not re.search(rf"(^|\b)(GH-|gh-|#){issue_number}\b", pull_request["body"]): + if not body or not re.search(body_search_pattern, body): return await gh.patch( - pull_request["url"], - data={"body": BODY.format(body=pull_request["body"], issue_number=issue_number)}, + pr_or_issue["url"], + data={ + "body": BODY.format( + body=body, + pr_or_issue_number=pr_or_issue_number, + key=key, + tag_type=key.lower(), + ) + }, ) - return + return None async def is_core_dev(gh, username): diff --git a/tests/test_gh_issue.py b/tests/test_gh_issue.py index 69813050..f4b9b78e 100644 --- a/tests/test_gh_issue.py +++ b/tests/test_gh_issue.py @@ -54,9 +54,11 @@ async def test_set_status_failure(action, monkeypatch): "title": "No issue in title", "issue_url": "issue URL", "url": "url", + "number": 1234, }, } issue_data = { + "url": "url", "labels": [ {"name": "non-trivial"}, ] @@ -84,9 +86,10 @@ async def test_set_status_failure_via_issue_not_found_on_github(action, monkeypa "title": "gh-123: Invalid issue number", "issue_url": "issue URL", "url": "url", + "number": 1234, }, } - issue_data = {"labels": []} + issue_data = {"url": "url", "labels": []} event = sansio.Event(data, event="pull_request", delivery_id="12345") gh = FakeGH(getitem=issue_data) async with aiohttp.ClientSession() as session: @@ -108,9 +111,10 @@ async def test_set_status_success_issue_found_on_bpo(action): "title": "bpo-12345: An issue on b.p.o", "issue_url": "issue URL", "url": "url", + "number": 1234, }, } - issue_data = {"labels": []} + issue_data = {"url": "url", "labels": []} event = sansio.Event(data, event="pull_request", delivery_id="12345") gh = FakeGH(getitem=issue_data) async with aiohttp.ClientSession() as session: @@ -138,9 +142,10 @@ async def test_set_status_success(action, monkeypatch): "url": "", "title": "[3.6] gh-1234: an issue!", "issue_url": "issue URL", + "number": 1234, }, } - issue_data = {"labels": []} + issue_data = {"url": "url", "labels": []} event = sansio.Event(data, event="pull_request", delivery_id="12345") gh = FakeGH(getitem=issue_data) await gh_issue.router.dispatch(event, gh, session=None) @@ -165,9 +170,10 @@ async def test_set_status_success_issue_found_on_gh(action, monkeypatch, issue_n "title": f"gh-{issue_number}: an issue!", "url": "url", "issue_url": "issue URL", + "number": 1234, }, } - issue_data = {"labels": []} + issue_data = {"url": "url", "labels": []} event = sansio.Event(data, event="pull_request", delivery_id="12345") gh = FakeGH(getitem=issue_data) async with aiohttp.ClientSession() as session: @@ -181,8 +187,10 @@ async def test_set_status_success_issue_found_on_gh(action, monkeypatch, issue_n assert len(gh.patch_data) > 0 assert f"" in gh.patch_data[0]["body"] - assert len(gh.patch_url) == 1 + assert f"" in gh.patch_data[1]["body"] + assert len(gh.patch_url) == 2 assert gh.patch_url[0] == data["pull_request"]["url"] + assert gh.patch_url[1] == issue_data["url"] @pytest.mark.asyncio @@ -197,9 +205,10 @@ async def test_set_status_success_issue_found_on_gh_ignore_case(action, monkeypa "title": f"GH-{issue_number}: an issue!", "url": "url", "issue_url": "issue URL", + "number": 1234, }, } - issue_data = {"labels": []} + issue_data = {"url": "url", "labels": []} event = sansio.Event(data, event="pull_request", delivery_id="12345") gh = FakeGH(getitem=issue_data) async with aiohttp.ClientSession() as session: @@ -213,8 +222,10 @@ async def test_set_status_success_issue_found_on_gh_ignore_case(action, monkeypa assert len(gh.patch_data) > 0 assert f"" in gh.patch_data[0]["body"] - assert len(gh.patch_url) == 1 + assert f"" in gh.patch_data[1]["body"] + assert len(gh.patch_url) == 2 assert gh.patch_url[0] == data["pull_request"]["url"] + assert gh.patch_url[1] == issue_data["url"] @pytest.mark.asyncio @@ -229,9 +240,11 @@ async def test_set_status_success_via_skip_issue_label(action, monkeypatch): "title": "No issue in title", "issue_url": "issue URL", "url": "url", + "number": 1234, }, } issue_data = { + "url": "url", "labels": [ {"name": "skip issue"}, ] @@ -260,9 +273,11 @@ async def test_set_status_success_via_skip_issue_label_pr_in_title(action, monke "title": "GH-93644: An issue with a PR as issue number", "issue_url": "issue URL", "url": "url", + "number": 1234, }, } issue_data = { + "url": "url", "labels": [ {"name": "skip issue"}, ] @@ -290,11 +305,12 @@ async def test_edit_title(monkeypatch, issue_number): "title": f"gh-{issue_number}: an issue!", "url": "url", "issue_url": "issue URL", + "number": 1234, }, "action": "edited", "changes": {"title": "thingy"}, } - issue_data = {"labels": []} + issue_data = {"url": "url", "labels": []} event = sansio.Event(data, event="pull_request", delivery_id="12345") gh = FakeGH(getitem=issue_data) await gh_issue.router.dispatch(event, gh, session=None) @@ -313,12 +329,13 @@ async def test_no_body_when_edit_title(monkeypatch, issue_number): "body": None, "issue_url": "issue URL", "statuses_url": "https://api.github.com/repos/python/cpython/statuses/98d60953c85df9f0f28e04322a4c4ebec7b180f4", + "number": 1234, }, "changes": { "title": f"gh-{issue_number}: Fix @asyncio.coroutine debug mode bug exposed by #5250." }, } - issue_data = {"labels": []} + issue_data = {"url": "url", "labels": []} event = sansio.Event(data, event="pull_request", delivery_id="12345") gh = FakeGH(getitem=issue_data) await gh_issue.router.dispatch(event, gh, session=None) @@ -326,8 +343,10 @@ async def test_no_body_when_edit_title(monkeypatch, issue_number): assert len(gh.patch_data) > 0 assert f"" in gh.patch_data[0]["body"] - assert len(gh.patch_url) == 1 + assert f"" in gh.patch_data[1]["body"] + assert len(gh.patch_url) == 2 assert gh.patch_url[0] == data["pull_request"]["url"] + assert gh.patch_url[1] == issue_data["url"] @pytest.mark.asyncio @@ -340,11 +359,12 @@ async def test_edit_other_than_title(monkeypatch): "title": "bpo-1234: an issue!", "url": "url", "issue_url": "issue URL", + "number": 1234, }, "action": "edited", "changes": {"stuff": "thingy"}, } - issue_data = {"labels": []} + issue_data = {"url": "url", "labels": []} event = sansio.Event(data, event="pull_request", delivery_id="12345") gh = FakeGH(getitem=issue_data) await gh_issue.router.dispatch(event, gh, session=None) @@ -365,9 +385,10 @@ async def test_new_label_skip_issue_no_issue(): "title": "An easy fix", "url": "url", "issue_url": "issue URL", + "number": 1234, }, } - issue_data = {"labels": []} + issue_data = {"url": "url", "labels": []} event = sansio.Event(data, event="pull_request", delivery_id="12345") gh = FakeGH(getitem=issue_data) await gh_issue.router.dispatch(event, gh) @@ -385,9 +406,10 @@ async def test_new_label_skip_issue_with_issue_number(): "title": "Revert gh-1234: revert an easy fix", "url": "url", "issue_url": "issue URL", + "number": 1234, }, } - issue_data = {"labels": []} + issue_data = {"url": "url", "labels": []} event = sansio.Event(data, event="pull_request", delivery_id="12345") gh = FakeGH(getitem=issue_data) await gh_issue.router.dispatch(event, gh) @@ -409,9 +431,10 @@ async def test_new_label_skip_issue_with_issue_number_ignore_case(): "title": "Revert Gh-1234: revert an easy fix", "url": "url", "issue_url": "issue URL", + "number": 1234, }, } - issue_data = {"labels": []} + issue_data = {"url": "url", "labels": []} event = sansio.Event(data, event="pull_request", delivery_id="12345") gh = FakeGH(getitem=issue_data) await gh_issue.router.dispatch(event, gh) @@ -431,9 +454,10 @@ async def test_new_label_not_skip_issue(): "statuses_url": "https://api.github.com/blah/blah/git-sha", "url": "url", "issue_url": "issue URL", + "number": 1234, }, } - issue_data = {"labels": []} + issue_data = {"url": "url", "labels": []} event = sansio.Event(data, event="pull_request", delivery_id="12345") gh = FakeGH(getitem=issue_data) await gh_issue.router.dispatch(event, gh) @@ -454,9 +478,10 @@ async def test_removed_label_from_label_deletion(monkeypatch): "title": "gh-1234: an issue!", "url": "url", "issue_url": "issue URL", + "number": 1234, }, } - issue_data = {"labels": []} + issue_data = {"url": "url", "labels": []} event = sansio.Event(data, event="pull_request", delivery_id="12345") gh = FakeGH(getitem=issue_data) await gh_issue.router.dispatch(event, gh, session=None) @@ -476,9 +501,10 @@ async def test_removed_label_skip_issue(monkeypatch): "title": "gh-1234: an issue!", "url": "url", "issue_url": "issue URL", + "number": 1234, }, } - issue_data = {"labels": []} + issue_data = {"url": "url", "labels": []} event = sansio.Event(data, event="pull_request", delivery_id="12345") gh = FakeGH(getitem=issue_data) await gh_issue.router.dispatch(event, gh, session=None) @@ -502,9 +528,10 @@ async def test_removed_label_non_skip_issue(monkeypatch): "statuses_url": "https://api.github.com/blah/blah/git-sha", "url": "url", "issue_url": "issue URL", + "number": 1234, }, } - issue_data = {"labels": []} + issue_data = {"url": "url", "labels": []} event = sansio.Event(data, event="pull_request", delivery_id="12345") gh = FakeGH(getitem=issue_data) await gh_issue.router.dispatch(event, gh, session=None) @@ -536,7 +563,11 @@ async def test_validate_issue_number_is_pr_on_github(): gh = FakeGH(getitem={ "number": 123, - "pull_request": {"html_url": "https://github.com/python/cpython/pull/123", "url": "url",} + "pull_request": { + "html_url": "https://github.com/python/cpython/pull/123", + "url": "url", + "number": 1234, + } }) async with aiohttp.ClientSession() as session: response = await gh_issue._validate_issue_number(gh, 123, session=session) diff --git a/tests/test_util.py b/tests/test_util.py index bfe3b92a..14ef038e 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -161,9 +161,9 @@ async def test_get_pr_for_commit_not_found(): async def test_patch_body_adds_issue_if_not_present(): - """Updates the description of a PR with the gh issue number if it exists. + """Updates the description of a PR/Issue with the gh issue/pr number if it exists. - returns if body exists with issue_number + returns if body exists with issue/pr number """ sha = "f2393593c99dd2d3ab8bfab6fcc5ddee540518a9" gh = FakeGH( @@ -179,27 +179,72 @@ async def test_patch_body_adds_issue_if_not_present(): vals["body"] = "GH-1234\n" with patch.object(gh, "patch") as mock: - await util.patch_body(gh, vals, "1234") + await util.patch_body(gh, vals, 1234, "Issue") mock.assert_not_called() with patch.object(gh, "patch") as mock: vals["body"] = "Multiple\nlines\nwith gh-1234 in some prose" - await util.patch_body(gh, vals, "1234") + await util.patch_body(gh, vals, 1234, "Issue") mock.assert_not_called() with patch.object(gh, "patch") as mock: vals["body"] = "#1234 in some prose" - await util.patch_body(gh, vals, "1234") + await util.patch_body(gh, vals, 1234, "Issue") mock.assert_not_called() with patch.object(gh, "patch") as mock: vals["body"] = "Some prose mentioning gh-12345 but not our issue" - await util.patch_body(gh, vals, "1234") + await util.patch_body(gh, vals, 1234, "Issue") mock.assert_called_once() with patch.object(gh, "patch") as mock: vals["body"] = None - await util.patch_body(gh, vals, "1234") + await util.patch_body(gh, vals, 1234, "Issue") mock.assert_called_once() with patch.object(gh, "patch") as mock: vals["body"] = "" - await util.patch_body(gh, vals, "1234") + await util.patch_body(gh, vals, 1234, "Issue") data = {"body": "\n\n\n* Issue: gh-1234\n\n"} mock.assert_called_once_with("https://fake.com", data=data) assert await gh.patch(vals["url"], data=vals) == None + + +async def test_patch_body_adds_pr_if_not_present(): + """Updates the description of a PR/Issue with the gh issue/pr number if it exists. + + returns if body exists with issue/pr number + """ + sha = "f2393593c99dd2d3ab8bfab6fcc5ddee540518a9" + gh = FakeGH( + getitem={ + f"https://api.github.com/search/issues?q=type:pr+repo:python/cpython+sha:{sha}": { + "total_count": 0, + "items": [], + } + } + ) + vals = {} + vals["url"] = "https://fake.com" + vals["body"] = "GH-1234\n" + + with patch.object(gh, "patch") as mock: + await util.patch_body(gh, vals, 1234, "PR") + mock.assert_not_called() + with patch.object(gh, "patch") as mock: + vals["body"] = "Multiple\nlines\nwith gh-1234 in some prose" + await util.patch_body(gh, vals, 1234, "PR") + mock.assert_not_called() + with patch.object(gh, "patch") as mock: + vals["body"] = "#1234 in some prose" + await util.patch_body(gh, vals, 1234, "PR") + mock.assert_not_called() + with patch.object(gh, "patch") as mock: + vals["body"] = "Some prose mentioning gh-12345 but not our issue" + await util.patch_body(gh, vals, 1234, "PR") + mock.assert_called_once() + with patch.object(gh, "patch") as mock: + vals["body"] = None + await util.patch_body(gh, vals, 1234, "PR") + mock.assert_called_once() + with patch.object(gh, "patch") as mock: + vals["body"] = "" + await util.patch_body(gh, vals, 1234, "PR") + data = {"body": "\n\n\n* PR: gh-1234\n\n"} + mock.assert_called_once_with("https://fake.com", data=data) + assert await gh.patch(vals["url"], data=vals) == None