diff --git a/tools/wptrunner/wptrunner/executors/executorwebdriver.py b/tools/wptrunner/wptrunner/executors/executorwebdriver.py index 994cdc2c6282ad..850d9fdfa10550 100644 --- a/tools/wptrunner/wptrunner/executors/executorwebdriver.py +++ b/tools/wptrunner/wptrunner/executors/executorwebdriver.py @@ -625,6 +625,8 @@ def connect(self): self.webdriver.start() def teardown(self): + if not self.webdriver: + return self.logger.debug("Hanging up on WebDriver session") try: self.webdriver.end() diff --git a/tools/wptrunner/wptrunner/testrunner.py b/tools/wptrunner/wptrunner/testrunner.py index c5b12ee1d93a72..0d982f85772fcf 100644 --- a/tools/wptrunner/wptrunner/testrunner.py +++ b/tools/wptrunner/wptrunner/testrunner.py @@ -110,7 +110,6 @@ def setup(self): def teardown(self): self.executor.teardown() - self.send_message("runner_teardown") self.result_queue = None self.command_queue = None self.browser = None @@ -140,12 +139,7 @@ def switch_executor(self, executor_implementation): def run(self): """Main loop accepting commands over the pipe and triggering the associated methods""" - try: - self.setup() - except Exception: - self.logger.warning("An error occured during executor setup:\n%s" % - traceback.format_exc()) - raise + self.setup() commands = {"run_test": self.run_test, "switch_executor": self.switch_executor, "reset": self.reset, @@ -531,9 +525,8 @@ def wait_event(self): RunnerManagerState.error: {}, RunnerManagerState.stop: {}, None: { - "runner_teardown": self.runner_teardown, "log": self.log, - "error": self.error + "error": self.error, } } try: @@ -553,6 +546,7 @@ def wait_event(self): self.logger.debug("Debugger exited") return RunnerManagerState.stop(False) + # `test_runner_proc` must be nonnull in the manager's `running` state. if (isinstance(self.state, RunnerManagerState.running) and not self.test_runner_proc.is_alive()): if not self.command_queue.empty(): @@ -956,34 +950,33 @@ def error(self, message): def stop_runner(self, force=False): """Stop the TestRunner and the browser binary.""" self.recording.set(["testrunner", "stop_runner"]) - if self.test_runner_proc is None: - return - - if self.test_runner_proc.is_alive(): - self.send_message("stop") try: - self.browser.stop(force=force) - self.ensure_runner_stopped() + # Stop the runner process before the browser process so that the + # former can gracefully tear down the protocol (e.g., closing an + # active WebDriver session). + self._ensure_runner_stopped() + # TODO(web-platform-tests/wpt#48030): Consider removing the + # `stop(force=...)` argument. + self.browser.stop(force=True) except (OSError, PermissionError): - self.logger.error("Failed to stop the runner") + self.logger.error("Failed to stop either the runner or the browser process", + exc_info=True) finally: self.cleanup() def teardown(self): self.logger.debug("TestRunnerManager teardown") - self.test_runner_proc = None self.command_queue.close() self.remote_queue.close() self.command_queue = None self.remote_queue = None self.recording.pause() - def ensure_runner_stopped(self): - self.logger.debug("ensure_runner_stopped") + def _ensure_runner_stopped(self): if self.test_runner_proc is None: return - - self.browser.stop(force=True) + self.logger.debug("Stopping runner process") + self.send_message("stop") self.test_runner_proc.join(10) mp = mpcontext.get_context() if self.test_runner_proc.is_alive(): @@ -1007,10 +1000,7 @@ def ensure_runner_stopped(self): self.remote_queue = mp.Queue() else: self.logger.debug("Runner process exited with code %i" % self.test_runner_proc.exitcode) - - def runner_teardown(self): - self.ensure_runner_stopped() - return RunnerManagerState.stop(False) + self.test_runner_proc = None def send_message(self, command, *args): """Send a message to the remote queue (to Executor).""" @@ -1034,11 +1024,6 @@ def cleanup(self): else: if cmd == "log": self.log(*data) - elif cmd == "runner_teardown": - # It's OK for the "runner_teardown" message to be left in - # the queue during cleanup, as we will already have tried - # to stop the TestRunner in `stop_runner`. - pass else: self.logger.warning(f"Command left in command_queue during cleanup: {cmd!r}, {data!r}") while True: