Stop metajob application of undefined attributes
Previously, metajobs set defaults for boolean attributes that were not
defined, and copied to them to matched jobs. This could result in a
metajob that did not define a voting attribute unintentially
overriding a matched job with the default voting value (True). This
change ensures that metajobs set values only for explicitly supplied
boolean attributes.
Change-Id: I99dc3a58c63946ddb54849b1629101b062159a69
Closes-Bug: #2000166
diff --git a/tests/test_model.py b/tests/test_model.py
index a97f0a0..2711618 100644
--- a/tests/test_model.py
+++ b/tests/test_model.py
@@ -43,3 +43,22 @@
job = model.Job('job')
job.copy(self.job)
self.assertTrue(job.skip_if_matcher)
+
+ def _assert_job_booleans_are_not_none(self, job):
+ self.assertIsNotNone(job.voting)
+ self.assertIsNotNone(job.hold_following_changes)
+
+ def test_job_sets_defaults_for_boolean_attributes(self):
+ job = model.Job('job')
+ self._assert_job_booleans_are_not_none(job)
+
+ def test_metajob_does_not_set_defaults_for_boolean_attributes(self):
+ job = model.Job('^job')
+ self.assertIsNone(job.voting)
+ self.assertIsNone(job.hold_following_changes)
+
+ def test_metajob_copy_does_not_set_undefined_boolean_attributes(self):
+ job = model.Job('job')
+ metajob = model.Job('^job')
+ job.copy(metajob)
+ self._assert_job_booleans_are_not_none(job)
diff --git a/zuul/model.py b/zuul/model.py
index 1786fd9..616ffba 100644
--- a/zuul/model.py
+++ b/zuul/model.py
@@ -452,8 +452,14 @@
self.failure_pattern = None
self.success_pattern = None
self.parameter_function = None
- self.hold_following_changes = False
- self.voting = True
+ # A metajob should only supply values for attributes that have
+ # been explicitly provided, so avoid setting boolean defaults.
+ if self.is_metajob:
+ self.hold_following_changes = None
+ self.voting = None
+ else:
+ self.hold_following_changes = False
+ self.voting = True
self.branches = []
self._branches = []
self.files = []
@@ -467,6 +473,10 @@
def __repr__(self):
return '<Job %s>' % (self.name)
+ @property
+ def is_metajob(self):
+ return self.name.startswith('^')
+
def copy(self, other):
if other.failure_message:
self.failure_message = other.failure_message
@@ -488,8 +498,11 @@
self.skip_if_matcher = other.skip_if_matcher.copy()
if other.swift:
self.swift.update(other.swift)
- self.hold_following_changes = other.hold_following_changes
- self.voting = other.voting
+ # Only non-None values should be copied for boolean attributes.
+ if other.hold_following_changes is not None:
+ self.hold_following_changes = other.hold_following_changes
+ if other.voting is not None:
+ self.voting = other.voting
def changeMatches(self, change):
matches_branch = False
@@ -1269,8 +1282,7 @@
if name in self.jobs:
return self.jobs[name]
job = Job(name)
- if name.startswith('^'):
- # This is a meta-job
+ if job.is_metajob:
regex = re.compile(name)
self.metajobs.append((regex, job))
else: