Implement github trigger requirement status

This allows using a connection specific requirement on the status of the
head of a PR. A list of statuses is accepted, just like the pipeline
requirement.

Update GithubRefFilter class to be more clear that it deals with
required statuses, whereas the event filter works with the status being
provided.

Change-Id: Ib91f6527bf1d8ff5fbc6434c8adaca1cd5e1ba6d

4 files changed, 72 insertions, 14 deletions

diff --git a/tests/fixtures/layouts/requirements-github.yaml b/tests/fixtures/layouts/requirements-github.yaml
index 5b92b58ac..addba1e63 100644
--- a/tests/fixtures/layouts/requirements-github.yaml
+++ b/tests/fixtures/layouts/requirements-github.yaml
@@ -14,6 +14,19 @@
         comment: true
 
 - pipeline:
+    name: trigger_status
+    manager: independent
+    trigger:
+      github:
+        - event: pull_request
+          action: comment
+          comment: 'trigger me'
+          require-status: "zuul:check:success"
+    success:
+      github:
+        comment: true
+
+- pipeline:
     name: trigger
     manager: independent
     trigger:
@@ -146,6 +159,9 @@
     pipeline:
       jobs:
         - project1-pipeline
+    trigger_status:
+      jobs:
+        - project1-pipeline
 
 - project:
     name: org/project2
diff --git a/tests/unit/test_github_requirements.py b/tests/unit/test_github_requirements.py
index 60bcf74ce..5f8f14ddd 100644
--- a/tests/unit/test_github_requirements.py
+++ b/tests/unit/test_github_requirements.py
@@ -49,6 +49,30 @@ class TestGithubRequirements(ZuulTestCase):
     @simple_layout('layouts/requirements-github.yaml', driver='github')
     def test_trigger_require_status(self):
         "Test trigger requirement: status"
+        A = self.fake_github.openFakePullRequest('org/project1', 'master', 'A')
+        # A comment event that we will keep submitting to trigger
+        comment = A.getCommentAddedEvent('trigger me')
+        self.fake_github.emitEvent(comment)
+        self.waitUntilSettled()
+        # No status from zuul so should not be enqueued
+        self.assertEqual(len(self.history), 0)
+
+        # An error status should not cause it to be enqueued
+        A.setStatus(A.head_sha, 'error', 'null', 'null', 'check')
+        self.fake_github.emitEvent(comment)
+        self.waitUntilSettled()
+        self.assertEqual(len(self.history), 0)
+
+        # A success status goes in
+        A.setStatus(A.head_sha, 'success', 'null', 'null', 'check')
+        self.fake_github.emitEvent(comment)
+        self.waitUntilSettled()
+        self.assertEqual(len(self.history), 1)
+        self.assertEqual(self.history[0].name, 'project1-pipeline')
+
+    @simple_layout('layouts/requirements-github.yaml', driver='github')
+    def test_trigger_on_status(self):
+        "Test trigger on: status"
         A = self.fake_github.openFakePullRequest('org/project2', 'master', 'A')
 
         # An error status should not cause it to be enqueued
diff --git a/zuul/driver/github/githubmodel.py b/zuul/driver/github/githubmodel.py
index bbacc9b1b..85738d8f7 100644
--- a/zuul/driver/github/githubmodel.py
+++ b/zuul/driver/github/githubmodel.py
@@ -59,10 +59,11 @@ class GithubTriggerEvent(TriggerEvent):
         return False
 
 
-class GithubReviewFilter(object):
-    def __init__(self, required_reviews=[]):
+class GithubCommonFilter(object):
+    def __init__(self, required_reviews=[], required_statuses=[]):
         self._required_reviews = copy.deepcopy(required_reviews)
         self.required_reviews = self._tidy_reviews(required_reviews)
+        self.required_statuses = required_statuses
 
     def _tidy_reviews(self, reviews):
         for r in reviews:
@@ -126,14 +127,27 @@ class GithubReviewFilter(object):
                 return False
         return True
 
+    def matchesRequiredStatuses(self, change):
+        # statuses are ORed
+        # A PR head can have multiple statuses on it. If the change
+        # statuses and the filter statuses are a null intersection, there
+        # are no matches and we return false
+        if self.required_statuses:
+            if set(change.status).isdisjoint(set(self.required_statuses)):
+                return False
+        return True
+
 
-class GithubEventFilter(EventFilter):
+class GithubEventFilter(EventFilter, GithubCommonFilter):
     def __init__(self, trigger, types=[], branches=[], refs=[],
                  comments=[], actions=[], labels=[], unlabels=[],
-                 states=[], statuses=[], ignore_deletes=True):
+                 states=[], statuses=[], required_statuses=[],
+                 ignore_deletes=True):
 
         EventFilter.__init__(self, trigger)
 
+        GithubCommonFilter.__init__(self, required_statuses=required_statuses)
+
         self._types = types
         self._branches = branches
         self._refs = refs
@@ -147,6 +161,7 @@ class GithubEventFilter(EventFilter):
         self.unlabels = unlabels
         self.states = states
         self.statuses = statuses
+        self.required_statuses = required_statuses
         self.ignore_deletes = ignore_deletes
 
     def __repr__(self):
@@ -172,6 +187,8 @@ class GithubEventFilter(EventFilter):
             ret += ' states: %s' % ', '.join(self.states)
         if self.statuses:
             ret += ' statuses: %s' % ', '.join(self.statuses)
+        if self.required_statuses:
+            ret += ' required_statuses: %s' % ', '.join(self.required_statuses)
         ret += '>'
 
         return ret
@@ -239,14 +256,18 @@ class GithubEventFilter(EventFilter):
         if self.statuses and event.status not in self.statuses:
             return False
 
+        if not self.matchesRequiredStatuses(change):
+            return False
+
         return True
 
 
-class GithubRefFilter(RefFilter, GithubReviewFilter):
+class GithubRefFilter(RefFilter, GithubCommonFilter):
     def __init__(self, statuses=[], required_reviews=[]):
         RefFilter.__init__(self)
 
-        GithubReviewFilter.__init__(self, required_reviews=required_reviews)
+        GithubCommonFilter.__init__(self, required_reviews=required_reviews,
+                                    required_statuses=statuses)
         self.statuses = statuses
 
     def __repr__(self):
@@ -263,13 +284,8 @@ class GithubRefFilter(RefFilter, GithubReviewFilter):
         return ret
 
     def matches(self, change):
-        # statuses are ORed
-        # A PR head can have multiple statuses on it. If the change
-        # statuses and the filter statuses are a null intersection, there
-        # are no matches and we return false
-        if self.statuses:
-            if set(change.status).isdisjoint(set(self.statuses)):
-                return False
+        if not self.matchesRequiredStatuses(change):
+            return False
 
         # required reviews are ANDed
         if not self.matchesReviews(change):
diff --git a/zuul/driver/github/githubtrigger.py b/zuul/driver/github/githubtrigger.py
index 3269c3658..4f015918b 100644
--- a/zuul/driver/github/githubtrigger.py
+++ b/zuul/driver/github/githubtrigger.py
@@ -42,7 +42,8 @@ class GithubTrigger(BaseTrigger):
                 labels=toList(trigger.get('label')),
                 unlabels=toList(trigger.get('unlabel')),
                 states=toList(trigger.get('state')),
-                statuses=toList(trigger.get('status'))
+                statuses=toList(trigger.get('status')),
+                required_statuses=toList(trigger.get('require-status'))
             )
             efilters.append(f)
 
@@ -68,6 +69,7 @@ def getSchema():
         'label': toList(str),
         'unlabel': toList(str),
         'state': toList(str),
+        'require-status': toList(str),
         'status': toList(str)
     }