Fix branch matching logic
Based on Jim's feedback, change the branch matching logic to always
have priority over ref matching. And v3 will always have refs, so no
need to check if that attribute exists. Also adds a test that checks the
current breakage of branch matching logic.
Change-Id: Iba148b73a77b3300ad84db1c05c083d2c82cd950
diff --git a/tests/base.py b/tests/base.py
index 2e3d682..c240a1b 100755
--- a/tests/base.py
+++ b/tests/base.py
@@ -2620,6 +2620,13 @@
return build
raise Exception("Unable to find build %s" % name)
+ def assertJobNotInHistory(self, name, project=None):
+ for job in self.history:
+ if (project is None or
+ job.parameters['zuul']['project']['name'] == project):
+ self.assertNotEqual(job.name, name,
+ 'Job %s found in history' % name)
+
def getJobFromHistory(self, name, project=None):
for job in self.history:
if (job.name == name and
diff --git a/tests/fixtures/layouts/matcher-test.yaml b/tests/fixtures/layouts/matcher-test.yaml
new file mode 100644
index 0000000..b511a2f
--- /dev/null
+++ b/tests/fixtures/layouts/matcher-test.yaml
@@ -0,0 +1,63 @@
+- pipeline:
+ name: check
+ manager: independent
+ trigger:
+ gerrit:
+ - event: patchset-created
+ success:
+ gerrit:
+ Verified: 1
+ failure:
+ gerrit:
+ Verified: -1
+
+- pipeline:
+ name: gate
+ manager: dependent
+ success-message: Build succeeded (gate).
+ trigger:
+ gerrit:
+ - event: comment-added
+ approval:
+ - Approved: 1
+ success:
+ gerrit:
+ Verified: 2
+ submit: true
+ failure:
+ gerrit:
+ Verified: -2
+ start:
+ gerrit:
+ Verified: 0
+ precedence: high
+
+- job:
+ name: base
+ parent: null
+
+- job:
+ name: project-test1
+ nodeset:
+ nodes:
+ - name: controller
+ label: label1
+
+- job:
+ name: ignore-branch
+ branches: ^(?!featureA).*$
+ nodeset:
+ nodes:
+ - name: controller
+ label: label2
+
+- project:
+ name: org/project
+ check:
+ jobs:
+ - project-test1
+ - ignore-branch
+ gate:
+ jobs:
+ - project-test1
+ - ignore-branch
diff --git a/tests/unit/test_change_matcher.py b/tests/unit/test_change_matcher.py
index 6b161a1..3d5345f 100644
--- a/tests/unit/test_change_matcher.py
+++ b/tests/unit/test_change_matcher.py
@@ -60,7 +60,7 @@
self.assertTrue(self.matcher.matches(self.change))
def test_matches_returns_true_on_matching_ref(self):
- self.change.branch = 'bar'
+ delattr(self.change, 'branch')
self.change.ref = 'foo'
self.assertTrue(self.matcher.matches(self.change))
@@ -69,11 +69,6 @@
self.change.ref = 'baz'
self.assertFalse(self.matcher.matches(self.change))
- def test_matches_returns_false_for_missing_attrs(self):
- delattr(self.change, 'branch')
- # ref is by default not an attribute
- self.assertFalse(self.matcher.matches(self.change))
-
class TestFileMatcher(BaseTestMatcher):
diff --git a/tests/unit/test_scheduler.py b/tests/unit/test_scheduler.py
index 9e5e36c..c15e62c 100755
--- a/tests/unit/test_scheduler.py
+++ b/tests/unit/test_scheduler.py
@@ -5642,3 +5642,30 @@
self.assertEqual(A.reported, 2)
self.assertEqual(B.reported, 2)
self.assertEqual(C.reported, 2)
+
+
+class TestSchedulerBranchMatcher(ZuulTestCase):
+
+ @simple_layout('layouts/matcher-test.yaml')
+ def test_job_branch_ignored(self):
+ '''
+ Test that branch matching logic works.
+
+ The 'ignore-branch' job has a branch matcher that is supposed to
+ match every branch except for the 'featureA' branch, so it should
+ not be run on a change to that branch.
+ '''
+ self.create_branch('org/project', 'featureA')
+ A = self.fake_gerrit.addFakeChange('org/project', 'featureA', 'A')
+ A.addApproval('Code-Review', 2)
+ self.fake_gerrit.addEvent(A.addApproval('Approved', 1))
+ self.waitUntilSettled()
+ self.printHistory()
+ self.assertEqual(self.getJobFromHistory('project-test1').result,
+ 'SUCCESS')
+ self.assertJobNotInHistory('ignore-branch')
+ self.assertEqual(A.data['status'], 'MERGED')
+ self.assertEqual(A.reported, 2,
+ "A should report start and success")
+ self.assertIn('gate', A.messages[1],
+ "A should transit gate")
diff --git a/zuul/change_matcher.py b/zuul/change_matcher.py
index baea217..7f6673d 100644
--- a/zuul/change_matcher.py
+++ b/zuul/change_matcher.py
@@ -60,11 +60,13 @@
class BranchMatcher(AbstractChangeMatcher):
def matches(self, change):
- return (
- (hasattr(change, 'branch') and self.regex.match(change.branch)) or
- (hasattr(change, 'ref') and
- change.ref is not None and self.regex.match(change.ref))
- )
+ if hasattr(change, 'branch'):
+ if self.regex.match(change.branch):
+ return True
+ return False
+ if self.regex.match(change.ref):
+ return True
+ return False
class FileMatcher(AbstractChangeMatcher):