Merge "Make file matchers overridable"

doc/source/user/config.rst

@@ -967,24 +967,21 @@ Here is an example of two job definitions:
       it will remain set for all child jobs and variants (it can not be
       set to ``false``).
 
-   .. _matchers:
-
-   The following job attributes are considered "matchers".  They are
-   not inherited in the usual manner, instead, these attributes are
-   used to determine whether a specific variant is used when
-   running a job.
-
    .. attr:: branches
 
       A regular expression (or list of regular expressions) which
       describe on what branches a job should run (or in the case of
-      variants: to alter the behavior of a job for a certain branch).
+      variants, to alter the behavior of a job for a certain branch).
+
+      This attribute is not inherited in the usual manner.  Instead,
+      it is used to determine whether each variant on which it appears
+      will be used when running the job.
 
       If there is no job definition for a given job which matches the
       branch of an item, then that job is not run for the item.
-      Otherwise, all of the job variants which match that branch (and
+      Otherwise, all of the job variants which match that branch are
-      any other selection criteria) are used when freezing the job.
+      used when freezing the job.  However, if
-      However, if :attr:`job.override-checkout` or
+      :attr:`job.override-checkout` or
       :attr:`job.required-projects.override-checkout` are set for a
       project, Zuul will attempt to use the job variants which match
       the values supplied in ``override-checkout`` for jobs defined in
@@ -1053,18 +1050,20 @@ Here is an example of two job definitions:
 
    .. attr:: files
 
-      This matcher indicates that the job should only run on changes
+      This indicates that the job should only run on changes where the
-      where the specified files are modified.  This is a regular
+      specified files are modified.  Unlike **branches**, this value
-      expression or list of regular expressions.
+      is subject to inheritance and overriding, so only the final
+      value is used to determine if the job should run. This is a
+      regular expression or list of regular expressions.
 
    .. attr:: irrelevant-files
 
-      This matcher is a negative complement of **files**.  It
+      This is a negative complement of **files**.  It indicates that
-      indicates that the job should run unless *all* of the files
+      the job should run unless *all* of the files changed match this
-      changed match this list.  In other words, if the regular
+      list.  In other words, if the regular expression ``docs/.*`` is
-      expression ``docs/.*`` is supplied, then this job will not run
+      supplied, then this job will not run if the only files changed
-      if the only files changed are in the docs directory.  A regular
+      are in the docs directory.  A regular expression or list of
-      expression or list of regular expressions.
+      regular expressions.
 
 .. _project:
 

releasenotes/notes/override-file-matchers-128731229d551d81.yaml

@@ -0,0 +1,8 @@
+features:
+  - |
+    Files (and irrelevant-files) matchers are now overridable.  Zuul
+    now uses only branch matchers to collect job variants.  Once those
+    variants are collected, they are combined, and the files and
+    irrelevant-files attributes are inherited and overridden as any
+    other job attribute.  The final values are used to determine
+    whether the job should ultimately run.

tests/fixtures/layouts/file-matchers.yaml

@@ -0,0 +1,63 @@
+- pipeline:
+    name: check
+    manager: independent
+    trigger:
+      gerrit:
+        - event: patchset-created
+    success:
+      gerrit:
+        Verified: 1
+    failure:
+      gerrit:
+        Verified: -1
+
+- job:
+    name: base
+    parent: null
+    run: playbooks/base.yaml
+
+- job:
+    name: parent-override-job
+    files: parent1.txt
+
+- job:
+    name: parent-override-job
+    files: parent2.txt
+
+- job:
+    name: child-job
+    parent: parent-override-job
+
+- job:
+    name: child-override-job
+    parent: parent-override-job
+    files: child.txt
+
+- job:
+    name: project-override-job
+    parent: parent-override-job
+
+- job:
+    name: irr-job
+    files:
+      - tests/.*
+    irrelevant-files:
+      - README
+
+- job:
+    name: irr-override-job
+    parent: irr-job
+    irrelevant-files:
+      - README
+      - tests/docs/.*
+
+- project:
+    name: org/project
+    check:
+      jobs:
+        - child-job  # parent2
+        - child-override-job  # child
+        - project-override-job:  # project
+            files: project.txt
+        - irr-job  # tests/docs/foo tests/foo
+        - irr-override-job  # tests/foo

tests/unit/test_model.py

@@ -82,22 +82,22 @@ class TestJob(BaseTestCase):
     def test_change_matches_returns_false_for_matched_skip_if(self):
         change = model.Change('project')
         change.files = ['/COMMIT_MSG', 'docs/foo']
-        self.assertFalse(self.job.changeMatches(change))
+        self.assertFalse(self.job.changeMatchesFiles(change))
 
     def test_change_matches_returns_false_for_single_matched_skip_if(self):
         change = model.Change('project')
         change.files = ['docs/foo']
-        self.assertFalse(self.job.changeMatches(change))
+        self.assertFalse(self.job.changeMatchesFiles(change))
 
     def test_change_matches_returns_true_for_unmatched_skip_if(self):
         change = model.Change('project')
         change.files = ['/COMMIT_MSG', 'foo']
-        self.assertTrue(self.job.changeMatches(change))
+        self.assertTrue(self.job.changeMatchesFiles(change))
 
     def test_change_matches_returns_true_for_single_unmatched_skip_if(self):
         change = model.Change('project')
         change.files = ['foo']
-        self.assertTrue(self.job.changeMatches(change))
+        self.assertTrue(self.job.changeMatchesFiles(change))
 
     def test_job_sets_defaults_for_boolean_attributes(self):
         self.assertIsNotNone(self.job.voting)
@@ -203,9 +203,9 @@ class TestJob(BaseTestCase):
         item = queue.enqueueChange(change)
         item.layout = self.layout
 
-        self.assertTrue(base.changeMatches(change))
+        self.assertTrue(base.changeMatchesBranch(change))
-        self.assertTrue(python27.changeMatches(change))
+        self.assertTrue(python27.changeMatchesBranch(change))
-        self.assertFalse(python27diablo.changeMatches(change))
+        self.assertFalse(python27diablo.changeMatchesBranch(change))
 
         item.freezeJobGraph()
         self.assertEqual(len(item.getJobs()), 1)
@@ -217,9 +217,9 @@ class TestJob(BaseTestCase):
         item = queue.enqueueChange(change)
         item.layout = self.layout
 
-        self.assertTrue(base.changeMatches(change))
+        self.assertTrue(base.changeMatchesBranch(change))
-        self.assertTrue(python27.changeMatches(change))
+        self.assertTrue(python27.changeMatchesBranch(change))
-        self.assertTrue(python27diablo.changeMatches(change))
+        self.assertTrue(python27diablo.changeMatchesBranch(change))
 
         item.freezeJobGraph()
         self.assertEqual(len(item.getJobs()), 1)
@@ -269,8 +269,8 @@ class TestJob(BaseTestCase):
         item = queue.enqueueChange(change)
         item.layout = self.layout
 
-        self.assertTrue(base.changeMatches(change))
+        self.assertTrue(base.changeMatchesFiles(change))
-        self.assertFalse(python27.changeMatches(change))
+        self.assertFalse(python27.changeMatchesFiles(change))
 
         item.freezeJobGraph()
         self.assertEqual([], item.getJobs())

tests/unit/test_scheduler.py

@@ -4893,10 +4893,61 @@ For CI problems and help debugging, contact ci@example.org"""
         self.waitUntilSettled()
 
         self.assertHistory([
-            dict(name='child-job', result='SUCCESS', changes='2,1'),
             dict(name='child-job', result='SUCCESS', changes='3,1'),
             dict(name='child-job', result='SUCCESS', changes='4,1'),
-        ])
+        ], ordered=False)
+
+    @simple_layout('layouts/file-matchers.yaml')
+    def test_file_matchers(self):
+        "Test several file matchers"
+        files = {'parent1.txt': ''}
+        A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A',
+                                           files=files)
+        self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
+        self.waitUntilSettled()
+
+        files = {'parent2.txt': ''}
+        B = self.fake_gerrit.addFakeChange('org/project', 'master', 'B',
+                                           files=files)
+        self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1))
+        self.waitUntilSettled()
+
+        files = {'child.txt': ''}
+        C = self.fake_gerrit.addFakeChange('org/project', 'master', 'C',
+                                           files=files)
+        self.fake_gerrit.addEvent(C.getPatchsetCreatedEvent(1))
+        self.waitUntilSettled()
+
+        files = {'project.txt': ''}
+        D = self.fake_gerrit.addFakeChange('org/project', 'master', 'D',
+                                           files=files)
+        self.fake_gerrit.addEvent(D.getPatchsetCreatedEvent(1))
+        self.waitUntilSettled()
+
+        files = {'tests/foo': ''}
+        E = self.fake_gerrit.addFakeChange('org/project', 'master', 'E',
+                                           files=files)
+        self.fake_gerrit.addEvent(E.getPatchsetCreatedEvent(1))
+        self.waitUntilSettled()
+
+        files = {'tests/docs/foo': ''}
+        F = self.fake_gerrit.addFakeChange('org/project', 'master', 'F',
+                                           files=files)
+        self.fake_gerrit.addEvent(F.getPatchsetCreatedEvent(1))
+        self.waitUntilSettled()
+
+        self.assertHistory([
+            dict(name='child-job', result='SUCCESS', changes='2,1'),
+
+            dict(name='child-override-job', result='SUCCESS', changes='3,1'),
+
+            dict(name='project-override-job', result='SUCCESS', changes='4,1'),
+
+            dict(name='irr-job', result='SUCCESS', changes='5,1'),
+            dict(name='irr-override-job', result='SUCCESS', changes='5,1'),
+
+            dict(name='irr-job', result='SUCCESS', changes='6,1'),
+        ], ordered=False)
 
     def test_trusted_project_dep_on_non_live_untrusted_project(self):
         # Test we get a layout for trusted projects when they depend on

zuul/model.py

@@ -934,8 +934,6 @@ class Job(ConfigObject):
             success_message=None,
             failure_url=None,
             success_url=None,
-            # Matchers.  These are separate so they can be individually
-            # overidden.
             branch_matcher=None,
             file_matcher=None,
             irrelevant_file_matcher=None,  # skip-if
@@ -1294,7 +1292,7 @@ class Job(ConfigObject):
 
         self.inheritance_path = self.inheritance_path + (repr(other),)
 
-    def changeMatches(self, change, override_branch=None):
+    def changeMatchesBranch(self, change, override_branch=None):
         if override_branch is None:
             branch_change = change
         else:
@@ -1307,7 +1305,9 @@ class Job(ConfigObject):
         if self.branch_matcher and not self.branch_matcher.matches(
                 branch_change):
             return False
+        return True
 
+    def changeMatchesFiles(self, change):
         if self.file_matcher and not self.file_matcher.matches(change):
             return False
 
@@ -3057,8 +3057,9 @@ class Layout(object):
                 branches = self.tenant.getProjectBranches(project)
                 if override_branch not in branches:
                     override_branch = None
-            if not variant.changeMatches(change,
+            if not variant.changeMatchesBranch(
-                                         override_branch=override_branch):
+                    change,
+                    override_branch=override_branch):
                 self.log.debug("Variant %s did not match %s", repr(variant),
                                change)
                 item.debug("Variant {variant} did not match".format(
@@ -3140,7 +3141,7 @@ class Layout(object):
             # jobs which match.
             override_checkouts = {}
             for variant in job_list.jobs[jobname]:
-                if variant.changeMatches(change):
+                if variant.changeMatchesBranch(change):
                     self._updateOverrideCheckouts(override_checkouts, variant)
             try:
                 variants = self.collectJobs(
@@ -3167,7 +3168,7 @@ class Layout(object):
             # variants
             matched = False
             for variant in job_list.jobs[jobname]:
-                if variant.changeMatches(change):
+                if variant.changeMatchesBranch(change):
                     frozen_job.applyVariant(variant, item.layout)
                     matched = True
                     self.log.debug("Pipeline variant %s matched %s",
@@ -3185,6 +3186,12 @@ class Layout(object):
                 item.debug("No matching pipeline variants for {jobname}".
                            format(jobname=jobname), indent=2)
                 continue
+            if not frozen_job.changeMatchesFiles(change):
+                self.log.debug("Job %s did not match files in %s",
+                               repr(frozen_job), change)
+                item.debug("Job {jobname} did not match files".
+                           format(jobname=jobname), indent=2)
+                continue
             if frozen_job.abstract:
                 raise Exception("Job %s is abstract and may not be "
                                 "directly run" %