[wptrunner] Decouple testdriver infrastructure from testharness

tools/wptrunner/wptrunner/environment.py

@@ -23,8 +23,6 @@
 sys.path.insert(0, repo_root)
 from tools import localpaths  # noqa: F401
 
-from wptserve.handlers import StringHandler
-
 serve = None
 
 
@@ -226,29 +224,47 @@ def get_routes(self):
             self.config.aliases,
             self.config)
 
+        testharnessreport_format_args = {
+            "output": self.pause_after_test,
+            "timeout_multiplier": self.testharness_timeout_multipler,
+            "explicit_timeout": "true" if self.debug_info is not None else "false",
+            "debug": "true" if self.debug_test else "false",
+        }
         for path, format_args, content_type, route in [
                 ("testharness_runner.html", {}, "text/html", "/testharness_runner.html"),
                 ("print_pdf_runner.html", {}, "text/html", "/print_pdf_runner.html"),
-                (os.path.join(here, "..", "..", "third_party", "pdf_js", "pdf.js"), None,
+                (os.path.join(here, "..", "..", "third_party", "pdf_js", "pdf.js"), {},
                  "text/javascript", "/_pdf_js/pdf.js"),
-                (os.path.join(here, "..", "..", "third_party", "pdf_js", "pdf.worker.js"), None,
+                (os.path.join(here, "..", "..", "third_party", "pdf_js", "pdf.worker.js"), {},
                  "text/javascript", "/_pdf_js/pdf.worker.js"),
-                (self.options.get("testharnessreport", "testharnessreport.js"),
-                 {"output": self.pause_after_test,
-                  "timeout_multiplier": self.testharness_timeout_multipler,
-                  "explicit_timeout": "true" if self.debug_info is not None else "false",
-                  "debug": "true" if self.debug_test else "false"},
-                 "text/javascript;charset=utf8",
-                 "/resources/testharnessreport.js")]:
-            path = os.path.normpath(os.path.join(here, path))
+                (
+                    self.options.get("testharnessreport", [
+                        # All testharness tests, even those that don't use testdriver, require
+                        # `message-queue.js` to signal completion.
+                        os.path.join("executors", "message-queue.js"),
+                        "testharnessreport.js"]),
+                    testharnessreport_format_args,
+                    "text/javascript;charset=utf8",
+                    "/resources/testharnessreport.js",
+                ),
+                (
+                    [os.path.join(repo_root, "resources", "testdriver.js"),
+                     # Include `message-queue.js` to support testdriver in non-testharness tests.
+                     os.path.join("executors", "message-queue.js"),
+                     "testdriver-extra.js"],
+                    {},
+                    "text/javascript",
+                    "/resources/testdriver.js",
+                ),
+        ]:
+            paths = [path] if isinstance(path, str) else path
+            abs_paths = [os.path.normpath(os.path.join(here, path)) for path in paths]
             # Note that .headers. files don't apply to static routes, so we need to
             # readd any static headers here.
             headers = {"Cache-Control": "max-age=3600"}
-            route_builder.add_static(path, format_args, content_type, route,
+            route_builder.add_static(abs_paths, format_args, content_type, route,
                                      headers=headers)
 
-        route_builder.add_handler("GET", "/resources/testdriver.js", TestdriverLoader())
-
         for url_base, test_root in self.test_paths.items():
             if url_base == "/":
                 continue
@@ -316,27 +332,6 @@ def test_servers(self):
         return failed, pending
 
 
-class TestdriverLoader:
-    """A special static handler for serving `/resources/testdriver.js`.
-
-    This handler lazily reads `testdriver{,-extra}.js` so that wptrunner doesn't
-    need to pass the entire file contents to child `wptserve` processes, which
-    can slow `wptserve` startup by several seconds (crbug.com/1479850).
-    """
-    def __init__(self):
-        self._handler = None
-
-    def __call__(self, request, response):
-        if not self._handler:
-            data = b""
-            with open(os.path.join(repo_root, "resources", "testdriver.js"), "rb") as fp:
-                data += fp.read()
-            with open(os.path.join(here, "testdriver-extra.js"), "rb") as fp:
-                data += fp.read()
-            self._handler = StringHandler(data, "text/javascript")
-        return self._handler(request, response)
-
-
 def wait_for_service(logger: StructuredLogger,
                      host: str,
                      port: int,

tools/wptrunner/wptrunner/executors/executorchrome.py

@@ -111,10 +111,10 @@ def send_message(self, cmd_id, message_type, status, message=None):
         else:
             self.webdriver.execute_script(message_script)
 
-    def _get_next_message_classic(self, url):
+    def _get_next_message_classic(self, url, script_resume):
         try:
             message_script, self._pending_message = self._pending_message, ""
-            return self.parent.base.execute_script(message_script + self.script_resume,
+            return self.parent.base.execute_script(message_script + script_resume,
                                                    asynchronous=True,
                                                    args=[strip_server(url)])
         except error.JavascriptErrorException as js_error:

tools/wptrunner/wptrunner/executors/executorwebdriver.py

@@ -437,24 +437,22 @@ def release(self):
 class WebDriverTestDriverProtocolPart(TestDriverProtocolPart):
     def setup(self):
         self.webdriver = self.parent.webdriver
-        with open(os.path.join(here, "testharness_webdriver_resume.js")) as f:
-            self.script_resume = f.read()
 
-    def get_next_message(self, url, test_window):
+    def get_next_message(self, url, script_resume, test_window):
         if hasattr(self.parent, "bidi_script"):
             # If `bidi_script` is available, the messages can be handled via BiDi.
-            return self._get_next_message_bidi(url, test_window)
+            return self._get_next_message_bidi(url, script_resume, test_window)
         else:
-            return self._get_next_message_classic(url)
+            return self._get_next_message_classic(url, script_resume)
 
-    def _get_next_message_classic(self, url):
+    def _get_next_message_classic(self, url, script_resume):
         """
         Get the next message from the test_driver using the classic WebDriver async script execution. This will block
         the event loop until the test_driver send a message.
         """
-        return self.parent.base.execute_script(self.script_resume, asynchronous=True, args=[strip_server(url)])
+        return self.parent.base.execute_script(script_resume, asynchronous=True, args=[strip_server(url)])
 
-    def _get_next_message_bidi(self, url, test_window):
+    def _get_next_message_bidi(self, url, script_resume, test_window):
         """
         Get the next message from the test_driver using async call. This will not block the event loop, which allows for
         processing the events from the test_runner to test_driver while waiting for the next test_driver commands.
@@ -470,7 +468,7 @@ def _get_next_message_bidi(self, url, test_window):
                                 %s
                             }).apply(null, args);
                         })
-                    }""" % self.script_resume
+                    }""" % script_resume
 
         bidi_url_argument = {
             "type": "string",
@@ -837,64 +835,19 @@ def run_func(self):
             self.result_flag.set()
 
 
-class WebDriverTestharnessExecutor(TestharnessExecutor):
-    supports_testdriver = True
-    protocol_cls = WebDriverProtocol
-
-    def __init__(self, logger, browser, server_config, timeout_multiplier=1,
-                 close_after_done=True, capabilities=None, debug_info=None,
-                 cleanup_after_test=True, **kwargs):
-        """WebDriver-based executor for testharness.js tests"""
-        TestharnessExecutor.__init__(self, logger, browser, server_config,
-                                     timeout_multiplier=timeout_multiplier,
-                                     debug_info=debug_info)
-        self.protocol = self.protocol_cls(self, browser, capabilities)
-        with open(os.path.join(here, "window-loaded.js")) as f:
-            self.window_loaded_script = f.read()
-
-
-        self.close_after_done = close_after_done
-        self.cleanup_after_test = cleanup_after_test
-
-    def is_alive(self):
-        return self.protocol.is_alive()
-
-    def on_environment_change(self, new_environment):
-        if new_environment["protocol"] != self.last_environment["protocol"]:
-            self.protocol.testharness.load_runner(new_environment["protocol"])
-
-    def do_test(self, test):
-        url = self.test_url(test)
-
-        success, data = WebDriverRun(self.logger,
-                                     self.do_testharness,
-                                     self.protocol,
-                                     url,
-                                     test.timeout * self.timeout_multiplier,
-                                     self.extra_timeout).run()
-
-        if success:
-            data, extra = data
-            return self.convert_result(test, data, extra=extra)
-
-        return (test.make_result(*data), [])
-
-    def do_testharness(self, protocol, url, timeout):
-        # The previous test may not have closed its old windows (if something
-        # went wrong or if cleanup_after_test was False), so clean up here.
-        protocol.testharness.close_old_windows()
+# TODO(web-platform-tests/wpt#13183): Add testdriver support to the other
+# executors.
+class TestDriverExecutorMixin:
+    def __init__(self, script_resume: str):
+        self.script_resume = script_resume
 
+    def run_testdriver(self, protocol, url, timeout):
         # If protocol implements `bidi_events`, remove all the existing subscriptions.
         if hasattr(protocol, 'bidi_events'):
             # Use protocol loop to run the async cleanup.
             protocol.loop.run_until_complete(protocol.bidi_events.unsubscribe_all())
 
-        # Now start the test harness
         test_window = self.get_or_create_test_window(protocol)
-        self.protocol.base.set_window(test_window)
-        # Wait until about:blank has been loaded
-        protocol.base.execute_script(self.window_loaded_script, asynchronous=True)
-
         # Exceptions occurred outside the main loop.
         unexpected_exceptions = []
 
@@ -948,7 +901,8 @@ async def process_bidi_event(method, params):
                 # TODO: what to do if there are more then 1 unexpected exceptions?
                 raise unexpected_exceptions[0]
 
-            test_driver_message = protocol.testdriver.get_next_message(url, test_window)
+            test_driver_message = protocol.testdriver.get_next_message(url, self.script_resume,
+                                                                       test_window)
             self.logger.debug("Receive message from testdriver: %s" % test_driver_message)
 
             # As of 2019-03-29, WebDriver does not define expected behavior for
@@ -981,36 +935,100 @@ async def process_bidi_event(method, params):
             # Use protocol loop to run the async cleanup.
             protocol.loop.run_until_complete(protocol.bidi_events.unsubscribe_all())
 
-        extra = {}
-        if leak_part := getattr(protocol, "leak", None):
-            testharness_window = protocol.base.current_window
-            extra_windows = set(protocol.base.window_handles())
-            extra_windows -= {protocol.testharness.runner_handle, testharness_window}
-            protocol.testharness.close_windows(extra_windows)
-            try:
-                protocol.base.set_window(testharness_window)
-                if counters := leak_part.check():
-                    extra["leak_counters"] = counters
-            except webdriver_error.NoSuchWindowException:
-                pass
-            finally:
-                protocol.base.set_window(protocol.testharness.runner_handle)
-
-        # Attempt to clean up any leftover windows, if allowed. This is
-        # preferable as it will blame the correct test if something goes wrong
-        # closing windows, but if the user wants to see the test results we
-        # have to leave the window(s) open.
-        if self.cleanup_after_test:
-            protocol.testharness.close_old_windows()
-
         if len(unexpected_exceptions) > 0:
             # TODO: what to do if there are more then 1 unexpected exceptions?
             raise unexpected_exceptions[0]
 
-        return rv, extra
+        return rv
+
+    def get_or_create_test_window(self, protocol):
+        return protocol.base.current_window
+
+
+class WebDriverTestharnessExecutor(TestharnessExecutor, TestDriverExecutorMixin):
+    supports_testdriver = True
+    protocol_cls = WebDriverProtocol
+
+    def __init__(self, logger, browser, server_config, timeout_multiplier=1,
+                 close_after_done=True, capabilities=None, debug_info=None,
+                 cleanup_after_test=True, **kwargs):
+        """WebDriver-based executor for testharness.js tests"""
+        TestharnessExecutor.__init__(self, logger, browser, server_config,
+                                     timeout_multiplier=timeout_multiplier,
+                                     debug_info=debug_info)
+        self.protocol = self.protocol_cls(self, browser, capabilities)
+        with open(os.path.join(here, "testharness_webdriver_resume.js")) as f:
+            script_resume = f.read()
+        TestDriverExecutorMixin.__init__(self, script_resume)
+        with open(os.path.join(here, "window-loaded.js")) as f:
+            self.window_loaded_script = f.read()
+
+        self.close_after_done = close_after_done
+        self.cleanup_after_test = cleanup_after_test
+
+    def is_alive(self):
+        return self.protocol.is_alive()
+
+    def on_environment_change(self, new_environment):
+        if new_environment["protocol"] != self.last_environment["protocol"]:
+            self.protocol.testharness.load_runner(new_environment["protocol"])
+
+    def do_test(self, test):
+        url = self.test_url(test)
+
+        success, data = WebDriverRun(self.logger,
+                                     self.do_testharness,
+                                     self.protocol,
+                                     url,
+                                     test.timeout * self.timeout_multiplier,
+                                     self.extra_timeout).run()
+
+        if success:
+            data, extra = data
+            return self.convert_result(test, data, extra=extra)
+
+        return (test.make_result(*data), [])
+
+    def do_testharness(self, protocol, url, timeout):
+        try:
+            # The previous test may not have closed its old windows (if something
+            # went wrong or if cleanup_after_test was False), so clean up here.
+            protocol.testharness.close_old_windows()
+            raw_results = self.run_testdriver(protocol, url, timeout)
+            extra = {}
+            if counters := self._check_for_leaks(protocol):
+                extra["leak_counters"] = counters
+            return raw_results, extra
+        finally:
+            # Attempt to clean up any leftover windows, if allowed. This is
+            # preferable as it will blame the correct test if something goes
+            # wrong closing windows, but if the user wants to see the test
+            # results we have to leave the window(s) open.
+            if self.cleanup_after_test:
+                protocol.testharness.close_old_windows()
+
+    def _check_for_leaks(self, protocol):
+        leak_part = getattr(protocol, "leak", None)
+        if not leak_part:
+            return None
+        testharness_window = protocol.base.current_window
+        extra_windows = set(protocol.base.window_handles())
+        extra_windows -= {protocol.testharness.runner_handle, testharness_window}
+        protocol.testharness.close_windows(extra_windows)
+        try:
+            protocol.base.set_window(testharness_window)
+            return leak_part.check()
+        except webdriver_error.NoSuchWindowException:
+            return None
+        finally:
+            protocol.base.set_window(protocol.testharness.runner_handle)
 
     def get_or_create_test_window(self, protocol):
-        return protocol.base.create_window()
+        test_window = protocol.base.create_window()
+        protocol.base.set_window(test_window)
+        # Wait until about:blank has been loaded
+        protocol.base.execute_script(self.window_loaded_script, asynchronous=True)
+        return test_window
 
 
 class WebDriverRefTestExecutor(RefTestExecutor):