Bug 1600314 - [manifestparser] Make 'ancestor_manifest' a relative path, r=gbrown

Also rename the key from 'ancestor-manifest' to 'ancestor_manifest' to be
consistent with other keys.

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

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

testing/mozbase/manifestparser/manifestparser/manifestparser.py

@@ -81,7 +81,7 @@ class ManifestParser(object):
         self.source_files = set()
         self.strict = strict
         self.rootdir = rootdir
-        self.relativeRoot = None
+        self._root = None
         self.finder = finder
         self._handle_defaults = handle_defaults
         if manifests:
@@ -92,6 +92,34 @@ class ManifestParser(object):
             return self.finder.get(path) is not None
         return os.path.exists(path)
 
+    @property
+    def root(self):
+        if not self._root:
+            if self.rootdir is None:
+                self._root = ""
+            else:
+                assert os.path.isabs(self.rootdir)
+                self._root = self.rootdir + os.path.sep
+        return self._root
+
+    def relative_to_root(self, path):
+        # Microoptimization, because relpath is quite expensive.
+        # We know that rootdir is an absolute path or empty. If path
+        # starts with rootdir, then path is also absolute and the tail
+        # of the path is the relative path (possibly non-normalized,
+        # when here is unknown).
+        # For this to work rootdir needs to be terminated with a path
+        # separator, so that references to sibling directories with
+        # a common prefix don't get misscomputed (e.g. /root and
+        # /rootbeer/file).
+        # When the rootdir is unknown, the relpath needs to be left
+        # unchanged. We use an empty string as rootdir in that case,
+        # which leaves relpath unchanged after slicing.
+        if path.startswith(self.root):
+            return path[len(self.root):]
+        else:
+            return relpath(path, self.root)
+
     # methods for reading manifests
 
     def _read(self, root, filename, defaults, defaults_only=False, parentmanifest=None):
@@ -140,14 +168,6 @@ class ManifestParser(object):
             filename = here = None
         defaults['here'] = here
 
-        # Rootdir is needed for relative path calculation. Precompute it for
-        # the microoptimization used below.
-        if self.rootdir is None:
-            rootdir = ""
-        else:
-            assert os.path.isabs(self.rootdir)
-            rootdir = self.rootdir + os.path.sep
-
         # read the configuration
         sections = read_ini(fp=fp, variables=defaults, strict=self.strict,
                             handle_defaults=self._handle_defaults)
@@ -201,29 +221,11 @@ class ManifestParser(object):
             test = data
             test['name'] = section
 
-            def relative_to_root(path):
-                # Microoptimization, because relpath is quite expensive.
-                # We know that rootdir is an absolute path or empty. If path
-                # starts with rootdir, then path is also absolute and the tail
-                # of the path is the relative path (possibly non-normalized,
-                # when here is unknown).
-                # For this to work rootdir needs to be terminated with a path
-                # separator, so that references to sibling directories with
-                # a common prefix don't get misscomputed (e.g. /root and
-                # /rootbeer/file).
-                # When the rootdir is unknown, the relpath needs to be left
-                # unchanged. We use an empty string as rootdir in that case,
-                # which leaves relpath unchanged after slicing.
-                if path.startswith(rootdir):
-                    return path[len(rootdir):]
-                else:
-                    return relpath(path, rootdir)
-
             # Will be None if the manifest being read is a file-like object.
             test['manifest'] = filename
             test['manifest_relpath'] = None
             if test['manifest']:
-                test['manifest_relpath'] = relative_to_root(normalize_path(test['manifest']))
+                test['manifest_relpath'] = self.relative_to_root(normalize_path(test['manifest']))
 
             # determine the path
             path = test.get('path', section)
@@ -238,7 +240,7 @@ class ManifestParser(object):
                     path = os.path.join(here, path)
                     if '..' in path:
                         path = os.path.normpath(path)
-                _relpath = relative_to_root(path)
+                _relpath = self.relative_to_root(path)
 
             test['path'] = path
             test['relpath'] = _relpath
@@ -248,7 +250,7 @@ class ManifestParser(object):
                 # indicate that in the test object for the sake of identifying
                 # a test, particularly in the case a test file is included by
                 # multiple manifests.
-                test['ancestor-manifest'] = parentmanifest
+                test['ancestor_manifest'] = self.relative_to_root(parentmanifest)
 
             # append the item
             self.tests.append(test)
@@ -505,7 +507,7 @@ class ManifestParser(object):
                 'manifest',
                 'manifest_relpath',
                 'relpath',
-                'ancestor-manifest'
+                'ancestor_manifest'
             ]
             for key in sorted(test.keys()):
                 if key in reserved:

testing/mozbase/manifestparser/tests/test_manifestparser.py

@@ -71,7 +71,7 @@ class TestManifestParser(unittest.TestCase):
                           ('flowers', 'foo.ini')])
 
         # The including manifest is always reported as a part of the generated test object.
-        self.assertTrue(all([t['ancestor-manifest'] == include_example
+        self.assertTrue(all([t['ancestor_manifest'] == 'include-example.ini'
                              for t in parser.tests if t['name'] != 'fleem']))
 
         # The manifests should be there too:
@@ -249,9 +249,8 @@ yellow = submarine
         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.assertFalse('ancestor_manifest' in isolated_test)
+        self.assertEqual(included_test['ancestor_manifest'], 'include-example.ini')
 
         self.assertTrue(include_example in parser.manifest_defaults)
         self.assertTrue(included_foo in parser.manifest_defaults)

testing/mozbase/moztest/moztest/resolve.py

@@ -398,11 +398,12 @@ class BuildBackendLoader(TestLoader):
             for metadata in tests:
                 defaults_manifests = [metadata['manifest']]
 
-                ancestor_manifest = metadata.get('ancestor-manifest')
+                ancestor_manifest = metadata.get('ancestor_manifest')
                 if ancestor_manifest:
                     # The (ancestor manifest, included manifest) tuple
                     # contains the defaults of the included manifest, so
                     # use it instead of [metadata['manifest']].
+                    ancestor_manifest = os.path.join(self.topsrcdir, ancestor_manifest)
                     defaults_manifests[0] = (ancestor_manifest, metadata['manifest'])
                     defaults_manifests.append(ancestor_manifest)
 

testing/xpcshell/runxpcshelltests.py

@@ -857,9 +857,9 @@ class XPCShellTests(object):
 
     def normalizeTest(self, root, test_object):
         path = test_object.get('file_relpath', test_object['relpath'])
-        if 'dupe-manifest' in test_object and 'ancestor-manifest' in test_object:
+        if 'dupe-manifest' in test_object and 'ancestor_manifest' in test_object:
             test_object['id'] = '%s:%s' % (os.path.basename
-                                           (test_object['ancestor-manifest']), path)
+                                           (test_object['ancestor_manifest']), path)
         else:
             test_object['id'] = path