diff --git a/tests/base.py b/tests/base.py index 50a37de584..abf2879888 100755 --- a/tests/base.py +++ b/tests/base.py @@ -660,6 +660,7 @@ class FakeGithubPullRequest(object): self.statuses = {} self.reviews = [] self.writers = [] + self.admins = [] self.updated_at = None self.head_sha = None self.is_merged = False @@ -1063,7 +1064,9 @@ class FakeGithubConnection(githubconnection.GithubConnection): for pr in self.pull_requests.values(): pr_owner, pr_project = pr.project.split('/') if (pr_owner == owner and proj == pr_project): - if login in pr.writers: + if login in pr.admins: + return 'admin' + elif login in pr.writers: return 'write' else: return 'read' diff --git a/tests/unit/test_github_requirements.py b/tests/unit/test_github_requirements.py index f125d1e4ac..c32f07e99e 100644 --- a/tests/unit/test_github_requirements.py +++ b/tests/unit/test_github_requirements.py @@ -252,6 +252,33 @@ class TestGithubRequirements(ZuulTestCase): self.assertEqual(len(self.history), 1) self.assertEqual(self.history[0].name, 'project5-reviewuserstate') + @simple_layout('layouts/requirements-github.yaml', driver='github') + def test_pipeline_require_review_write_perms(self): + "Test pipeline requirement: review from user with write" + + A = self.fake_github.openFakePullRequest('org/project4', 'master', 'A') + # Add herp to admins + A.admins.append('herp') + # A comment event that we will keep submitting to trigger + comment = A.getCommentAddedEvent('test me') + self.fake_github.emitEvent(comment) + self.waitUntilSettled() + # No positive review from derp so should not be enqueued + self.assertEqual(len(self.history), 0) + + # The first review is from a reader, and thus should not be enqueued + A.addReview('derp', 'APPROVED') + self.fake_github.emitEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 0) + + # A positive review from herp should cause it to be enqueued + A.addReview('herp', 'APPROVED') + self.fake_github.emitEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 1) + self.assertEqual(self.history[0].name, 'project4-reviewreq') + @simple_layout('layouts/requirements-github.yaml', driver='github') def test_pipeline_require_review_comment_masked(self): "Test pipeline requirement: review comments on top of votes" diff --git a/zuul/driver/github/githubmodel.py b/zuul/driver/github/githubmodel.py index 0731dd7338..3b0618f96b 100644 --- a/zuul/driver/github/githubmodel.py +++ b/zuul/driver/github/githubmodel.py @@ -110,10 +110,15 @@ class GithubCommonFilter(object): return False elif k == 'permission': # If permission is read, we've matched. You must have read - # to provide a review. Write or admin permission is different. + # to provide a review. if v != 'read': - if review['permission'] != v: - return False + # Admins have implicit write. + if v == 'write': + if review['permission'] not in ('write', 'admin'): + return False + elif v == 'admin': + if review['permission'] != 'admin': + return False return True def matchesReviews(self, change):