[DRAFT] Implement r+ await

homu/comments.py

@@ -45,6 +45,33 @@ def render(self):
         )
 
 
+class ApprovedAwait(Comment):
+    def __init__(self, bot=None, **args):
+        # Because homu needs to leave a comment for itself to kick off a build,
+        # we need to know the correct botname to use. However, we don't want to
+        # save that botname in our state JSON. So we need a custom constructor
+        # to grab the botname and delegate the rest of the keyword args to the
+        # Comment constructor.
+        super().__init__(**args)
+        self.bot = bot
+
+    params = ["sha", "approver"]
+
+    def render(self):
+        # The comment here is required because Homu wants a full, unambiguous,
+        # pinned commit hash to kick off the build, and this note-to-self is
+        # how it gets it. This is to safeguard against situations where Homu
+        # reloads and another commit has been pushed since the approval.
+        message = ":pushpin: Commit {sha} will be " + \
+            "approved by `{approver}` once CI passes\n\n" + \
+            "<!-- @{bot} r={approver} {sha} await -->"
+        return message.format(
+            sha=self.sha,
+            approver=self.approver,
+            bot=self.bot
+        )
+
+
 class ApprovalIgnoredWip(Comment):
     def __init__(self, wip_keyword=None, **args):
         # We want to use the wip keyword in the message, but not in the json

homu/main.py

@@ -465,6 +465,7 @@ class AuthState(IntEnum):
 
 class LabelEvent(Enum):
     APPROVED = 'approved'
+    APPROVED_AWAITING_CI = 'approved_awaiting_ci'
     REJECTED = 'rejected'
     CONFLICT = 'conflict'
     SUCCEED = 'succeed'
@@ -520,95 +521,33 @@ def parse_commands(body, username, user_id, repo_label, repo_cfg, state,
     for command in commands:
         found = True
         if command.action == 'approve':
-            if not _reviewer_auth_verified():
+            result = _approve_common(
+                _reviewer_auth_verified=_reviewer_auth_verified,
+                command=command,
+                states=states,
+                state=state,
+                username=username,
+                my_username=my_username,
+                comment_factory=comments.Approved,
+                success_label=LabelEvent.APPROVED,
+            )
+            if result == 'skip':
                 continue
 
-            approver = command.actor
-            cur_sha = command.commit
-
-            # Ignore WIP PRs
-            is_wip = False
-            for wip_kw in ['WIP', 'TODO', '[WIP]', '[TODO]', '[DO NOT MERGE]']:
-                if state.title.upper().startswith(wip_kw):
-                    if realtime:
-                        state.add_comment(comments.ApprovalIgnoredWip(
-                            sha=state.head_sha,
-                            wip_keyword=wip_kw,
-                        ))
-                    is_wip = True
-                    break
-            if is_wip:
+        elif command.action == 'approve-await':
+            result = _approve_common(
+                _reviewer_auth_verified=_reviewer_auth_verified,
+                command=command,
+                states=states,
+                state=state,
+                username=username,
+                my_username=my_username,
+                comment_factory=comments.ApprovedAwait,
+                success_label=LabelEvent.APPROVED_AWAITING_CI,
+            )
+            if result == 'skip':
                 continue
 
-            # Sometimes, GitHub sends the head SHA of a PR as 0000000
-            # through the webhook. This is called a "null commit", and
-            # seems to happen when GitHub internally encounters a race
-            # condition. Last time, it happened when squashing commits
-            # in a PR. In this case, we just try to retrieve the head
-            # SHA manually.
-            if all(x == '0' for x in state.head_sha):
-                if realtime:
-                    state.add_comment(
-                        ':bangbang: Invalid head SHA found, retrying: `{}`'
-                        .format(state.head_sha)
-                    )
-
-                state.head_sha = state.get_repo().pull_request(state.num).head.sha  # noqa
-                state.save()
-
-                assert any(x != '0' for x in state.head_sha)
-
-            if state.approved_by and realtime and username != my_username:
-                for _state in states[state.repo_label].values():
-                    if _state.status == 'pending':
-                        break
-                else:
-                    _state = None
-
-                lines = []
-
-                if state.status in ['failure', 'error']:
-                    lines.append('- This pull request previously failed. You should add more commits to fix the bug, or use `retry` to trigger a build again.')  # noqa
-
-                if _state:
-                    if state == _state:
-                        lines.append('- This pull request is currently being tested. If there\'s no response from the continuous integration service, you may use `retry` to trigger a build again.')  # noqa
-                    else:
-                        lines.append('- There\'s another pull request that is currently being tested, blocking this pull request: #{}'.format(_state.num))  # noqa
-
-                if lines:
-                    lines.insert(0, '')
-                lines.insert(0, ':bulb: This pull request was already approved, no need to approve it again.')  # noqa
-
-                state.add_comment('\n'.join(lines))
-
-            if sha_cmp(cur_sha, state.head_sha):
-                state.approved_by = approver
-                state.try_ = False
-                state.set_status('')
-
-                state.save()
-            elif realtime and username != my_username:
-                if cur_sha:
-                    msg = '`{}` is not a valid commit SHA.'.format(cur_sha)
-                    state.add_comment(
-                        ':scream_cat: {} Please try again with `{}`.'
-                        .format(msg, state.head_sha)
-                    )
-                else:
-                    state.add_comment(comments.Approved(
-                        sha=state.head_sha,
-                        approver=approver,
-                        bot=my_username,
-                    ))
-                    treeclosed = state.blocked_by_closed_tree()
-                    if treeclosed:
-                        state.add_comment(
-                            ':evergreen_tree: The tree is currently closed for pull requests below priority {}, this pull request will be tested once the tree is reopened'  # noqa
-                            .format(treeclosed)
-                        )
-                    state.change_labels(LabelEvent.APPROVED)
-
         elif command.action == 'unapprove':
             # Allow the author of a pull request to unapprove their own PR. The
             # author can already perform other actions that effectively
@@ -807,6 +746,107 @@ def parse_commands(body, username, user_id, repo_label, repo_cfg, state,
     return state_changed
 
 
+def _approve_common(
+    _reviewer_auth_verified,
+    command,
+    states,
+    state,
+    username,
+    my_username,
+    realtime,
+    comment_factory,
+    success_label,
+):
+    if not _reviewer_auth_verified():
+        return 'skip'
+
+    approver = command.actor
+    cur_sha = command.commit
+
+    # Ignore WIP PRs
+    is_wip = False
+    for wip_kw in ['WIP', 'TODO', '[WIP]', '[TODO]', '[DO NOT MERGE]']:
+        if state.title.upper().startswith(wip_kw):
+            if realtime:
+                state.add_comment(comments.ApprovalIgnoredWip(
+                    sha=state.head_sha,
+                    wip_keyword=wip_kw,
+                ))
+            is_wip = True
+            break
+    if is_wip:
+        return 'skip'
+
+    # Sometimes, GitHub sends the head SHA of a PR as 0000000
+    # through the webhook. This is called a "null commit", and
+    # seems to happen when GitHub internally encounters a race
+    # condition. Last time, it happened when squashing commits
+    # in a PR. In this case, we just try to retrieve the head
+    # SHA manually.
+    if all(x == '0' for x in state.head_sha):
+        if realtime:
+            state.add_comment(
+                ':bangbang: Invalid head SHA found, retrying: `{}`'
+                .format(state.head_sha)
+            )
+
+        state.head_sha = state.get_repo().pull_request(state.num).head.sha  # noqa
+        state.save()
+
+        assert any(x != '0' for x in state.head_sha)
+
+    if state.approved_by and realtime and username != my_username:
+        for _state in states[state.repo_label].values():
+            if _state.status == 'pending':
+                break
+        else:
+            _state = None
+
+        lines = []
+
+        if state.status in ['failure', 'error']:
+            lines.append('- This pull request previously failed. You should add more commits to fix the bug, or use `retry` to trigger a build again.')  # noqa
+
+        if _state:
+            if state == _state:
+                lines.append('- This pull request is currently being tested. If there\'s no response from the continuous integration service, you may use `retry` to trigger a build again.')  # noqa
+            else:
+                lines.append('- There\'s another pull request that is currently being tested, blocking this pull request: #{}'.format(_state.num))  # noqa
+
+        if lines:
+            lines.insert(0, '')
+        lines.insert(0, ':bulb: This pull request was already approved, no need to approve it again.')  # noqa
+
+        state.add_comment('\n'.join(lines))
+
+    if sha_cmp(cur_sha, state.head_sha):
+        state.approved_by = approver
+        state.try_ = False
+        state.set_status('')
+
+        state.save()
+    elif realtime and username != my_username:
+        if cur_sha:
+            msg = '`{}` is not a valid commit SHA.'.format(cur_sha)
+            state.add_comment(
+                ':scream_cat: {} Please try again with `{}`.'
+                .format(msg, state.head_sha)
+            )
+        else:
+            state.add_comment(comment_factory(
+                sha=state.head_sha,
+                approver=approver,
+                bot=my_username,
+            ))
+            treeclosed = state.blocked_by_closed_tree()
+            if treeclosed:
+                state.add_comment(
+                    ':evergreen_tree: The tree is currently closed for pull requests below priority {}, this pull request will be tested once the tree is reopened'  # noqa
+                    .format(treeclosed)
+                )
+            state.change_labels(success_label)
+
+
 def handle_hook_response(state, hook_cfg, body, extra_data):
     post_data = {}
     post_data["pull"] = state.num

homu/parse_issue_comment.py

@@ -28,6 +28,13 @@ def approve(cls, approver, commit):
         command.actor = approver.lstrip('@')
         return command
 
+    @classmethod
+    def approve_await(cls, approver, commit):
+        command = cls('approve-await')
+        command.commit = commit
+        command.actor = approver.lstrip('@')
+        return command
+
     @classmethod
     def unapprove(cls):
         return cls('unapprove')
@@ -183,15 +190,26 @@ def parse_issue_comment(username, body, sha, botname, hooks=[]):
             if i + 1 < len(words) and is_sha(words[i + 1]):
                 approved_sha = words[i + 1]
                 words[i + 1] = None
+                i += 1
 
             approver = word[len('r='):] if word.startswith('r=') else username
 
             # Ignore "r=me"
             if approver == 'me':
                 continue
 
-            commands.append(
-                    IssueCommentCommand.approve(approver, approved_sha))
+            await_ci = False
+            if i + 1 < len(words) and words[i + 1] == 'await':
+                await_ci = True
+                words[i + 1] = None
+                i += 1
+
+            if await_ci:
+                commands.append(
+                        IssueCommentCommand.approve_await(approver, approved_sha))  # noqa
+            else:
+                commands.append(
+                        IssueCommentCommand.approve(approver, approved_sha))
 
         elif word == 'r-':
             commands.append(IssueCommentCommand.unapprove())

homu/tests/test_parse_issue_comment.py

@@ -53,6 +53,37 @@ def test_r_plus_with_sha():
     assert command.commit == other_commit
 
 
+def test_r_plus_await():
+    """
+    @bors r+ await
+    """
+
+    author = "jack"
+    body = "@bors r+ await"
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'approve-await'
+    assert command.actor == 'jack'
+
+
+def test_r_plus_await_with_sha():
+    """
+    @bors r+ {sha} await
+    """
+
+    author = "jack"
+    body = "@bors r+ {} await".format(other_commit)
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'approve-await'
+    assert command.actor == 'jack'
+    assert command.commit == other_commit
+
+
 def test_r_equals():
     """
     @bors r=jill
@@ -83,6 +114,36 @@ def test_r_equals_at_user():
     assert command.actor == 'jill'
 
 
+def test_r_equals_await():
+    """
+    @bors r=jill await
+    """
+
+    author = "jack"
+    body = "@bors r=jill await"
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'approve-await'
+    assert command.actor == 'jill'
+
+
+def test_r_equals_at_user_await():
+    """
+    @bors r=@jill await
+    """
+
+    author = "jack"
+    body = "@bors r=@jill await"
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'approve-await'
+    assert command.actor == 'jill'
+
+
 def test_hidden_r_equals():
     author = "bors"
     body = """