Merge "Implement pipeline reject filter for github" into feature/zuulv3
diff --git a/tests/unit/test_github_requirements.py b/tests/unit/test_github_requirements.py
index 301ea2f..43bdfc2 100644
--- a/tests/unit/test_github_requirements.py
+++ b/tests/unit/test_github_requirements.py
@@ -142,7 +142,7 @@
A = self.fake_github.openFakePullRequest('org/project4', 'master', 'A')
# Add derp to writers
- A.writers.append('derp')
+ A.writers.extend(('derp', 'werp'))
# A comment event that we will keep submitting to trigger
comment = A.getCommentAddedEvent('test me')
self.fake_github.emitEvent(comment)
@@ -156,16 +156,29 @@
self.waitUntilSettled()
self.assertEqual(len(self.history), 0)
+ # A negative review from werp should not cause it to be enqueued
+ A.addReview('werp', 'CHANGES_REQUESTED')
+ self.fake_github.emitEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 0)
+
# A positive from nobody should not cause it to be enqueued
A.addReview('nobody', 'APPROVED')
self.fake_github.emitEvent(comment)
self.waitUntilSettled()
self.assertEqual(len(self.history), 0)
- # A positive review from derp should cause it to be enqueued
+ # A positive review from derp should still be blocked by the
+ # negative review from werp
A.addReview('derp', 'APPROVED')
self.fake_github.emitEvent(comment)
self.waitUntilSettled()
+ self.assertEqual(len(self.history), 0)
+
+ # A positive review from werp should cause it to be enqueued
+ A.addReview('werp', 'APPROVED')
+ self.fake_github.emitEvent(comment)
+ self.waitUntilSettled()
self.assertEqual(len(self.history), 1)
self.assertEqual(self.history[0].name, 'project4-reviewreq')
diff --git a/zuul/driver/github/githubmodel.py b/zuul/driver/github/githubmodel.py
index 9516097..cfd1bc0 100644
--- a/zuul/driver/github/githubmodel.py
+++ b/zuul/driver/github/githubmodel.py
@@ -60,9 +60,12 @@
class GithubCommonFilter(object):
- def __init__(self, required_reviews=[], required_statuses=[]):
+ def __init__(self, required_reviews=[], required_statuses=[],
+ reject_reviews=[]):
self._required_reviews = copy.deepcopy(required_reviews)
+ self._reject_reviews = copy.deepcopy(reject_reviews)
self.required_reviews = self._tidy_reviews(required_reviews)
+ self.reject_reviews = self._tidy_reviews(reject_reviews)
self.required_statuses = required_statuses
def _tidy_reviews(self, reviews):
@@ -109,15 +112,17 @@
return True
def matchesReviews(self, change):
- if self.required_reviews:
+ if self.required_reviews or self.reject_reviews:
if not hasattr(change, 'number'):
# not a PR, no reviews
return False
- if not change.reviews:
- # No reviews means no matching
+ if self.required_reviews and not change.reviews:
+ # No reviews means no matching of required bits
+ # having reject reviews but no reviews on the change is okay
return False
- return self.matchesRequiredReviews(change)
+ return (self.matchesRequiredReviews(change) and
+ self.matchesNoRejectReviews(change))
def matchesRequiredReviews(self, change):
for rreview in self.required_reviews:
@@ -131,6 +136,14 @@
return False
return True
+ def matchesNoRejectReviews(self, change):
+ for rreview in self.reject_reviews:
+ for review in change.reviews:
+ if self._match_review_required_review(rreview, review):
+ # A review matched, we can reject right away
+ return False
+ return True
+
def matchesRequiredStatuses(self, change):
# statuses are ORed
# A PR head can have multiple statuses on it. If the change
@@ -271,10 +284,11 @@
class GithubRefFilter(RefFilter, GithubCommonFilter):
def __init__(self, connection_name, statuses=[], required_reviews=[],
- open=None, current_patchset=None):
+ reject_reviews=[], open=None, current_patchset=None):
RefFilter.__init__(self, connection_name)
GithubCommonFilter.__init__(self, required_reviews=required_reviews,
+ reject_reviews=reject_reviews,
required_statuses=statuses)
self.statuses = statuses
self.open = open
@@ -289,6 +303,9 @@
if self.required_reviews:
ret += (' required-reviews: %s' %
str(self.required_reviews))
+ if self.reject_reviews:
+ ret += (' reject-reviews: %s' %
+ str(self.reject_reviews))
if self.open:
ret += ' open: %s' % self.open
if self.current_patchset:
@@ -320,7 +337,7 @@
else:
return False
- # required reviews are ANDed
+ # required reviews are ANDed (reject reviews are ORed)
if not self.matchesReviews(change):
return False
diff --git a/zuul/driver/github/githubsource.py b/zuul/driver/github/githubsource.py
index 1c2f727..519ebf1 100644
--- a/zuul/driver/github/githubsource.py
+++ b/zuul/driver/github/githubsource.py
@@ -101,7 +101,11 @@
return [f]
def getRejectFilters(self, config):
- return []
+ f = GithubRefFilter(
+ connection_name=self.connection.connection_name,
+ reject_reviews=to_list(config.get('review'))
+ )
+ return [f]
review = v.Schema({'username': str,