Merge "Fix branch matching logic" into feature/zuulv3

tests/base.py

@@ -2648,6 +2648,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

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

tests/unit/test_change_matcher.py

@@ -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):
 

tests/unit/test_scheduler.py

@@ -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")

zuul/change_matcher.py

@@ -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):