Bug 1334767 - Make wpt manifest lint only flag changes that will affect the tests, r=Ms2ger
authorJames Graham <james@hoppipolla.co.uk>
Sat, 28 Jan 2017 08:49:11 +0000
changeset 331761 a16de69b0207951be269b3070b67860a57bf5c62
parent 331760 37d919b2c97ab3b123d82e878d2917f28658ceb0
child 331762 01aad8181f5d02e797b24d0b46ceeb727e762f12
push id31285
push usercbook@mozilla.com
push dateTue, 31 Jan 2017 14:53:43 +0000
treeherdermozilla-central@1d5f138d4af7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMs2ger
bugs1334767
milestone54.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 1334767 - Make wpt manifest lint only flag changes that will affect the tests, r=Ms2ger This is needed because we changed the manifest format to store file hashes for faster updating. But keeping that up to date requires the manifest to be regenerated too often so we instead just check for changes to the actual tests that will run. MozReview-Commit-ID: FYU5Vr6cXwd
testing/web-platform/manifestupdate.py
--- a/testing/web-platform/manifestupdate.py
+++ b/testing/web-platform/manifestupdate.py
@@ -1,16 +1,23 @@
 import argparse
 import imp
 import os
 import sys
+from collections import defaultdict
 
 from mozlog.structured import commandline
 from wptrunner.wptcommandline import get_test_paths, set_from_config
-from wptrunner.testloader import ManifestLoader
+
+manifest = None
+
+def do_delayed_imports(wpt_dir):
+    global manifest
+    sys.path.insert(0, os.path.join(wpt_dir, "tools", "manifest"))
+    import manifest
 
 def create_parser():
     p = argparse.ArgumentParser()
     p.add_argument("--check-clean", action="store_true",
                    help="Check that updating the manifest doesn't lead to any changes")
     commandline.add_logging_group(p)
 
     return p
@@ -22,68 +29,115 @@ def update(logger, wpt_dir, check_clean=
     kwargs = {"config": os.path.join(wpt_dir, "wptrunner.ini"),
               "tests_root": None,
               "metadata_root": None}
 
     set_from_config(kwargs)
     config = kwargs["config"]
     test_paths = get_test_paths(config)
 
+    do_delayed_imports(wpt_dir)
+
     if check_clean:
-        old_manifests = {}
-        for data in test_paths.itervalues():
-            path = os.path.join(data["metadata_path"], "MANIFEST.json")
-            with open(path) as f:
-                old_manifests[path] = f.readlines()
+        return _check_clean(logger, test_paths)
+
+    return _update(logger, test_paths)
+
 
-    try:
-        ManifestLoader(test_paths, force_manifest_update=True).load()
+def _update(logger, test_paths):
+    for url_base, paths in test_paths.iteritems():
+        manifest_path = os.path.join(paths["metadata_path"], "MANIFEST.json")
+        m = manifest.manifest.load(paths["tests_path"], manifest_path)
+        manifest.update.update(paths["tests_path"], m, working_copy=True)
+        manifest.manifest.write(m, manifest_path)
+    return 0
+
 
-        rv = 0
+def _check_clean(logger, test_paths):
+    manifests_by_path = {}
+    rv = 0
+    for url_base, paths in test_paths.iteritems():
+        tests_path = paths["tests_path"]
+        manifest_path = os.path.join(paths["metadata_path"], "MANIFEST.json")
+        old_manifest = manifest.manifest.load(tests_path, manifest_path)
+        new_manifest = manifest.manifest.Manifest.from_json(tests_path,
+                                                            old_manifest.to_json())
+        manifest.update.update(tests_path, new_manifest, working_copy=True)
+        manifests_by_path[manifest_path] = (old_manifest, new_manifest)
 
-        if check_clean:
-            clean = diff_manifests(logger, old_manifests)
-            if not clean:
-                rv = 1
-    finally:
-        if check_clean:
-            for path, data in old_manifests.iteritems():
-                logger.info("Restoring manifest %s" % path)
-                with open(path, "w") as f:
-                    f.writelines(data)
+    for manifest_path, (old_manifest, new_manifest) in manifests_by_path.iteritems():
+        if not diff_manifests(logger, manifest_path, old_manifest, new_manifest):
+            rv = 1
+    if rv:
+        logger.error("Manifest %s is outdated, use |mach wpt-manifest-update| to fix." % manifest_path)
 
     return rv
 
-def diff_manifests(logger, old_manifests):
-    logger.info("Diffing old and new manifests")
-    import difflib
+
+def diff_manifests(logger, manifest_path, old_manifest, new_manifest):
+    """Lint the differences between old and new versions of a
+    manifest. Differences are considered significant (and so produce
+    lint errors) if they produce a meaningful difference in the actual
+    tests run.
+
+    :param logger: mozlog logger to use for output
+    :param manifest_path: Path to the manifest being linted
+    :param old_manifest: Manifest object representing the initial manifest
+    :param new_manifest: Manifest object representing the updated manifest
+    """
+    logger.info("Diffing old and new manifests %s" % manifest_path)
+    old_items, new_items = defaultdict(set), defaultdict(set)
+    for manifest, items in [(old_manifest, old_items),
+                            (new_manifest, new_items)]:
+        for test_type, path, tests in manifest:
+            for test in tests:
+                test_id = [test.id]
+                test_id.extend(tuple(item) if isinstance(item, list) else item
+                               for item in test.meta_key())
+                if hasattr(test, "references"):
+                    test_id.extend(tuple(item) for item in test.references)
+                test_id = tuple(test_id)
+                items[path].add((test_type, test_id))
+
+    old_paths = set(old_items.iterkeys())
+    new_paths = set(new_items.iterkeys())
+
+    added_paths = new_paths - old_paths
+    deleted_paths = old_paths - new_paths
+
+    common_paths = new_paths & old_paths
 
     clean = True
-    for path, old in old_manifests.iteritems():
-        with open(path) as f:
-            new = f.readlines()
 
-        if old != new:
+    for path in added_paths:
+        clean = False
+        log_error(logger, manifest_path, "%s in source but not in manifest." % path)
+    for path in deleted_paths:
+        clean = False
+        log_error(logger, manifest_path, "%s in manifest but removed from source." % path)
+
+    for path in common_paths:
+        old_tests = old_items[path]
+        new_tests = new_items[path]
+        added_tests = new_tests - old_tests
+        removed_tests = old_tests - new_tests
+        if added_tests or removed_tests:
             clean = False
-            sm = difflib.SequenceMatcher(a=old, b=new)
-            for group in sm.get_grouped_opcodes():
-                logged = False
-                message = []
-                for op, old_0, old_1, new_0, new_1 in group:
-                    if op != "equal" and not logged:
-                        logged = True
-                        logger.lint_error(path=path,
-                                          message="Manifest changed",
-                                          lineno=(old_0 + 1),
-                                          source="\n".join(old[old_0:old_1]),
-                                          linter="wpt-manifest")
-                    if op == "equal":
-                        message.extend(' ' + line for line in old[old_0:old_1])
-                    if op in ('replace', 'delete'):
-                        message.extend('-' + line for line in old[old_0:old_1])
-                    if op in ('replace', 'insert'):
-                        message.extend('+' + line for line in new[new_0:new_1])
-                logger.info("".join(message))
+            log_error(logger, manifest_path, "%s changed test types or metadata" % path)
+
     if clean:
-        logger.info("No differences found")
+        # Manifest currently has some list vs tuple inconsistencies that break
+        # a simple equality comparison.
+        new_paths = {(key, value[0], value[1])
+                     for (key, value) in new_manifest.to_json()["paths"].iteritems()}
+        old_paths = {(key, value[0], value[1])
+                     for (key, value) in old_manifest.to_json()["paths"].iteritems()}
+        if old_paths != new_paths:
+            logger.warning("Manifest %s contains correct tests but file hashes changed; please update" % manifest_path)
 
     return clean
 
+def log_error(logger, manifest_path, msg):
+    logger.lint_error(path=manifest_path,
+                      message=msg,
+                      lineno=0,
+                      source="",
+                      linter="wpt-manifest")