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.

Author: bryanburgers

.gitignore

@@ -5,3 +5,4 @@
 /homu.egg-info/
 /main.db
 /cache
+*.pyc

.travis.yml

@@ -6,4 +6,6 @@ python:
 install:
   - pip install flake8
 script:
-  - flake8 homu
+  - flake8 homu
+  - pip install -e .
+  - python setup.py test

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,61 +715,58 @@ 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
 
         if found:
             state_changed = True
 
-            words[i] = ''
-
     return state_changed