Bug 1497898 - Allow the gitignore filter to work on name components only, r=ato
authorJames Graham <james@hoppipolla.co.uk>
Fri, 16 Nov 2018 18:48:36 +0000
changeset 503249 46c50835b9972454afc33df9df569afd657430ca
parent 503248 284749dd9ee9d5e693cd86887934092a8889e1fc
child 503250 215f0ae6129888a0865d151835a86aee34368156
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersato
bugs1497898
milestone65.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 1497898 - Allow the gitignore filter to work on name components only, r=ato We end up with a lot of rules like (?:.*)/.*\.ext which are basically trying to find the last component in a path and match against that. These are rather slow to run so the easiest thing tdo is just pass in the last component of the path when we know that's the only thing the rule can match. The changes to surrounding code to use this API will be made in future commits. Depends on D8222 Differential Revision: https://phabricator.services.mozilla.com/D8223
testing/web-platform/tests/tools/gitignore/gitignore.py
testing/web-platform/tests/tools/manifest/manifest.py
testing/web-platform/tests/tools/wptrunner/wptrunner/testloader.py
--- a/testing/web-platform/tests/tools/gitignore/gitignore.py
+++ b/testing/web-platform/tests/tools/gitignore/gitignore.py
@@ -1,26 +1,31 @@
 import re
 import os
 
 end_space = re.compile(r"([^\\]\s)*$")
 
 
-def fnmatch_translate(pat, path_name=False):
+def fnmatch_translate(pat, allow_component_only=True):
     parts = []
     seq = False
     i = 0
-    if pat[0] == "/" or path_name:
+    component_pattern = False
+    if pat[0] == "/":
         parts.append("^")
         any_char = "[^/]"
         if pat[0] == "/":
             pat = pat[1:]
     else:
         any_char = "."
-        parts.append("^(?:.*/)?")
+        if allow_component_only and "/" not in pat:
+            component_pattern = True
+            parts.append("^")
+        else:
+            parts.append("^(?:.*/)?")
     if pat[-1] == "/":
         # If the last character is / match this directory or any subdirectory
         pat = pat[:-1]
         suffix = "(?:/|$)"
     else:
         suffix = "$"
     while i < len(pat):
         c = pat[i]
@@ -39,17 +44,17 @@ def fnmatch_translate(pat, path_name=Fal
                 if parts[-1] == "[":
                     parts = parts[:-1]
                 elif parts[-1] == "^" and parts[-2] == "[":
                     parts = parts[:-2]
                 else:
                     parts.append(c)
             elif c == "-":
                 parts.append(c)
-            elif not (path_name and c == "/"):
+            else:
                 parts += re.escape(c)
         elif c == "[":
             parts.append("[")
             if i < len(pat) - 1 and pat[i+1] in ("!", "^"):
                 parts.append("^")
                 i += 1
             seq = True
         elif c == "*":
@@ -65,17 +70,17 @@ def fnmatch_translate(pat, path_name=Fal
         else:
             parts.append(re.escape(c))
         i += 1
 
     if seq:
         raise ValueError
     parts.append(suffix)
     try:
-        return re.compile("".join(parts))
+        return component_pattern, re.compile("".join(parts))
     except Exception:
         raise
 
 
 def parse_line(line):
     line = line.rstrip()
     if not line or line[0] == "#":
         return
--- a/testing/web-platform/tests/tools/manifest/manifest.py
+++ b/testing/web-platform/tests/tools/manifest/manifest.py
@@ -38,34 +38,33 @@ item_classes = {"testharness": Testharne
                 "stub": Stub,
                 "wdspec": WebDriverSpecTest,
                 "conformancechecker": ConformanceCheckerTest,
                 "visual": VisualTest,
                 "support": SupportFile}
 
 
 class TypeData(object):
-    def __init__(self, manifest, type_cls):
+    def __init__(self, manifest, type_cls, meta_filters):
         """Dict-like object containing the TestItems for each test type.
 
         Loading an actual Item class for each test is unnecessarily
         slow, so this class allows lazy-loading of the test
         items. When the manifest is loaded we store the raw json
         corresponding to the test type, and only create an Item
         subclass when the test is accessed. In order to remain
         API-compatible with consumers that depend on getting an Item
         from iteration, we do egerly load all items when iterating
-        over the class.
-
-        """
+        over the class."""
         self.manifest = manifest
         self.type_cls = type_cls
+        self.json_data = {}
+        self.tests_root = None
         self.data = {}
-        self.json_data = None
-        self.tests_root = None
+        self.meta_filters = meta_filters or []
 
     def __getitem__(self, key):
         if key not in self.data:
             self.load(key)
         return self.data[key]
 
     def __bool__(self):
         return bool(self.data)
@@ -103,25 +102,24 @@ class TypeData(object):
             return default
 
     def itervalues(self):
         self.load_all()
         return itervalues(self.data)
 
     def iteritems(self):
         self.load_all()
-        for path, tests in iteritems(self.data):
-            yield path, tests
+        return iteritems(self.data)
 
     def load(self, key):
         """Load a specific Item given a path"""
         if self.json_data is not None:
             data = set()
             path = from_os_path(key)
-            for test in self.json_data.get(path, []):
+            for test in iterfilter(self.meta_filters, self.json_data.get(path, [])):
                 manifest_item = self.type_cls.from_json(self.manifest,
                                                         self.tests_root,
                                                         path,
                                                         test)
                 data.add(manifest_item)
             self.data[key] = data
         else:
             raise ValueError
@@ -129,17 +127,17 @@ class TypeData(object):
     def load_all(self):
         """Load all test items in this class"""
         if self.json_data is not None:
             for path, value in iteritems(self.json_data):
                 key = to_os_path(path)
                 if key in self.data:
                     continue
                 data = set()
-                for test in self.json_data.get(path, []):
+                for test in iterfilter(self.meta_filters, self.json_data.get(path, [])):
                     manifest_item = self.type_cls.from_json(self.manifest,
                                                             self.tests_root,
                                                             path,
                                                             test)
                     data.add(manifest_item)
                 self.data[key] = data
             self.json_data = None
 
@@ -152,17 +150,16 @@ class TypeData(object):
     def paths(self):
         """Get a list of all paths containing items of this type,
         without actually constructing all the items"""
         rv = set(iterkeys(self.data))
         if self.json_data:
             rv |= set(to_os_path(item) for item in iterkeys(self.json_data))
         return rv
 
-
 class ManifestData(dict):
     def __init__(self, manifest, meta_filters=None):
         """Dictionary subclass containing a TypeData instance for each test type,
         keyed by type name"""
         self.initialized = False
         for key, value in item_classes.iteritems():
             self[key] = TypeData(manifest, value, meta_filters=meta_filters)
         self.initialized = True
@@ -178,31 +175,32 @@ class ManifestData(dict):
         without actually constructing all the items"""
         rv = set()
         for item_data in itervalues(self):
             rv |= set(item_data.paths())
         return rv
 
 
 class Manifest(object):
-    def __init__(self, url_base="/"):
+    def __init__(self, url_base="/", meta_filters=None):
         assert url_base is not None
         self._path_hash = {}
-        self._data = ManifestData(self)
+        self._data = ManifestData(self, meta_filters)
         self._reftest_nodes_by_url = None
         self.url_base = url_base
 
     def __iter__(self):
         return self.itertypes()
 
     def itertypes(self, *types):
         if not types:
             types = sorted(self._data.keys())
         for item_type in types:
-            for path, tests in sorted(self._data[item_type]):
+            for path in sorted(self._data[item_type]):
+                tests = self._data[item_type][path]
                 yield item_type, path, tests
 
     def iterpath(self, path):
         for type_tests in self._data.values():
             for test in type_tests.get(path, set()):
                 yield test
 
     def iterdir(self, dir_name):
@@ -346,24 +344,22 @@ class Manifest(object):
         return rv
 
     @classmethod
     def from_json(cls, tests_root, obj, types=None, meta_filters=None):
         version = obj.get("version")
         if version != CURRENT_VERSION:
             raise ManifestVersionMismatch
 
-        self = cls(url_base=obj.get("url_base", "/"))
+        self = cls(url_base=obj.get("url_base", "/"), meta_filters=meta_filters)
         if not hasattr(obj, "items") and hasattr(obj, "paths"):
             raise ManifestError
 
         self._path_hash = {to_os_path(k): v for k, v in iteritems(obj["paths"])}
 
-        meta_filters = meta_filters or []
-
         for test_type, type_paths in iteritems(obj["items"]):
             if test_type not in item_classes:
                 raise ManifestError
 
             if types and test_type not in types:
                 continue
 
             self._data[test_type].set_json(tests_root, type_paths)
@@ -377,25 +373,31 @@ def load(tests_root, manifest, types=Non
     # "manifest" is a path or file-like object.
     if isinstance(manifest, string_types):
         if os.path.exists(manifest):
             logger.debug("Opening manifest at %s" % manifest)
         else:
             logger.debug("Creating new manifest at %s" % manifest)
         try:
             with open(manifest) as f:
-                rv = Manifest.from_json(tests_root, json.load(f), types=types, meta_filters=meta_filters)
+                rv = Manifest.from_json(tests_root,
+                                        json.load(f),
+                                        types=types,
+                                        meta_filters=meta_filters)
         except IOError:
             return None
         except ValueError:
             logger.warning("%r may be corrupted", manifest)
             return None
         return rv
 
-    return Manifest.from_json(tests_root, json.load(manifest), types=types, meta_filters=meta_filters)
+    return Manifest.from_json(tests_root,
+                              json.load(manifest),
+                              types=types,
+                              meta_filters=meta_filters)
 
 
 def write(manifest, manifest_path):
     dir_name = os.path.dirname(manifest_path)
     if not os.path.exists(dir_name):
         os.makedirs(dir_name)
     with open(manifest_path, "wb") as f:
         json.dump(manifest.to_json(), f, sort_keys=True, indent=1)
--- a/testing/web-platform/tests/tools/wptrunner/wptrunner/testloader.py
+++ b/testing/web-platform/tests/tools/wptrunner/wptrunner/testloader.py
@@ -397,17 +397,17 @@ class ManifestLoader(object):
             rv[manifest_file] = path_data
         return rv
 
     def create_manifest(self, manifest_path, tests_path, url_base="/"):
         self.update_manifest(manifest_path, tests_path, url_base, recreate=True,
                              download=self.manifest_download)
 
     def update_manifest(self, manifest_path, tests_path, url_base="/",
-                        recreate=False, download=False):
+                        meta_filters=None, recreate=False, download=False):
         self.logger.info("Updating test manifest %s" % manifest_path)
 
         json_data = None
         if download:
             # TODO: make this not github-specific
             download_from_github(manifest_path, tests_path)
 
         if not recreate:
@@ -419,32 +419,45 @@ class ManifestLoader(object):
             except ValueError:
                 self.logger.info("Unable to parse test manifest")
 
         if not json_data:
             self.logger.info("Creating test manifest")
             manifest_file = manifest.Manifest(url_base)
         else:
             try:
-                manifest_file = manifest.Manifest.from_json(tests_path, json_data)
+                manifest_file = manifest.Manifest.from_json(tests_path,
+                                                            json_data,
+                                                            meta_filters=meta_filters)
             except manifest.ManifestVersionMismatch:
-                manifest_file = manifest.Manifest(url_base)
+                manifest_file = manifest.Manifest(url_base, meta_filters=meta_filters)
 
         manifest_update.update(tests_path, manifest_file, True)
 
         manifest.write(manifest_file, manifest_path)
+        return manifest_file
 
     def load_manifest(self, tests_path, manifest_path, url_base="/", **kwargs):
-        if (not os.path.exists(manifest_path) or
-            self.force_manifest_update):
-            self.update_manifest(manifest_path, tests_path, url_base, download=self.manifest_download)
         try:
-            manifest_file = manifest.load(tests_path, manifest_path, types=self.types, meta_filters=self.meta_filters)
+            if (not os.path.exists(manifest_path) or
+                self.force_manifest_update):
+                manifest_file = self.update_manifest(manifest_path,
+                                                     tests_path,
+                                                     url_base,
+                                                     meta_filters=self.meta_filters,
+                                                     download=self.manifest_download)
+
+            else:
+                manifest_file = manifest.load(tests_path,
+                                              manifest_path,
+                                              types=self.types,
+                                              meta_filters=self.meta_filters)
         except manifest.ManifestVersionMismatch:
             manifest_file = manifest.Manifest(url_base)
+
         if manifest_file.url_base != url_base:
             self.logger.info("Updating url_base in manifest from %s to %s" % (manifest_file.url_base,
                                                                               url_base))
             manifest_file.url_base = url_base
             manifest.write(manifest_file, manifest_path)
 
         return manifest_file