Consolidate logic for multi-global tests.

Instead of a series of if-conditions create some functions for
working with multi-global tests and reuse them as far as possible in
all the different places where we were previously repeating the
logic. This should make the tests easier to maintain in the future
particularly if we want to add more types of globals or different ways
of referring to different subsets.

Also fix handling of .https. and .any. so that the test author doesn't
have to carefully specify flags in the correct order for things to
work properly.

tools/lint/lint.py

@@ -18,7 +18,7 @@
 from ..gitignore.gitignore import PathFilter
 from ..wpt import testfiles
 
-from manifest.sourcefile import SourceFile, js_meta_re, python_meta_re, space_chars
+from manifest.sourcefile import SourceFile, js_meta_re, python_meta_re, space_chars, any_variants, skip_variants
 from six import binary_type, iteritems, itervalues
 from six.moves import range
 from six.moves.urllib.parse import urlsplit, urljoin
@@ -635,17 +635,36 @@ def check_script_metadata(repo_root, path, f, css_mode):
         if m:
             key, value = m.groups()
             if key == b"global":
-                valid_global_values = ["window", "worker", "serviceworker", "!serviceworker"]
-                global_values = value.split(b",")
+                global_values = {item.strip() for item in value.split(",")}
                 for global_value in global_values:
-                    if global_value not in valid_global_values:
-                        errors.append(("UNKNOWN-GLOBAL-METADATA", "Unexpected value for global metadata", path, idx + 1))
-                    if global_value == b"serviceworker" and b"!serviceworker" in global_values:
-                        errors.append(("BROKEN-GLOBAL-METADATA", "Cannot specify both serviceworker and !serviceworker", path, idx + 1))
-                    elif global_value == b"serviceworker" and b"worker" in global_values:
-                        errors.append(("BROKEN-GLOBAL-METADATA", "Cannot specify both serviceworker and worker", path, idx + 1))
-                    elif global_value == b"!serviceworker" and b"worker" not in global_values:
-                         errors.append(("BROKEN-GLOBAL-METADATA", "Cannot specify !serviceworker without worker", path, idx + 1))
+                    if global_value not in any_variants and global_value not in skip_variants:
+                        errors.append(("UNKNOWN-GLOBAL-METADATA",
+                                       "Unexpected value for global metadata",
+                                       path, idx + 1))
+                    if global_value in skip_variants:
+                        if global_value[1:] in global_values:
+                            errors.append(("BROKEN-GLOBAL-METADATA",
+                                           "Cannot specify both %s and %s" % (global_value, global_value[1:]),
+                                           path, idx + 1))
+                        else:
+                            # Check for a !value actually corresponds to something in an expansion
+                            valid_keys = {key for key, values in iteritems(any_variants)
+                                          if global_value[1:] in values.get("globals", {})}
+                            if not valid_keys & global_values:
+                                errors.append(("BROKEN-GLOBAL-METADATA",
+                                               "Cannot specify %s without %s" % (global_value,
+                                                                                 "or ".join(valid_keys)),
+                                               path, idx + 1))
+                    else:
+                        # Check we don't have a value as a top-level key and an expansion
+                        invalid_keys = {key for key, values in iteritems(any_variants)
+                                        if global_value in values.get("globals", {}) and
+                                        key != global_value}
+                        if invalid_keys:
+                            errors.append(("BROKEN-GLOBAL-METADATA",
+                                           "Cannot specify both %s and %s" % (global_value,
+                                                                              "or ".join(invalid_keys)),
+                                           path, idx + 1))
             elif key == b"timeout":
                 if value != b"long":
                     errors.append(("UNKNOWN-TIMEOUT-METADATA", "Unexpected value for timeout metadata", path, idx + 1))

tools/manifest/sourcefile.py

@@ -1,7 +1,7 @@
 import hashlib
 import re
 import os
-from six import binary_type
+from six import binary_type, itervalues
 from six.moves.urllib.parse import urljoin
 from fnmatch import fnmatch
 try:
@@ -49,6 +49,44 @@ def read_script_metadata(f, regexp):
         yield (m.groups()[0], m.groups()[1])
 
 
+any_variants = {"window": {"suffix": ".any.html"},
+                "serviceworker": {},
+                "worker": {"globals": {"worker", "sharedworker", "serviceworker"}}}
+skip_variants = {"!serviceworker"}
+force_https = {"serviceworker"}
+
+
+def global_variants(value):
+    rv = {}
+    global_values = set(item.strip() for item in value.split(","))
+    for item in global_values:
+        if item in any_variants:
+            variant = any_variants[item]
+            global_types = variant.get("globals", {item})
+            for global_type in global_types:
+                suffix = variant.get("suffix", ".any.%s.html" % global_type)
+                if global_type in force_https:
+                    suffix = ".https" + suffix
+                rv[global_type] = suffix
+    for item in skip_variants:
+        variant = item[1:]
+        if item in global_values and variant in rv:
+            del rv[variant]
+    return rv
+
+
+def global_variant_url(url, suffix):
+    # Ensure that .any. is the last flag
+    if not url.endswith(".any.js"):
+        url = url.replace(".any.", ".")
+        replace_end(url, ".js", ".any.js")
+    # If the url must be loaded over https, ensure that it will have
+    # the form .https.any.js
+    if ".https." in url and suffix.startswith(".https."):
+        url = url.replace(".https.", ".")
+    return replace_end(url, ".any.js", suffix)
+
+
 class SourceFile(object):
     parsers = {"html":lambda x:html5lib.parse(x, treebuilder="etree"),
                "xhtml":lambda x:ElementTree.parse(x, XMLParser.XMLParser()),
@@ -504,23 +542,14 @@ def manifest_items(self):
 
         elif self.name_is_any:
             rv = TestharnessTest.item_type, []
-            global_value_paths = []
             for (key, value) in self.script_metadata:
                 if key == b"global":
-                    global_values = value.split(b",")
-                    for global_value in global_values:
-                        if global_value == b"window":
-                            global_value_paths.append(".any.html")
-                        elif global_value == b"serviceworker":
-                            global_value_paths.append(".any.serviceworker.https.html")
-                        elif global_value == b"worker":
-                            global_value_paths.append(".any.worker.html")
-                            global_value_paths.append(".any.sharedworker.html")
-                            if b"!serviceworker" not in global_values:
-                                global_value_paths.append(".any.serviceworker.https.html")
-            for path in global_value_paths:
-                rv[1].append(TestharnessTest(self, replace_end(self.url, ".any.js", path), timeout=self.timeout))
-
+                    for suffix in itervalues(global_variants(value)):
+                        url = global_variant_url(self.url, suffix)
+                        rv[1].append(TestharnessTest(self, url,
+                                                     timeout=self.timeout))
+                    break
+            print rv
         elif self.name_is_window:
             rv = TestharnessTest.item_type, [
                 TestharnessTest(self, replace_end(self.url, ".window.js", ".window.html"),

tools/serve/serve.py

@@ -20,15 +20,16 @@
 from localpaths import repo_root
 
 import sslutils
-from manifest.sourcefile import read_script_metadata, js_meta_re
+from manifest.sourcefile import read_script_metadata, js_meta_re, global_variants
 from wptserve import server as wptserve, handlers
 from wptserve import stash
 from wptserve import config
 from wptserve.logger import set_logger
 from wptserve.handlers import filesystem_path, wrap_pipeline
-from wptserve.utils import get_port
+from wptserve.utils import get_port, HTTPException
 from mod_pywebsocket import standalone as pywebsocket
 
+
 def replace_end(s, old, new):
     """
     Given a string `s` that ends with `old`, replace that occurrence of `old`
@@ -85,6 +86,9 @@ def _get_path(self, path, resource_path):
                 path = replace_end(path, src, dest)
         return path
 
+    def _validate_meta(self, key, value):
+        pass
+
     def _get_meta(self, request):
         """Get an iterator over strings to inject into the wrapper document
         based on //META comments in the associated js file.
@@ -94,6 +98,7 @@ def _get_meta(self, request):
         path = self._get_path(filesystem_path(self.base_path, request, self.url_base), False)
         with open(path, "rb") as f:
             for key, value in read_script_metadata(f, js_meta_re):
+                self._validate_meta(key, value)
                 replacement = self._meta_replacement(key, value)
                 if replacement:
                     yield replacement
@@ -118,7 +123,16 @@ def _meta_replacement(self, key, value):
         # a specific metadata key: value pair.
         pass
 
+
 class HtmlWrapperHandler(WrapperHandler):
+    global_type = None
+
+    def _validate_meta(self, key, value):
+        if key == b"global":
+            if self.global_type not in global_variants(value):
+                raise HTTPException(404, "This test cannot be loaded in %s mode" %
+                                    self.global_type)
+
     def _meta_replacement(self, key, value):
         if key == b"timeout":
             if value == b"long":
@@ -128,7 +142,9 @@ def _meta_replacement(self, key, value):
             return '<script src="%s"></script>' % attribute
         return None
 
+
 class WindowHandler(HtmlWrapperHandler):
+    global_type = "window"
     path_replace = [(".any.html", ".any.js"),
                     (".window.html", ".window.js")]
     wrapper = """<!doctype html>
@@ -146,14 +162,9 @@ class WindowHandler(HtmlWrapperHandler):
 <script src="%(path)s"></script>
 """
 
-    def _meta_replacement(self, key, value):
-        if key == b"global":
-            if b"window" not in value.split(b","):
-                raise ValueError("This test cannot be loaded in window mode")
-        else:
-            HtmlWrapperHandler._meta_replacement(self, key, value)
 
 class WorkerHandler(HtmlWrapperHandler):
+    global_type = "worker"
     path_replace = [(".any.worker.html", ".any.js", ".any.worker.js")]
     wrapper = """<!doctype html>
 <meta charset=utf-8>
@@ -166,14 +177,9 @@ class WorkerHandler(HtmlWrapperHandler):
 </script>
 """
 
-    def _meta_replacement(self, key, value):
-        if key == b"global":
-            if b"worker" not in value.split(b","):
-                raise ValueError("This test cannot be loaded in dedicated/shared worker mode")
-        else:
-            HtmlWrapperHandler._meta_replacement(self, key, value)
 
 class SharedWorkerHandler(WorkerHandler):
+    global_type = "sharedworker"
     path_replace = [(".any.sharedworker.html", ".any.js", ".any.worker.js")]
     wrapper = """<!doctype html>
 <meta charset=utf-8>
@@ -186,8 +192,10 @@ class SharedWorkerHandler(WorkerHandler):
 </script>
 """
 
+
 class ServiceWorkerHandler(HtmlWrapperHandler):
-    path_replace = [(".any.serviceworker.https.html", ".any.js", ".any.worker.js")]
+    global_type = "serviceworker"
+    path_replace = [(".https.any.serviceworker.html", ".any.js", ".any.worker.js")]
     wrapper = """<!doctype html>
 <meta charset=utf-8>
 %(meta)s
@@ -205,13 +213,6 @@ class ServiceWorkerHandler(HtmlWrapperHandler):
 </script>
 """
 
-    def _meta_replacement(self, key, value):
-        if key == b"global":
-            values = value.split(b",")
-            if (b"worker" not in values and b"serviceworker" not in values) or b"!serviceworker" in values:
-                raise ValueError("This test cannot be loaded in service worker mode")
-        else:
-            HtmlWrapperHandler._meta_replacement(self, key, value)
 
 class WorkerJavaScriptHandler(WrapperHandler):
     headers = [('Content-Type', 'text/javascript')]
@@ -227,10 +228,6 @@ class WorkerJavaScriptHandler(WrapperHandler):
 """
 
     def _meta_replacement(self, key, value):
-        if key == b"global":
-            values = value.split(b",")
-            if b"worker" not in values and "serviceworker" not in values:
-                raise ValueError("This test doesn't need a worker script")
         if key == b"script":
             attribute = value.decode('utf-8').replace("\\", "\\\\").replace('"', '\\"')
             return 'importScripts("%s")' % attribute
@@ -284,7 +281,7 @@ def add_mount_point(self, url_base, path):
             ("GET", "*.window.html", WindowHandler),
             ("GET", "*.any.worker.html", WorkerHandler),
             ("GET", "*.any.sharedworker.html", SharedWorkerHandler),
-            ("GET", "*.any.serviceworker.https.html", ServiceWorkerHandler),
+            ("GET", "*.https.any.serviceworker.html", ServiceWorkerHandler),
             ("GET", "*.any.worker.js", WorkerJavaScriptHandler),
             ("GET", "*.asis", handlers.AsIsHandler),
             ("*", "*.py", handlers.PythonScriptHandler),

tools/wptserve/wptserve/handlers.py

@@ -260,6 +260,8 @@ def __init__(self, func):
     def __call__(self, request, response):
         try:
             rv = self.func(request, response)
+        except HTTPException:
+            raise
         except Exception:
             msg = traceback.format_exc()
             raise HTTPException(500, message=msg)