Add support for submitting reviews on GitHub

GitHub has added a lot of controls around the review object, so it is
useful to be able to run tests and then submit a review rather than
simply merge. One use-case is also to be able to self-approve with a
comment, such as is done in the test code added.

Change-Id: I16872062e627b385f78023878bea348555ec5348

doc/source/admin/drivers/github.rst

@@ -327,6 +327,17 @@ itself. Status name, description, and context is taken from the pipeline.
       comment to the pipeline status to the github pull request. Only
       used for Pull Request based items.
 
+   .. attr:: review
+
+      One of `approve`, `comment`, or `request-changes` that causes the
+      reporter to submit a review with the specified status on Pull Request
+      based items. Has no effect on other items.
+
+   .. attr:: review-body
+
+      Text that will be submitted as the body of the review. Required if review
+      is set to `comment` or `request-changes`.
+
    .. attr:: merge
       :default: false
 

tests/base.py

@@ -799,15 +799,6 @@ class GithubChangeReference(git.Reference):
     _points_to_commits_only = True
 
 
-class FakeGHReview(object):
-
-    def __init__(self, data):
-        self.data = data
-
-    def as_dict(self):
-        return self.data
-
-
 class FakeGithubPullRequest(object):
 
     def __init__(self, github, number, project, branch,
@@ -1074,7 +1065,7 @@ class FakeGithubPullRequest(object):
             submitted_at = time.strftime(
                 gh_time_format, granted_on.timetuple())
 
-        self.reviews.append(FakeGHReview({
+        self.reviews.append(tests.fakegithub.FakeGHReview({
             'state': state,
             'user': {
                 'login': user,

tests/fakegithub.py

@@ -16,6 +16,8 @@
 
 import github3.exceptions
 import re
+import time
+
 
 FAKE_BASE_URL = 'https://example.com/api/v3/'
 
@@ -59,6 +61,15 @@ class FakeStatus(object):
         }
 
 
+class FakeGHReview(object):
+
+    def __init__(self, data):
+        self.data = data
+
+    def as_dict(self):
+        return self.data
+
+
 class FakeCombinedStatus(object):
     def __init__(self, sha, statuses):
         self.sha = sha
@@ -292,6 +303,18 @@ class FakePull(object):
     def reviews(self):
         return self._fake_pull_request.reviews
 
+    def create_review(self, body, commit_id, event):
+        review = FakeGHReview({
+            'state': event,
+            'user': {
+                'login': 'fakezuul',
+                'email': 'fakezuul@fake.test',
+            },
+            'submitted_at': time.gmtime(),
+        })
+        self._fake_pull_request.reviews.append(review)
+        return review
+
     @property
     def head(self):
         client = FakeGithubClient(self._fake_pull_request.github.github_data)

tests/fixtures/layouts/reviews-github.yaml

@@ -11,6 +11,18 @@
         label:
           - tests passed
 
+- pipeline:
+    name: selfies
+    manager: independent
+    trigger:
+      github:
+        - event: pull_request
+          action: comment
+          comment: "I solemnly swear that I am up to no good"
+    success:
+      github:
+        review: approve
+
 - job:
     name: base
     parent: null
@@ -22,6 +34,9 @@
 
 - project:
     name: org/project
+    selfies:
+      jobs:
+        - project-reviews
     reviews:
       jobs:
         - project-reviews

tests/unit/test_github_driver.py

@@ -287,7 +287,7 @@ class TestGithubDriver(ZuulTestCase):
         self.assertEqual(['other label'], C.labels)
 
     @simple_layout('layouts/reviews-github.yaml', driver='github')
-    def test_review_event(self):
+    def test_reviews(self):
         A = self.fake_github.openFakePullRequest('org/project', 'master', 'A')
         self.fake_github.emitEvent(A.getReviewAddedEvent('approve'))
         self.waitUntilSettled()
@@ -301,6 +301,15 @@ class TestGithubDriver(ZuulTestCase):
         self.waitUntilSettled()
         self.assertEqual(1, len(self.history))
 
+        # test sending reviews
+        C = self.fake_github.openFakePullRequest('org/project', 'master', 'C')
+        self.fake_github.emitEvent(C.getCommentAddedEvent(
+            "I solemnly swear that I am up to no good"))
+        self.waitUntilSettled()
+        self.assertEqual('project-reviews', self.history[0].name)
+        self.assertEqual(1, len(C.reviews))
+        self.assertEqual('APPROVE', C.reviews[0].as_dict()['state'])
+
     @simple_layout('layouts/basic-github.yaml', driver='github')
     def test_timer_event(self):
         self.executor_server.hold_jobs_in_build = True

zuul/driver/github/githubconnection.py

@@ -1445,6 +1445,14 @@ class GithubConnection(BaseConnection):
                        state, sha, project)
         self.log_rate_limit(self.log, github)
 
+    def reviewPull(self, project, pr_number, sha, review, body):
+        github = self.getGithubClient(project)
+        owner, proj = project.split('/')
+        pull_request = github.pull_request(owner, proj, pr_number)
+        event = review.replace('-', '_')
+        event = event.upper()
+        pull_request.create_review(body=body, commit_id=sha, event=event)
+
     def labelPull(self, project, pr_number, label):
         github = self.getGithubClient(project)
         owner, proj = project.split('/')

zuul/driver/github/githubreporter.py

@@ -37,6 +37,8 @@ class GithubReporter(BaseReporter):
         if not isinstance(self._labels, list):
             self._labels = [self._labels]
         self._unlabels = self.config.get('unlabel', [])
+        self._review = self.config.get('review')
+        self._review_body = self.config.get('review-body')
         if not isinstance(self._unlabels, list):
             self._unlabels = [self._unlabels]
         self.context = "{}/{}".format(pipeline.tenant.name, pipeline.name)
@@ -70,6 +72,8 @@ class GithubReporter(BaseReporter):
                 self.addPullComment(item)
             if self._labels or self._unlabels:
                 self.setLabels(item)
+            if self._review:
+                self.addReview(item)
             if (self._merge):
                 self.mergePull(item)
                 if not item.change.is_merged:
@@ -150,6 +154,21 @@ class GithubReporter(BaseReporter):
             'Merge of change %s failed after 2 attempts, giving up' %
             item.change)
 
+    def addReview(self, item):
+        project = item.change.project.name
+        pr_number = item.change.number
+        sha = item.change.patchset
+        self.log.debug('Reporting change %s, params %s, review:\n%s' %
+                       (item.change, self.config, self._review))
+        self.connection.reviewPull(
+            project,
+            pr_number,
+            sha,
+            self._review,
+            self._review_body)
+        for label in self._unlabels:
+            self.connection.unlabelPull(project, pr_number, label)
+
     def setLabels(self, item):
         project = item.change.project.name
         pr_number = item.change.number
@@ -214,6 +233,8 @@ def getSchema():
         'comment': bool,
         'merge': bool,
         'label': scalar_or_list(str),
-        'unlabel': scalar_or_list(str)
+        'unlabel': scalar_or_list(str),
+        'review': v.Any('approve', 'request-changes', 'comment'),
+        'review-body': str
     })
     return github_reporter