Merge "Support negated regexes in files/irrelevant-files"

releasenotes/notes/negate-files-4edc64fc264f3fb0.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    The :attr:`job.files` and :attr:`job.irrelevant-files` attributes
+    now fully support the new :ref:`regular expression <regex>`
+    syntax, including negation.

tests/fixtures/layouts/files-negate.yaml

@@ -0,0 +1,30 @@
+- 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: project-test-files
+    run: playbooks/project-test-files.yaml
+
+- project:
+    name: org/project
+    check:
+      jobs:
+        - project-test-files:
+            files:
+              - regex: ^(dontrun|README|/COMMIT_MSG)$
+                negate: true

tests/fixtures/layouts/irrelevant-files-negate.yaml

@@ -0,0 +1,30 @@
+- 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: project-test-irrelevant-files
+    run: playbooks/project-test-irrelevant-files.yaml
+
+- project:
+    name: org/project
+    check:
+      jobs:
+        - project-test-irrelevant-files:
+            irrelevant-files:
+              - regex: ^respectme$
+                negate: true

tests/unit/test_change_matcher.py

@@ -121,6 +121,39 @@ class TestMatchAllFiles(BaseTestFilesMatcher):
         self._test_matches(True, files=['docs/foo'])
 
 
+class TestMatchAllFilesNegate(BaseTestFilesMatcher):
+
+    def setUp(self):
+        super().setUp()
+        self.matcher = cm.MatchAllFiles(
+            [cm.FileMatcher(ZuulRegex('^docs/.*$', negate=True))])
+
+    def test_matches_returns_false_when_files_attr_missing(self):
+        delattr(self.change, 'files')
+        self._test_matches(False)
+
+    def test_matches_returns_false_when_no_files(self):
+        self._test_matches(False)
+
+    def test_matches_returns_false_when_not_all_files_match(self):
+        self._test_matches(False, files=['/COMMIT_MSG', 'docs/foo', 'foo/bar'])
+
+    def test_matches_returns_false_when_single_file_does_not_match(self):
+        self._test_matches(False, files=['docs/foo'])
+
+    def test_matches_returns_false_when_commit_message_matches(self):
+        self._test_matches(False, files=['/COMMIT_MSG'])
+
+    def test_matches_returns_false_when_all_files_match(self):
+        self._test_matches(False, files=['/COMMIT_MSG', 'docs/foo'])
+
+    def test_matches_returns_false_when_single_file_matches(self):
+        self._test_matches(False, files=['docs/foo'])
+
+    def test_matches_returns_true_when_no_files_match(self):
+        self._test_matches(True, files=['foo'])
+
+
 class TestMatchAnyFiles(BaseTestFilesMatcher):
 
     def setUp(self):
@@ -148,6 +181,33 @@ class TestMatchAnyFiles(BaseTestFilesMatcher):
         self._test_matches(False, files=['/COMMIT_MSG', 'foo/bar'])
 
 
+class TestMatchAnyFilesNegate(BaseTestFilesMatcher):
+
+    def setUp(self):
+        super().setUp()
+        self.matcher = cm.MatchAnyFiles(
+            [cm.FileMatcher(ZuulRegex('^docs/.*$', negate=True))])
+
+    def test_matches_returns_true_when_files_attr_missing(self):
+        delattr(self.change, 'files')
+        self._test_matches(True)
+
+    def test_matches_returns_true_when_no_files(self):
+        self._test_matches(True)
+
+    def test_matches_returns_true_when_only_commit_message(self):
+        self._test_matches(True, files=['/COMMIT_MSG'])
+
+    def test_matches_returns_true_when_some_files_match(self):
+        self._test_matches(True, files=['/COMMIT_MSG', 'docs/foo', 'foo/bar'])
+
+    def test_matches_returns_false_when_single_file_matches(self):
+        self._test_matches(False, files=['docs/foo'])
+
+    def test_matches_returns_true_when_no_matching_files(self):
+        self._test_matches(True, files=['/COMMIT_MSG', 'foo/bar'])
+
+
 class TestMatchAll(BaseTestMatcher):
 
     def test_matches_returns_true(self):

tests/unit/test_scheduler.py

@@ -3467,6 +3467,36 @@ class TestScheduler(ZuulTestCase):
         self.assertEqual(B.data['status'], 'MERGED')
         self.assertEqual(B.reported, 2)
 
+    def _test_files_negated_jobs(self, should_skip):
+        "Test that jobs with negated files filter run only when appropriate"
+        if should_skip:
+            files = {'dontrun': 'me\n'}
+        else:
+            files = {'dorun': 'please!\n'}
+
+        change = self.fake_gerrit.addFakeChange('org/project',
+                                                'master',
+                                                'test files',
+                                                files=files)
+        self.fake_gerrit.addEvent(change.getPatchsetCreatedEvent(1))
+        self.waitUntilSettled()
+
+        tested_change_ids = [x.changes[0] for x in self.history
+                             if x.name == 'project-test-files']
+
+        if should_skip:
+            self.assertEqual([], tested_change_ids)
+        else:
+            self.assertIn(change.data['number'], tested_change_ids)
+
+    @simple_layout('layouts/files-negate.yaml')
+    def test_files_negated_no_match_skips_job(self):
+        self._test_files_negated_jobs(should_skip=True)
+
+    @simple_layout('layouts/files-negate.yaml')
+    def test_files_negated_match_runs_job(self):
+        self._test_files_negated_jobs(should_skip=False)
+
     def _test_irrelevant_files_jobs(self, should_skip):
         "Test that jobs with irrelevant-files filter run only when appropriate"
         if should_skip:
@@ -3497,6 +3527,42 @@ class TestScheduler(ZuulTestCase):
     def test_irrelevant_files_no_match_runs_job(self):
         self._test_irrelevant_files_jobs(should_skip=False)
 
+    def _test_irrelevant_files_negated_jobs(self, should_skip):
+        "Test that jobs with irrelevant-files filter run only when appropriate"
+        if should_skip:
+            files = {'ignoreme': 'ignored\n'}
+        else:
+            files = {'respectme': 'please!\n'}
+
+        change = self.fake_gerrit.addFakeChange('org/project',
+                                                'master',
+                                                'test irrelevant-files',
+                                                files=files)
+        self.fake_gerrit.addEvent(change.getPatchsetCreatedEvent(1))
+        self.waitUntilSettled()
+
+        tested_change_ids = [x.changes[0] for x in self.history
+                             if x.name == 'project-test-irrelevant-files']
+
+        if should_skip:
+            self.assertEqual([], tested_change_ids)
+        else:
+            self.assertIn(change.data['number'], tested_change_ids)
+
+    @simple_layout('layouts/irrelevant-files-negate.yaml')
+    def test_irrelevant_files_negated_match_skips_job(self):
+        # Anything other than "respectme" is irrelevant. This adds
+        # "README" which is irrelevant, and "ignoreme" which is
+        # irrelevant, so the job should not run.
+        self._test_irrelevant_files_negated_jobs(should_skip=True)
+
+    @simple_layout('layouts/irrelevant-files-negate.yaml')
+    def test_irrelevant_files_negated_no_match_runs_job(self):
+        # Anything other than "respectme" is irrelevant. This adds
+        # "README" which is irrelevant, and "respecme" which *is*
+        # relevant, so the job should run.
+        self._test_irrelevant_files_negated_jobs(should_skip=False)
+
     @simple_layout('layouts/inheritance.yaml')
     def test_inherited_jobs_keep_matchers(self):
         files = {'ignoreme': 'ignored\n'}

zuul/configloader.py

@@ -571,9 +571,9 @@ class JobParser(object):
                       'semaphores': to_list(vs.Any(semaphore, str)),
                       'tags': to_list(str),
                       'branches': to_list(vs.Any(ZUUL_REGEX, str)),
-                      'files': to_list(str),
+                      'files': to_list(vs.Any(ZUUL_REGEX, str)),
                       'secrets': to_list(vs.Any(secret, str)),
-                      'irrelevant-files': to_list(str),
+                      'irrelevant-files': to_list(vs.Any(ZUUL_REGEX, str)),
                       # validation happens in NodeSetParser
                       'nodeset': vs.Any(dict, str),
                       'timeout': int,
@@ -941,9 +941,18 @@ class JobParser(object):
         if branches:
             job.setBranchMatcher(branches)
         if 'files' in conf:
-            job.setFileMatcher(as_list(conf['files']))
+            with self.pcontext.confAttr(conf, 'files') as conf_files:
+                job.setFileMatcher([
+                    make_regex(x, self.pcontext)
+                    for x in as_list(conf_files)
+                ])
         if 'irrelevant-files' in conf:
-            job.setIrrelevantFileMatcher(as_list(conf['irrelevant-files']))
+            with self.pcontext.confAttr(conf,
+                                        'irrelevant-files') as conf_ifiles:
+                job.setIrrelevantFileMatcher([
+                    make_regex(x, self.pcontext)
+                    for x in as_list(conf_ifiles)
+                ])
         if 'failure-output' in conf:
             failure_output = as_list(conf['failure-output'])
             # Test compilation to detect errors, but the zuul_stream

zuul/lib/re2util.py

@@ -93,6 +93,14 @@ class ZuulRegex:
             return not self.re.search(subject)
         return self.re.search(subject)
 
+    def toDict(self):
+        # This is used in user-facing serialization, like zuul-web, to
+        # match job syntax.
+        return {
+            "regex": self.pattern,
+            "negate": self.negate,
+        }
+
     def serialize(self):
         return {
             "pattern": self.pattern,

zuul/model.py

@@ -42,7 +42,6 @@ import jsonpath_rw
 
 from zuul import change_matcher
 from zuul.lib.config import get_default
-from zuul.lib.re2util import ZuulRegex
 from zuul.lib.result_data import get_artifacts_from_result_data
 from zuul.lib.logutil import get_annotated_logger
 from zuul.lib.capabilities import capabilities_registry
@@ -3169,18 +3168,20 @@ class Job(ConfigObject):
 
     def setFileMatcher(self, files):
         # Set the file matcher to match any of the change files
-        self._files = files
+        # Input is a list of ZuulRegex objects
+        self._files = [x.toDict() for x in files]
         matchers = []
-        for fn in files:
-            matchers.append(change_matcher.FileMatcher(ZuulRegex(fn)))
+        for zuul_regex in files:
+            matchers.append(change_matcher.FileMatcher(zuul_regex))
         self.file_matcher = change_matcher.MatchAnyFiles(matchers)
 
     def setIrrelevantFileMatcher(self, irrelevant_files):
         # Set the irrelevant file matcher to match any of the change files
-        self._irrelevant_files = irrelevant_files
+        # Input is a list of ZuulRegex objects
+        self._irrelevant_files = [x.toDict() for x in irrelevant_files]
         matchers = []
-        for fn in irrelevant_files:
-            matchers.append(change_matcher.FileMatcher(ZuulRegex(fn)))
+        for zuul_regex in irrelevant_files:
+            matchers.append(change_matcher.FileMatcher(zuul_regex))
         self.irrelevant_file_matcher = change_matcher.MatchAllFiles(matchers)
 
     def updateVariables(self, other_vars, other_extra_vars, other_host_vars,