Skip to content

Commit

Permalink
Revert "Do not block waiting for chromedriver to start up (#46126)" (#…
Browse files Browse the repository at this point in the history
…46739)

We are seeing "connection refused" error on Safari and Chrome Mac and
Windows. Revert for now and we will see if we can have a better way to
do that.

This reverts commit f9487b0.
This reverts commit d6b7c6e.
  • Loading branch information
WeizhongX authored and pull[bot] committed Nov 7, 2024
1 parent c8625ce commit 1221529
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 29 deletions.
22 changes: 21 additions & 1 deletion tools/wptrunner/wptrunner/browsers/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,16 @@
import os
import platform
import socket
import time
import traceback
from abc import ABCMeta, abstractmethod
from typing import cast, Any, List, Mapping, Optional, Tuple, Type

import mozprocess
from mozdebug import DebuggerInfo
from mozlog.structuredlog import StructuredLogger

from ..environment import wait_for_service
from ..testloader import GroupMetadata
from ..wptcommandline import require_arg # noqa: F401
from ..wpttest import Test
Expand Down Expand Up @@ -316,6 +319,7 @@ def __init__(self,
self.env = os.environ.copy() if env is None else env
self.webdriver_args = webdriver_args if webdriver_args is not None else []

self.init_deadline: Optional[float] = None
self._output_handler: Optional[OutputHandler] = None
self._cmd = None
self._proc: Optional[mozprocess.ProcessHandler] = None
Expand All @@ -326,6 +330,7 @@ def make_command(self) -> List[str]:
return [self.webdriver_binary] + self.webdriver_args

def start(self, group_metadata: GroupMetadata, **kwargs: Any) -> None:
self.init_deadline = time.time() + self.init_timeout
try:
self._run_server(group_metadata, **kwargs)
except KeyboardInterrupt:
Expand All @@ -340,6 +345,7 @@ def create_output_handler(self, cmd: List[str]) -> OutputHandler:
return OutputHandler(self.logger, cmd)

def _run_server(self, group_metadata: GroupMetadata, **kwargs: Any) -> None:
assert self.init_deadline is not None
cmd = self.make_command()
self._output_handler = self.create_output_handler(cmd)

Expand All @@ -359,7 +365,21 @@ def _run_server(self, group_metadata: GroupMetadata, **kwargs: Any) -> None:
raise
self._output_handler.after_process_start(self._proc.pid)

self._output_handler.start(group_metadata=group_metadata, **kwargs)
try:
wait_for_service(
self.logger,
self.host,
self.port,
timeout=self.init_deadline - time.time(),
server_process=self._proc,
)
except Exception:
self.logger.error(
"WebDriver was not accessible "
f"within the timeout:\n{traceback.format_exc()}")
raise
finally:
self._output_handler.start(group_metadata=group_metadata, **kwargs)
self.logger.debug("_run complete")

def stop(self, force: bool = False) -> bool:
Expand Down
18 changes: 0 additions & 18 deletions tools/wptrunner/wptrunner/browsers/chrome_ios.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
# mypy: allow-untyped-defs

import traceback
from .base import WebDriverBrowser, require_arg
from .base import get_timeout_multiplier # noqa: F401
from ..environment import wait_for_service
from ..executors import executor_kwargs as base_executor_kwargs
from ..executors.base import WdspecExecutor # noqa: F401
from ..executors.executorwebdriver import (WebDriverCrashtestExecutor, # noqa: F401
Expand Down Expand Up @@ -60,19 +58,3 @@ class ChromeiOSBrowser(WebDriverBrowser):
def make_command(self):
return ([self.webdriver_binary, f"--port={self.port}"] +
self.webdriver_args)

def start(self, group_metadata, **kwargs):
super().start(group_metadata, **kwargs)
try:
wait_for_service(
self.logger,
self.host,
self.port,
timeout=self.init_timeout,
server_process=self._proc,
)
except Exception:
self.logger.error(
"WebDriver was not accessible "
f"within the timeout:\n{traceback.format_exc()}")
raise
2 changes: 0 additions & 2 deletions tools/wptrunner/wptrunner/browsers/firefox.py
Original file line number Diff line number Diff line change
Expand Up @@ -910,7 +910,6 @@ def __init__(self, logger, binary, package_name, prefs_root, webdriver_binary, w
self.binary = binary
self.package_name = package_name
self.webdriver_binary = webdriver_binary
self.init_deadline = None

self.stackfix_dir = stackfix_dir
self.symbols_path = symbols_path
Expand Down Expand Up @@ -959,7 +958,6 @@ def create_output_handler(self, cmd):

def start(self, group_metadata, **kwargs):
self.leak_report_file = setup_leak_report(self.leak_check, self.profile, self.env)
self.init_deadline = time.time() + self.init_timeout
super().start(group_metadata, **kwargs)

def stop(self, force=False):
Expand Down
22 changes: 14 additions & 8 deletions tools/wptrunner/wptrunner/tests/browsers/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import sys
from os.path import dirname, join
from unittest import mock

import pytest

Expand Down Expand Up @@ -29,14 +30,19 @@ def test_logging_immediate_exit():
handler = MozLogTestHandler()
logger.add_handler(handler)

browser = base.WebDriverBrowser(
logger, webdriver_binary="echo", webdriver_args=["sample output"]
)
try:
browser.start(group_metadata={})
finally:
# Ensure the `echo` process actually exits
browser._proc.wait()
class CustomException(Exception):
pass

with mock.patch.object(base, "wait_for_service", side_effect=CustomException):
browser = base.WebDriverBrowser(
logger, webdriver_binary="echo", webdriver_args=["sample output"]
)
try:
with pytest.raises(CustomException):
browser.start(group_metadata={})
finally:
# Ensure the `echo` process actually exits
browser._proc.wait()

process_output_actions = [
data for data in handler.items if data["action"] == "process_output"
Expand Down

0 comments on commit 1221529

Please sign in to comment.