From e885dd7099ac740a21f30ee9d5a091de2f002b7c Mon Sep 17 00:00:00 2001
From: Bryan Burgers <bryan@burgers.io>
Date: Wed, 22 May 2019 09:51:28 -0500
Subject: [PATCH 1/4] Test the command parsing logic

Start adding some tests to Homu by testing the issue comment
command-parsing logic.

Take `parse_commands` and break it apart into two phases

* Parsing phase
* Execution phase

The parsing phase returns a list of commands with action names (ideally,
this would be a Rust enum, but to simulate that, we use action names on
a single class) that are returned regardless of the current state.  So
for example, `@bors retry` will return a `retry` command regardless of
the current state of `realtime`.

The execution phase then inspects the list of commands and decides what
to do with them. So for example, the `retry` command will be skipped if
`realtime == False`.

This has the positive result of having a parsing phase that has no
side-effects, which makes it much easier to test. This can lead to
higher confidence that the code works as expected without the high cost
of testing in production and possibly disrupting the build flow.

This has the negative result of adding a lot of lines of code to achieve
command parsing, which we already do successfully without the tests.
---
 .gitignore                             |   1 +
 .travis.yml                            |   4 +-
 homu/main.py                           | 125 +++---
 homu/parse_issue_comment.py            | 246 ++++++++++++
 homu/tests/__init__.py                 |   0
 homu/tests/test_parse_issue_comment.py | 505 +++++++++++++++++++++++++
 setup.cfg                              |   2 +
 setup.py                               |   7 +
 8 files changed, 820 insertions(+), 70 deletions(-)
 create mode 100644 homu/parse_issue_comment.py
 create mode 100644 homu/tests/__init__.py
 create mode 100644 homu/tests/test_parse_issue_comment.py
 create mode 100644 setup.cfg

diff --git a/.gitignore b/.gitignore
index 258e93f..8be4882 100644
--- a/.gitignore
+++ b/.gitignore
@@ -5,3 +5,4 @@
 /homu.egg-info/
 /main.db
 /cache
+*.pyc
diff --git a/.travis.yml b/.travis.yml
index 5f35324..9a8a947 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -6,4 +6,6 @@ python:
 install:
   - pip install flake8
 script:
-  - flake8 homu
\ No newline at end of file
+  - flake8 homu
+  - pip install -e .
+  - python setup.py test
diff --git a/homu/main.py b/homu/main.py
index 97ed61b..59b8c00 100644
--- a/homu/main.py
+++ b/homu/main.py
@@ -6,6 +6,7 @@
 import functools
 from . import comments
 from . import utils
+from .parse_issue_comment import parse_issue_comment
 from .auth import verify as verify_auth
 from .utils import lazy_debug
 import logging
@@ -15,7 +16,6 @@
 import sqlite3
 import requests
 from contextlib import contextmanager
-from itertools import chain
 from queue import Queue
 import os
 import sys
@@ -476,28 +476,20 @@ def parse_commands(body, username, repo_label, repo_cfg, state, my_username,
         my_username,
     )
 
-    words = list(chain.from_iterable(re.findall(r'\S+', x) for x in body.splitlines() if '@' + my_username in x))  # noqa
-    if words[1:] == ["are", "you", "still", "there?"] and realtime:
-        state.add_comment(
-            ":cake: {}\n\n![]({})".format(
-                random.choice(PORTAL_TURRET_DIALOG), PORTAL_TURRET_IMAGE)
-            )
-    for i, word in reversed(list(enumerate(words))):
+    hooks = []
+    if 'hooks' in global_cfg:
+        hooks = list(global_cfg['hooks'].keys())
+
+    commands = parse_issue_comment(username, body, sha, my_username, hooks)
+
+    for command in commands:
         found = True
-        if word == 'r+' or word.startswith('r='):
+        if command.action == 'approve':
             if not _reviewer_auth_verified():
                 continue
 
-            if not sha and i + 1 < len(words):
-                cur_sha = sha_or_blank(words[i + 1])
-            else:
-                cur_sha = sha
-
-            approver = word[len('r='):] if word.startswith('r=') else username
-
-            # Ignore "r=me"
-            if approver == 'me':
-                continue
+            approver = command.actor
+            cur_sha = command.commit
 
             # Ignore WIP PRs
             is_wip = False
@@ -582,7 +574,7 @@ def parse_commands(body, username, repo_label, repo_cfg, state, my_username,
                         )
                     state.change_labels(LabelEvent.APPROVED)
 
-        elif word == 'r-':
+        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
             # unapprove the PR (change the target branch, push more commits,
@@ -601,14 +593,12 @@ def parse_commands(body, username, repo_label, repo_cfg, state, my_username,
             if realtime:
                 state.change_labels(LabelEvent.REJECTED)
 
-        elif word.startswith('p='):
+        elif command.action == 'prioritize':
             if not verify_auth(username, repo_label, repo_cfg, state,
                                AuthState.TRY, realtime, my_username):
                 continue
-            try:
-                pvalue = int(word[len('p='):])
-            except ValueError:
-                continue
+
+            pvalue = command.priority
 
             if pvalue > global_cfg['max_priority']:
                 if realtime:
@@ -620,12 +610,12 @@ def parse_commands(body, username, repo_label, repo_cfg, state, my_username,
             state.priority = pvalue
             state.save()
 
-        elif word.startswith('delegate='):
+        elif command.action == 'delegate':
             if not verify_auth(username, repo_label, repo_cfg, state,
                                AuthState.REVIEWER, realtime, my_username):
                 continue
 
-            state.delegate = word[len('delegate='):]
+            state.delegate = command.delegate_to
             state.save()
 
             if realtime:
@@ -634,14 +624,14 @@ def parse_commands(body, username, repo_label, repo_cfg, state, my_username,
                     delegate=state.delegate
                 ))
 
-        elif word == 'delegate-':
+        elif command.action == 'undelegate':
             # TODO: why is this a TRY?
             if not _try_auth_verified():
                 continue
             state.delegate = ''
             state.save()
 
-        elif word == 'delegate+':
+        elif command.action == 'delegate-author':
             if not _reviewer_auth_verified():
                 continue
 
@@ -654,7 +644,7 @@ def parse_commands(body, username, repo_label, repo_cfg, state, my_username,
                     delegate=state.delegate
                 ))
 
-        elif word == 'retry' and realtime:
+        elif command.action == 'retry' and realtime:
             if not _try_auth_verified():
                 continue
             state.set_status('')
@@ -663,7 +653,7 @@ def parse_commands(body, username, repo_label, repo_cfg, state, my_username,
                 state.record_retry_log(command_src, body)
                 state.change_labels(event)
 
-        elif word in ['try', 'try-'] and realtime:
+        elif command.action in ['try', 'untry'] and realtime:
             if not _try_auth_verified():
                 continue
             if state.status == '' and state.approved_by:
@@ -674,7 +664,7 @@ def parse_commands(body, username, repo_label, repo_cfg, state, my_username,
                 )
                 continue
 
-            state.try_ = word == 'try'
+            state.try_ = command.action == 'try'
 
             state.merge_sha = ''
             state.init_build_res([])
@@ -689,14 +679,14 @@ def parse_commands(body, username, repo_label, repo_cfg, state, my_username,
                 # any meaningful labeling events.
                 state.change_labels(LabelEvent.TRY)
 
-        elif word in WORDS_TO_ROLLUP:
+        elif command.action == 'rollup':
             if not _try_auth_verified():
                 continue
-            state.rollup = WORDS_TO_ROLLUP[word]
+            state.rollup = command.rollup_value
 
             state.save()
 
-        elif word == 'force' and realtime:
+        elif command.action == 'force' and realtime:
             if not _try_auth_verified():
                 continue
             if 'buildbot' in repo_cfg:
@@ -725,52 +715,51 @@ def parse_commands(body, username, repo_label, repo_cfg, state, my_username,
                     ':bomb: Buildbot returned an error: `{}`'.format(err)
                 )
 
-        elif word == 'clean' and realtime:
+        elif command.action == 'clean' and realtime:
             if not _try_auth_verified():
                 continue
             state.merge_sha = ''
             state.init_build_res([])
 
             state.save()
-        elif (word == 'hello?' or word == 'ping') and realtime:
-            state.add_comment(":sleepy: I'm awake I'm awake")
-        elif word.startswith('treeclosed='):
+
+        elif command.action == 'ping' and realtime:
+            if command.ping_type == 'portal':
+                state.add_comment(
+                    ":cake: {}\n\n![]({})".format(
+                        random.choice(PORTAL_TURRET_DIALOG),
+                        PORTAL_TURRET_IMAGE)
+                    )
+            else:
+                state.add_comment(":sleepy: I'm awake I'm awake")
+
+        elif command.action == 'treeclosed':
             if not _reviewer_auth_verified():
                 continue
-            try:
-                treeclosed = int(word[len('treeclosed='):])
-                state.change_treeclosed(treeclosed, command_src)
-            except ValueError:
-                pass
+            state.change_treeclosed(command.treeclosed_value, command_src)
             state.save()
-        elif word == 'treeclosed-':
+
+        elif command.action == 'untreeclosed':
             if not _reviewer_auth_verified():
                 continue
             state.change_treeclosed(-1, None)
             state.save()
-        elif 'hooks' in global_cfg:
-            hook_found = False
-            for hook in global_cfg['hooks']:
-                hook_cfg = global_cfg['hooks'][hook]
-                if hook_cfg['realtime'] and not realtime:
+
+        elif command.action == 'hook':
+            hook = command.hook_name
+            hook_cfg = global_cfg['hooks'][hook]
+            if hook_cfg['realtime'] and not realtime:
+                continue
+            if hook_cfg['access'] == "reviewer":
+                if not _reviewer_auth_verified():
                     continue
-                if word == hook or word.startswith('%s=' % hook):
-                    if hook_cfg['access'] == "reviewer":
-                        if not _reviewer_auth_verified():
-                            continue
-                    else:
-                        if not _try_auth_verified():
-                            continue
-                    hook_found = True
-                    extra_data = ""
-                    if word.startswith('%s=' % hook):
-                        extra_data = word.split("=")[1]
-                    Thread(
-                        target=handle_hook_response,
-                        args=[state, hook_cfg, body, extra_data]
-                    ).start()
-            if not hook_found:
-                found = False
+            else:
+                if not _try_auth_verified():
+                    continue
+            Thread(
+                target=handle_hook_response,
+                args=[state, hook_cfg, body, command.hook_extra]
+            ).start()
 
         else:
             found = False
@@ -778,8 +767,6 @@ def parse_commands(body, username, repo_label, repo_cfg, state, my_username,
         if found:
             state_changed = True
 
-            words[i] = ''
-
     return state_changed
 
 
diff --git a/homu/parse_issue_comment.py b/homu/parse_issue_comment.py
new file mode 100644
index 0000000..7fdd3c5
--- /dev/null
+++ b/homu/parse_issue_comment.py
@@ -0,0 +1,246 @@
+from itertools import chain
+import re
+
+
+class IssueCommentCommand:
+    """
+    A command that has been parsed out of a GitHub issue comment.
+
+    E.g., `@bors r+` => an issue command with action == 'approve'
+    """
+
+    def __init__(self, action):
+        self.action = action
+
+    @classmethod
+    def approve(cls, approver, commit):
+        command = cls('approve')
+        command.commit = commit
+        command.actor = approver
+        return command
+
+    @classmethod
+    def unapprove(cls):
+        return cls('unapprove')
+
+    @classmethod
+    def prioritize(cls, priority):
+        command = cls('prioritize')
+        command.priority = priority
+        return command
+
+    @classmethod
+    def delegate_author(cls):
+        return cls('delegate-author')
+
+    @classmethod
+    def delegate(cls, delegate_to):
+        command = cls('delegate')
+        command.delegate_to = delegate_to
+        return command
+
+    @classmethod
+    def undelegate(cls):
+        return cls('undelegate')
+
+    @classmethod
+    def retry(cls):
+        return cls('retry')
+
+    @classmethod
+    def try_(cls):
+        return cls('try')
+
+    @classmethod
+    def untry(cls):
+        return cls('untry')
+
+    @classmethod
+    def rollup(cls, rollup_value):
+        command = cls('rollup')
+        command.rollup_value = rollup_value
+        return command
+
+    @classmethod
+    def force(cls):
+        return cls('force')
+
+    @classmethod
+    def clean(cls):
+        return cls('clean')
+
+    @classmethod
+    def ping(cls, ping_type='standard'):
+        command = cls('ping')
+        command.ping_type = ping_type
+        return command
+
+    @classmethod
+    def treeclosed(cls, treeclosed_value):
+        command = cls('treeclosed')
+        command.treeclosed_value = treeclosed_value
+        return command
+
+    @classmethod
+    def untreeclosed(cls):
+        return cls('untreeclosed')
+
+    @classmethod
+    def hook(cls, hook_name, hook_extra=None):
+        command = cls('hook')
+        command.hook_name = hook_name
+        command.hook_extra = hook_extra
+        return command
+
+
+WORDS_TO_ROLLUP = {
+    'rollup-': 0,
+    'rollup': 1,
+    'rollup=maybe': 0,
+    'rollup=never': -1,
+    'rollup=always': 1,
+}
+
+
+def is_sha(sha):
+    """
+    Try to determine if the input is a git sha
+    """
+    return re.match(r'^[0-9a-f]{4,}$', sha)
+
+
+def hook_with_extra_is_in_hooks(word, hooks):
+    """
+    Determine if the word given is the name of a valid hook, with extra data
+    hanging off of it (e.g., `validhookname=extradata`).
+
+       hook_with_extra_is_in_hooks(
+         'validhookname=stuff',
+         ['validhookname', 'other'])
+       #=> True
+
+       hook_with_extra_is_in_hooks(
+         'invalidhookname=stuff',
+         ['validhookname', 'other'])
+       #=> False
+
+       hook_with_extra_is_in_hooks(
+         'validhookname',
+         ['validhookname', 'other'])
+       #=> False
+    """
+    for hook in hooks:
+        if word.startswith('{}='.format(hook)):
+            return True
+
+    return False
+
+
+def parse_issue_comment(username, body, sha, botname, hooks=[]):
+    """
+    Parse an issue comment looking for commands that Homu should handle
+
+    Parameters:
+    username: the username of the user that created the issue comment.
+           This is without the leading @
+    body: the full body of the comment (markdown)
+    sha: the commit that the comment applies to
+    botname: the name of bot. This is without the leading @.
+           So if we should respond to `@bors {command}`, botname will be `bors`
+    hooks: a list of strings that are valid hook names.
+           E.g. `['hook1', 'hook2', 'hook3']`
+    """
+
+    words = list(chain.from_iterable(re.findall(r'\S+', x) for x in body.splitlines() if '@' + botname in x))  # noqa
+
+    commands = []
+
+    if words[1:] == ["are", "you", "still", "there?"]:
+        commands.append(IssueCommentCommand.ping('portal'))
+
+    for i, word in reversed(list(enumerate(words))):
+        found = True
+        if word == 'r+' or word.startswith('r='):
+            approved_sha = sha
+
+            if i + 1 < len(words) and is_sha(words[i + 1]):
+                approved_sha = words[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))
+
+        elif word == 'r-':
+            commands.append(IssueCommentCommand.unapprove())
+
+        elif word.startswith('p='):
+            try:
+                pvalue = int(word[len('p='):])
+            except ValueError:
+                continue
+
+            commands.append(IssueCommentCommand.prioritize(pvalue))
+
+        elif word.startswith('delegate='):
+            delegate = word[len('delegate='):]
+            commands.append(IssueCommentCommand.delegate(delegate))
+
+        elif word == 'delegate-':
+            commands.append(IssueCommentCommand.undelegate())
+
+        elif word == 'delegate+':
+            commands.append(IssueCommentCommand.delegate_author())
+
+        elif word == 'retry':
+            commands.append(IssueCommentCommand.retry())
+
+        elif word == 'try':
+            commands.append(IssueCommentCommand.try_())
+
+        elif word == 'try-':
+            commands.append(IssueCommentCommand.untry())
+
+        elif word in WORDS_TO_ROLLUP:
+            rollup_value = WORDS_TO_ROLLUP[word]
+            commands.append(IssueCommentCommand.rollup(rollup_value))
+
+        elif word == 'force':
+            commands.append(IssueCommentCommand.force())
+
+        elif word == 'clean':
+            commands.append(IssueCommentCommand.clean())
+
+        elif (word == 'hello?' or word == 'ping'):
+            commands.append(IssueCommentCommand.ping())
+
+        elif word.startswith('treeclosed='):
+            try:
+                treeclosed = int(word[len('treeclosed='):])
+                commands.append(IssueCommentCommand.treeclosed(treeclosed))
+            except ValueError:
+                pass
+
+        elif word == 'treeclosed-':
+            commands.append(IssueCommentCommand.untreeclosed())
+
+        elif word in hooks:
+            commands.append(IssueCommentCommand.hook(word))
+
+        elif hook_with_extra_is_in_hooks(word, hooks):
+            # word is like `somehook=data` and `somehook` is in our list of
+            # potential hooks
+            (hook_name, hook_extra) = word.split('=', 2)
+            commands.append(IssueCommentCommand.hook(hook_name, hook_extra))
+
+        else:
+            found = False
+
+        if found:
+            words[i] = ''
+
+    return commands
diff --git a/homu/tests/__init__.py b/homu/tests/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git a/homu/tests/test_parse_issue_comment.py b/homu/tests/test_parse_issue_comment.py
new file mode 100644
index 0000000..3db4a77
--- /dev/null
+++ b/homu/tests/test_parse_issue_comment.py
@@ -0,0 +1,505 @@
+from homu.parse_issue_comment import parse_issue_comment
+
+# Random commit number. Just so that we don't need to come up with a new one
+# for every test.
+commit = "5ffafdb1e94fa87334d4851a57564425e11a569e"
+other_commit = "4e4c9ddd781729173df2720d83e0f4d1b0102a94"
+
+
+def test_r_plus():
+    """
+    @bors r+
+    """
+
+    author = "jack"
+    body = "@bors r+"
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'approve'
+    assert command.actor == 'jack'
+
+
+def test_r_plus_with_sha():
+    """
+    @bors r+ {sha}
+    """
+
+    author = "jack"
+    body = "@bors r+ {}".format(other_commit)
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'approve'
+    assert command.actor == 'jack'
+    assert command.commit == other_commit
+
+
+def test_r_equals():
+    """
+    @bors r=jill
+    """
+
+    author = "jack"
+    body = "@bors r=jill"
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'approve'
+    assert command.actor == 'jill'
+
+
+def test_r_me():
+    """
+    Ignore r=me
+    """
+
+    author = "jack"
+    body = "@bors r=me"
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    # r=me is not a valid command, so no valid commands.
+    assert len(commands) == 0
+
+
+def test_r_minus():
+    """
+    @bors r-
+    """
+
+    author = "jack"
+    body = "@bors r-"
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'unapprove'
+
+
+def test_priority():
+    """
+    @bors p=5
+    """
+
+    author = "jack"
+    body = "@bors p=5"
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'prioritize'
+    assert command.priority == 5
+
+
+def test_approve_and_priority():
+    """
+    @bors r+ p=5
+    """
+
+    author = "jack"
+    body = "@bors r+ p=5"
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 2
+    approve_commands = [command for command in commands
+                        if command.action == 'approve']
+    prioritize_commands = [command for command in commands
+                           if command.action == 'prioritize']
+    assert len(approve_commands) == 1
+    assert len(prioritize_commands) == 1
+
+    assert approve_commands[0].actor == 'jack'
+    assert prioritize_commands[0].priority == 5
+
+
+def test_approve_specific_and_priority():
+    """
+    @bors r+ {sha} p=5
+    """
+
+    author = "jack"
+    body = "@bors r+ {} p=5".format(other_commit)
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 2
+    approve_commands = [command for command in commands
+                        if command.action == 'approve']
+    prioritize_commands = [command for command in commands
+                           if command.action == 'prioritize']
+    assert len(approve_commands) == 1
+    assert len(prioritize_commands) == 1
+
+    assert approve_commands[0].actor == 'jack'
+    assert approve_commands[0].commit == other_commit
+    assert prioritize_commands[0].priority == 5
+
+
+def test_delegate_plus():
+    """
+    @bors delegate+
+    """
+
+    author = "jack"
+    body = "@bors delegate+"
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'delegate-author'
+
+
+def test_delegate_equals():
+    """
+    @bors delegate={username}
+    """
+
+    author = "jack"
+    body = "@bors delegate=jill"
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'delegate'
+    assert command.delegate_to == 'jill'
+
+
+def test_delegate_minus():
+    """
+    @bors delegate-
+    """
+
+    author = "jack"
+    body = "@bors delegate-"
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'undelegate'
+
+
+def test_retry():
+    """
+    @bors retry
+    """
+
+    author = "jack"
+    body = "@bors retry"
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'retry'
+
+
+def test_try():
+    """
+    @bors try
+    """
+
+    author = "jack"
+    body = "@bors try"
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'try'
+
+
+def test_try_minus():
+    """
+    @bors try-
+    """
+
+    author = "jack"
+    body = "@bors try-"
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'untry'
+
+
+def test_rollup():
+    """
+    @bors rollup
+    """
+
+    author = "jack"
+    body = "@bors rollup"
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'rollup'
+    assert command.rollup_value == 1
+
+
+def test_rollup_minus():
+    """
+    @bors rollup-
+    """
+
+    author = "jack"
+    body = "@bors rollup-"
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'rollup'
+    assert command.rollup_value == 0
+
+
+def test_rollup_never():
+    """
+    @bors rollup=never
+    """
+
+    author = "jack"
+    body = "@bors rollup=never"
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'rollup'
+    assert command.rollup_value == -1
+
+
+def test_rollup_maybe():
+    """
+    @bors rollup=maybe
+    """
+
+    author = "jack"
+    body = "@bors rollup=maybe"
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'rollup'
+    assert command.rollup_value == 0
+
+
+def test_rollup_always():
+    """
+    @bors rollup=always
+    """
+
+    author = "jack"
+    body = "@bors rollup=always"
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'rollup'
+    assert command.rollup_value == 1
+
+
+def test_force():
+    """
+    @bors force
+    """
+
+    author = "jack"
+    body = "@bors force"
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'force'
+
+
+def test_clean():
+    """
+    @bors clean
+    """
+
+    author = "jack"
+    body = "@bors clean"
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'clean'
+
+
+def test_ping():
+    """
+    @bors ping
+    """
+
+    author = "jack"
+    body = "@bors ping"
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'ping'
+    assert command.ping_type == 'standard'
+
+
+def test_hello():
+    """
+    @bors hello?
+    """
+
+    author = "jack"
+    body = "@bors hello?"
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'ping'
+    assert command.ping_type == 'standard'
+
+
+def test_portal_ping():
+    """
+    @bors are you still there?
+    """
+
+    author = "jack"
+    body = "@bors are you still there?"
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'ping'
+    assert command.ping_type == 'portal'
+
+
+def test_treeclosed():
+    """
+    @bors treeclosed=50
+    """
+
+    author = "jack"
+    body = "@bors treeclosed=50"
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'treeclosed'
+    assert command.treeclosed_value == 50
+
+
+def test_treeclosed_minus():
+    """
+    @bors treeclosed-
+    """
+
+    author = "jack"
+    body = "@bors treeclosed-"
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'untreeclosed'
+
+
+def test_hook():
+    """
+    Test hooks that are defined in the configuration
+
+    @bors secondhook
+    """
+
+    author = "jack"
+    body = "@bors secondhook"
+    commands = parse_issue_comment(
+            author, body, commit, "bors",
+            ['firsthook', 'secondhook', 'thirdhook'])
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'hook'
+    assert command.hook_name == 'secondhook'
+    assert command.hook_extra is None
+
+
+def test_hook_equals():
+    """
+    Test hooks that are defined in the configuration
+
+    @bors secondhook=extra
+    """
+
+    author = "jack"
+    body = "@bors secondhook=extra"
+    commands = parse_issue_comment(
+            author, body, commit, "bors",
+            ['firsthook', 'secondhook', 'thirdhook'])
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'hook'
+    assert command.hook_name == 'secondhook'
+    assert command.hook_extra == 'extra'
+
+
+def test_multiple_hooks():
+    """
+    Test hooks that are defined in the configuration
+
+    @bors thirdhook secondhook=extra
+    """
+
+    author = "jack"
+    body = "@bors thirdhook secondhook=extra"
+    commands = parse_issue_comment(
+            author, body, commit, "bors",
+            ['firsthook', 'secondhook', 'thirdhook'])
+
+    assert len(commands) == 2
+    secondhook_commands = [command for command in commands
+                           if command.action == 'hook'
+                           and command.hook_name == 'secondhook']
+    thirdhook_commands = [command for command in commands
+                          if command.action == 'hook'
+                          and command.hook_name == 'thirdhook']
+    assert len(secondhook_commands) == 1
+    assert len(thirdhook_commands) == 1
+    assert secondhook_commands[0].hook_extra == 'extra'
+    assert thirdhook_commands[0].hook_extra is None
+
+
+def test_ignore_commands_before_bors_line():
+    """
+    Test that when command-like statements appear before the @bors part,
+    they don't get parsed
+    """
+
+    author = "jack"
+    body = """
+    A sentence that includes command-like statements, like r- or ping or delegate+ or the like.
+
+    @bors r+
+    """ # noqa
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'approve'
+    assert command.actor == 'jack'
+
+
+def test_ignore_commands_after_bors_line():
+    """
+    Test that when command-like statements appear after the @bors part,
+    they don't get parsed
+    """
+
+    author = "jack"
+    body = """
+    @bors r+
+
+    A sentence that includes command-like statements, like r- or ping or delegate+ or the like.
+    """ # noqa
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'approve'
+    assert command.actor == 'jack'
diff --git a/setup.cfg b/setup.cfg
new file mode 100644
index 0000000..b7e4789
--- /dev/null
+++ b/setup.cfg
@@ -0,0 +1,2 @@
+[aliases]
+test=pytest
diff --git a/setup.py b/setup.py
index 96b4561..d69845c 100644
--- a/setup.py
+++ b/setup.py
@@ -5,6 +5,7 @@
     version='0.3.0',
     author='Barosl Lee',
     url='https://github.com/barosl/homu',
+    test_suite='homu.tests',
     description=('A bot that integrates with GitHub '
                  'and your favorite continuous integration service'),
 
@@ -18,6 +19,12 @@
         'waitress',
         'retrying',
     ],
+    setup_requires=[
+        'pytest-runner',
+    ],
+    tests_require=[
+        'pytest',
+    ],
     package_data={
         'homu': [
             'html/*.html',

From 0194c3af720ea5a080243951d838f5bc1d72494a Mon Sep 17 00:00:00 2001
From: Bryan Burgers <bryan@burgers.io>
Date: Wed, 12 Jun 2019 12:10:09 -0500
Subject: [PATCH 2/4] Stop parsing when we reach an unknown word

When parsing a @bors command, stop parsing when we reach any unknown
word. This enables commands like

    @bors retry (yielding priority to the rollup)

to include a description of why we retried (which gets put in the retry
log) without also setting `rollup`.

The only tricky bit to this is a command like

    @bors r=person 0123456789abcdef

where the commit sha is part of the `r=` command, and we need to parse
it that way.
---
 homu/parse_issue_comment.py            | 18 ++++++++++++------
 homu/tests/test_parse_issue_comment.py | 25 +++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/homu/parse_issue_comment.py b/homu/parse_issue_comment.py
index 7fdd3c5..d53c2ea 100644
--- a/homu/parse_issue_comment.py
+++ b/homu/parse_issue_comment.py
@@ -158,13 +158,21 @@ def parse_issue_comment(username, body, sha, botname, hooks=[]):
     if words[1:] == ["are", "you", "still", "there?"]:
         commands.append(IssueCommentCommand.ping('portal'))
 
-    for i, word in reversed(list(enumerate(words))):
-        found = True
+    for i, word in enumerate(words):
+        if word is None:
+            # We already parsed the next word, and we set it to an empty string
+            # to signify that we did.
+            continue
+
+        if word == '@' + botname:
+            continue
+
         if word == 'r+' or word.startswith('r='):
             approved_sha = sha
 
             if i + 1 < len(words) and is_sha(words[i + 1]):
                 approved_sha = words[i + 1]
+                words[i + 1] = None
 
             approver = word[len('r='):] if word.startswith('r=') else username
 
@@ -238,9 +246,7 @@ def parse_issue_comment(username, body, sha, botname, hooks=[]):
             commands.append(IssueCommentCommand.hook(hook_name, hook_extra))
 
         else:
-            found = False
-
-        if found:
-            words[i] = ''
+            # First time we reach an unknown word, stop parsing.
+            break
 
     return commands
diff --git a/homu/tests/test_parse_issue_comment.py b/homu/tests/test_parse_issue_comment.py
index 3db4a77..5525b26 100644
--- a/homu/tests/test_parse_issue_comment.py
+++ b/homu/tests/test_parse_issue_comment.py
@@ -465,6 +465,31 @@ def test_multiple_hooks():
     assert thirdhook_commands[0].hook_extra is None
 
 
+def test_parse_up_to_first_unknown_word():
+    """
+    Test that when parsing, once we arrive at an unknown word, we stop parsing
+    """
+
+    author = "jack"
+    body = """
+    @bors retry -- yielding priority to the rollup
+    """
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'retry'
+
+    body = """
+    @bors retry (yielding priority to the rollup)
+    """
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'retry'
+
+
 def test_ignore_commands_before_bors_line():
     """
     Test that when command-like statements appear before the @bors part,

From dc435f6b138f0bcfed70b5deaacbf173d7dd48f0 Mon Sep 17 00:00:00 2001
From: Bryan Burgers <bryan@burgers.io>
Date: Wed, 12 Jun 2019 16:01:52 -0500
Subject: [PATCH 3/4] Make sure Homu hidden approval command parses

Homu leaves self-approvals in comments, like

    <!-- @bors r=someone abcdef -->

and we need to make sure those still parse correctly.
---
 homu/parse_issue_comment.py            |  9 ++++++++-
 homu/tests/test_parse_issue_comment.py | 16 ++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/homu/parse_issue_comment.py b/homu/parse_issue_comment.py
index d53c2ea..e43133f 100644
--- a/homu/parse_issue_comment.py
+++ b/homu/parse_issue_comment.py
@@ -151,7 +151,14 @@ def parse_issue_comment(username, body, sha, botname, hooks=[]):
            E.g. `['hook1', 'hook2', 'hook3']`
     """
 
-    words = list(chain.from_iterable(re.findall(r'\S+', x) for x in body.splitlines() if '@' + botname in x))  # noqa
+    botname_regex = re.compile(r'^.*(?=@' + botname + ')')
+
+    # All of the 'words' after and including the botname
+    words = list(chain.from_iterable(
+                     re.findall(r'\S+', re.sub(botname_regex, '', x))
+                 for x
+                 in body.splitlines()
+                 if '@' + botname in x))  # noqa
 
     commands = []
 
diff --git a/homu/tests/test_parse_issue_comment.py b/homu/tests/test_parse_issue_comment.py
index 5525b26..e7e475a 100644
--- a/homu/tests/test_parse_issue_comment.py
+++ b/homu/tests/test_parse_issue_comment.py
@@ -52,6 +52,22 @@ def test_r_equals():
     assert command.actor == 'jill'
 
 
+def test_hidden_r_equals():
+    author = "bors"
+    body = """
+    :pushpin: Commit {0} has been approved by `jack`
+    <!-- @bors r=jack {0} -->
+    """.format(commit)
+
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'approve'
+    assert command.actor == 'jack'
+    assert command.commit == commit
+
+
 def test_r_me():
     """
     Ignore r=me

From 69f5fd185b172f323f92d694dfb883de960df42d Mon Sep 17 00:00:00 2001
From: Bryan Burgers <bryan@burgers.io>
Date: Fri, 14 Jun 2019 06:35:06 -0500
Subject: [PATCH 4/4] Command parser: allow `@bors:`

Commands that started with the botname *and then a colon* were being
dropped, because the parser didn't recognize the command and didn't
continue. Fix this by explicitely allowing a colon after the botname.
---
 homu/parse_issue_comment.py            |  3 +++
 homu/tests/test_parse_issue_comment.py | 16 ++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/homu/parse_issue_comment.py b/homu/parse_issue_comment.py
index e43133f..8d53bc0 100644
--- a/homu/parse_issue_comment.py
+++ b/homu/parse_issue_comment.py
@@ -174,6 +174,9 @@ def parse_issue_comment(username, body, sha, botname, hooks=[]):
         if word == '@' + botname:
             continue
 
+        if word == '@' + botname + ':':
+            continue
+
         if word == 'r+' or word.startswith('r='):
             approved_sha = sha
 
diff --git a/homu/tests/test_parse_issue_comment.py b/homu/tests/test_parse_issue_comment.py
index e7e475a..e7da8ef 100644
--- a/homu/tests/test_parse_issue_comment.py
+++ b/homu/tests/test_parse_issue_comment.py
@@ -21,6 +21,22 @@ def test_r_plus():
     assert command.actor == 'jack'
 
 
+def test_r_plus_with_colon():
+    """
+    @bors: r+
+    """
+
+    author = "jack"
+    body = "@bors: r+"
+    commands = parse_issue_comment(author, body, commit, "bors")
+
+    assert len(commands) == 1
+    command = commands[0]
+    assert command.action == 'approve'
+    assert command.actor == 'jack'
+    assert command.commit == commit
+
+
 def test_r_plus_with_sha():
     """
     @bors r+ {sha}