Don't run jobs if only their file matchers are updated

Normally, if a change is made to a job configuration, we want to
ignore the file matchers and run the job so that we can see the
update in practice.  However, we don't want to do so if the
*only* change to the job configuration is the file matchers.  The
result is no different than before, and running the job tells us
nothing about the value of the change.

This behavior was already the case, but due to a bug.  This
change both fixes the bug and intentionally restores the
behavior.

The bug that is corrected is that when variants were applied, the
actual file matchers attached to the frozen job were updated, but
not their string/list representation (stored in the '_files' and
'_irrelevant_files' attributes).  This change moves those
attributes to the list of context attributes (which are
automatically copied on inheritance) -- the same as the actual
matcher attributes.

We also do the same for the '_branches' attribute (the text
version of the branch matcher).  These changes make the
serialized form of the frozen job (which is returned by the web
api) more accurate.  And in the case of the files matchers,
causes Zuul to correctly detect that the job has changed when
they are updated.

To restore the current (accidental but desired) behavior, we now
pop the files and irrelevant-files keys from the serialized
representation of the job before comparison when deciding whether
to run the job during a config update.  This means that it will
only run if something other than a files matcher is changed.

This change also removes the _implied_branch job attribute
because it is no longer used anywhere (implied branches now
arrive via projects and project templates).

Change-Id: Id1d595873fe87ca980b2f33832f55542f9e5d496

tests/unit/test_scheduler.py

@@ -6256,6 +6256,33 @@ class TestJobUpdateFileMatcher(ZuulTestCase):
             dict(name='existing-irr', result='SUCCESS', changes='1,1'),
         ], ordered=False)
 
+    def test_job_update_files(self):
+        "Test that changes to file matchers themselves don't run jobs"
+        # Normally we want to ignore file matchers and run jobs if the
+        # job config changes, but if the only thing about the job
+        # config that changes *is* the file matchers, then we don't
+        # want to run it.
+        in_repo_conf = textwrap.dedent(
+            """
+            - job:
+                name: existing-files
+                files: 'doesnotexist'
+            - job:
+                name: existing-irr
+                irrelevant-files:
+                  - README
+                  - ^zuul.d/.*$
+                  - newthing
+            """)
+
+        file_dict = {'zuul.d/new.yaml': in_repo_conf}
+        A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A',
+                                           files=file_dict)
+        self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
+        self.waitUntilSettled()
+
+        self.assertHistory([])
+
     def test_new_job(self):
         "Test matchers are overridden when creating a new job"
         in_repo_conf = textwrap.dedent(

tests/unit/test_web.py

@@ -336,7 +336,6 @@ class TestWeb(BaseTestWeb):
                 'irrelevant_files': [],
                 'match_on_config_updates': True,
                 'final': False,
-                'implied_branch': None,
                 'nodeset': {
                     'groups': [],
                     'name': '',
@@ -378,7 +377,6 @@ class TestWeb(BaseTestWeb):
                 'irrelevant_files': [],
                 'match_on_config_updates': True,
                 'final': False,
-                'implied_branch': None,
                 'nodeset': {
                     'groups': [],
                     'name': '',
@@ -422,7 +420,6 @@ class TestWeb(BaseTestWeb):
                 'description': None,
                 'files': [],
                 'final': False,
-                'implied_branch': None,
                 'irrelevant_files': [],
                 'match_on_config_updates': True,
                 'name': 'test-job',
@@ -541,7 +538,6 @@ class TestWeb(BaseTestWeb):
                   'description': None,
                   'files': [],
                   'final': False,
-                  'implied_branch': None,
                   'irrelevant_files': [],
                   'match_on_config_updates': True,
                   'name': 'project-merge',
@@ -576,7 +572,6 @@ class TestWeb(BaseTestWeb):
                   'description': None,
                   'files': [],
                   'final': False,
-                  'implied_branch': None,
                   'irrelevant_files': [],
                   'match_on_config_updates': True,
                   'name': 'project-test1',
@@ -611,7 +606,6 @@ class TestWeb(BaseTestWeb):
                   'description': None,
                   'files': [],
                   'final': False,
-                  'implied_branch': None,
                   'irrelevant_files': [],
                   'match_on_config_updates': True,
                   'name': 'project-test2',
@@ -646,7 +640,6 @@ class TestWeb(BaseTestWeb):
                   'description': None,
                   'files': [],
                   'final': False,
-                  'implied_branch': None,
                   'irrelevant_files': [],
                   'match_on_config_updates': True,
                   'name': 'project1-project2-integration',
@@ -701,7 +694,6 @@ class TestWeb(BaseTestWeb):
                              'description': None,
                              'files': [],
                              'final': False,
-                             'implied_branch': None,
                              'irrelevant_files': [],
                              'match_on_config_updates': True,
                              'name': 'project-post',

web/src/containers/job/JobVariant.jsx

@@ -84,7 +84,7 @@ class JobVariant extends React.Component {
 
     const jobInfos = [
       'description', 'context', 'builds', 'status',
-      'parent', 'attempts', 'timeout', 'semaphore', 'implied_branch',
+      'parent', 'attempts', 'timeout', 'semaphore',
       'nodeset', 'variables', 'override_checkout',
     ]
     jobInfos.forEach(key => {

zuul/model.py

@@ -1160,8 +1160,11 @@ class Job(ConfigObject):
             failure_url=None,
             success_url=None,
             branch_matcher=None,
+            _branches=(),
             file_matcher=None,
+            _files=(),
             irrelevant_file_matcher=None,  # skip-if
+            _irrelevant_files=(),
             match_on_config_updates=True,
             tags=frozenset(),
             provides=frozenset(),
@@ -1215,10 +1218,6 @@ class Job(ConfigObject):
             description=None,
             variant_description=None,
             protected_origin=None,
-            _branches=(),
-            _implied_branch=None,
-            _files=(),
-            _irrelevant_files=(),
             secrets=(),  # secrets aren't inheritable
             queued=False,
         )
@@ -1243,7 +1242,6 @@ class Job(ConfigObject):
         d['files'] = self._files
         d['irrelevant_files'] = self._irrelevant_files
         d['variant_description'] = self.variant_description
-        d['implied_branch'] = self._implied_branch
         if self.source_context:
             d['source_context'] = self.source_context.toDict()
         else:
@@ -3002,8 +3000,14 @@ class QueueItem(object):
             if old_job is None:
                 log.debug("Found a newly created job")
                 return True  # A newly created job
-            if (job.toDict(self.pipeline.tenant) !=
-                old_job.toDict(self.pipeline.tenant)):
+            old_job_dict = old_job.toDict(self.pipeline.tenant)
+            new_job_dict = job.toDict(self.pipeline.tenant)
+            # Ignore changes to file matchers since they don't affect
+            # the content of the job.
+            for attr in ['files', 'irrelevant_files']:
+                old_job_dict.pop(attr, None)
+                new_job_dict.pop(attr, None)
+            if (new_job_dict != old_job_dict):
                 log.debug("Found an updated job")
                 return True  # This job's configuration has changed
         return False