Don't immediately delete PR's branch in case it's close&reopen (#483)

* Don't immediately delete PR's branch in case it's close&reopen

* Add missing import

* Add test

Co-authored-by: Jelle Zijlstra <[email protected]>
Co-authored-by: Mariatta Wijaya <[email protected]>

Author: 3 people

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)

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",