Bug 1514456 [wpt PR 14537] - Cache manifest everywhere, a=testonly
authorRobert Ma <robertma@chromium.org>
Thu, 31 Jan 2019 12:14:08 +0000
changeset 457779 c61e1ff60a77e10b9e20029e28a9724234a33475
parent 457778 7f5ee45f4443cbecedf305c93c01636e13453b62
child 457780 a3338f338d4082b62b4d83add8e00a8fd0c151fe
push id35518
push useropoprus@mozilla.com
push dateFri, 08 Feb 2019 09:55:14 +0000
treeherdermozilla-central@3a3e393396f4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstestonly
bugs1514456, 14537, 14421
milestone67.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 1514456 [wpt PR 14537] - Cache manifest everywhere, a=testonly Automatic update from web-platform-tests Cache manifest everywhere Move the cache layer of manifest objects from wpt.testfiles to manifest.manifest so that all users of the manifest module can benefit. This prevents us from having two in-memory copies of the manifest when running affected tests (one loaded by wpt.testfiles and the other loaded by wptrunner.testloader). Partly fixes #14421 (the memory usage part of it, but not the code health part). -- Fix test_wpt.py The test module uses the same temporary manifest in all test cases, but some test cases modify the manifest, which causes inconsistency in the cache. This commit changes the test setup to initialize a persistent manifest on the module level, and each test case will then make a temporary copy of it. Also speed up tools/wpt tests on Azure by updating the manifest first. -- wpt-commits: 46bac0d389a48d590631de2f06534c62d170671d, 5d720552ca9e7bc3bf9442571a7ab1b763cdc02c wpt-pr: 14537
testing/web-platform/tests/.azure-pipelines.yml
testing/web-platform/tests/tools/manifest/manifest.py
testing/web-platform/tests/tools/wpt/testfiles.py
testing/web-platform/tests/tools/wpt/tests/test_wpt.py
--- a/testing/web-platform/tests/.azure-pipelines.yml
+++ b/testing/web-platform/tests/.azure-pipelines.yml
@@ -112,11 +112,12 @@ jobs:
   condition: dependencies.decision.outputs['test_jobs.wpt_integration']
   pool:
     vmImage: 'macOS-10.13'
   steps:
   # full checkout required
   - template: tools/ci/azure/install_chrome.yml
   - template: tools/ci/azure/install_firefox.yml
   - template: tools/ci/azure/update_hosts.yml
+  - template: tools/ci/azure/update_manifest.yml
   - template: tools/ci/azure/tox_pytest.yml
     parameters:
       directory: tools/wpt/
--- a/testing/web-platform/tests/tools/manifest/manifest.py
+++ b/testing/web-platform/tests/tools/manifest/manifest.py
@@ -397,40 +397,50 @@ class Manifest(object):
 
 def load(tests_root, manifest, types=None, meta_filters=None):
     logger = get_logger()
 
     logger.warning("Prefer load_and_update instead")
     return _load(logger, tests_root, manifest, types, meta_filters)
 
 
+__load_cache = {}
+
+
 def _load(logger, tests_root, manifest, types=None, meta_filters=None):
     # "manifest" is a path or file-like object.
+    manifest_path = (manifest if isinstance(manifest, string_types)
+                     else manifest.name)
+    if manifest_path in __load_cache:
+        return __load_cache[manifest_path]
+
     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)
         except IOError:
             return None
         except ValueError:
             logger.warning("%r may be corrupted", manifest)
             return None
-        return rv
+    else:
+        rv = 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)
+    __load_cache[manifest_path] = rv
+    return rv
 
 
 def load_and_update(tests_root,
                     manifest_path,
                     url_base,
                     update=True,
                     rebuild=False,
                     metadata_path=None,
--- a/testing/web-platform/tests/tools/wpt/testfiles.py
+++ b/testing/web-platform/tests/tools/wpt/testfiles.py
@@ -3,18 +3,16 @@ import logging
 import os
 import re
 import subprocess
 import sys
 
 from collections import OrderedDict
 from six import iteritems
 
-from ..manifest import manifest
-
 here = os.path.dirname(__file__)
 wpt_root = os.path.abspath(os.path.join(here, os.pardir, os.pardir))
 
 logger = logging.getLogger()
 
 
 def get_git_cmd(repo_path):
     """Create a function for invoking git commands as a subprocess."""
@@ -178,34 +176,25 @@ def files_changed(revish, ignore_rules=N
 
 
 def _in_repo_root(full_path):
     rel_path = os.path.relpath(full_path, wpt_root)
     path_components = rel_path.split(os.sep)
     return len(path_components) < 2
 
 
-def _init_manifest_cache():
-    c = {}
-
-    def load(manifest_path=None, manifest_update=True):
-        if manifest_path is None:
-            manifest_path = os.path.join(wpt_root, "MANIFEST.json")
-        if c.get(manifest_path):
-            return c[manifest_path]
-        # cache at most one path:manifest
-        c.clear()
-        wpt_manifest = manifest.load_and_update(wpt_root, manifest_path, "/",
-                                                update=manifest_update)
-        c[manifest_path] = wpt_manifest
-        return c[manifest_path]
-    return load
-
-
-load_manifest = _init_manifest_cache()
+def load_manifest(manifest_path=None, manifest_update=True):
+    # Delay import after localpaths sets up sys.path, because otherwise the
+    # import path will be "..manifest" and Python will treat it as a different
+    # manifest module.
+    from manifest import manifest
+    if manifest_path is None:
+        manifest_path = os.path.join(wpt_root, "MANIFEST.json")
+    return manifest.load_and_update(wpt_root, manifest_path, "/",
+                                    update=manifest_update)
 
 
 def affected_testfiles(files_changed, skip_dirs=None,
                        manifest_path=None, manifest_update=True):
     """Determine and return list of test files that reference changed files."""
     if skip_dirs is None:
         skip_dirs = set(["conformance-checkers", "docs", "tools"])
     affected_testfiles = set()
--- a/testing/web-platform/tests/tools/wpt/tests/test_wpt.py
+++ b/testing/web-platform/tests/tools/wpt/tests/test_wpt.py
@@ -25,37 +25,39 @@ def is_port_8000_in_use():
             return True
         else:
             raise e
     finally:
         s.close()
     return False
 
 
-@pytest.fixture(scope="module")
+def get_persistent_manifest_path():
+    directory = ("~/meta" if os.environ.get('TRAVIS') == "true"
+                 else wpt.localpaths.repo_root)
+    return os.path.join(directory, "MANIFEST.json")
+
+
+@pytest.fixture(scope="module", autouse=True)
+def init_manifest():
+    with pytest.raises(SystemExit) as excinfo:
+        wpt.main(argv=["manifest", "--no-download",
+                       "--path", get_persistent_manifest_path()])
+    assert excinfo.value.code == 0
+
+
+@pytest.fixture
 def manifest_dir():
-    def update_manifest():
-        with pytest.raises(SystemExit) as excinfo:
-            wpt.main(argv=["manifest", "--no-download", "--path", os.path.join(path, "MANIFEST.json")])
-        assert excinfo.value.code == 0
-
-    if os.environ.get('TRAVIS') == "true":
-        path = "~/meta"
-        update_manifest()
+    try:
+        path = tempfile.mkdtemp()
+        shutil.copyfile(get_persistent_manifest_path(),
+                        os.path.join(path, "MANIFEST.json"))
         yield path
-    else:
-        try:
-            path = tempfile.mkdtemp()
-            old_path = os.path.join(wpt.localpaths.repo_root, "MANIFEST.json")
-            if os.path.exists(os.path.join(wpt.localpaths.repo_root, "MANIFEST.json")):
-                shutil.copyfile(old_path, os.path.join(path, "MANIFEST.json"))
-            update_manifest()
-            yield path
-        finally:
-            shutil.rmtree(path)
+    finally:
+        shutil.rmtree(path)
 
 
 @pytest.fixture
 def temp_test():
     os.makedirs("../../.tools-tests")
     test_count = {"value": 0}
 
     def make_test(body):