Add a test to verify push reports only set status

Also fix reporting on push along the way, as it turns out this wasn't
really working to begin with.

Within the fake github connection class, keep track of all the
reportings that have happened. These are just a simple list of tuples
about the reports, and we're only checking them in one test case for
now. This may be more useful down the road in other tests.

Change-Id: I85bb7a580c21f0762187f2176b74983baf6e8509

tests/base.py

@@ -868,6 +868,7 @@ class FakeGithubConnection(githubconnection.GithubConnection):
         self.upstream_root = upstream_root
         self.merge_failure = False
         self.merge_not_allowed_count = 0
+        self.reports = []
 
     def openFakePullRequest(self, project, branch, subject, files=[]):
         self.pr_number += 1
@@ -979,10 +980,14 @@ class FakeGithubConnection(githubconnection.GithubConnection):
         return ['master']
 
     def commentPull(self, project, pr_number, message):
+        # record that this got reported
+        self.reports.append((project, pr_number, 'comment'))
         pull_request = self.pull_requests[pr_number - 1]
         pull_request.addComment(message)
 
     def mergePull(self, project, pr_number, commit_message='', sha=None):
+        # record that this got reported
+        self.reports.append((project, pr_number, 'merge'))
         pull_request = self.pull_requests[pr_number - 1]
         if self.merge_failure:
             raise Exception('Pull request was not merged')
@@ -998,6 +1003,8 @@ class FakeGithubConnection(githubconnection.GithubConnection):
 
     def setCommitStatus(self, project, sha, state, url='', description='',
                         context='default', user='zuul'):
+        # record that this got reported
+        self.reports.append((project, sha, 'status', (user, context, state)))
         # always insert a status to the front of the list, to represent
         # the last status provided for a commit.
         # Since we're bypassing github API, which would require a user, we
@@ -1014,10 +1021,14 @@ class FakeGithubConnection(githubconnection.GithubConnection):
         })
 
     def labelPull(self, project, pr_number, label):
+        # record that this got reported
+        self.reports.append((project, pr_number, 'label', label))
         pull_request = self.pull_requests[pr_number - 1]
         pull_request.addLabel(label)
 
     def unlabelPull(self, project, pr_number, label):
+        # record that this got reported
+        self.reports.append((project, pr_number, 'unlabel', label))
         pull_request = self.pull_requests[pr_number - 1]
         pull_request.removeLabel(label)
 

tests/fixtures/layouts/reporting-github.yaml

@@ -34,6 +34,29 @@
       github:
         comment: false
 
+- pipeline:
+    name: push-reporting
+    description: Uncommon reporting
+    manager: independent
+    trigger:
+      github:
+        - event: push
+        - event: pull_request
+          action: opened
+    start:
+      github:
+        comment: true
+        status: 'pending'
+    success:
+      github:
+        comment: true
+        status: 'success'
+        merge: true
+    failure:
+      github:
+        comment: true
+        status: 'failure'
+
 - job:
     name: project-test1
 
@@ -45,3 +68,9 @@
     reporting:
       jobs:
         - project-test1
+
+- project:
+    name: org/project2
+    push-reporting:
+      jobs:
+        - project-test1

tests/unit/test_github_driver.py

@@ -320,6 +320,46 @@ class TestGithubDriver(ZuulTestCase):
         self.assertThat(report_status['url'][len(base):],
                         MatchesRegex('^[a-fA-F0-9]{32}\/$'))
 
+    @simple_layout('layouts/reporting-github.yaml', driver='github')
+    def test_push_reporting(self):
+        project = 'org/project2'
+        # pipeline reports pull status both on start and success
+        self.executor_server.hold_jobs_in_build = True
+        pevent = self.fake_github.getPushEvent(project=project,
+                                               ref='refs/heads/master')
+
+        self.fake_github.emitEvent(pevent)
+        self.waitUntilSettled()
+
+        # there should only be one report, a status
+        self.assertEqual(1, len(self.fake_github.reports))
+        # Verify the user/context/state of the status
+        status = ('zuul', 'tenant-one/push-reporting', 'pending')
+        self.assertEqual(status, self.fake_github.reports[0][-1])
+
+        # free the executor, allow the build to finish
+        self.executor_server.hold_jobs_in_build = False
+        self.executor_server.release()
+        self.waitUntilSettled()
+
+        # Now there should be a second report, the success of the build
+        self.assertEqual(2, len(self.fake_github.reports))
+        # Verify the user/context/state of the status
+        status = ('zuul', 'tenant-one/push-reporting', 'success')
+        self.assertEqual(status, self.fake_github.reports[-1][-1])
+
+        # now make a PR which should also comment
+        self.executor_server.hold_jobs_in_build = True
+        A = self.fake_github.openFakePullRequest(project, 'master', 'A')
+        self.fake_github.emitEvent(A.getPullRequestOpenedEvent())
+        self.waitUntilSettled()
+
+        # Now there should be a four reports, a new comment
+        # and status
+        self.assertEqual(4, len(self.fake_github.reports))
+        self.executor_server.release()
+        self.waitUntilSettled()
+
     @simple_layout('layouts/merging-github.yaml', driver='github')
     def test_report_pull_merge(self):
         # pipeline merges the pull request on success

zuul/driver/github/githubreporter.py

@@ -43,10 +43,13 @@ class GithubReporter(BaseReporter):
         """Report on an event."""
         # order is important for github branch protection.
         # A status should be set before a merge attempt
-        if (self._commit_status is not None and
-            hasattr(item.change, 'patchset') and
-            item.change.patchset is not None):
-            self.setCommitStatus(item)
+        if self._commit_status is not None:
+            if (hasattr(item.change, 'patchset') and
+                    item.change.patchset is not None):
+                self.setCommitStatus(item)
+            elif (hasattr(item.change, 'newrev') and
+                    item.change.newrev is not None):
+                self.setCommitStatus(item)
         # Comments, labels, and merges can only be performed on pull requests.
         # If the change is not a pull request (e.g. a push) skip them.
         if hasattr(item.change, 'number'):
@@ -71,7 +74,10 @@ class GithubReporter(BaseReporter):
 
     def setCommitStatus(self, item):
         project = item.change.project.name
-        sha = item.change.patchset
+        if hasattr(item.change, 'patchset'):
+            sha = item.change.patchset
+        elif hasattr(item.change, 'newrev'):
+            sha = item.change.newrev
         context = '%s/%s' % (item.pipeline.layout.tenant.name,
                              item.pipeline.name)
         state = self._commit_status