diff --git a/tests/base.py b/tests/base.py index a6cc2f6781..869835f204 100644 --- a/tests/base.py +++ b/tests/base.py @@ -784,6 +784,15 @@ 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, @@ -1050,14 +1059,14 @@ class FakeGithubPullRequest(object): submitted_at = time.strftime( gh_time_format, granted_on.timetuple()) - self.reviews.append({ + self.reviews.append(FakeGHReview({ 'state': state, 'user': { 'login': user, 'email': user + "@derp.com", }, 'submitted_at': submitted_at, - }) + })) def getPRReference(self): return '%s/head' % self.number @@ -1229,10 +1238,6 @@ class FakeGithubConnection(githubconnection.GithubConnection): super(FakeGithubConnection, self).addProject(project) self.getGithubClient(project).addProject(project) - def _getPullReviews(self, owner, project, number): - pr = self.pull_requests[int(number)] - return pr.reviews - def getGitUrl(self, project): if self.git_url_with_auth: auth_token = ''.join( diff --git a/tests/fakegithub.py b/tests/fakegithub.py index fb0df30221..93d280c442 100644 --- a/tests/fakegithub.py +++ b/tests/fakegithub.py @@ -296,6 +296,9 @@ class FakePull(object): return [FakeFile(fn) for fn in sorted(self._fake_pull_request.files)][:300] + def reviews(self): + return self._fake_pull_request.reviews + @property def head(self): client = FakeGithubClient(self._fake_pull_request.github.github_data) diff --git a/zuul/driver/github/githubconnection.py b/zuul/driver/github/githubconnection.py index d34dfbf5be..2019f264cf 100644 --- a/zuul/driver/github/githubconnection.py +++ b/zuul/driver/github/githubconnection.py @@ -417,7 +417,8 @@ class GithubEventProcessor(object): def _issue_to_pull_request(self, body): number = body.get('issue').get('number') project_name = body.get('repository').get('full_name') - pr_body = self.connection.getPull(project_name, number, self.log) + pr_body, pr_obj = self.connection.getPull( + project_name, number, self.log) if pr_body is None: self.log.debug('Pull request #%s not found in project %s' % (number, project_name)) @@ -963,7 +964,7 @@ class GithubConnection(BaseConnection): def _updateChange(self, change): self.log.info("Updating %s" % (change,)) - change.pr = self.getPull(change.project.name, change.number) + change.pr, pr_obj = self.getPull(change.project.name, change.number) change.ref = "refs/pull/%s/head" % change.number change.branch = change.pr.get('base').get('ref') @@ -989,7 +990,7 @@ class GithubConnection(BaseConnection): change.is_merged = change.pr.get('merged') change.status = self._get_statuses(change.project, change.patchset) - change.reviews = self.getPullReviews(change.project, + change.reviews = self.getPullReviews(pr_obj, change.project, change.number) change.labels = change.pr.get('labels') # ensure message is at least an empty string @@ -1151,7 +1152,7 @@ class GithubConnection(BaseConnection): log.debug('Got PR %s#%s', project_name, number) self.log_rate_limit(self.log, github) - return pr + return (pr, probj) def canMerge(self, change, allow_needs): # NOTE: The mergeable call may get a false (null) while GitHub is @@ -1191,7 +1192,8 @@ class GithubConnection(BaseConnection): raise Exception('Multiple pulls found with head sha %s' % sha) if len(cached_pr_numbers) == 1: for pr in cached_pr_numbers: - return self.getPull(project, pr, log) + pr_body, pr_obj = self.getPull(project, pr, log) + return pr_body pulls = [] project_name = project @@ -1216,10 +1218,11 @@ class GithubConnection(BaseConnection): return None return pulls.pop() - def getPullReviews(self, project, number): - owner, proj = project.name.split('/') - - revs = self._getPullReviews(owner, proj, number) + def getPullReviews(self, pr_obj, project, number): + # make a list out of the reviews so that we complete our + # API transaction + revs = [review.as_dict() for review in pr_obj.reviews()] + self.log.debug('Got reviews for PR %s#%s', project, number) permissions = {} reviews = {} @@ -1325,17 +1328,6 @@ class GithubConnection(BaseConnection): # we allow additional successful status contexts we don't care about. return required_contexts.issubset(successful) - def _getPullReviews(self, owner, project, number): - # make a list out of the reviews so that we complete our - # API transaction - github = self.getGithubClient("%s/%s" % (owner, project)) - reviews = [review.as_dict() for review in - github.pull_request(owner, project, number).reviews()] - - self.log.debug('Got reviews for PR %s/%s#%s', owner, project, number) - self.log_rate_limit(self.log, github) - return reviews - def getUser(self, login, project): return GithubUser(login, self, project) diff --git a/zuul/driver/github/githubsource.py b/zuul/driver/github/githubsource.py index 91fb84a422..1c0dda91c5 100644 --- a/zuul/driver/github/githubsource.py +++ b/zuul/driver/github/githubsource.py @@ -81,7 +81,7 @@ class GithubSource(BaseSource): num = int(m.group(3)) except ValueError: return None - pull = self.connection.getPull('%s/%s' % (org, proj), int(num)) + pull, pr_obj = self.connection.getPull('%s/%s' % (org, proj), int(num)) if not pull: return None proj = pull.get('base').get('repo').get('full_name')