diff options
author | James E. Blair <jeblair@redhat.com> | 2017-12-01 15:12:04 -0800 |
---|---|---|
committer | James E. Blair <jeblair@redhat.com> | 2017-12-01 15:25:13 -0800 |
commit | 9ab8db4e1383ee20b2acffabddcfe8bbabe839ad (patch) | |
tree | 5defddebedb3322ec27865a715c9182b9f97e0e5 | |
parent | 6a125faf9dceb09f6c84a53d7d8b39f945f52700 (diff) | |
download | zuul-9ab8db4e1383ee20b2acffabddcfe8bbabe839ad.tar.gz |
Fix complex branch matchers in project configs
A recent change to always apply an implied branch matcher to in-tree
project configs on projects with branches had an optimization to
avoid adding unecessary jobs which was too simple. It assumed that
if the raw text of the matcher on the job did not match the name
of the branch, then there was no need to add the job. That only
works for branch matchers that are simple branch names. One that
is a regex may or may not match the implied branch.
To correct this, when building the job list, run the implied branch
name against the branch matcher regex (if there is one) for the job
to determine whether it will not match.
Change-Id: Ibbe097801a45928bc9942991d868a78f5f441887
7 files changed, 73 insertions, 4 deletions
diff --git a/tests/fixtures/config/branch-negative/git/org_project/.zuul.yaml b/tests/fixtures/config/branch-negative/git/org_project/.zuul.yaml new file mode 100644 index 000000000..f02f449b3 --- /dev/null +++ b/tests/fixtures/config/branch-negative/git/org_project/.zuul.yaml @@ -0,0 +1,10 @@ +- job: + name: test-job + run: playbooks/test-job.yaml + +- project: + name: org/project + check: + jobs: + - test-job: + branches: ^(?!stable) diff --git a/tests/fixtures/config/branch-negative/git/org_project/README b/tests/fixtures/config/branch-negative/git/org_project/README new file mode 100644 index 000000000..9daeafb98 --- /dev/null +++ b/tests/fixtures/config/branch-negative/git/org_project/README @@ -0,0 +1 @@ +test diff --git a/tests/fixtures/config/branch-negative/git/org_project/playbooks/test-job.yaml b/tests/fixtures/config/branch-negative/git/org_project/playbooks/test-job.yaml new file mode 100644 index 000000000..f679dceae --- /dev/null +++ b/tests/fixtures/config/branch-negative/git/org_project/playbooks/test-job.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] diff --git a/tests/fixtures/config/branch-negative/git/project-config/zuul.yaml b/tests/fixtures/config/branch-negative/git/project-config/zuul.yaml new file mode 100644 index 000000000..dc4a18221 --- /dev/null +++ b/tests/fixtures/config/branch-negative/git/project-config/zuul.yaml @@ -0,0 +1,26 @@ +- pipeline: + name: check + manager: independent + trigger: + gerrit: + - event: patchset-created + success: + gerrit: + Verified: 1 + failure: + gerrit: + Verified: -1 + +- job: + name: base + parent: null + +- project: + name: project-config + check: + jobs: [] + +- project: + name: org/project + check: + jobs: [] diff --git a/tests/fixtures/config/branch-negative/main.yaml b/tests/fixtures/config/branch-negative/main.yaml new file mode 100644 index 000000000..0ac232fbb --- /dev/null +++ b/tests/fixtures/config/branch-negative/main.yaml @@ -0,0 +1,8 @@ +- tenant: + name: tenant-one + source: + gerrit: + config-projects: + - project-config + untrusted-projects: + - org/project diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py index e2da808cb..70d921123 100755 --- a/tests/unit/test_v3.py +++ b/tests/unit/test_v3.py @@ -157,6 +157,27 @@ class TestFinal(ZuulTestCase): self.assertIn('Unable to modify final job', A.messages[0]) +class TestBranchNegative(ZuulTestCase): + tenant_config_file = 'config/branch-negative/main.yaml' + + def test_negative_branch_match(self): + # Test that a negative branch matcher works with implied branches. + self.create_branch('org/project', 'stable/pike') + self.fake_gerrit.addEvent( + self.fake_gerrit.getFakeBranchCreatedEvent( + 'org/project', 'stable/pike')) + self.waitUntilSettled() + + A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A') + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + B = self.fake_gerrit.addFakeChange('org/project', 'stable/pike', 'A') + self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + self.assertHistory([ + dict(name='test-job', result='SUCCESS', changes='1,1')]) + + class TestBranchTemplates(ZuulTestCase): tenant_config_file = 'config/branch-templates/main.yaml' diff --git a/zuul/model.py b/zuul/model.py index 11fa38395..8fe4d1489 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -961,7 +961,7 @@ class Job(object): m = m.matchers[0] if not isinstance(m, change_matcher.BranchMatcher): return None - return m._regex + return m def addBranchMatcher(self, branch): # Add a branch matcher that combines as a boolean *and* with @@ -1129,9 +1129,10 @@ class JobList(object): # ensure that it still only affects this branch # (whatever else it may do). simple_branch = job.getSimpleBranchMatcher() - if simple_branch and simple_branch != implied_branch: - # Job is for a different branch, don't add it. - continue + if simple_branch: + if not simple_branch.regex.match(implied_branch): + # This branch will never match, don't add it. + continue if not simple_branch: # The branch matcher could be complex, or # missing. Add our implied matcher. |