Skip to content

Commit 3016b20

Browse files
committed
WebDriverBrowser.stop() should only check the WebDriver server
Previously, we'd check any overridden `is_alive` method; thus we might not stop the WebDriver server process if any part of the browser was not still alive. For example, if the SafariBrowser class had: ``` def is_alive(self): return ( process_is_alive("Safari") and process_is_alive("webinspectord") and super().is_alive() ) def stop(self, force=False): terminate_process("Safari") super().stop(force=force) ``` Then we would never terminate `safaridriver` in the superclass, because we would've already have terminated Safari and thus the browser would report itself as no longer alive, which seems like the desired behaviour given it's no longer in a state to run tests.
1 parent cc793b8 commit 3016b20

File tree

1 file changed

+2
-2
lines changed
  • tools/wptrunner/wptrunner/browsers

1 file changed

+2
-2
lines changed

tools/wptrunner/wptrunner/browsers/base.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -389,7 +389,7 @@ def _run_server(self, group_metadata: GroupMetadata, **kwargs: Any) -> None:
389389
def stop(self, force: bool = False) -> bool:
390390
self.logger.debug("Stopping WebDriver")
391391
clean = True
392-
if self.is_alive():
392+
if WebDriverBrowser.is_alive(self):
393393
proc = cast(mozprocess.ProcessHandler, self._proc)
394394
# Pass a timeout value to mozprocess Processhandler.kill()
395395
# to ensure it always returns within it.
@@ -398,7 +398,7 @@ def stop(self, force: bool = False) -> bool:
398398
if force and kill_result != 0:
399399
clean = False
400400
proc.kill(9, timeout=5)
401-
success = not self.is_alive()
401+
success = not WebDriverBrowser.is_alive(self)
402402
if success and self._output_handler is not None:
403403
# Only try to do output post-processing if we managed to shut down
404404
self._output_handler.after_process_stop(clean)

0 commit comments

Comments
 (0)