Chromedriver fixes
- Fix chromedriver binary extraction (LICENSE.chromedriver was
wrongly detected as binary)
- Use absolute browser / driver paths
Drive-by-fix:
- Log error if there are more than 50 existing results
Change-Id: I2d10615465f0b8c2a9783cd0f2563ec812c2a0d1
Reviewed-on: https://chromium-review.googlesource.com/c/crossbench/+/4203147
Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
diff --git a/crossbench/browsers/base.py b/crossbench/browsers/base.py
index 64e9de7..814bcba 100644
--- a/crossbench/browsers/base.py
+++ b/crossbench/browsers/base.py
@@ -56,6 +56,7 @@
self.major_version: int = 0
if path:
self.path = self._resolve_binary(path)
+ assert self.path.is_absolute()
self.version = self._extract_version()
self.major_version = int(self.version.split(".")[0])
self.unique_name = f"{self.type}_v{self.major_version}_{self.label}"
@@ -114,6 +115,7 @@
return self.log_file.with_suffix(".stdout.log")
def _resolve_binary(self, path: pathlib.Path) -> pathlib.Path:
+ path = path.absolute()
assert path.exists(), f"Binary at path={path} does not exist."
self.app_path = path
self.app_name = self.app_path.stem
diff --git a/crossbench/browsers/chromium.py b/crossbench/browsers/chromium.py
index 56fcb1b..23d51c6 100644
--- a/crossbench/browsers/chromium.py
+++ b/crossbench/browsers/chromium.py
@@ -396,10 +396,16 @@
zip_ref.extractall(zip_file.parent)
zip_file.unlink()
maybe_driver = None
- maybe_drivers: List[pathlib.Path] = [
+ candidates: List[pathlib.Path] = [
path for path in zip_file.parent.glob("**/*")
if path.is_file() and "chromedriver" in path.name
]
+ # Find exact match first:
+ maybe_drivers: List[pathlib.Path] = [
+ path for path in candidates if path.stem == "chromedriver"
+ ]
+ # Backup less strict matching:
+ maybe_drivers += candidates
if len(maybe_drivers) > 0:
maybe_driver = maybe_drivers[0]
assert maybe_driver and maybe_driver.is_file(), (
diff --git a/crossbench/browsers/webdriver.py b/crossbench/browsers/webdriver.py
index 82bbf7e..517a3c4 100644
--- a/crossbench/browsers/webdriver.py
+++ b/crossbench/browsers/webdriver.py
@@ -35,7 +35,7 @@
return log_file.with_suffix(".driver.log")
def setup_binary(self, runner: Runner) -> None:
- self._driver_path = self._find_driver()
+ self._driver_path = self._find_driver().absolute()
assert self._driver_path.exists(), (
f"Webdriver path '{self._driver_path}' does not exist")
@@ -50,6 +50,7 @@
def start(self, run: Run) -> None:
assert not self._is_running
assert self._driver_path
+ assert self._driver_path.is_absolute()
self._check_driver_version()
self._driver = self._start_driver(run, self._driver_path)
if hasattr(self._driver, "service"):
diff --git a/crossbench/env.py b/crossbench/env.py
index 4fb5f1c..b87e443 100644
--- a/crossbench/env.py
+++ b/crossbench/env.py
@@ -402,10 +402,14 @@
return
results = [path for path in results_dir.iterdir() if path.is_dir()]
num_results = len(results)
- if num_results > 20:
- logging.warning(
- "Found %d existing crossbench results. "
- "Consider cleaning stale results in '%s'", num_results, results_dir)
+ if num_results < 20:
+ return
+ message = (f"Found {num_results} existing crossbench results. "
+ f"Consider cleaning stale results in '{results_dir}'")
+ if num_results > 50:
+ logging.error(message)
+ else:
+ logging.warning(message)
def setup(self) -> None:
self.validate()
diff --git a/tests/test_env.py b/tests/test_env.py
index bbf6b71..efb8963 100644
--- a/tests/test_env.py
+++ b/tests/test_env.py
@@ -290,12 +290,21 @@
env.validate()
cm.assert_not_called()
+ def test_results_dir_many(self):
+ # Create fake test result dirs:
+ for i in range(30):
+ (self.out_dir.parent / str(i)).mkdir()
+ env = cb.env.HostEnvironment(self.mock_runner)
+ with mock.patch("logging.warning") as cm:
+ env.validate()
+ cm.assert_called_once()
+
def test_results_dir_too_many(self):
# Create fake test result dirs:
for i in range(100):
(self.out_dir.parent / str(i)).mkdir()
env = cb.env.HostEnvironment(self.mock_runner)
- with mock.patch("logging.warning") as cm:
+ with mock.patch("logging.error") as cm:
env.validate()
cm.assert_called_once()
