From c66002dc20ec63c61294054efc32ed24fa6a89ba Mon Sep 17 00:00:00 2001 From: Andrew Halberstadt Date: Fri, 29 Nov 2019 16:16:09 +0000 Subject: [PATCH] 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 --- .../manifestparser/manifestparser.py | 64 ++++++++++--------- .../tests/test_manifestparser.py | 7 +- testing/mozbase/moztest/moztest/resolve.py | 3 +- testing/xpcshell/runxpcshelltests.py | 4 +- 4 files changed, 40 insertions(+), 38 deletions(-) diff --git a/testing/mozbase/manifestparser/manifestparser/manifestparser.py b/testing/mozbase/manifestparser/manifestparser/manifestparser.py index 83d398ccfd2f..4b9dfb255774 100755 --- a/testing/mozbase/manifestparser/manifestparser/manifestparser.py +++ b/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: diff --git a/testing/mozbase/manifestparser/tests/test_manifestparser.py b/testing/mozbase/manifestparser/tests/test_manifestparser.py index d3d94e32d92f..7b257726b8c5 100755 --- a/testing/mozbase/manifestparser/tests/test_manifestparser.py +++ b/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) diff --git a/testing/mozbase/moztest/moztest/resolve.py b/testing/mozbase/moztest/moztest/resolve.py index 6c9a312747bb..6014d513a85b 100644 --- a/testing/mozbase/moztest/moztest/resolve.py +++ b/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) diff --git a/testing/xpcshell/runxpcshelltests.py b/testing/xpcshell/runxpcshelltests.py index 931be85a7e0e..60e6f473df70 100755 --- a/testing/xpcshell/runxpcshelltests.py +++ b/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