Make TestRoot into a class
This is easier to type annotate than a TypedDict, and overall slightly
nicer design.
diff --git a/tools/wpt/update.py b/tools/wpt/update.py
index 41faeac..4dba7e6 100644
--- a/tools/wpt/update.py
+++ b/tools/wpt/update.py
@@ -13,8 +13,8 @@
from manifest import manifest # type: ignore
for url_base, paths in test_paths.items():
manifest.load_and_update(
- paths["tests_path"],
- paths["manifest_path"],
+ paths.tests_path,
+ paths.manifest_path,
url_base)
diff --git a/tools/wptrunner/wptrunner/config.py b/tools/wptrunner/wptrunner/config.py
index ba51fbb..d78efbf 100644
--- a/tools/wptrunner/wptrunner/config.py
+++ b/tools/wptrunner/wptrunner/config.py
@@ -4,17 +4,17 @@
import os
import sys
from collections import OrderedDict
-from typing import Any, Dict, Mapping
+from typing import Dict, Mapping, Optional
here = os.path.dirname(__file__)
-class ConfigDict(Dict[str, Any]):
- def __init__(self, base_path: str, *args: Any, **kwargs: Any):
+class ConfigDict(Dict[str, str]):
+ def __init__(self, base_path: str, *args: str, **kwargs: str):
self.base_path = base_path
dict.__init__(self, *args, **kwargs)
- def get_path(self, key: str, default:Any = None) -> Any:
+ def get_path(self, key: str, default:Optional[str] = None) -> Optional[str]:
if key not in self:
return default
path = self[key]
diff --git a/tools/wptrunner/wptrunner/environment.py b/tools/wptrunner/wptrunner/environment.py
index de96f79..e206e42 100644
--- a/tools/wptrunner/wptrunner/environment.py
+++ b/tools/wptrunner/wptrunner/environment.py
@@ -45,7 +45,7 @@
def serve_path(test_paths):
- return test_paths["/"]["tests_path"]
+ return test_paths["/"].tests_path
def webtranport_h3_server_is_running(host, port, timeout):
@@ -249,10 +249,10 @@
route_builder.add_handler("GET", "/resources/testdriver.js", TestdriverLoader())
- for url_base, paths in self.test_paths.items():
+ for url_base, test_root in self.test_paths.items():
if url_base == "/":
continue
- route_builder.add_mount_point(url_base, paths["tests_path"])
+ route_builder.add_mount_point(url_base, test_root.tests_path)
if "/" not in self.test_paths:
del route_builder.mountpoint_routes["/"]
diff --git a/tools/wptrunner/wptrunner/testloader.py b/tools/wptrunner/wptrunner/testloader.py
index bfa553e..aa26654 100644
--- a/tools/wptrunner/wptrunner/testloader.py
+++ b/tools/wptrunner/wptrunner/testloader.py
@@ -326,19 +326,20 @@
def load(self):
rv = {}
- for url_base, paths in self.test_paths.items():
- manifest_file = self.load_manifest(url_base=url_base,
- **paths)
- path_data = {"url_base": url_base}
- path_data.update(paths)
+ for url_base, test_root in self.test_paths.items():
+ manifest_file = self.load_manifest(url_base, test_root)
+ path_data = {"url_base": url_base,
+ "tests_path": test_root.tests_path,
+ "metadata_path": test_root.metadata_path,
+ "manifest_path": test_root.manifest_path}
rv[manifest_file] = path_data
return rv
- def load_manifest(self, tests_path, manifest_path, metadata_path, url_base="/", **kwargs):
- cache_root = os.path.join(metadata_path, ".cache")
+ def load_manifest(self, url_base, test_root):
+ cache_root = os.path.join(test_root.metadata_path, ".cache")
if self.manifest_download:
- download_from_github(manifest_path, tests_path)
- return manifest.load_and_update(tests_path, manifest_path, url_base,
+ download_from_github(test_root.manifest_path, test_root.tests_path)
+ return manifest.load_and_update(test_root.tests_path, test_root.manifest_path, url_base,
cache_root=cache_root, update=self.force_manifest_update,
types=self.types)
diff --git a/tools/wptrunner/wptrunner/tests/browsers/test_webkitgtk.py b/tools/wptrunner/wptrunner/tests/browsers/test_webkitgtk.py
index 324142c..b5fc5fc 100644
--- a/tools/wptrunner/wptrunner/tests/browsers/test_webkitgtk.py
+++ b/tools/wptrunner/wptrunner/tests/browsers/test_webkitgtk.py
@@ -7,9 +7,11 @@
from wptserve.config import ConfigBuilder
from ..base import active_products
-from wptrunner import environment, products, testloader
+from wptrunner import environment, products, testloader, wptcommandline
-test_paths = {"/": {"tests_path": join(dirname(__file__), "..", "..", "..", "..", "..")}} # repo root
+wpt_root = join(dirname(__file__), "..", "..", "..", "..")
+
+test_paths = {"/": wptcommandline.TestRoot(wpt_root, wpt_root)}
environment.do_delayed_imports(None, test_paths)
logger = logging.getLogger()
diff --git a/tools/wptrunner/wptrunner/tests/test_products.py b/tools/wptrunner/wptrunner/tests/test_products.py
index 4317511..0781ec5 100644
--- a/tools/wptrunner/wptrunner/tests/test_products.py
+++ b/tools/wptrunner/wptrunner/tests/test_products.py
@@ -8,8 +8,11 @@
from .base import all_products, active_products
from .. import environment
from .. import products
+from .. import wptcommandline
-test_paths = {"/": {"tests_path": join(dirname(__file__), "..", "..", "..", "..")}} # repo root
+wpt_root = join(dirname(__file__), "..", "..", "..", "..")
+
+test_paths = {"/": wptcommandline.TestRoot(wpt_root, wpt_root)}
environment.do_delayed_imports(None, test_paths)
diff --git a/tools/wptrunner/wptrunner/tests/test_update.py b/tools/wptrunner/wptrunner/tests/test_update.py
index e569db9..59aaaea 100644
--- a/tools/wptrunner/wptrunner/tests/test_update.py
+++ b/tools/wptrunner/wptrunner/tests/test_update.py
@@ -8,7 +8,7 @@
import pytest
-from .. import metadata, manifestupdate, wptmanifest
+from .. import metadata, manifestupdate, wptcommandline, wptmanifest
from ..update.update import WPTUpdate
from ..update.base import StepRunner, Step
from mozlog import structuredlog, handlers, formatters
@@ -1771,13 +1771,14 @@
def test_update_pickle():
logger = structuredlog.StructuredLogger("expected_test")
+ wpt_root = os.path.abspath(os.path.join(here,
+ os.pardir,
+ os.pardir,
+ os.pardir,
+ os.pardir))
args = {
"test_paths": {
- "/": {"tests_path": os.path.abspath(os.path.join(here,
- os.pardir,
- os.pardir,
- os.pardir,
- os.pardir))},
+ "/": wptcommandline.TestRoot(wpt_root, wpt_root),
},
"abort": False,
"continue": False,
diff --git a/tools/wptrunner/wptrunner/update/update.py b/tools/wptrunner/wptrunner/update/update.py
index 1e9be41..d519c20 100644
--- a/tools/wptrunner/wptrunner/update/update.py
+++ b/tools/wptrunner/wptrunner/update/update.py
@@ -25,8 +25,8 @@
"path": state.kwargs["sync_path"]}
state.paths = state.kwargs["test_paths"]
- state.tests_path = state.paths["/"]["tests_path"]
- state.metadata_path = state.paths["/"]["metadata_path"]
+ state.tests_path = state.paths["/"].tests_path
+ state.metadata_path = state.paths["/"].metadata_path
assert os.path.isabs(state.tests_path)
@@ -108,12 +108,12 @@
return
paths = state.kwargs["test_paths"]
- state.tests_path = state.paths["/"]["tests_path"]
- state.metadata_path = state.paths["/"]["metadata_path"]
+ state.tests_path = state.paths["/"].tests_path
+ state.metadata_path = state.paths["/"].metadata_path
for url_paths in paths.values():
- tests_path = url_paths["tests_path"]
- metadata_path = url_paths["metadata_path"]
+ tests_path = url_paths.tests_path
+ metadata_path = url_paths.metadata_path
for dirpath, dirnames, filenames in os.walk(metadata_path):
for filename in filenames:
if filename == "__dir__.ini":
@@ -144,7 +144,7 @@
:param kwargs: Command line arguments
"""
self.runner_cls = runner_cls
- self.serve_root = kwargs["test_paths"]["/"]["tests_path"]
+ self.serve_root = kwargs["test_paths"]["/"].tests_path
if not kwargs["sync"]:
setup_paths(self.serve_root)
diff --git a/tools/wptrunner/wptrunner/wptcommandline.py b/tools/wptrunner/wptrunner/wptcommandline.py
index fb0dca7..87f51d6 100644
--- a/tools/wptrunner/wptrunner/wptcommandline.py
+++ b/tools/wptrunner/wptrunner/wptcommandline.py
@@ -6,13 +6,7 @@
from collections import OrderedDict
from shutil import which
from datetime import timedelta
-from typing import cast, Mapping
-
-try:
- from typing import TypedDict
-except ImportError:
- # For Python 3.7 we don't have TypedDict do alias it to a generic Mapping
- TypedDict = Mapping
+from typing import Mapping, Optional
from . import config
from . import products
@@ -491,39 +485,34 @@
new_value = kwargs["config"].get(section, config.ConfigDict({})).get_path(config_value)
kwargs[kw_value] = new_value
- kwargs["test_paths"] = get_test_paths(kwargs["config"])
-
- if kwargs["tests_root"]:
- if "/" not in kwargs["test_paths"]:
- kwargs["test_paths"]["/"] = {}
- kwargs["test_paths"]["/"]["tests_path"] = kwargs["tests_root"]
-
- if kwargs["metadata_root"]:
- if "/" not in kwargs["test_paths"]:
- kwargs["test_paths"]["/"] = {}
- kwargs["test_paths"]["/"]["metadata_path"] = kwargs["metadata_root"]
-
- if kwargs.get("manifest_path"):
- if "/" not in kwargs["test_paths"]:
- kwargs["test_paths"]["/"] = {}
- kwargs["test_paths"]["/"]["manifest_path"] = kwargs["manifest_path"]
+ test_paths = get_test_paths(kwargs["config"],
+ kwargs["tests_root"],
+ kwargs["metadata_root"],
+ kwargs["manifest_path"])
+ check_paths(test_paths)
+ kwargs["test_paths"] = test_paths
kwargs["suite_name"] = kwargs["config"].get("web-platform-tests", {}).get("name", "web-platform-tests")
- check_paths(kwargs)
+class TestRoot:
+ def __init__(self, tests_path: str, metadata_path: str, manifest_path: Optional[str] = None):
+ self.tests_path = tests_path
+ self.metadata_path = metadata_path
+ if manifest_path is None:
+ manifest_path = os.path.join(metadata_path, "MANIFEST.json")
-class TestRoot(TypedDict):
- tests_path: str
- metadata_path: str
- manifest_path: str
+ self.manifest_path = manifest_path
TestPaths = Mapping[str, TestRoot]
-def get_test_paths(config: Mapping[str, config.ConfigDict]) -> TestPaths:
+def get_test_paths(config: Mapping[str, config.ConfigDict],
+ tests_path_override: Optional[str] = None,
+ metadata_path_override: Optional[str] = None,
+ manifest_path_override: Optional[str] = None) -> TestPaths:
# Set up test_paths
test_paths = OrderedDict()
@@ -531,35 +520,47 @@
if section.startswith("manifest:"):
manifest_opts = config[section]
url_base = manifest_opts.get("url_base", "/")
- test_root = {
- "tests_path": manifest_opts.get_path("tests"),
- "metadata_path": manifest_opts.get_path("metadata"),
- }
- if "manifest" in manifest_opts:
- test_root["manifest_path"] = manifest_opts.get_path("manifest")
- test_paths[url_base] = cast(TestRoot, test_root)
+ tests_path = manifest_opts.get_path("tests")
+ if tests_path is None:
+ raise ValueError(f"Missing `tests` key in configuration with url_base {url_base}")
+ metadata_path = manifest_opts.get_path("metadata")
+ if metadata_path is None:
+ raise ValueError(f"Missing `metadata` key in configuration with url_base {url_base}")
+ manifest_path = manifest_opts.get_path("manifest")
+
+ if url_base == "/":
+ if tests_path_override is not None:
+ tests_path = tests_path_override
+ if metadata_path_override is not None:
+ metadata_path = metadata_path_override
+ if manifest_path_override is not None:
+ manifest_path = manifest_path_override
+
+ test_paths[url_base] = TestRoot(tests_path, metadata_path, manifest_path)
+
+ if "/" not in test_paths:
+ if tests_path_override is None or metadata_path_override is None:
+ raise ValueError("No ini file configures the root url, "
+ "so --tests and --metadata arguments are mandatory")
+ test_paths["/"] = TestRoot(tests_path_override,
+ metadata_path_override,
+ manifest_path_override)
return test_paths
-def exe_path(name):
+def exe_path(name: Optional[str]) -> Optional[str]:
if name is None:
- return
+ return None
return which(name)
-def check_paths(kwargs):
- for test_paths in kwargs["test_paths"].values():
- if not ("tests_path" in test_paths and
- "metadata_path" in test_paths):
- print("Fatal: must specify both a test path and metadata path")
- sys.exit(1)
- if "manifest_path" not in test_paths:
- test_paths["manifest_path"] = os.path.join(test_paths["metadata_path"],
- "MANIFEST.json")
- for key, path in test_paths.items():
+def check_paths(test_paths: TestPaths) -> None:
+ for test_root in test_paths.values():
+ for key in ["tests_path", "metadata_path", "manifest_path"]:
name = key.split("_", 1)[0]
+ path = getattr(test_root, key)
if name == "manifest":
# For the manifest we can create it later, so just check the path
@@ -712,7 +713,7 @@
sys.exit(1)
if kwargs["properties_file"] is None and not kwargs["no_properties_file"]:
- default_file = os.path.join(kwargs["test_paths"]["/"]["metadata_path"],
+ default_file = os.path.join(kwargs["test_paths"]["/"].metadata_path,
"update_properties.json")
if os.path.exists(default_file):
kwargs["properties_file"] = default_file
diff --git a/tools/wptrunner/wptrunner/wptrunner.py b/tools/wptrunner/wptrunner/wptrunner.py
index 0658b58..a4ad268 100644
--- a/tools/wptrunner/wptrunner/wptrunner.py
+++ b/tools/wptrunner/wptrunner/wptrunner.py
@@ -390,7 +390,7 @@
env_extras.append(FontInstaller(
logger,
font_dir=kwargs["font_dir"],
- ahem=os.path.join(test_paths["/"]["tests_path"], "fonts/Ahem.ttf")
+ ahem=os.path.join(test_paths["/"].tests_path, "fonts/Ahem.ttf")
))
recording.set(["startup", "load_tests"])
