diff --git a/bedevere/util.py b/bedevere/util.py index a18250a9..97dddffe 100644 --- a/bedevere/util.py +++ b/bedevere/util.py @@ -3,7 +3,18 @@ import gidgethub + +DEFAULT_BODY = "" +TAG_NAME = "gh-issue-number" NEWS_NEXT_DIR = "Misc/NEWS.d/next/" +CLOSING_TAG = f"" +BODY = f"""\ +{{body}} + + +* gh-{{issue_number}} +{CLOSING_TAG} +""" @enum.unique @@ -67,7 +78,10 @@ async def files_for_PR(gh, pull_request): data = [] async for filedata in gh.getiter(files_url): # pragma: no branch data.append( - {"file_name": filedata["filename"], "patch": filedata.get("patch", ""),} + { + "file_name": filedata["filename"], + "patch": filedata.get("patch", ""), + } ) return data @@ -77,6 +91,24 @@ 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. + + returns if body exists with issue_number + """ + if "body" not in pull_request or pull_request["body"] is None: + return await gh.patch( + pull_request["url"], + data=BODY.format(body=DEFAULT_BODY, issue_number=issue_number), + ) + if f"GH-{issue_number}\n" not in pull_request["body"]: + return await gh.patch( + pull_request["url"], + data=BODY.format(body=pull_request["body"], issue_number=issue_number), + ) + return + + async def is_core_dev(gh, username): """Check if the user is a CPython core developer.""" org_teams = "/orgs/python/teams" diff --git a/tests/test_gh_issue.py b/tests/test_gh_issue.py index 8a47d6c2..729bc258 100644 --- a/tests/test_gh_issue.py +++ b/tests/test_gh_issue.py @@ -12,11 +12,14 @@ class FakeGH: - def __init__(self, *, getitem=None, post=None): + def __init__(self, *, getitem=None, post=None, patch=None): self._getitem_return = getitem self._post_return = post + self._patch_return = patch self.post_url = [] self.post_data = [] + self.patch_url = [] + self.patch_data = [] async def getitem(self, url): if isinstance(self._getitem_return, Exception): @@ -28,7 +31,6 @@ async def post(self, url, *, data): self.post_data.append(data) return self._post_return - @pytest.mark.asyncio @pytest.mark.parametrize("action", ["opened", "synchronize", "reopened"]) async def test_set_status_failure(action, monkeypatch): @@ -40,6 +42,7 @@ async def test_set_status_failure(action, monkeypatch): "statuses_url": "https://api.github.com/blah/blah/git-sha", "title": "No issue in title", "issue_url": "issue URL", + "url": "url", }, } issue_data = { @@ -68,6 +71,8 @@ async def test_set_status_failure_via_issue_not_found_on_github(action, monkeypa "pull_request": { "statuses_url": "https://api.github.com/blah/blah/git-sha", "title": "gh-123: Invalid issue number", + "issue_url": "issue URL", + "url": "url", }, } event = sansio.Event(data, event="pull_request", delivery_id="12345") @@ -88,7 +93,9 @@ async def test_set_status_success_issue_found_on_bpo(action): "action": action, "pull_request": { "statuses_url": "https://api.github.com/blah/blah/git-sha", - "title": "bpo-12345: an issue!", + "title": "bpo-12345: An issue on b.p.o", + "issue_url": "issue URL", + "url": "url", }, } event = sansio.Event(data, event="pull_request", delivery_id="12345") @@ -112,6 +119,7 @@ async def test_set_status_success(action, monkeypatch): "action": action, "pull_request": { "statuses_url": "https://api.github.com/blah/blah/git-sha", + "url": "", "title": "[3.6] gh-1234: an issue!", }, } @@ -137,6 +145,7 @@ async def test_set_status_success_issue_found_on_gh(action, monkeypatch): "pull_request": { "statuses_url": "https://api.github.com/blah/blah/git-sha", "title": "gh-12345: an issue!", + "url": "url", }, } event = sansio.Event(data, event="pull_request", delivery_id="12345") @@ -161,6 +170,7 @@ async def test_set_status_success_issue_found_on_gh_ignore_case(action, monkeypa "pull_request": { "statuses_url": "https://api.github.com/blah/blah/git-sha", "title": "GH-12345: an issue!", + "url": "url", }, } event = sansio.Event(data, event="pull_request", delivery_id="12345") @@ -186,6 +196,7 @@ async def test_set_status_success_via_skip_issue_label(action, monkeypatch): "statuses_url": "https://api.github.com/blah/blah/git-sha", "title": "No issue in title", "issue_url": "issue URL", + "url": "url", }, } issue_data = { @@ -211,6 +222,7 @@ async def test_edit_title(monkeypatch): "pull_request": { "statuses_url": "https://api.github.com/blah/blah/git-sha", "title": "gh-1234: an issue!", + "url": "url", }, "action": "edited", "changes": {"title": "thingy"}, @@ -251,6 +263,7 @@ async def test_edit_other_than_title(monkeypatch): "pull_request": { "statuses_url": "https://api.github.com/blah/blah/git-sha", "title": "bpo-1234: an issue!", + "url": "url", }, "action": "edited", "changes": {"stuff": "thingy"}, @@ -270,6 +283,7 @@ async def test_new_label_skip_issue_no_issue(): "pull_request": { "statuses_url": "https://api.github.com/blah/blah/git-sha", "title": "An easy fix", + "url": "url", }, } event = sansio.Event(data, event="pull_request", delivery_id="12345") @@ -287,6 +301,7 @@ async def test_new_label_skip_issue_with_issue_number(): "pull_request": { "statuses_url": "https://api.github.com/blah/blah/git-sha", "title": "Revert gh-1234: revert an easy fix", + "url": "url", }, } event = sansio.Event(data, event="pull_request", delivery_id="12345") @@ -308,6 +323,7 @@ async def test_new_label_skip_issue_with_issue_number_ignore_case(): "pull_request": { "statuses_url": "https://api.github.com/blah/blah/git-sha", "title": "Revert Gh-1234: revert an easy fix", + "url": "url", }, } event = sansio.Event(data, event="pull_request", delivery_id="12345") @@ -327,6 +343,7 @@ async def test_new_label_not_skip_issue(): "label": {"name": "non-trivial"}, "pull_request": { "statuses_url": "https://api.github.com/blah/blah/git-sha", + "url": "url", }, } event = sansio.Event(data, event="pull_request", delivery_id="12345") @@ -347,6 +364,7 @@ async def test_removed_label_from_label_deletion(monkeypatch): "pull_request": { "statuses_url": "https://api.github.com/blah/blah/git-sha", "title": "gh-1234: an issue!", + "url": "url", }, } event = sansio.Event(data, event="pull_request", delivery_id="12345") @@ -366,6 +384,7 @@ async def test_removed_label_skip_issue(monkeypatch): "pull_request": { "statuses_url": "https://api.github.com/blah/blah/git-sha", "title": "gh-1234: an issue!", + "url": "url", }, } event = sansio.Event(data, event="pull_request", delivery_id="12345") @@ -389,6 +408,7 @@ async def test_removed_label_non_skip_issue(monkeypatch): "label": {"name": "non-trivial"}, "pull_request": { "statuses_url": "https://api.github.com/blah/blah/git-sha", + "url": "url", }, } event = sansio.Event(data, event="pull_request", delivery_id="12345") @@ -422,7 +442,7 @@ 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"} + "pull_request": {"html_url": "https://github.com/python/cpython/pull/123", "url": "url",} }) async with aiohttp.ClientSession() as session: response = await gh_issue._validate_issue_number(gh, 123, session=session) diff --git a/tests/test_stage.py b/tests/test_stage.py index af450001..fd9bb814 100644 --- a/tests/test_stage.py +++ b/tests/test_stage.py @@ -18,6 +18,7 @@ def __init__(self, *, getiter=None, getitem=None, delete=None, post=None): self.getitem_url = None self.delete_url = None self.post_ = [] + self.patch_ = [] async def getiter(self, url, url_vars={}): self.getiter_url = sansio.format_url(url, url_vars) @@ -40,6 +41,10 @@ async def post(self, url, url_vars={}, *, data): post_url = sansio.format_url(url, url_vars) self.post_.append((post_url, data)) + async def patch(self, url, url_vars={}, *, data): + patch_url = sansio.format_url(url, url_vars) + self.patch_.append((patch_url, data)) + async def test_stage(): # Skip changing labels if the label is already set. @@ -73,7 +78,12 @@ async def test_opened_pr(): issue_url = "https://api.github.com/issue/42" data = { "action": "opened", - "pull_request": {"user": {"login": username,}, "issue_url": issue_url,}, + "pull_request": { + "user": { + "login": username, + }, + "issue_url": issue_url, + }, } event = sansio.Event(data, event="pull_request", delivery_id="12345") teams = [{"name": "python core", "id": 6}] @@ -95,7 +105,12 @@ async def test_opened_pr(): issue_url = "https://api.github.com/issue/42" data = { "action": "opened", - "pull_request": {"user": {"login": username,}, "issue_url": issue_url,}, + "pull_request": { + "user": { + "login": username, + }, + "issue_url": issue_url, + }, } event = sansio.Event(data, event="pull_request", delivery_id="12345") teams = [{"name": "python core", "id": 6}] @@ -120,7 +135,12 @@ async def test_new_review(): username = "andreamcinnes" data = { "action": "submitted", - "review": {"state": "approved", "user": {"login": username,},}, + "review": { + "state": "approved", + "user": { + "login": username, + }, + }, "pull_request": { "url": "https://api.github.com/pr/42", "issue_url": "https://api.github.com/issue/42", @@ -176,7 +196,12 @@ async def test_new_review(): # First comment review from a non-core dev. data = { "action": "submitted", - "review": {"state": "comment", "user": {"login": username,},}, + "review": { + "state": "comment", + "user": { + "login": username, + }, + }, "pull_request": { "url": "https://api.github.com/pr/42", "issue_url": "https://api.github.com/issue/42", @@ -208,7 +233,12 @@ async def test_new_review(): username = "brettcannon" data = { "action": "submitted", - "review": {"user": {"login": username,}, "state": "APPROVED",}, + "review": { + "user": { + "login": username, + }, + "state": "APPROVED", + }, "pull_request": { "url": "https://api.github.com/pr/42", "issue_url": "https://api.github.com/issue/42", @@ -239,7 +269,12 @@ async def test_new_review(): username = "brettcannon" data = { "action": "submitted", - "review": {"user": {"login": username,}, "state": "APPROVED",}, + "review": { + "user": { + "login": username, + }, + "state": "APPROVED", + }, "pull_request": { "url": "https://api.github.com/pr/42", "issue_url": "https://api.github.com/issue/42", @@ -266,7 +301,12 @@ async def test_new_review(): # Core dev requests changes. data = { "action": "submitted", - "review": {"user": {"login": username,}, "state": "changes_requested".upper(),}, + "review": { + "user": { + "login": username, + }, + "state": "changes_requested".upper(), + }, "pull_request": { "url": "https://api.github.com/pr/42", "issue_url": "https://api.github.com/issue/42", @@ -299,7 +339,12 @@ async def test_new_review(): # Comment reviews do nothing. data = { "action": "submitted", - "review": {"user": {"login": username,}, "state": "commented".upper(),}, + "review": { + "user": { + "login": username, + }, + "state": "commented".upper(), + }, "pull_request": { "url": "https://api.github.com/pr/42", "issue_url": "https://api.github.com/issue/42", @@ -315,7 +360,12 @@ async def test_new_review(): # Skip commenting if "awaiting changes" is already set. data = { "action": "submitted", - "review": {"user": {"login": username,}, "state": "changes_requested".upper(),}, + "review": { + "user": { + "login": username, + }, + "state": "changes_requested".upper(), + }, "pull_request": { "url": "https://api.github.com/pr/42", "issue_url": "https://api.github.com/issue/42", @@ -354,7 +404,12 @@ async def test_non_core_dev_does_not_downgrade(): # Approval from a core dev changes the state to "Awaiting merge". data = { "action": "submitted", - "review": {"state": "approved", "user": {"login": core_dev,},}, + "review": { + "state": "approved", + "user": { + "login": core_dev, + }, + }, "pull_request": { "url": "https://api.github.com/pr/42", "issue_url": "https://api.github.com/issue/42", @@ -378,7 +433,12 @@ async def test_non_core_dev_does_not_downgrade(): # Non-comment review from a non-core dev doesn't "downgrade" the PR's state. data = { "action": "submitted", - "review": {"state": "approved", "user": {"login": non_core_dev,},}, + "review": { + "state": "approved", + "user": { + "login": non_core_dev, + }, + }, "pull_request": { "url": "https://api.github.com/pr/42", "issue_url": "https://api.github.com/issue/42", @@ -515,7 +575,9 @@ async def test_change_requested_for_core_dev(): data = { "action": "submitted", "review": { - "user": {"login": "gvanrossum",}, + "user": { + "login": "gvanrossum", + }, "state": "changes_requested".upper(), }, "pull_request": { @@ -561,7 +623,9 @@ async def test_change_requested_for_non_core_dev(): data = { "action": "submitted", "review": { - "user": {"login": "gvanrossum",}, + "user": { + "login": "gvanrossum", + }, "state": "changes_requested".upper(), }, "pull_request": { @@ -621,7 +685,10 @@ async def test_awaiting_label_removed_when_pr_merged(label): issue_url = "https://api.github.com/repos/org/proj/issues/3749" data = { "action": "closed", - "pull_request": {"merged": True, "issue_url": issue_url,}, + "pull_request": { + "merged": True, + "issue_url": issue_url, + }, } event = sansio.Event(data, event="pull_request", delivery_id="12345") @@ -657,7 +724,10 @@ async def test_awaiting_label_not_removed_when_pr_not_merged(label): issue_url = "https://api.github.com/repos/org/proj/issues/3749" data = { "action": "closed", - "pull_request": {"merged": False, "issue_url": issue_url,}, + "pull_request": { + "merged": False, + "issue_url": issue_url, + }, } event = sansio.Event(data, event="pull_request", delivery_id="12345") @@ -699,7 +769,14 @@ async def test_new_commit_pushed_to_approved_pr(): "number": 5547, "title": "[3.6] bpo-32720: Fixed the replacement field grammar documentation. (GH-5544)", "body": "\n\n`arg_name` and `element_index` are defined as `digit`+ instead of `integer`.\n(cherry picked from commit 7a561afd2c79f63a6008843b83733911d07f0119)\n\nCo-authored-by: Mariatta ", - "labels": [{"name": "CLA signed",}, {"name": "awaiting merge",}], + "labels": [ + { + "name": "CLA signed", + }, + { + "name": "awaiting merge", + }, + ], "issue_url": "/repos/python/cpython/issues/5547", } ], @@ -761,7 +838,14 @@ async def test_new_commit_pushed_to_not_approved_pr(): "number": 5547, "title": "[3.6] bpo-32720: Fixed the replacement field grammar documentation. (GH-5544)", "body": "\n\n`arg_name` and `element_index` are defined as `digit`+ instead of `integer`.\n(cherry picked from commit 7a561afd2c79f63a6008843b83733911d07f0119)\n\nCo-authored-by: Mariatta ", - "labels": [{"name": "CLA signed",}, {"name": "awaiting review",}], + "labels": [ + { + "name": "CLA signed", + }, + { + "name": "awaiting review", + }, + ], "issue_url": "/repos/python/cpython/issues/5547", } ], diff --git a/tests/test_util.py b/tests/test_util.py index bbd34803..7f275b91 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -2,6 +2,7 @@ import gidgethub import pytest +from unittest.mock import patch from bedevere import util @@ -157,3 +158,36 @@ async def test_get_pr_for_commit_not_found(): result = await util.get_pr_for_commit(gh, sha) assert result is None + + +async def test_patch_body_adds_issue_if_not_present(): + """Updates the description of a PR with the gh issue number if it exists. + + returns if body exists with issue_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") + mock.assert_not_called() + with patch.object(gh, "patch") as mock: + vals["body"] = None + await util.patch_body(gh, vals, "1234") + mock.assert_called_once() + with patch.object(gh, "patch") as mock: + vals["body"] = "" + await util.patch_body(gh, vals, "1234") + data = "\n\n\n* gh-1234\n\n" + mock.assert_called_once_with("https://fake.com", data=data) + assert await gh.patch(vals["url"], data=vals) == None