[wptrunner] Force stopping the browser when wdspec test hits external timeout.
Web-platform tests use a timeout per test module while pytest
supports a timeout for test methods. As long as a file wide
timeout isn't supported wptrunner has to force kill the browser
when a wdspec test runs into an external timeout.
Differential Revision: https://phabricator.services.mozilla.com/D139714
bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1754712
gecko-commit: 21dda7d9394c8678424160933c1a487661d59bd6
gecko-reviewers: jgraham
diff --git a/tools/wptrunner/wptrunner/browsers/firefox.py b/tools/wptrunner/wptrunner/browsers/firefox.py
index 0c103bd..afc7b58 100644
--- a/tools/wptrunner/wptrunner/browsers/firefox.py
+++ b/tools/wptrunner/wptrunner/browsers/firefox.py
@@ -883,12 +883,13 @@
super().start(group_metadata, **kwargs)
def stop(self, force=False):
- # Initially wait for any WebDriver session to cleanly shutdown
- # When this is called the executor is usually sending a end session
+ # Initially wait for any WebDriver session to cleanly shutdown if the
+ # process doesn't have to be force stopped.
+ # When this is called the executor is usually sending an end session
# command to the browser. We don't have a synchronisation mechanism
# that allows us to know that process is ongoing, so poll the status
# endpoint until there isn't a session, before killing the driver.
- if self.is_alive():
+ if self.is_alive() and not force:
end_time = time.time() + BrowserInstance.shutdown_timeout
while time.time() < end_time:
self.logger.debug("Waiting for WebDriver session to end")
diff --git a/tools/wptrunner/wptrunner/testrunner.py b/tools/wptrunner/wptrunner/testrunner.py
index ca214b7..0c75683 100644
--- a/tools/wptrunner/wptrunner/testrunner.py
+++ b/tools/wptrunner/wptrunner/testrunner.py
@@ -247,9 +247,9 @@
initializing = namedtuple("initializing",
["test", "test_group", "group_metadata", "failure_count"])
running = namedtuple("running", ["test", "test_group", "group_metadata"])
- restarting = namedtuple("restarting", ["test", "test_group", "group_metadata"])
+ restarting = namedtuple("restarting", ["test", "test_group", "group_metadata", "force_stop"])
error = namedtuple("error", [])
- stop = namedtuple("stop", [])
+ stop = namedtuple("stop", ["force_stop"])
RunnerManagerState = _RunnerManagerState()
@@ -351,7 +351,7 @@
RunnerManagerState.before_init: self.start_init,
RunnerManagerState.initializing: self.init,
RunnerManagerState.running: self.run_test,
- RunnerManagerState.restarting: self.restart_runner
+ RunnerManagerState.restarting: self.restart_runner,
}
self.state = RunnerManagerState.before_init()
@@ -386,8 +386,8 @@
raise
finally:
self.logger.debug("TestRunnerManager main loop terminating, starting cleanup")
- clean = isinstance(self.state, RunnerManagerState.stop)
- self.stop_runner(force=not clean)
+ force_stop = not isinstance(self.state, RunnerManagerState.stop) or self.state.force_stop
+ self.stop_runner(force=force_stop)
self.teardown()
self.logger.debug("TestRunnerManager main loop terminated")
@@ -418,12 +418,15 @@
self.logger.debug("Got command: %r" % command)
except IOError:
self.logger.error("Got IOError from poll")
- return RunnerManagerState.restarting(0)
+ return RunnerManagerState.restarting(self.state.test,
+ self.state.test_group,
+ self.state.group_metadata,
+ False)
except Empty:
if (self.debug_info and self.debug_info.interactive and
self.browser.started and not self.browser.is_alive()):
self.logger.debug("Debugger exited")
- return RunnerManagerState.stop()
+ return RunnerManagerState.stop(False)
if (isinstance(self.state, RunnerManagerState.running) and
not self.test_runner_proc.is_alive()):
@@ -443,7 +446,10 @@
self.logger.critical("Last test did not complete")
return RunnerManagerState.error()
self.logger.warning("More tests found, but runner process died, restarting")
- return RunnerManagerState.restarting(0)
+ return RunnerManagerState.restarting(self.state.test,
+ self.state.test_group,
+ self.state.group_metadata,
+ False)
else:
f = (dispatch.get(self.state.__class__, {}).get(command) or
dispatch.get(None, {}).get(command))
@@ -460,7 +466,7 @@
test, test_group, group_metadata = self.get_next_test()
self.recording.set(["testrunner", "init"])
if test is None:
- return RunnerManagerState.stop()
+ return RunnerManagerState.stop(True)
else:
return RunnerManagerState.initializing(test, test_group, group_metadata, 0)
@@ -551,7 +557,8 @@
self.logger.info("Restarting browser for new test environment")
return RunnerManagerState.restarting(self.state.test,
self.state.test_group,
- self.state.group_metadata)
+ self.state.group_metadata,
+ False)
self.recording.set(["testrunner", "test"] + self.state.test.id.split("/")[1:])
self.logger.test_start(self.state.test.id)
@@ -685,6 +692,7 @@
file_result.status in ("CRASH", "EXTERNAL-TIMEOUT", "INTERNAL-ERROR") or
((subtest_unexpected or is_unexpected) and
self.restart_on_unexpected))
+ force_stop = test.test_type == "wdspec" and file_result.status == "EXTERNAL-TIMEOUT"
self.recording.set(["testrunner", "after-test"])
if (not file_result.status == "CRASH" and
@@ -693,7 +701,7 @@
self.logger.info("Pausing until the browser exits")
self.send_message("wait")
else:
- return self.after_test_end(test, restart_before_next)
+ return self.after_test_end(test, restart_before_next, force_stop=force_stop)
def wait_finished(self, rerun=False):
assert isinstance(self.state, RunnerManagerState.running)
@@ -703,7 +711,7 @@
# post-stop processing
return self.after_test_end(self.state.test, not rerun, force_rerun=rerun)
- def after_test_end(self, test, restart, force_rerun=False):
+ def after_test_end(self, test, restart, force_rerun=False, force_stop=False):
assert isinstance(self.state, RunnerManagerState.running)
# Mixing manual reruns and automatic reruns is confusing; we currently assume
# that as long as we've done at least the automatic run count in total we can
@@ -711,7 +719,7 @@
if not force_rerun and self.run_count >= self.rerun:
test, test_group, group_metadata = self.get_next_test()
if test is None:
- return RunnerManagerState.stop()
+ return RunnerManagerState.stop(force_stop)
if test_group is not self.state.test_group:
# We are starting a new group of tests, so force a restart
self.logger.info("Restarting browser for new test group")
@@ -720,14 +728,14 @@
test_group = self.state.test_group
group_metadata = self.state.group_metadata
if restart:
- return RunnerManagerState.restarting(test, test_group, group_metadata)
+ return RunnerManagerState.restarting(test, test_group, group_metadata, force_stop)
else:
return RunnerManagerState.running(test, test_group, group_metadata)
def restart_runner(self):
"""Stop and restart the TestRunner"""
assert isinstance(self.state, RunnerManagerState.restarting)
- self.stop_runner()
+ self.stop_runner(force=self.state.force_stop)
return RunnerManagerState.initializing(self.state.test, self.state.test_group, self.state.group_metadata, 0)
def log(self, data):
@@ -794,7 +802,7 @@
def runner_teardown(self):
self.ensure_runner_stopped()
- return RunnerManagerState.stop()
+ return RunnerManagerState.stop(False)
def send_message(self, command, *args):
"""Send a message to the remote queue (to Executor)."""
