diff --git a/miss_islington/delete_branch.py b/miss_islington/delete_branch.py index de0a3ed9e..b9c7c6222 100644 --- a/miss_islington/delete_branch.py +++ b/miss_islington/delete_branch.py @@ -1,3 +1,5 @@ +import asyncio + import gidgethub.routing router = gidgethub.routing.Router() @@ -11,4 +13,12 @@ async def delete_branch(event, gh, *args, **kwargs): if event.data["pull_request"]["user"]["login"] == "miss-islington": branch_name = event.data["pull_request"]["head"]["ref"] url = f"/repos/miss-islington/cpython/git/refs/heads/{branch_name}" - await gh.delete(url) + if event.data["pull_request"]["merged"]: + await gh.delete(url) + else: + # this is delayed to ensure that the bot doesn't remove the branch + # if PR was closed and reopened to rerun checks (or similar) + await asyncio.sleep(60) + updated_data = await gh.getitem(event.data["pull_request"]["url"]) + if updated_data["state"] == "closed": + await gh.delete(url) diff --git a/tests/test_delete_branch.py b/tests/test_delete_branch.py index 7b7a48936..32c8fa183 100644 --- a/tests/test_delete_branch.py +++ b/tests/test_delete_branch.py @@ -1,16 +1,29 @@ +import asyncio + from gidgethub import sansio from miss_islington import delete_branch class FakeGH: - def __init__(self): + def __init__(self, *, getitem=None): + self._getitem_return = getitem + self.getitem_url = None self.post_data = None + async def getitem(self, url): + self.getitem_url = url + to_return = self._getitem_return[self.getitem_url] + return to_return + async def delete(self, url): self.delete_url = url +async def noop_sleep(delay, result=None): + pass + + async def test_branch_deleted_when_pr_merged(): data = { "action": "closed", @@ -77,7 +90,7 @@ async def test_branch_deleted_and_thanks(): ) -async def test_branch_deleted_when_pr_closed(): +async def test_branch_deleted_when_pr_closed(monkeypatch): data = { "action": "closed", "pull_request": { @@ -86,11 +99,16 @@ async def test_branch_deleted_when_pr_closed(): "merged": False, "merged_by": {"login": None}, "head": {"ref": "backport-17ab8f0-3.7"}, + "url": "https://api.github.com/repos/python/cpython/pulls/5722", }, } event = sansio.Event(data, event="pull_request", delivery_id="1") + getitem = { + "https://api.github.com/repos/python/cpython/pulls/5722": {"state": "closed"}, + } - gh = FakeGH() + monkeypatch.setattr(asyncio, "sleep", noop_sleep) + gh = FakeGH(getitem=getitem) await delete_branch.router.dispatch(event, gh) assert gh.post_data is None # does not leave a comment assert ( @@ -99,6 +117,30 @@ async def test_branch_deleted_when_pr_closed(): ) +async def test_branch_not_deleted_when_pr_closed_and_reopened(monkeypatch): + data = { + "action": "closed", + "pull_request": { + "number": 5722, + "user": {"login": "miss-islington"}, + "merged": False, + "merged_by": {"login": None}, + "head": {"ref": "backport-17ab8f0-3.7"}, + "url": "https://api.github.com/repos/python/cpython/pulls/5722", + }, + } + event = sansio.Event(data, event="pull_request", delivery_id="1") + getitem = { + "https://api.github.com/repos/python/cpython/pulls/5722": {"state": "opened"}, + } + + monkeypatch.setattr(asyncio, "sleep", noop_sleep) + gh = FakeGH(getitem=getitem) + await delete_branch.router.dispatch(event, gh) + assert gh.post_data is None # does not leave a comment + assert not hasattr(gh, "delete_url") + + async def test_ignore_non_miss_islingtons_prs(): data = { "action": "closed",