From 9fa93fdefa44b59abd111400c76c475800d839fc Mon Sep 17 00:00:00 2001
From: Tobias Henkel <tobias.henkel@bmw.de>
Date: Fri, 7 Dec 2018 08:50:43 +0100
Subject: [PATCH] Use combined status for Github status checks

When checking the required status checks we currently get all statuses
and get the successful of them. However Github returns all historic
status changes there. So a change that get a successful check, then a
recheck and then a failed check still enters the gate but will be
prohibited by Github to merge in the end. However github also offers
us a combined status call that only returns the current state of the
statuses. Using this fixes the issue.

Change-Id: Iec3b2a3dfc8626870381604badd40de71e7257b9
---
 tests/fakegithub.py                    | 19 +++++++++++++++++++
 tests/unit/test_github_driver.py       | 19 ++++++++++++++++++-
 zuul/driver/github/githubconnection.py |  4 ++--
 3 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/tests/fakegithub.py b/tests/fakegithub.py
index df34c694b6..fb0df30221 100644
--- a/tests/fakegithub.py
+++ b/tests/fakegithub.py
@@ -59,6 +59,12 @@ class FakeStatus(object):
         }
 
 
+class FakeCombinedStatus(object):
+    def __init__(self, sha, statuses):
+        self.sha = sha
+        self.statuses = statuses
+
+
 class FakeCommit(object):
     def __init__(self, sha):
         self._statuses = []
@@ -74,6 +80,19 @@ class FakeCommit(object):
     def statuses(self):
         return self._statuses
 
+    def status(self):
+        '''
+        Returns the combined status wich only contains the latest statuses of
+        the commit together with some other information that we don't need
+        here.
+        '''
+        latest_statuses_by_context = {}
+        for status in self._statuses:
+            if status.context not in latest_statuses_by_context:
+                latest_statuses_by_context[status.context] = status
+        combined_statuses = latest_statuses_by_context.values()
+        return FakeCombinedStatus(self.sha, combined_statuses)
+
 
 class FakeRepository(object):
     def __init__(self, name, data):
diff --git a/tests/unit/test_github_driver.py b/tests/unit/test_github_driver.py
index 2b2f9bd3da..28b7b4a3fb 100644
--- a/tests/unit/test_github_driver.py
+++ b/tests/unit/test_github_driver.py
@@ -896,8 +896,25 @@ class TestGithubDriver(ZuulTestCase):
         # job is expected
         self.assertEqual(0, len(self.history))
 
-        # now set the required status 'tenant-one/check'
+        # now set a failing status 'tenant-one/check'
         repo = github.repo_from_project('org/project')
+        repo.create_status(A.head_sha, 'failed', 'example.com', 'description',
+                           'tenant-one/check')
+        self.fake_github.emitEvent(A.getPullRequestOpenedEvent())
+        self.waitUntilSettled()
+        self.assertEqual(0, len(self.history))
+
+        # now set a successful status followed by a failing status to check
+        # that the later failed status wins
+        repo.create_status(A.head_sha, 'success', 'example.com', 'description',
+                           'tenant-one/check')
+        repo.create_status(A.head_sha, 'failed', 'example.com', 'description',
+                           'tenant-one/check')
+        self.fake_github.emitEvent(A.getPullRequestOpenedEvent())
+        self.waitUntilSettled()
+        self.assertEqual(0, len(self.history))
+
+        # now set the required status 'tenant-one/check'
         repo.create_status(A.head_sha, 'success', 'example.com', 'description',
                            'tenant-one/check')
 
diff --git a/zuul/driver/github/githubconnection.py b/zuul/driver/github/githubconnection.py
index 58c38183b0..95eb7f5bdd 100644
--- a/zuul/driver/github/githubconnection.py
+++ b/zuul/driver/github/githubconnection.py
@@ -1245,8 +1245,8 @@ class GithubConnection(BaseConnection):
                 break
 
         # Get successful statuses
-        successful = set(
-            [s.context for s in commit.statuses() if s.state == 'success'])
+        successful = set([s.context for s in commit.status().statuses
+                          if s.state == 'success'])
 
         # Required contexts must be a subset of the successful contexts as
         # we allow additional successful status contexts we don't care about.