Merge "Add support for submitting reviews on GitHub"

This commit is contained in:
Zuul 2019-05-16 21:32:46 +00:00 committed by Gerrit Code Review
commit c58b410cb4
7 changed files with 90 additions and 12 deletions

View File

@ -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 comment to the pipeline status to the github pull request. Only
used for Pull Request based items. 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 .. attr:: merge
:default: false :default: false

View File

@ -802,15 +802,6 @@ class GithubChangeReference(git.Reference):
_points_to_commits_only = True _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): class FakeGithubPullRequest(object):
def __init__(self, github, number, project, branch, def __init__(self, github, number, project, branch,
@ -1077,7 +1068,7 @@ class FakeGithubPullRequest(object):
submitted_at = time.strftime( submitted_at = time.strftime(
gh_time_format, granted_on.timetuple()) gh_time_format, granted_on.timetuple())
self.reviews.append(FakeGHReview({ self.reviews.append(tests.fakegithub.FakeGHReview({
'state': state, 'state': state,
'user': { 'user': {
'login': user, 'login': user,

View File

@ -16,6 +16,8 @@
import github3.exceptions import github3.exceptions
import re import re
import time
FAKE_BASE_URL = 'https://example.com/api/v3/' FAKE_BASE_URL = 'https://example.com/api/v3/'
@ -60,6 +62,15 @@ class FakeStatus(object):
} }
class FakeGHReview(object):
def __init__(self, data):
self.data = data
def as_dict(self):
return self.data
class FakeCombinedStatus(object): class FakeCombinedStatus(object):
def __init__(self, sha, statuses): def __init__(self, sha, statuses):
self.sha = sha self.sha = sha
@ -293,6 +304,18 @@ class FakePull(object):
def reviews(self): def reviews(self):
return self._fake_pull_request.reviews 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 @property
def head(self): def head(self):
client = FakeGithubClient(self._fake_pull_request.github.github_data) client = FakeGithubClient(self._fake_pull_request.github.github_data)

View File

@ -11,6 +11,18 @@
label: label:
- tests passed - 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: - job:
name: base name: base
parent: null parent: null
@ -22,6 +34,9 @@
- project: - project:
name: org/project name: org/project
selfies:
jobs:
- project-reviews
reviews: reviews:
jobs: jobs:
- project-reviews - project-reviews

View File

@ -287,7 +287,7 @@ class TestGithubDriver(ZuulTestCase):
self.assertEqual(['other label'], C.labels) self.assertEqual(['other label'], C.labels)
@simple_layout('layouts/reviews-github.yaml', driver='github') @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') A = self.fake_github.openFakePullRequest('org/project', 'master', 'A')
self.fake_github.emitEvent(A.getReviewAddedEvent('approve')) self.fake_github.emitEvent(A.getReviewAddedEvent('approve'))
self.waitUntilSettled() self.waitUntilSettled()
@ -301,6 +301,15 @@ class TestGithubDriver(ZuulTestCase):
self.waitUntilSettled() self.waitUntilSettled()
self.assertEqual(1, len(self.history)) 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') @simple_layout('layouts/basic-github.yaml', driver='github')
def test_timer_event(self): def test_timer_event(self):
self.executor_server.hold_jobs_in_build = True self.executor_server.hold_jobs_in_build = True

View File

@ -1443,6 +1443,14 @@ class GithubConnection(BaseConnection):
state, sha, project) state, sha, project)
self.log_rate_limit(self.log, github) 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): def labelPull(self, project, pr_number, label):
github = self.getGithubClient(project) github = self.getGithubClient(project)
owner, proj = project.split('/') owner, proj = project.split('/')

View File

@ -37,6 +37,8 @@ class GithubReporter(BaseReporter):
if not isinstance(self._labels, list): if not isinstance(self._labels, list):
self._labels = [self._labels] self._labels = [self._labels]
self._unlabels = self.config.get('unlabel', []) 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): if not isinstance(self._unlabels, list):
self._unlabels = [self._unlabels] self._unlabels = [self._unlabels]
self.context = "{}/{}".format(pipeline.tenant.name, pipeline.name) self.context = "{}/{}".format(pipeline.tenant.name, pipeline.name)
@ -70,6 +72,8 @@ class GithubReporter(BaseReporter):
self.addPullComment(item) self.addPullComment(item)
if self._labels or self._unlabels: if self._labels or self._unlabels:
self.setLabels(item) self.setLabels(item)
if self._review:
self.addReview(item)
if (self._merge): if (self._merge):
self.mergePull(item) self.mergePull(item)
if not item.change.is_merged: if not item.change.is_merged:
@ -150,6 +154,21 @@ class GithubReporter(BaseReporter):
'Merge of change %s failed after 2 attempts, giving up' % 'Merge of change %s failed after 2 attempts, giving up' %
item.change) 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): def setLabels(self, item):
project = item.change.project.name project = item.change.project.name
pr_number = item.change.number pr_number = item.change.number
@ -213,6 +232,8 @@ def getSchema():
'comment': bool, 'comment': bool,
'merge': bool, 'merge': bool,
'label': scalar_or_list(str), '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 return github_reporter