Cleanup approval requirement testing
On further inspection, trigger requirements will still be required
for parts of the kind of pipeline construction that we are using in
OpenStack; the idea that they could be completely replaced by
pipeline requirements is incorrect. To that end, un-deprecate
trigger requirements and cleanup the testing of both of them so that
each requirement is tested (relatively) independently. A small
amount of requirement composition is also tested, though all possible
combinations (a lot!) are not.
Move this testing into a new file for organizational purposes.
Also, support multiple vote values (eg, "+1" or "+2") when requiring
an approval.
Change-Id: I683c9a574ced0e27ced59e62b8059fef2dfd8b20
diff --git a/doc/source/zuul.rst b/doc/source/zuul.rst
index 4c5a624..e3ad2e8 100644
--- a/doc/source/zuul.rst
+++ b/doc/source/zuul.rst
@@ -423,13 +423,11 @@
containing 'retrigger' somewhere in the comment text are added to a
change.
- *require-approval* (deprecated)
+ *require-approval*
This may be used for any event. It requires that a certain kind
of approval be present for the current patchset of the change (the
approval could be added by the event in question). It follows the
- same syntax as the "approval" pipeline requirement below. This
- form should be considered deprecated and the pipeline requirement
- used instead.
+ same syntax as the "approval" pipeline requirement below.
**timer**
This trigger will run based on a cron-style time specification.
@@ -475,7 +473,9 @@
Any other field is interpreted as a review category and value
pair. For example ``verified: 1`` would require that the approval
- be for a +1 vote in the "Verified" column.
+ be for a +1 vote in the "Verified" column. The value may either
+ be a single value or a list: ``verified: [1, 2]`` would match
+ either a +1 or +2 vote.
**open**
A boolean value (``true`` or ``false``) that indicates whether the change
diff --git a/tests/fixtures/layout-pipeline-requirements.yaml b/tests/fixtures/layout-pipeline-requirements.yaml
deleted file mode 100644
index 1826d88..0000000
--- a/tests/fixtures/layout-pipeline-requirements.yaml
+++ /dev/null
@@ -1,59 +0,0 @@
-includes:
- - python-file: custom_functions.py
-
-pipelines:
- - name: check
- manager: IndependentPipelineManager
- require:
- approval:
- - email-filter: jenkins@example.com
- older-than: 48h
- open: True
- trigger:
- gerrit:
- - event: patchset-created
- - event: comment-added
- success:
- gerrit:
- verified: 1
- failure:
- gerrit:
- verified: -1
-
- - name: gate
- manager: DependentPipelineManager
- failure-message: Build failed. For information on how to proceed, see http://wiki.example.org/Test_Failures
- require:
- status:
- - NEW
- approval:
- - verified: 1
- username: jenkins
- newer-than: 48h
- trigger:
- gerrit:
- - event: comment-added
- approval:
- - approved: 1
- - event: comment-added
- approval:
- - verified: 1
- success:
- gerrit:
- verified: 2
- submit: true
- failure:
- gerrit:
- verified: -2
- start:
- gerrit:
- verified: 0
- precedence: high
-
-projects:
- - name: org/project
- merge-mode: cherry-pick
- check:
- - project-check
- gate:
- - project-gate
diff --git a/tests/fixtures/layout-require-approval.yaml b/tests/fixtures/layout-require-approval.yaml
deleted file mode 100644
index 18eee99..0000000
--- a/tests/fixtures/layout-require-approval.yaml
+++ /dev/null
@@ -1,58 +0,0 @@
-includes:
- - python-file: custom_functions.py
-
-pipelines:
- - name: check
- manager: IndependentPipelineManager
- trigger:
- gerrit:
- - event: patchset-created
- - event: comment-added
- require-approval:
- - email-filter: jenkins@example.com
- older-than: 48h
- success:
- gerrit:
- verified: 1
- failure:
- gerrit:
- verified: -1
-
- - name: gate
- manager: DependentPipelineManager
- failure-message: Build failed. For information on how to proceed, see http://wiki.example.org/Test_Failures
- trigger:
- gerrit:
- - event: comment-added
- require-approval:
- - verified: 1
- username: jenkins
- newer-than: 48h
- approval:
- - approved: 1
- - event: comment-added
- require-approval:
- - verified: 1
- username: jenkins
- newer-than: 48h
- approval:
- - verified: 1
- success:
- gerrit:
- verified: 2
- submit: true
- failure:
- gerrit:
- verified: -2
- start:
- gerrit:
- verified: 0
- precedence: high
-
-projects:
- - name: org/project
- merge-mode: cherry-pick
- check:
- - project-check
- gate:
- - project-gate
diff --git a/tests/fixtures/layout-current-patchset.yaml b/tests/fixtures/layout-requirement-current-patchset.yaml
similarity index 82%
rename from tests/fixtures/layout-current-patchset.yaml
rename to tests/fixtures/layout-requirement-current-patchset.yaml
index dc8f768..405077e 100644
--- a/tests/fixtures/layout-current-patchset.yaml
+++ b/tests/fixtures/layout-requirement-current-patchset.yaml
@@ -1,6 +1,3 @@
-includes:
- - python-file: custom_functions.py
-
pipelines:
- name: check
manager: IndependentPipelineManager
@@ -19,6 +16,5 @@
projects:
- name: org/project
- merge-mode: cherry-pick
check:
- project-check
diff --git a/tests/fixtures/layout-requirement-email.yaml b/tests/fixtures/layout-requirement-email.yaml
new file mode 100644
index 0000000..6ed7603
--- /dev/null
+++ b/tests/fixtures/layout-requirement-email.yaml
@@ -0,0 +1,37 @@
+pipelines:
+ - name: pipeline
+ manager: IndependentPipelineManager
+ require:
+ approval:
+ - email-filter: jenkins@example.com
+ trigger:
+ gerrit:
+ - event: comment-added
+ success:
+ gerrit:
+ verified: 1
+ failure:
+ gerrit:
+ verified: -1
+
+ - name: trigger
+ manager: IndependentPipelineManager
+ trigger:
+ gerrit:
+ - event: comment-added
+ require-approval:
+ - email-filter: jenkins@example.com
+ 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-newer-than.yaml b/tests/fixtures/layout-requirement-newer-than.yaml
new file mode 100644
index 0000000..b6beb35
--- /dev/null
+++ b/tests/fixtures/layout-requirement-newer-than.yaml
@@ -0,0 +1,39 @@
+pipelines:
+ - name: pipeline
+ manager: IndependentPipelineManager
+ require:
+ approval:
+ - username: jenkins
+ newer-than: 48h
+ trigger:
+ gerrit:
+ - event: comment-added
+ success:
+ gerrit:
+ verified: 1
+ failure:
+ gerrit:
+ verified: -1
+
+ - name: trigger
+ manager: IndependentPipelineManager
+ trigger:
+ gerrit:
+ - event: comment-added
+ require-approval:
+ - username: jenkins
+ newer-than: 48h
+ 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-older-than.yaml b/tests/fixtures/layout-requirement-older-than.yaml
new file mode 100644
index 0000000..2edf9df
--- /dev/null
+++ b/tests/fixtures/layout-requirement-older-than.yaml
@@ -0,0 +1,39 @@
+pipelines:
+ - name: pipeline
+ manager: IndependentPipelineManager
+ require:
+ approval:
+ - username: jenkins
+ older-than: 48h
+ trigger:
+ gerrit:
+ - event: comment-added
+ success:
+ gerrit:
+ verified: 1
+ failure:
+ gerrit:
+ verified: -1
+
+ - name: trigger
+ manager: IndependentPipelineManager
+ trigger:
+ gerrit:
+ - event: comment-added
+ require-approval:
+ - username: jenkins
+ older-than: 48h
+ 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-current-patchset.yaml b/tests/fixtures/layout-requirement-open.yaml
similarity index 75%
copy from tests/fixtures/layout-current-patchset.yaml
copy to tests/fixtures/layout-requirement-open.yaml
index dc8f768..e62719d 100644
--- a/tests/fixtures/layout-current-patchset.yaml
+++ b/tests/fixtures/layout-requirement-open.yaml
@@ -1,11 +1,8 @@
-includes:
- - python-file: custom_functions.py
-
pipelines:
- name: check
manager: IndependentPipelineManager
require:
- current-patchset: True
+ open: True
trigger:
gerrit:
- event: patchset-created
@@ -19,6 +16,5 @@
projects:
- name: org/project
- merge-mode: cherry-pick
check:
- project-check
diff --git a/tests/fixtures/layout-current-patchset.yaml b/tests/fixtures/layout-requirement-status.yaml
similarity index 75%
copy from tests/fixtures/layout-current-patchset.yaml
copy to tests/fixtures/layout-requirement-status.yaml
index dc8f768..af33468 100644
--- a/tests/fixtures/layout-current-patchset.yaml
+++ b/tests/fixtures/layout-requirement-status.yaml
@@ -1,11 +1,8 @@
-includes:
- - python-file: custom_functions.py
-
pipelines:
- name: check
manager: IndependentPipelineManager
require:
- current-patchset: True
+ status: NEW
trigger:
gerrit:
- event: patchset-created
@@ -19,6 +16,5 @@
projects:
- name: org/project
- merge-mode: cherry-pick
check:
- project-check
diff --git a/tests/fixtures/layout-requirement-username.yaml b/tests/fixtures/layout-requirement-username.yaml
new file mode 100644
index 0000000..7a549f0
--- /dev/null
+++ b/tests/fixtures/layout-requirement-username.yaml
@@ -0,0 +1,37 @@
+pipelines:
+ - name: pipeline
+ manager: IndependentPipelineManager
+ require:
+ approval:
+ - 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-approval:
+ - username: jenkins
+ 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-vote.yaml b/tests/fixtures/layout-requirement-vote.yaml
new file mode 100644
index 0000000..7ccadff
--- /dev/null
+++ b/tests/fixtures/layout-requirement-vote.yaml
@@ -0,0 +1,39 @@
+pipelines:
+ - name: pipeline
+ manager: IndependentPipelineManager
+ require:
+ approval:
+ - username: jenkins
+ verified: 1
+ trigger:
+ gerrit:
+ - event: comment-added
+ success:
+ gerrit:
+ verified: 1
+ failure:
+ gerrit:
+ verified: -1
+
+ - name: trigger
+ manager: IndependentPipelineManager
+ trigger:
+ gerrit:
+ - event: comment-added
+ require-approval:
+ - username: jenkins
+ verified: 1
+ 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-vote1.yaml b/tests/fixtures/layout-requirement-vote1.yaml
new file mode 100644
index 0000000..7ccadff
--- /dev/null
+++ b/tests/fixtures/layout-requirement-vote1.yaml
@@ -0,0 +1,39 @@
+pipelines:
+ - name: pipeline
+ manager: IndependentPipelineManager
+ require:
+ approval:
+ - username: jenkins
+ verified: 1
+ trigger:
+ gerrit:
+ - event: comment-added
+ success:
+ gerrit:
+ verified: 1
+ failure:
+ gerrit:
+ verified: -1
+
+ - name: trigger
+ manager: IndependentPipelineManager
+ trigger:
+ gerrit:
+ - event: comment-added
+ require-approval:
+ - username: jenkins
+ verified: 1
+ 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-vote2.yaml b/tests/fixtures/layout-requirement-vote2.yaml
new file mode 100644
index 0000000..33d84d1
--- /dev/null
+++ b/tests/fixtures/layout-requirement-vote2.yaml
@@ -0,0 +1,39 @@
+pipelines:
+ - name: pipeline
+ manager: IndependentPipelineManager
+ require:
+ approval:
+ - username: jenkins
+ 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-approval:
+ - username: jenkins
+ 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/test_requirements.py b/tests/test_requirements.py
new file mode 100644
index 0000000..120e37e
--- /dev/null
+++ b/tests/test_requirements.py
@@ -0,0 +1,323 @@
+#!/usr/bin/env python
+
+# Copyright 2012-2014 Hewlett-Packard Development Company, L.P.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+import logging
+import time
+
+from tests.base import ZuulTestCase
+
+logging.basicConfig(level=logging.DEBUG,
+ format='%(asctime)s %(name)-32s '
+ '%(levelname)-8s %(message)s')
+
+
+class TestRequirements(ZuulTestCase):
+ """Test pipeline and trigger requirements"""
+
+ def test_pipeline_require_approval_newer_than(self):
+ "Test pipeline requirement: approval newer than"
+ return self._test_require_approval_newer_than('org/project1',
+ 'project1-pipeline')
+
+ def test_trigger_require_approval_newer_than(self):
+ "Test trigger requirement: approval newer than"
+ return self._test_require_approval_newer_than('org/project2',
+ 'project2-trigger')
+
+ def _test_require_approval_newer_than(self, project, job):
+ self.config.set('zuul', 'layout_config',
+ 'tests/fixtures/layout-requirement-newer-than.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', 2, username='nobody')
+ self.fake_gerrit.addEvent(comment)
+ self.waitUntilSettled()
+ # No +1 from Jenkins so should not be enqueued
+ self.assertEqual(len(self.history), 0)
+
+ # Add a too-old +1, should not be enqueued
+ A.addApproval('VRFY', 1, granted_on=time.time() - 72 * 60 * 60)
+ self.fake_gerrit.addEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 0)
+
+ # Add a recent +1
+ self.fake_gerrit.addEvent(A.addApproval('VRFY', 1))
+ self.fake_gerrit.addEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 1)
+ self.assertEqual(self.history[0].name, job)
+
+ def test_pipeline_require_approval_older_than(self):
+ "Test pipeline requirement: approval older than"
+ return self._test_require_approval_older_than('org/project1',
+ 'project1-pipeline')
+
+ def test_trigger_require_approval_older_than(self):
+ "Test trigger requirement: approval older than"
+ return self._test_require_approval_older_than('org/project2',
+ 'project2-trigger')
+
+ def _test_require_approval_older_than(self, project, job):
+ self.config.set('zuul', 'layout_config',
+ 'tests/fixtures/layout-requirement-older-than.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', 2, username='nobody')
+ self.fake_gerrit.addEvent(comment)
+ self.waitUntilSettled()
+ # No +1 from Jenkins so should not be enqueued
+ self.assertEqual(len(self.history), 0)
+
+ # Add a recent +1 which should not be enqueued
+ A.addApproval('VRFY', 1)
+ self.fake_gerrit.addEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 0)
+
+ # Add an old +1 which should be enqueued
+ A.addApproval('VRFY', 1, granted_on=time.time() - 72 * 60 * 60)
+ self.fake_gerrit.addEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 1)
+ self.assertEqual(self.history[0].name, job)
+
+ def test_pipeline_require_approval_username(self):
+ "Test pipeline requirement: approval username"
+ return self._test_require_approval_username('org/project1',
+ 'project1-pipeline')
+
+ def test_trigger_require_approval_username(self):
+ "Test trigger requirement: approval username"
+ return self._test_require_approval_username('org/project2',
+ 'project2-trigger')
+
+ def _test_require_approval_username(self, project, job):
+ self.config.set('zuul', 'layout_config',
+ 'tests/fixtures/layout-requirement-username.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', 2, username='nobody')
+ self.fake_gerrit.addEvent(comment)
+ self.waitUntilSettled()
+ # No approval from Jenkins so should not be enqueued
+ self.assertEqual(len(self.history), 0)
+
+ # Add an approval from Jenkins
+ A.addApproval('VRFY', 1)
+ self.fake_gerrit.addEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 1)
+ self.assertEqual(self.history[0].name, job)
+
+ def test_pipeline_require_approval_email(self):
+ "Test pipeline requirement: approval email"
+ return self._test_require_approval_email('org/project1',
+ 'project1-pipeline')
+
+ def test_trigger_require_approval_email(self):
+ "Test trigger requirement: approval email"
+ return self._test_require_approval_email('org/project2',
+ 'project2-trigger')
+
+ def _test_require_approval_email(self, project, job):
+ self.config.set('zuul', 'layout_config',
+ 'tests/fixtures/layout-requirement-email.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', 2, username='nobody')
+ self.fake_gerrit.addEvent(comment)
+ self.waitUntilSettled()
+ # No approval from Jenkins so should not be enqueued
+ self.assertEqual(len(self.history), 0)
+
+ # Add an approval from Jenkins
+ A.addApproval('VRFY', 1)
+ self.fake_gerrit.addEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 1)
+ self.assertEqual(self.history[0].name, job)
+
+ def test_pipeline_require_approval_vote1(self):
+ "Test pipeline requirement: approval vote with one value"
+ return self._test_require_approval_vote1('org/project1',
+ 'project1-pipeline')
+
+ def test_trigger_require_approval_vote1(self):
+ "Test trigger requirement: approval vote with one value"
+ return self._test_require_approval_vote1('org/project2',
+ 'project2-trigger')
+
+ def _test_require_approval_vote1(self, project, job):
+ self.config.set('zuul', 'layout_config',
+ 'tests/fixtures/layout-requirement-vote1.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', 2, 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 not cause it to be enqueued
+ A.addApproval('VRFY', -1)
+ self.fake_gerrit.addEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 0)
+
+ # A +1 should allow it to be enqueued
+ A.addApproval('VRFY', 1)
+ self.fake_gerrit.addEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 1)
+ self.assertEqual(self.history[0].name, job)
+
+ def test_pipeline_require_approval_vote2(self):
+ "Test pipeline requirement: approval vote with two values"
+ return self._test_require_approval_vote2('org/project1',
+ 'project1-pipeline')
+
+ def test_trigger_require_approval_vote2(self):
+ "Test trigger requirement: approval vote with two values"
+ return self._test_require_approval_vote2('org/project2',
+ 'project2-trigger')
+
+ def _test_require_approval_vote2(self, project, job):
+ self.config.set('zuul', 'layout_config',
+ 'tests/fixtures/layout-requirement-vote2.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', 2, 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 not cause it to be enqueued
+ A.addApproval('VRFY', -1)
+ self.fake_gerrit.addEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 0)
+
+ # A -2 from jenkins should not cause it to be enqueued
+ A.addApproval('VRFY', -2)
+ self.fake_gerrit.addEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 0)
+
+ # A +1 should allow it to be enqueued
+ A.addApproval('VRFY', 1)
+ self.fake_gerrit.addEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 1)
+ self.assertEqual(self.history[0].name, job)
+
+ # A +2 should allow it to be enqueued
+ B = self.fake_gerrit.addFakeChange(project, 'master', 'B')
+ # A comment event that we will keep submitting to trigger
+ comment = B.addApproval('CRVW', 2, username='nobody')
+ self.fake_gerrit.addEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 1)
+
+ B.addApproval('VRFY', 2)
+ self.fake_gerrit.addEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 2)
+ self.assertEqual(self.history[1].name, job)
+
+ def test_pipeline_require_current_patchset(self):
+ "Test pipeline requirement: current-patchset"
+ self.config.set('zuul', 'layout_config',
+ 'tests/fixtures/layout-requirement-'
+ 'current-patchset.yaml')
+ self.sched.reconfigure(self.config)
+ self.registerJobs()
+ # Create two patchsets and let their tests settle out. Then
+ # comment on first patchset and check that no additional
+ # jobs are run.
+ A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
+ self.fake_gerrit.addEvent(A.addApproval('CRVW', 1))
+ self.waitUntilSettled()
+ A.addPatchset()
+ self.fake_gerrit.addEvent(A.addApproval('CRVW', 1))
+ self.waitUntilSettled()
+
+ self.assertEqual(len(self.history), 2) # one job for each ps
+ self.fake_gerrit.addEvent(A.getChangeCommentEvent(1))
+ self.waitUntilSettled()
+
+ # Assert no new jobs ran after event for old patchset.
+ self.assertEqual(len(self.history), 2)
+
+ # Make sure the same event on a new PS will trigger
+ self.fake_gerrit.addEvent(A.getChangeCommentEvent(2))
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 3)
+
+ def test_pipeline_require_open(self):
+ "Test pipeline requirement: open"
+ self.config.set('zuul', 'layout_config',
+ 'tests/fixtures/layout-requirement-open.yaml')
+ self.sched.reconfigure(self.config)
+ self.registerJobs()
+
+ A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A',
+ status='MERGED')
+ self.fake_gerrit.addEvent(A.addApproval('CRVW', 2))
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 0)
+
+ B = self.fake_gerrit.addFakeChange('org/project', 'master', 'B')
+ self.fake_gerrit.addEvent(B.addApproval('CRVW', 2))
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 1)
+
+ def test_pipeline_require_status(self):
+ "Test pipeline requirement: status"
+ self.config.set('zuul', 'layout_config',
+ 'tests/fixtures/layout-requirement-status.yaml')
+ self.sched.reconfigure(self.config)
+ self.registerJobs()
+
+ A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A',
+ status='MERGED')
+ self.fake_gerrit.addEvent(A.addApproval('CRVW', 2))
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 0)
+
+ B = self.fake_gerrit.addFakeChange('org/project', 'master', 'B')
+ self.fake_gerrit.addEvent(B.addApproval('CRVW', 2))
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 1)
diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py
index 6e65774..70a9ec1 100755
--- a/tests/test_scheduler.py
+++ b/tests/test_scheduler.py
@@ -664,31 +664,6 @@
self.assertTrue(trigger.canMerge(a, mgr.getSubmitAllowNeeds()))
trigger.maintainCache([])
- def test_pipeline_requirements_closed_change(self):
- "Test that pipeline requirements for closed changes are effective"
- self.config.set('zuul', 'layout_config',
- 'tests/fixtures/layout-pipeline-requirements.yaml')
- self.sched.reconfigure(self.config)
-
- A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A',
- status='MERGED')
- self.fake_gerrit.addEvent(A.addApproval('CRVW', 2))
- self.waitUntilSettled()
- self.assertEqual(len(self.history), 0)
- self.assertEqual(len(self.builds), 0)
-
- B = self.fake_gerrit.addFakeChange('org/project', 'master', 'B',
- status='MERGED')
- B.addApproval('CRVW', 2)
- B.addApproval('VRFY', 1)
- self.fake_gerrit.addEvent(B.addApproval('APRV', 1))
- self.waitUntilSettled()
- self.assertEqual(len(self.history), 0)
- self.assertEqual(len(self.builds), 0)
-
- for pipeline in self.sched.layout.pipelines.values():
- pipeline.trigger.maintainCache([])
-
def test_build_configuration(self):
"Test that zuul merges the right commits for testing"
@@ -1651,135 +1626,6 @@
self.assertEqual(D.data['status'], 'MERGED')
self.assertEqual(D.reported, 2)
- def test_pipeline_requirements_approval_check_and_gate(self):
- "Test pipeline requirements triggers both check and gate"
- self.config.set('zuul', 'layout_config',
- 'tests/fixtures/layout-pipeline-requirements.yaml')
- self.sched.reconfigure(self.config)
- self.registerJobs()
- self._test_required_approval_check_and_gate()
-
- def test_required_approval_check_and_gate(self):
- "Test required-approval triggers both check and gate"
- self.config.set('zuul', 'layout_config',
- 'tests/fixtures/layout-require-approval.yaml')
- self.sched.reconfigure(self.config)
- self.registerJobs()
- self._test_required_approval_check_and_gate()
-
- def _test_required_approval_check_and_gate(self):
- A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
- A.addApproval('CRVW', 2)
- # Add a too-old +1
- A.addApproval('VRFY', 1, granted_on=time.time() - 72 * 60 * 60)
-
- aprv = A.addApproval('APRV', 1)
- self.fake_gerrit.addEvent(aprv)
- self.waitUntilSettled()
- # Should have run a check job
- self.assertEqual(len(self.history), 1)
- self.assertEqual(self.history[0].name, 'project-check')
-
- # Report the result of that check job (overrides previous vrfy)
- # Skynet alert: this should trigger a gate job now that
- # all reqs are met
- self.fake_gerrit.addEvent(A.addApproval('VRFY', 1))
- self.waitUntilSettled()
- self.assertEqual(len(self.history), 2)
- self.assertEqual(self.history[1].name, 'project-gate')
-
- def test_pipeline_requirements_approval_newer(self):
- "Test pipeline requirements newer trigger parameter"
- self.config.set('zuul', 'layout_config',
- 'tests/fixtures/layout-pipeline-requirements.yaml')
- self.sched.reconfigure(self.config)
- self.registerJobs()
- self._test_required_approval_newer()
-
- def test_required_approval_newer(self):
- "Test required-approval newer trigger parameter"
- self.config.set('zuul', 'layout_config',
- 'tests/fixtures/layout-require-approval.yaml')
- self.sched.reconfigure(self.config)
- self.registerJobs()
- self._test_required_approval_newer()
-
- def _test_required_approval_newer(self):
- self.config.set('zuul', 'layout_config',
- 'tests/fixtures/layout-require-approval.yaml')
- self.sched.reconfigure(self.config)
- self.registerJobs()
-
- A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
- A.addApproval('CRVW', 2)
- aprv = A.addApproval('APRV', 1)
- self.fake_gerrit.addEvent(aprv)
- self.waitUntilSettled()
- # No +1 from Jenkins so should not be enqueued
- self.assertEqual(len(self.history), 0)
-
- # Add a too-old +1, should trigger check but not gate
- A.addApproval('VRFY', 1, granted_on=time.time() - 72 * 60 * 60)
- self.fake_gerrit.addEvent(aprv)
- self.waitUntilSettled()
- self.assertEqual(len(self.history), 1)
- self.assertEqual(self.history[0].name, 'project-check')
-
- # Add a recent +1
- self.fake_gerrit.addEvent(A.addApproval('VRFY', 1))
- self.fake_gerrit.addEvent(aprv)
- self.waitUntilSettled()
- self.assertEqual(len(self.history), 2)
- self.assertEqual(self.history[1].name, 'project-gate')
-
- def test_pipeline_requirements_approval_older(self):
- "Test pipeline requirements older trigger parameter"
- self.config.set('zuul', 'layout_config',
- 'tests/fixtures/layout-pipeline-requirements.yaml')
- self.sched.reconfigure(self.config)
- self.registerJobs()
- self._test_required_approval_older()
-
- def test_required_approval_older(self):
- "Test required-approval older trigger parameter"
- self.config.set('zuul', 'layout_config',
- 'tests/fixtures/layout-require-approval.yaml')
- self.sched.reconfigure(self.config)
- self.registerJobs()
- self._test_required_approval_older()
-
- def _test_required_approval_older(self):
- self.config.set('zuul', 'layout_config',
- 'tests/fixtures/layout-require-approval.yaml')
- self.sched.reconfigure(self.config)
- self.registerJobs()
-
- A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
- crvw = A.addApproval('CRVW', 2)
- self.fake_gerrit.addEvent(crvw)
- self.waitUntilSettled()
- # No +1 from Jenkins so should not be enqueued
- self.assertEqual(len(self.history), 0)
-
- # Add an old +1 and trigger check with a comment
- A.addApproval('VRFY', 1, granted_on=time.time() - 72 * 60 * 60)
- self.fake_gerrit.addEvent(crvw)
- self.waitUntilSettled()
- self.assertEqual(len(self.history), 1)
- self.assertEqual(self.history[0].name, 'project-check')
-
- # Add a recent +1 and make sure nothing changes
- A.addApproval('VRFY', 1)
- self.fake_gerrit.addEvent(crvw)
- self.waitUntilSettled()
- self.assertEqual(len(self.history), 1)
-
- # The last thing we did was query a change then do nothing
- # with a pipeline, so it will be in the cache; clean it up so
- # it does not fail the test.
- for pipeline in self.sched.layout.pipelines.values():
- pipeline.trigger.maintainCache([])
-
def test_rerun_on_error(self):
"Test that if a worker fails to run a job, it is run again"
self.worker.hold_jobs_in_build = True
@@ -2921,35 +2767,3 @@
self.getJobFromHistory('experimental-project-test').result,
'SUCCESS')
self.assertEqual(A.reported, 1)
-
- def test_old_patchset_doesnt_trigger(self):
- "Test that jobs never run against old patchsets"
- self.config.set('zuul', 'layout_config',
- 'tests/fixtures/layout-current-patchset.yaml')
- self.sched.reconfigure(self.config)
- self.registerJobs()
- # Create two patchsets and let their tests settle out. Then
- # comment on first patchset and check that no additional
- # jobs are run.
- A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
- # Added because the layout file really wants an approval but this
- # doesn't match anyways.
- self.fake_gerrit.addEvent(A.addApproval('CRVW', 1))
- self.waitUntilSettled()
- A.addPatchset()
- self.fake_gerrit.addEvent(A.addApproval('CRVW', 1))
- self.waitUntilSettled()
-
- old_history_count = len(self.history)
- self.assertEqual(old_history_count, 2) # one job for each ps
- self.fake_gerrit.addEvent(A.getChangeCommentEvent(1))
- self.waitUntilSettled()
-
- # Assert no new jobs ran after event for old patchset.
- self.assertEqual(len(self.history), old_history_count)
-
- # The last thing we did was add an event for a change then do
- # nothing with a pipeline, so it will be in the cache;
- # clean it up so it does not fail the test.
- for pipeline in self.sched.layout.pipelines.values():
- pipeline.trigger.maintainCache([])
diff --git a/zuul/model.py b/zuul/model.py
index a97c541..4a9a9f6 100644
--- a/zuul/model.py
+++ b/zuul/model.py
@@ -979,10 +979,66 @@
return change
-class EventFilter(object):
+class BaseFilter(object):
+ def __init__(self, required_approvals=[]):
+ self.required_approvals = required_approvals
+
+ for a in self.required_approvals:
+ for k, v in a.items():
+ if k == 'username':
+ pass
+ elif k == 'email-filter':
+ a[k] = re.compile(v)
+ elif k == 'newer-than':
+ a[k] = time_to_seconds(v)
+ elif k == 'older-than':
+ a[k] = time_to_seconds(v)
+ else:
+ if not isinstance(v, list):
+ a[k] = [v]
+
+ def matchesRequiredApprovals(self, change):
+ now = time.time()
+ for rapproval in self.required_approvals:
+ matches_approval = False
+ for approval in change.approvals:
+ if 'description' not in approval:
+ continue
+ found_approval = True
+ by = approval.get('by', {})
+ for k, v in rapproval.items():
+ if k == 'username':
+ if (by.get('username', '') != v):
+ found_approval = False
+ elif k == 'email-filter':
+ if (not v.search(by.get('email', ''))):
+ found_approval = False
+ elif k == 'newer-than':
+ t = now - v
+ if (approval['grantedOn'] < t):
+ found_approval = False
+ elif k == 'older-than':
+ t = now - v
+ if (approval['grantedOn'] >= t):
+ found_approval = False
+ else:
+ if (normalizeCategory(approval['description']) != k or
+ int(approval['value']) not in v):
+ found_approval = False
+ if found_approval:
+ matches_approval = True
+ break
+ if not matches_approval:
+ return False
+ return True
+
+
+class EventFilter(BaseFilter):
def __init__(self, types=[], branches=[], refs=[], event_approvals={},
comment_filters=[], email_filters=[], username_filters=[],
- timespecs=[], require_approvals=[]):
+ timespecs=[], required_approvals=[]):
+ super(EventFilter, self).__init__(
+ required_approvals=required_approvals)
self._types = types
self._branches = branches
self._refs = refs
@@ -996,17 +1052,8 @@
self.email_filters = [re.compile(x) for x in email_filters]
self.username_filters = [re.compile(x) for x in username_filters]
self.event_approvals = event_approvals
- self.require_approvals = require_approvals
self.timespecs = timespecs
- for a in self.require_approvals:
- if 'older-than' in a:
- a['older-than'] = time_to_seconds(a['older-than'])
- if 'newer-than' in a:
- a['newer-than'] = time_to_seconds(a['newer-than'])
- if 'email-filter' in a:
- a['email-filter'] = re.compile(a['email-filter'])
-
def __repr__(self):
ret = '<EventFilter'
@@ -1019,9 +1066,9 @@
if self.event_approvals:
ret += ' event_approvals: %s' % ', '.join(
['%s:%s' % a for a in self.event_approvals.items()])
- if self.require_approvals:
- ret += ' require_approvals: %s' % ', '.join(
- ['%s' % a for a in self.require_approvals])
+ if self.required_approvals:
+ ret += ' required_approvals: %s' % ', '.join(
+ ['%s' % a for a in self.required_approvals])
if self._comment_filters:
ret += ' comment_filters: %s' % ', '.join(self._comment_filters)
if self._email_filters:
@@ -1101,42 +1148,13 @@
if not matches_approval:
return False
- if self.require_approvals and not change.approvals:
+ if self.required_approvals and not change.approvals:
# A change with no approvals can not match
return False
- now = time.time()
- for rapproval in self.require_approvals:
- matches_approval = False
- for approval in change.approvals:
- if 'description' not in approval:
- continue
- found_approval = True
- by = approval.get('by', {})
- for k, v in rapproval.items():
- if k == 'username':
- if (by.get('username', '') != v):
- found_approval = False
- elif k == 'email-filter':
- if (not v.search(by.get('email', ''))):
- found_approval = False
- elif k == 'newer-than':
- t = now - v
- if (approval['grantedOn'] < t):
- found_approval = False
- elif k == 'older-than':
- t = now - v
- if (approval['grantedOn'] >= t):
- found_approval = False
- else:
- if (normalizeCategory(approval['description']) != k or
- int(approval['value']) != v):
- found_approval = False
- if found_approval:
- matches_approval = True
- break
- if not matches_approval:
- return False
+ # required approvals are ANDed
+ if not self.matchesRequiredApprovals(change):
+ return False
# timespecs are ORed
matches_timespec = False
@@ -1149,21 +1167,14 @@
return True
-class ChangeishFilter(object):
+class ChangeishFilter(BaseFilter):
def __init__(self, open=None, current_patchset=None,
- statuses=[], approvals=[]):
+ statuses=[], required_approvals=[]):
+ super(ChangeishFilter, self).__init__(
+ required_approvals=required_approvals)
self.open = open
self.current_patchset = current_patchset
self.statuses = statuses
- self.approvals = approvals
-
- for a in self.approvals:
- if 'older-than' in a:
- a['older-than'] = time_to_seconds(a['older-than'])
- if 'newer-than' in a:
- a['newer-than'] = time_to_seconds(a['newer-than'])
- if 'email-filter' in a:
- a['email-filter'] = re.compile(a['email-filter'])
def __repr__(self):
ret = '<ChangeishFilter'
@@ -1174,8 +1185,8 @@
ret += ' current-patchset: %s' % self.current_patchset
if self.statuses:
ret += ' statuses: %s' % ', '.join(self.statuses)
- if self.approvals:
- ret += ' approvals: %s' % str(self.approvals)
+ if self.required_approvals:
+ ret += ' required_approvals: %s' % str(self.required_approvals)
ret += '>'
return ret
@@ -1193,42 +1204,13 @@
if change.status not in self.statuses:
return False
- if self.approvals and not change.approvals:
+ if self.required_approvals and not change.approvals:
# A change with no approvals can not match
return False
- now = time.time()
- for rapproval in self.approvals:
- matches_approval = False
- for approval in change.approvals:
- if 'description' not in approval:
- continue
- found_approval = True
- by = approval.get('by', {})
- for k, v in rapproval.items():
- if k == 'username':
- if (by.get('username', '') != v):
- found_approval = False
- elif k == 'email-filter':
- if (not v.search(by.get('email', ''))):
- found_approval = False
- elif k == 'newer-than':
- t = now - v
- if (approval['grantedOn'] < t):
- found_approval = False
- elif k == 'older-than':
- t = now - v
- if (approval['grantedOn'] >= t):
- found_approval = False
- else:
- if (normalizeCategory(approval['description']) != k or
- int(approval['value']) != v):
- found_approval = False
- if found_approval:
- matches_approval = True
- break
- if not matches_approval:
- return False
+ # required approvals are ANDed
+ if not self.matchesRequiredApprovals(change):
+ return False
return True
diff --git a/zuul/scheduler.py b/zuul/scheduler.py
index 09fda72..e919b0c 100644
--- a/zuul/scheduler.py
+++ b/zuul/scheduler.py
@@ -280,7 +280,7 @@
open=require.get('open'),
current_patchset=require.get('current-patchset'),
statuses=toList(require.get('status')),
- approvals=toList(require.get('approval')))
+ required_approvals=toList(require.get('approval')))
manager.changeish_filters.append(f)
# TODO: move this into triggers (may require pluggable
@@ -302,7 +302,7 @@
toList(trigger.get('email_filter')),
username_filters=
toList(trigger.get('username_filter')),
- require_approvals=
+ required_approvals=
toList(trigger.get('require-approval')))
manager.event_filters.append(f)
elif 'timer' in conf_pipeline['trigger']: