Bug 1583353 - [manifestparser] Support manifests in the 'pathprefix' filter, r=egao
authorAndrew Halberstadt <ahalberstadt@mozilla.com>
Fri, 08 Nov 2019 18:58:09 +0000
changeset 501588 98bbf6967fd6bfc2ede0ef46bbf4bd5518d31bc6
parent 501587 ca54f70e0297399b460682c0d189ba17b55560e7
child 501589 d741ca0e07b1ebc23654dd2f33ad51fdd10ebaae
push id100300
push userahalberstadt@mozilla.com
push dateTue, 12 Nov 2019 15:57:39 +0000
treeherderautoland@7ea00823df0a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersegao
bugs1583353
milestone72.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1583353 - [manifestparser] Support manifests in the 'pathprefix' filter, r=egao Allows 'paths' passed into the pathprefix filter to be manifests. Any path that ends with '.ini' is considered a manifest. Depends on D51899 Differential Revision: https://phabricator.services.mozilla.com/D51900
.flake8
testing/mozbase/manifestparser/manifestparser/filters.py
testing/mozbase/manifestparser/manifestparser/manifestparser.py
testing/mozbase/manifestparser/tests/test_filters.py
--- a/.flake8
+++ b/.flake8
@@ -102,16 +102,17 @@ per-file-ignores =
     # These paths are intentionally excluded.
     ipc/ipdl/*: F403, F405
     layout/tools/reftest/selftest/conftest.py: F811
     # cpp_eclipse has a lot of multi-line embedded XML which exceeds line length
     python/mozbuild/mozbuild/backend/cpp_eclipse.py: E501
     testing/firefox-ui/**/__init__.py: F401
     testing/marionette/**/__init__.py: F401
     testing/mochitest/tests/python/conftest.py: F811
+    testing/mozbase/manifestparser/tests/test_filters.py: E731
     testing/mozharness/configs/*: E124, E127, E128, E131, E231, E261, E265, E266, E501, W391
 
     # These paths contain Python-2 only syntax which cause errors since flake8
     # is run with Python 3.
     browser/app/macversion.py: F633
     build/**: F633, F821
     config/**: F633, F821
     ipc/pull-chromium.py: F633
--- a/testing/mozbase/manifestparser/manifestparser/filters.py
+++ b/testing/mozbase/manifestparser/manifestparser/filters.py
@@ -369,36 +369,44 @@ class tags(InstanceFilter):
             if any(t in self.tags for t in test_tags):
                 yield test
 
 
 class pathprefix(InstanceFilter):
     """
     Removes tests that don't start with any of the given test paths.
 
-    :param paths: A list of test paths to filter on
+    :param paths: A list of test paths (or manifests) to filter on
     """
 
     def __init__(self, paths):
         InstanceFilter.__init__(self, paths)
         if isinstance(paths, string_types):
             paths = [paths]
         self.paths = paths
 
     def __call__(self, tests, values):
         for test in tests:
             for tp in self.paths:
                 tp = os.path.normpath(tp)
 
-                path = test['relpath']
-                if os.path.isabs(tp):
-                    path = test['path']
+                if tp.endswith('.ini'):
+                    mpath = test['manifest'] if os.path.isabs(tp) else test['manifest_relpath']
 
-                if not os.path.normpath(path).startswith(tp):
-                    continue
+                    # only return tests that are in this manifest
+                    if os.path.normpath(mpath) != tp:
+                        continue
+                else:
+                    # only return tests that start with this path
+                    path = test['relpath']
+                    if os.path.isabs(tp):
+                        path = test['path']
+
+                    if not os.path.normpath(path).startswith(tp):
+                        continue
 
                 # any test path that points to a single file will be run no
                 # matter what, even if it's disabled
                 if 'disabled' in test and os.path.normpath(test['relpath']) == tp:
                     del test['disabled']
                 yield test
                 break
 
--- a/testing/mozbase/manifestparser/manifestparser/manifestparser.py
+++ b/testing/mozbase/manifestparser/manifestparser/manifestparser.py
@@ -196,49 +196,54 @@ class ManifestParser(object):
 
             # otherwise an item
             # apply ancestor defaults, while maintaining current file priority
             data = dict(self._ancestor_defaults.items() + data.items())
 
             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']))
 
             # determine the path
             path = test.get('path', section)
             _relpath = path
             if '://' not in path:  # don't futz with URLs
                 path = normalize_path(path)
                 if here and not os.path.isabs(path):
                     # Profiling indicates 25% of manifest parsing is spent
                     # in this call to normpath, but almost all calls return
                     # their argument unmodified, so we avoid the call if
                     # '..' if not present in the path.
                     path = os.path.join(here, path)
                     if '..' in path:
                         path = os.path.normpath(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):
-                    _relpath = path[len(rootdir):]
-                else:
-                    _relpath = relpath(path, rootdir)
+                _relpath = relative_to_root(path)
 
             test['path'] = path
             test['relpath'] = _relpath
 
             if parentmanifest is not None:
                 # If a test was included by a parent manifest we may need to
                 # indicate that in the test object for the sake of identifying
                 # a test, particularly in the case a test file is included by
@@ -488,17 +493,25 @@ class ManifestParser(object):
             if not os.path.isabs(path):
                 path = test['path']
                 if self.rootdir:
                     path = relpath(test['path'], self.rootdir)
                 path = denormalize_path(path)
             print('[%s]' % path, file=fp)
 
             # reserved keywords:
-            reserved = ['path', 'name', 'here', 'manifest', 'relpath', 'ancestor-manifest']
+            reserved = [
+                'path',
+                'name',
+                'here',
+                'manifest',
+                'manifest_relpath',
+                'relpath',
+                'ancestor-manifest'
+            ]
             for key in sorted(test.keys()):
                 if key in reserved:
                     continue
                 if key in global_kwargs:
                     continue
                 if key in global_tags and not test[key]:
                     continue
                 print('%s = %s' % (key, test[key]), file=fp)
--- a/testing/mozbase/manifestparser/tests/test_filters.py
+++ b/testing/mozbase/manifestparser/tests/test_filters.py
@@ -1,27 +1,29 @@
 #!/usr/bin/env python
-# flake8: noqa
+
+from __future__ import absolute_import, print_function
 
-from __future__ import absolute_import
-
+import os
 from copy import deepcopy
-import os
+from pprint import pprint
 
+import mozpack.path as mozpath
 import mozunit
 import pytest
 
 from manifestparser.filters import (
+    enabled,
+    fail_if,
+    filterlist,
+    pathprefix,
+    run_if,
+    skip_if,
     subsuite,
     tags,
-    skip_if,
-    run_if,
-    fail_if,
-    enabled,
-    filterlist,
 )
 
 here = os.path.dirname(os.path.abspath(__file__))
 
 
 def test_data_model():
     foo = lambda x, y: x
     bar = lambda x, y: x
@@ -92,27 +94,60 @@ def test_filters_run_in_order():
 
     fl = filterlist([a, b])
     fl.append(c)
     fl.extend([d, e])
     fl += [f]
     assert [i for i in fl] == [a, b, c, d, e, f]
 
 
+@pytest.fixture(scope='module')
+def create_tests():
+
+    def inner(*paths, **defaults):
+        tests = []
+        for path in paths:
+            if isinstance(path, tuple):
+                path, kwargs = path
+            else:
+                kwargs = {}
+
+            path = mozpath.normpath(path)
+            manifest = kwargs.pop('manifest', defaults.pop('manifest',
+                                  mozpath.join(mozpath.dirname(path), 'manifest.ini')))
+            test = {
+                'name': mozpath.basename(path),
+                'path': '/root/' + path,
+                'relpath': path,
+                'manifest': '/root/' + manifest,
+                'manifest_relpath': manifest,
+            }
+            test.update(**defaults)
+            test.update(**kwargs)
+            tests.append(test)
+
+        # dump tests to stdout for easier debugging on failure
+        print("The 'create_tests' fixture returned:")
+        pprint(tests, indent=2)
+        return tests
+
+    return inner
+
+
 @pytest.fixture
-def tests():
-    return (
-        {"name": "test0"},
-        {"name": "test1", "skip-if": "foo == 'bar'"},
-        {"name": "test2", "run-if": "foo == 'bar'"},
-        {"name": "test3", "fail-if": "foo == 'bar'"},
-        {"name": "test4", "disabled": "some reason"},
-        {"name": "test5", "subsuite": "baz"},
-        {"name": "test6", "subsuite": "baz,foo == 'bar'"},
-        {"name": "test7", "tags": "foo bar"},
+def tests(create_tests):
+    return create_tests(
+        "test0",
+        ("test1", {"skip-if": "foo == 'bar'"}),
+        ("test2", {"run-if": "foo == 'bar'"}),
+        ("test3", {"fail-if": "foo == 'bar'"}),
+        ("test4", {"disabled": "some reason"}),
+        ("test5", {"subsuite": "baz"}),
+        ("test6", {"subsuite": "baz,foo == 'bar'"}),
+        ("test7", {"tags": "foo bar"}),
     )
 
 
 def test_skip_if(tests):
     ref = deepcopy(tests)
     tests = list(skip_if(tests, {}))
     assert len(tests) == len(ref)
 
@@ -188,10 +223,47 @@ def test_tags(tests):
     assert len(tests) == 0
 
     tests = deepcopy(ref)
     tests = list(ftags2(tests, {}))
     assert len(tests) == 1
     assert ref[7] in tests
 
 
+def test_pathprefix(create_tests):
+    tests = create_tests(
+        'test0',
+        'subdir/test1',
+        'subdir/test2',
+        ('subdir/test3', {'manifest': 'manifest.ini'}),
+    )
+
+    def names(items):
+        return sorted(i['name'] for i in items)
+
+    # relative directory
+    prefix = pathprefix('subdir')
+    filtered = prefix(tests, {})
+    assert names(filtered) == ['test1', 'test2', 'test3']
+
+    # absolute directory
+    prefix = pathprefix(['/root/subdir'])
+    filtered = prefix(tests, {})
+    assert names(filtered) == ['test1', 'test2', 'test3']
+
+    # relative manifest
+    prefix = pathprefix(['subdir/manifest.ini'])
+    filtered = prefix(tests, {})
+    assert names(filtered) == ['test1', 'test2']
+
+    # absolute manifest
+    prefix = pathprefix(['/root/subdir/manifest.ini'])
+    filtered = prefix(tests, {})
+    assert names(filtered) == ['test1', 'test2']
+
+    # mixed test and manifest
+    prefix = pathprefix(['subdir/test2', 'manifest.ini'])
+    filtered = prefix(tests, {})
+    assert names(filtered) == ['test0', 'test2', 'test3']
+
+
 if __name__ == '__main__':
     mozunit.main()