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
This commit is contained in:
parent
9129ed7250
commit
f6dc176f0e
|
@ -2620,6 +2620,13 @@ class ZuulTestCase(BaseTestCase):
|
|||
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
|
||||
|
|
|
@ -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
|
|
@ -60,7 +60,7 @@ class TestBranchMatcher(BaseTestMatcher):
|
|||
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 @@ class TestBranchMatcher(BaseTestMatcher):
|
|||
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):
|
||||
|
||||
|
|
|
@ -5642,3 +5642,30 @@ class TestSemaphoreInRepo(ZuulTestCase):
|
|||
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")
|
||||
|
|
|
@ -60,11 +60,13 @@ class ProjectMatcher(AbstractChangeMatcher):
|
|||
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):
|
||||
|
|
Loading…
Reference in New Issue