Bug 1498636 - Separate "include" variables from manifest defaults r=ahal

Test manifests may be included by multiple other manifests, optionally
with additional variables below the `[include:...]` section header.
These additional variables are specific to the manifest that contained
the "include" section, and should not inadvertently be shared with other
manifests that also happen to include this manifest.

To achieve that, store the defaults for included manifests in a (path to
parent manifest, path to included manifest) tuple instead of just the
included manifest.

Differential Revision: https://phabricator.services.mozilla.com/D18086

--HG--
extra : moz-landing-system : lando

testing/mozbase/manifestparser/manifestparser/manifestparser.py

@@ -147,10 +147,20 @@ class ManifestParser(object):
             assert os.path.isabs(self.rootdir)
             rootdir = self.rootdir + os.path.sep
 
+        if parentmanifest and filename:
+            # A manifest can be read multiple times, via "include:", optionally
+            # with section-specific variables. These variables only apply to
+            # the included manifest when included via the same parent manifest,
+            # so they must be associated with (parentmanifest, filename).
+            # |defaults| contains the defaults of the parent manifest, plus any
+            # variables from the "[include:...]" section.
+            self.manifest_defaults[(parentmanifest, filename)] = defaults.copy()
+
         # read the configuration
         sections = read_ini(fp=fp, variables=defaults, strict=self.strict,
                             handle_defaults=self._handle_defaults)
-        self.manifest_defaults[filename] = defaults
+        if parentmanifest is None:
+            self.manifest_defaults[filename] = defaults
 
         parent_section_found = False
 
@@ -341,10 +351,17 @@ class ManifestParser(object):
     def manifests(self, tests=None):
         """
         return manifests in order in which they appear in the tests
+        If |tests| is not set, the order of the manifests is unspecified.
         """
         if tests is None:
+            manifests = []
             # Make sure to return all the manifests, even ones without tests.
-            return self.manifest_defaults.keys()
+            for manifest in self.manifest_defaults.keys():
+                if isinstance(manifest, tuple):
+                    (parentmanifest, manifest) = manifest
+                if manifest not in manifests:
+                    manifests.append(manifest)
+            return manifests
 
         manifests = []
         for test in tests:

testing/mozbase/manifestparser/tests/test_manifestparser.py

@@ -128,6 +128,121 @@ yellow = submarine"""  # noqa
         self.assertEqual(buffer.getvalue().strip(),
                          expected_output)
 
+    def test_include_manifest_defaults(self):
+        """
+        Test that manifest_defaults and manifests() are correctly populated
+        when includes are used.
+        """
+
+        include_example = os.path.join(here, 'include-example.ini')
+        noinclude_example = os.path.join(here, 'just-defaults.ini')
+        bar_path = os.path.join(here, 'include', 'bar.ini')
+        foo_path = os.path.join(here, 'include', 'foo.ini')
+
+        parser = ManifestParser(manifests=(include_example, noinclude_example))
+
+        # Standalone manifests must be appear as-is.
+        self.assertTrue(include_example in parser.manifest_defaults)
+        self.assertTrue(noinclude_example in parser.manifest_defaults)
+
+        # Included manifests must only appear together with the parent manifest
+        # that included the manifest.
+        self.assertFalse(bar_path in parser.manifest_defaults)
+        self.assertFalse(foo_path in parser.manifest_defaults)
+        self.assertTrue((include_example, bar_path) in parser.manifest_defaults)
+        self.assertTrue((include_example, foo_path) in parser.manifest_defaults)
+
+        # manifests() must only return file paths (strings).
+        manifests = parser.manifests()
+        self.assertEqual(len(manifests), 4)
+        self.assertIn(foo_path, manifests)
+        self.assertIn(bar_path, manifests)
+        self.assertIn(include_example, manifests)
+        self.assertIn(noinclude_example, manifests)
+
+    def test_include_repeated(self):
+        """
+        Test that repeatedly included manifests are independent of each other.
+        """
+        include_example = os.path.join(here, 'include-example.ini')
+        included_foo = os.path.join(here, 'include', 'foo.ini')
+
+        # In the expected output, blue and yellow have the values from foo.ini
+        # (ocean, submarine) instead of the ones from include-example.ini
+        # (violets, daffodils), because the defaults in the included file take
+        # precedence over the values from the parent.
+        include_output = """[include/crash-handling]
+foo = fleem
+
+[fleem]
+foo = bar
+
+[include/flowers]
+blue = ocean
+foo = bar
+red = roses
+yellow = submarine
+
+"""
+        included_output = """[include/flowers]
+blue = ocean
+yellow = submarine
+
+"""
+
+        parser = ManifestParser(manifests=(include_example, included_foo),
+                                rootdir=here)
+        self.assertEqual(parser.get('name'),
+                         ['crash-handling', 'fleem', 'flowers', 'flowers'])
+        self.assertEqual([(test['name'], os.path.basename(test['manifest']))
+                          for test in parser.tests],
+                         [('crash-handling', 'bar.ini'),
+                          ('fleem', 'include-example.ini'),
+                          ('flowers', 'foo.ini'),
+                          ('flowers', 'foo.ini')])
+        self.check_included_repeat(parser, parser.tests[3], parser.tests[2],
+                                   "%s%s" % (include_output, included_output))
+
+        # Same tests, but with the load order of the manifests swapped.
+        parser = ManifestParser(manifests=(included_foo, include_example),
+                                rootdir=here)
+        self.assertEqual(parser.get('name'),
+                         ['flowers', 'crash-handling', 'fleem', 'flowers'])
+        self.assertEqual([(test['name'], os.path.basename(test['manifest']))
+                          for test in parser.tests],
+                         [('flowers', 'foo.ini'),
+                          ('crash-handling', 'bar.ini'),
+                          ('fleem', 'include-example.ini'),
+                          ('flowers', 'foo.ini')])
+        self.check_included_repeat(parser, parser.tests[0], parser.tests[3],
+                                   "%s%s" % (included_output, include_output))
+
+    def check_included_repeat(self, parser, isolated_test, included_test,
+                              expected_output):
+        include_example = os.path.join(here, 'include-example.ini')
+        included_foo = os.path.join(here, 'include', 'foo.ini')
+        manifest_default_key = (include_example, included_foo)
+
+        self.assertFalse('ancestor-manifest' in isolated_test)
+        self.assertEqual(included_test['ancestor-manifest'],
+                         os.path.join(here, 'include-example.ini'))
+
+        self.assertTrue(include_example in parser.manifest_defaults)
+        self.assertTrue(included_foo in parser.manifest_defaults)
+        self.assertTrue(manifest_default_key in parser.manifest_defaults)
+        self.assertEqual(parser.manifest_defaults[manifest_default_key],
+                         {
+                             'foo': 'bar',
+                             'here': os.path.join(here, 'include'),
+                             'red': 'roses',
+                             'blue': 'violets',
+                             'yellow': 'daffodils',
+                         })
+
+        buffer = StringIO()
+        parser.write(fp=buffer)
+        self.assertEqual(buffer.getvalue(), expected_output)
+
     def test_invalid_path(self):
         """
         Test invalid path should not throw when not strict

testing/mozbase/moztest/moztest/resolve.py

@@ -331,7 +331,7 @@ class TestMetadata(object):
 
                     ancestor_manifest = metadata.get('ancestor-manifest')
                     if ancestor_manifest:
-                        defaults_manifests.append(ancestor_manifest)
+                        defaults_manifests.append((ancestor_manifest, metadata['manifest']))
 
                     for manifest in defaults_manifests:
                         manifest_defaults = defaults.get(manifest)