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()