Browse Source

Fix reject clauses in the absence of approvals

This change fixes a behavior that might be surprising to a
user: If a reject clause is configured for a pipeline, the reject clause
is always true even if no approval is set. If, however, an unrelated
approval is added to the change, the reject clause is evaluated as
expected.

The root cause was a check that assumed that the absence of approvals
will lead to a negative filter result. However, this is only true for
requirements.

Change-Id: I220bbed931ced7e807febefbf2b1b33af53a8da0
changes/62/589762/4
Markus Hosch 2 years ago
parent
commit
89359b161b
6 changed files with 64 additions and 2 deletions
  1. +2
    -0
      tests/fixtures/config/requirements/reject/git/common-config/playbooks/project3-job.yaml
  2. +29
    -0
      tests/fixtures/config/requirements/reject/git/common-config/zuul.yaml
  3. +1
    -0
      tests/fixtures/config/requirements/reject/git/org_project3/README
  4. +1
    -0
      tests/fixtures/config/requirements/reject/main.yaml
  5. +30
    -0
      tests/unit/test_requirements.py
  6. +1
    -2
      zuul/driver/gerrit/gerritmodel.py

+ 2
- 0
tests/fixtures/config/requirements/reject/git/common-config/playbooks/project3-job.yaml View File

@ -0,0 +1,2 @@
- hosts: all
tasks: []

+ 29
- 0
tests/fixtures/config/requirements/reject/git/common-config/zuul.yaml View File

@ -24,6 +24,25 @@
gerrit:
Verified: -1
- pipeline:
name: rejectline
manager: independent
reject:
gerrit:
approval:
- Verified:
- -1
- -2
trigger:
gerrit:
- event: comment-added
success:
gerrit:
Verified: 1
failure:
gerrit:
Verified: -1
- pipeline:
name: trigger
manager: independent
@ -58,6 +77,10 @@
name: project2-job
run: playbooks/project2-job.yaml
- job:
name: project3-job
run: playbooks/project3-job.yaml
- project:
name: org/project1
pipeline:
@ -69,3 +92,9 @@
trigger:
jobs:
- project2-job
- project:
name: org/project3
rejectline:
jobs:
- project3-job

+ 1
- 0
tests/fixtures/config/requirements/reject/git/org_project3/README View File

@ -0,0 +1 @@
test

+ 1
- 0
tests/fixtures/config/requirements/reject/main.yaml View File

@ -7,3 +7,4 @@
untrusted-projects:
- org/project1
- org/project2
- org/project3

+ 30
- 0
tests/unit/test_requirements.py View File

@ -411,3 +411,33 @@ class TestRequirementsReject(ZuulTestCase):
def test_trigger_require_reject(self):
"Test trigger requirement: rejections absent"
return self._test_require_reject('org/project2', 'project2-job')
def test_pipeline_requirement_reject_unrelated(self):
"Test if reject is obeyed if another unrelated approval is present"
# Having no approvals whatsoever shall not reject the change
A = self.fake_gerrit.addFakeChange('org/project3', 'master', 'A')
self.fake_gerrit.addEvent(A.getChangeCommentEvent(1))
self.waitUntilSettled()
self.assertEqual(len(self.history), 1)
# Setting another unrelated approval shall not change the behavior of
# the configured reject.
comment = A.addApproval('Approved', 1, username='reviewer_e')
self.fake_gerrit.addEvent(comment)
self.waitUntilSettled()
self.assertEqual(len(self.history), 2)
# Setting the approval 'Verified' to a rejected value shall not lead to
# a build.
comment = A.addApproval('Verified', -1, username='jenkins')
self.fake_gerrit.addEvent(comment)
self.waitUntilSettled()
self.assertEqual(len(self.history), 2)
# Setting the approval 'Verified' to an accepted value shall lead to
# a build.
comment = A.addApproval('Verified', 1, username='jenkins')
self.fake_gerrit.addEvent(comment)
self.waitUntilSettled()
self.assertEqual(len(self.history), 3)

+ 1
- 2
zuul/driver/gerrit/gerritmodel.py View File

@ -113,8 +113,7 @@ class GerritApprovalFilter(object):
if not hasattr(change, 'number'):
# Not a change, no reviews
return False
if (self.required_approvals and not change.approvals
or self.reject_approvals and not change.approvals):
if self.required_approvals and not change.approvals:
# A change with no approvals can not match
return False


Loading…
Cancel
Save