Bug 1334767 - Make wpt manifest lint only flag changes that will affect the tests, r=Ms2ger draft
authorJames Graham <james@hoppipolla.co.uk>
Sat, 28 Jan 2017 08:49:11 +0000
changeset 468374 f53b89c2b88b9739ddad9d640e6c23b66d7dd972
parent 468373 bb64fb1dcca06a5bd583c0d833363e0e6c191366
child 543926 491d595e14148aaed41b378256fd6069e95010d1
push id43442
push userbmo:james@hoppipolla.co.uk
push dateTue, 31 Jan 2017 06:24:29 +0000
reviewersMs2ger
bugs1334767
milestone54.0a1
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")