Add support for negative requirements
This change adds support for false matching of requirements. To make
this useful you can now require all approvals match a requirement or
only one (ie any).
Therefore depreciate require-approvals, replacing with
require-approvals-any and a new require-approvals-all.
Change-Id: I458e677315ccb90d64cd0c0e734951141324a9c3
diff --git a/tests/fixtures/layout-requirement-all.yaml b/tests/fixtures/layout-requirement-all.yaml
new file mode 100644
index 0000000..968739d
--- /dev/null
+++ b/tests/fixtures/layout-requirement-all.yaml
@@ -0,0 +1,41 @@
+pipelines:
+ - name: pipeline
+ manager: IndependentPipelineManager
+ require:
+ all-approvals:
+ - username: jenkins
+ verified: [1, 2]
+ - verified: "![-1, -2]"
+ trigger:
+ gerrit:
+ - event: comment-added
+ success:
+ gerrit:
+ verified: 1
+ failure:
+ gerrit:
+ verified: -1
+
+ - name: trigger
+ manager: IndependentPipelineManager
+ trigger:
+ gerrit:
+ - event: comment-added
+ require-all-approvals:
+ - username: jenkins
+ verified: [1, 2]
+ - verified: "![-1, -2]"
+ success:
+ gerrit:
+ verified: 1
+ failure:
+ gerrit:
+ verified: -1
+
+projects:
+ - name: org/project1
+ pipeline:
+ - project1-pipeline
+ - name: org/project2
+ trigger:
+ - project2-trigger
diff --git a/tests/fixtures/layout-requirement-any.yaml b/tests/fixtures/layout-requirement-any.yaml
new file mode 100644
index 0000000..6275d8d
--- /dev/null
+++ b/tests/fixtures/layout-requirement-any.yaml
@@ -0,0 +1,43 @@
+pipelines:
+ - name: pipeline
+ manager: IndependentPipelineManager
+ require:
+ any-approval:
+ - username: jenkins
+ verified: [1, 2]
+ - username: core-reviewer
+ code-review: "![-1, -2]"
+ trigger:
+ gerrit:
+ - event: comment-added
+ success:
+ gerrit:
+ verified: 1
+ failure:
+ gerrit:
+ verified: -1
+
+ - name: trigger
+ manager: IndependentPipelineManager
+ trigger:
+ gerrit:
+ - event: comment-added
+ require-any-approval:
+ - username: jenkins
+ verified: [1, 2]
+ - username: core-reviewer
+ code-review: "![-1, -2]"
+ success:
+ gerrit:
+ verified: 1
+ failure:
+ gerrit:
+ verified: -1
+
+projects:
+ - name: org/project1
+ pipeline:
+ - project1-pipeline
+ - name: org/project2
+ trigger:
+ - project2-trigger
diff --git a/tests/fixtures/layout-requirement-email.yaml b/tests/fixtures/layout-requirement-email.yaml
index 4bfb733..dadcd6c 100644
--- a/tests/fixtures/layout-requirement-email.yaml
+++ b/tests/fixtures/layout-requirement-email.yaml
@@ -2,7 +2,7 @@
- name: pipeline
manager: IndependentPipelineManager
require:
- approval:
+ any-approval:
- email: jenkins@example.com
trigger:
gerrit:
@@ -19,7 +19,7 @@
trigger:
gerrit:
- event: comment-added
- require-approval:
+ require-any-approval:
- email: jenkins@example.com
success:
gerrit:
diff --git a/tests/fixtures/layout-requirement-negative-username.yaml b/tests/fixtures/layout-requirement-negative-username.yaml
new file mode 100644
index 0000000..f542b86
--- /dev/null
+++ b/tests/fixtures/layout-requirement-negative-username.yaml
@@ -0,0 +1,37 @@
+pipelines:
+ - name: pipeline
+ manager: IndependentPipelineManager
+ require:
+ all-approvals:
+ - username: '!jenkins'
+ trigger:
+ gerrit:
+ - event: comment-added
+ success:
+ gerrit:
+ verified: 1
+ failure:
+ gerrit:
+ verified: -1
+
+ - name: trigger
+ manager: IndependentPipelineManager
+ trigger:
+ gerrit:
+ - event: comment-added
+ require-all-approvals:
+ - username: '!jenkins'
+ success:
+ gerrit:
+ verified: 1
+ failure:
+ gerrit:
+ verified: -1
+
+projects:
+ - name: org/project1
+ pipeline:
+ - project1-pipeline
+ - name: org/project2
+ trigger:
+ - project2-trigger
\ No newline at end of file
diff --git a/tests/fixtures/layout-requirement-newer-than.yaml b/tests/fixtures/layout-requirement-newer-than.yaml
index b6beb35..f723c79 100644
--- a/tests/fixtures/layout-requirement-newer-than.yaml
+++ b/tests/fixtures/layout-requirement-newer-than.yaml
@@ -2,7 +2,7 @@
- name: pipeline
manager: IndependentPipelineManager
require:
- approval:
+ any-approval:
- username: jenkins
newer-than: 48h
trigger:
@@ -20,7 +20,7 @@
trigger:
gerrit:
- event: comment-added
- require-approval:
+ require-any-approval:
- username: jenkins
newer-than: 48h
success:
diff --git a/tests/fixtures/layout-requirement-older-than.yaml b/tests/fixtures/layout-requirement-older-than.yaml
index 2edf9df..0e011cc 100644
--- a/tests/fixtures/layout-requirement-older-than.yaml
+++ b/tests/fixtures/layout-requirement-older-than.yaml
@@ -2,7 +2,7 @@
- name: pipeline
manager: IndependentPipelineManager
require:
- approval:
+ any-approval:
- username: jenkins
older-than: 48h
trigger:
@@ -20,7 +20,7 @@
trigger:
gerrit:
- event: comment-added
- require-approval:
+ require-any-approval:
- username: jenkins
older-than: 48h
success:
diff --git a/tests/fixtures/layout-requirement-username.yaml b/tests/fixtures/layout-requirement-username.yaml
index 7a549f0..8520179 100644
--- a/tests/fixtures/layout-requirement-username.yaml
+++ b/tests/fixtures/layout-requirement-username.yaml
@@ -2,7 +2,7 @@
- name: pipeline
manager: IndependentPipelineManager
require:
- approval:
+ any-approval:
- username: jenkins
trigger:
gerrit:
@@ -19,7 +19,7 @@
trigger:
gerrit:
- event: comment-added
- require-approval:
+ require-any-approval:
- username: jenkins
success:
gerrit:
diff --git a/tests/fixtures/layout-requirement-vote.yaml b/tests/fixtures/layout-requirement-vote.yaml
index 7ccadff..6736e98 100644
--- a/tests/fixtures/layout-requirement-vote.yaml
+++ b/tests/fixtures/layout-requirement-vote.yaml
@@ -2,7 +2,7 @@
- name: pipeline
manager: IndependentPipelineManager
require:
- approval:
+ any-approval:
- username: jenkins
verified: 1
trigger:
@@ -20,7 +20,7 @@
trigger:
gerrit:
- event: comment-added
- require-approval:
+ require-any-approval:
- username: jenkins
verified: 1
success:
diff --git a/tests/fixtures/layout-requirement-vote1.yaml b/tests/fixtures/layout-requirement-vote1.yaml
index 7ccadff..6736e98 100644
--- a/tests/fixtures/layout-requirement-vote1.yaml
+++ b/tests/fixtures/layout-requirement-vote1.yaml
@@ -2,7 +2,7 @@
- name: pipeline
manager: IndependentPipelineManager
require:
- approval:
+ any-approval:
- username: jenkins
verified: 1
trigger:
@@ -20,7 +20,7 @@
trigger:
gerrit:
- event: comment-added
- require-approval:
+ require-any-approval:
- username: jenkins
verified: 1
success:
diff --git a/tests/fixtures/layout-requirement-vote2.yaml b/tests/fixtures/layout-requirement-vote2.yaml
index 33d84d1..a6cd6a3 100644
--- a/tests/fixtures/layout-requirement-vote2.yaml
+++ b/tests/fixtures/layout-requirement-vote2.yaml
@@ -2,7 +2,7 @@
- name: pipeline
manager: IndependentPipelineManager
require:
- approval:
+ any-approval:
- username: jenkins
verified: [1, 2]
trigger:
@@ -20,7 +20,7 @@
trigger:
gerrit:
- event: comment-added
- require-approval:
+ require-any-approval:
- username: jenkins
verified: [1, 2]
success:
diff --git a/tests/test_requirements.py b/tests/test_requirements.py
index 4316925..52e3973 100644
--- a/tests/test_requirements.py
+++ b/tests/test_requirements.py
@@ -323,3 +323,131 @@
self.fake_gerrit.addEvent(B.addApproval('CRVW', 2))
self.waitUntilSettled()
self.assertEqual(len(self.history), 1)
+
+ def test_pipeline_require_negative_username(self):
+ "Test negative pipeline requirement: no comment from jenkins"
+ return self._test_require_negative_username('org/project1',
+ 'project1-pipeline')
+
+ def test_trigger_require_negative_username(self):
+ "Test negative trigger requirement: no comment from jenkins"
+ return self._test_require_negative_username('org/project2',
+ 'project2-trigger')
+
+ def _test_require_negative_username(self, project, job):
+ "Test negative username's match"
+ # Should only trigger if Jenkins hasn't voted.
+ self.config.set(
+ 'zuul', 'layout_config',
+ 'tests/fixtures/layout-requirement-negative-username.yaml')
+ self.sched.reconfigure(self.config)
+ self.registerJobs()
+
+ # add in a change with no comments
+ A = self.fake_gerrit.addFakeChange(project, 'master', 'A')
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 0)
+
+ # add in a comment that will trigger
+ self.fake_gerrit.addEvent(A.addApproval('CRVW', 1,
+ username='reviewer'))
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 1)
+ self.assertEqual(self.history[0].name, job)
+
+ # add in a comment from jenkins user which shouldn't trigger
+ self.fake_gerrit.addEvent(A.addApproval('VRFY', 1, username='jenkins'))
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 1)
+
+ # Check future reviews also won't trigger as a 'jenkins' user has
+ # commented previously
+ self.fake_gerrit.addEvent(A.addApproval('CRVW', 1,
+ username='reviewer'))
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 1)
+
+ def test_pipeline_require_any(self):
+ "Test pipeline requirement: any requirement passes"
+ return self._test_require_any('org/project1', 'project1-pipeline')
+
+ def test_trigger_require_any(self):
+ "Test trigger requirement: any requirement passes"
+ return self._test_require_any('org/project2', 'project2-trigger')
+
+ def _test_require_any(self, project, job):
+ "Test any of the given requirements are matched"
+ self.config.set(
+ 'zuul', 'layout_config',
+ 'tests/fixtures/layout-requirement-any.yaml')
+ self.sched.reconfigure(self.config)
+ self.registerJobs()
+
+ A = self.fake_gerrit.addFakeChange(project, 'master', 'A')
+ # A comment event that we will keep submitting to trigger
+ comment = A.addApproval('CRVW', 1, username='nobody')
+ self.fake_gerrit.addEvent(comment)
+ self.waitUntilSettled()
+ # No approval from Jenkins so should not be enqueued
+ self.assertEqual(len(self.history), 0)
+
+ # A +1 from jenkins should allow it to be enqueued
+ A.addApproval('VRFY', 1, username='jenkins')
+ self.fake_gerrit.addEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 1)
+ self.assertEqual(self.history[0].name, job)
+
+ # A non-negative from a non-core should not queue
+ B = self.fake_gerrit.addFakeChange(project, 'master', 'B')
+ # A comment event that we will keep submitting to trigger
+ comment = B.addApproval('CRVW', 1, username='nobody')
+ self.fake_gerrit.addEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 1)
+
+ # A non-negative from a core member should queue
+ B.addApproval('CRVW', 2, username='core-reviewer')
+ self.fake_gerrit.addEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 2)
+ self.assertEqual(self.history[1].name, job)
+
+ def test_pipeline_require_all(self):
+ "Test pipeline requirement: all requirements pass"
+ return self._test_require_all('org/project1', 'project1-pipeline')
+
+ def test_trigger_require_all(self):
+ "Test trigger requirement: all requirements pass"
+ return self._test_require_all('org/project2', 'project2-trigger')
+
+ def _test_require_all(self, project, job):
+ "Test all of the given requirements are matched"
+ self.config.set(
+ 'zuul', 'layout_config',
+ 'tests/fixtures/layout-requirement-all.yaml')
+ self.sched.reconfigure(self.config)
+ self.registerJobs()
+
+ A = self.fake_gerrit.addFakeChange(project, 'master', 'A')
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 0)
+
+ # A +2 from 'nobody' only satisfies the non-negative requirement,
+ # not the requirement to be from 'jenkins'
+ comment = A.addApproval('VRFY', 1, username='nobody')
+ self.fake_gerrit.addEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 0)
+
+ B = self.fake_gerrit.addFakeChange(project, 'master', 'A')
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 0)
+
+ # A +2 from Jenkins satisfies both the user condition and the
+ # non-negative condition
+ comment = B.addApproval('VRFY', 2, username='jenkins')
+ self.fake_gerrit.addEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 1)
+ self.assertEqual(self.history[0].name, job)