From dbe6fab14f446b1929b2b30c31a18d489d16b272 Mon Sep 17 00:00:00 2001 From: Alexander Evseev Date: Thu, 19 Nov 2015 12:46:34 +0300 Subject: [PATCH] Don't take into account commit message for skip-if filter If there is some skip-if condition containing all-files-match-any, then Zuul skips jobs for changes without modified files (merge commits), because it always matches '/COMMIT_MSG'. So test files for regexes only if CR has more than one modified file, because '/COMMIT_MSG' is always included even for empty merge commits. Change-Id: Iad78d9eb8212beea3238728321c1ba74efa991e2 --- doc/source/zuul.rst | 5 ++++- tests/test_change_matcher.py | 8 ++++---- tests/test_model.py | 4 ++-- zuul/change_matcher.py | 2 +- 4 files changed, 11 insertions(+), 8 deletions(-) diff --git a/doc/source/zuul.rst b/doc/source/zuul.rst index 98e4bb8a2a..7132407125 100644 --- a/doc/source/zuul.rst +++ b/doc/source/zuul.rst @@ -759,7 +759,10 @@ each job as it builds a list from the project specification. expressions. The pattern for '/COMMIT_MSG' is always matched on and does not - have to be included. + have to be included. Exception is merge commits (without modified + files), in this case '/COMMIT_MSG' is not matched, and job is not + skipped. In case of merge commits it's assumed that list of modified + files isn't predictible and CI should be run. **voting (optional)** Boolean value (``true`` or ``false``) that indicates whatever diff --git a/tests/test_change_matcher.py b/tests/test_change_matcher.py index 1f4ab93d61..05853223a5 100644 --- a/tests/test_change_matcher.py +++ b/tests/test_change_matcher.py @@ -123,13 +123,13 @@ class TestMatchAllFiles(BaseTestMatcher): self._test_matches(False) def test_matches_returns_false_when_not_all_files_match(self): - self._test_matches(False, files=['docs/foo', 'foo/bar']) + self._test_matches(False, files=['/COMMIT_MSG', 'docs/foo', 'foo/bar']) - def test_matches_returns_true_when_commit_message_matches(self): - self._test_matches(True, files=['/COMMIT_MSG']) + def test_matches_returns_false_when_commit_message_matches(self): + self._test_matches(False, files=['/COMMIT_MSG']) def test_matches_returns_true_when_all_files_match(self): - self._test_matches(True, files=['docs/foo']) + self._test_matches(True, files=['/COMMIT_MSG', 'docs/foo']) class TestMatchAll(BaseTestMatcher): diff --git a/tests/test_model.py b/tests/test_model.py index 271161869f..739eef3714 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -31,12 +31,12 @@ class TestJob(BaseTestCase): def test_change_matches_returns_false_for_matched_skip_if(self): change = model.Change('project') - change.files = ['docs/foo'] + change.files = ['/COMMIT_MSG', 'docs/foo'] self.assertFalse(self.job.changeMatches(change)) def test_change_matches_returns_true_for_unmatched_skip_if(self): change = model.Change('project') - change.files = ['foo'] + change.files = ['/COMMIT_MSG', 'foo'] self.assertTrue(self.job.changeMatches(change)) def test_copy_retains_skip_if(self): diff --git a/zuul/change_matcher.py b/zuul/change_matcher.py index ed380f0ae5..ca2d93f375 100644 --- a/zuul/change_matcher.py +++ b/zuul/change_matcher.py @@ -101,7 +101,7 @@ class MatchAllFiles(AbstractMatcherCollection): yield self.commit_regex def matches(self, change): - if not (hasattr(change, 'files') and change.files): + if not (hasattr(change, 'files') and len(change.files) > 1): return False for file_ in change.files: matched_file = False