[py]: Propagate stderr to exceptions when selenium manager fails (#11329)

* [py]: Capture `stderr` when selenium manager goes wrong and some tests

* [py]: Apply `linting`

* [py]: tidy up `selenium manager` unit tests

* [py]: Remove unused fixtures and improve naming of manager unit tests

* [py] Raise `SeleniumManagerException` when binary cannot be found on disk

* Delete py/selenium/webdriver/common/linux directory

* [py]: Handle exceptions better in `service` classes from `selenium-manager`

* [py]: Remove `selenium-manager` binary

* [py]: simplify command and logging in selenium manager `.run()`

* [py]: update `.gitignore` to prevent `selenium-manager` in python

Author: symonk

.gitignore

@@ -73,6 +73,9 @@ py/selenium/webdriver/remote/isDisplayed.js
 py/docs/build/
 py/build/
 py/LICENSE
+py/selenium/webdriver/common/linux/
+py/selenium/webdriver/common/windows/
+py/selenium/webdriver/common/macos/
 selenium.egg-info/
 third_party/java/jetty/jetty-repacked.jar
 *.user

py/selenium/common/__init__.py

@@ -39,6 +39,7 @@
 from .exceptions import NoSuchShadowRootException
 from .exceptions import NoSuchWindowException
 from .exceptions import ScreenshotException
+from .exceptions import SeleniumManagerException
 from .exceptions import SessionNotCreatedException
 from .exceptions import StaleElementReferenceException
 from .exceptions import TimeoutException
@@ -81,4 +82,5 @@
     "InvalidSessionIdException",
     "SessionNotCreatedException",
     "UnknownMethodException",
+    "SeleniumManagerException",
 ]

py/selenium/common/exceptions.py

@@ -298,3 +298,7 @@ class UnknownMethodException(WebDriverException):
     """
     The requested command matched a known URL but did not match any methods for that URL.
     """
+
+
+class SeleniumManagerException(WebDriverException):
+    """Raised when an issue interacting with selenium manager occurs."""

py/selenium/webdriver/common/selenium_manager.py

@@ -15,13 +15,12 @@
 # specific language governing permissions and limitations
 # under the License.
 import logging
-import re
 import subprocess
 import sys
 from pathlib import Path
 from typing import Tuple
 
-from selenium.common.exceptions import WebDriverException
+from selenium.common.exceptions import SeleniumManagerException
 
 logger = logging.getLogger(__name__)
 
@@ -32,6 +31,9 @@ class SeleniumManager:
     This implementation is still in beta, and may change.
     """
 
+    def __init__(self) -> None:
+        pass
+
     @staticmethod
     def get_binary() -> Path:
         """
@@ -49,26 +51,27 @@ def get_binary() -> Path:
         path = Path(__file__).parent.joinpath(directory, file)
 
         if not path.is_file():
-            raise WebDriverException("Unable to obtain Selenium Manager")
+            tracker = "https://github.com/SeleniumHQ/selenium/issues"
+            raise SeleniumManagerException(f"{path} is missing.  Please open an issue on {tracker}")
 
         return path
 
-    @staticmethod
-    def driver_location(browser: str) -> str:
+    def driver_location(self, browser: str) -> str:
         """
         Determines the path of the correct driver.
         :Args:
          - browser: which browser to get the driver path for.
         :Returns: The driver path to use
         """
-        if browser not in ("chrome", "firefox", "edge", "ie"):
-            raise WebDriverException(f"Unable to locate driver associated with browser name: {browser}")
+        allowed = ("chrome", "firefox", "edge", "ie")
+        if browser not in allowed:
+            raise SeleniumManagerException(f"{browser} is not a valid browser.  Choose one of: {allowed}")
 
-        args = (str(SeleniumManager.get_binary()), "--browser", browser)
-        result = SeleniumManager.run(args)
-        command = result.split("\t")[-1].strip()
-        logger.debug(f"Using driver at: {command}")
-        return command
+        binary, flag, browser = str(self.get_binary()), "--browser", browser
+        result = self.run((binary, flag, browser))
+        executable = result.split("\t")[-1].strip()
+        logger.debug(f"Using driver at: {executable}")
+        return executable
 
     @staticmethod
     def run(args: Tuple[str, str, str]) -> str:
@@ -78,10 +81,13 @@ def run(args: Tuple[str, str, str]) -> str:
          - args: the components of the command being executed.
         :Returns: The log string containing the driver location.
         """
-        logger.debug(f"Executing selenium manager with: {args}")
-        result = subprocess.run(args, stdout=subprocess.PIPE).stdout.decode("utf-8")
-
-        if not re.match("^INFO\t", result):
-            raise WebDriverException(f"Unsuccessful command executed: {args}")
-
-        return result
+        command = " ".join(args)
+        logger.debug(f"Executing: {command}")
+        completed_proc = subprocess.run(args, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+        stdout = completed_proc.stdout.decode("utf-8").rstrip("\n")
+        stderr = completed_proc.stderr.decode("utf-8").rstrip("\n")
+        if completed_proc.returncode:
+            raise SeleniumManagerException(f"Selenium manager failed for: {command}. {stderr}")
+        else:
+            # selenium manager exited 0 successfully, parse the executable path from stdout.
+            return stdout.split("\t")[-1].strip()

py/selenium/webdriver/common/service.py

@@ -93,7 +93,7 @@ def start(self) -> None:
                 logger.debug("driver not found in PATH, trying Selenium Manager")
                 browser = self.__class__.__module__.split(".")[-2]
                 try:
-                    path = SeleniumManager.driver_location(browser)
+                    path = SeleniumManager().driver_location(browser)
                 except WebDriverException as new_err:
                     logger.debug("Unable to obtain driver using Selenium Manager: " + new_err.msg)
                     raise err

py/test/selenium/webdriver/common/selenium_manager_tests.py

@@ -0,0 +1,35 @@
+#  Licensed to the Software Freedom Conservancy (SFC) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The SFC licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing,
+#  software distributed under the License is distributed on an
+#  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+#  KIND, either express or implied.  See the License for the
+#  specific language governing permissions and limitations
+#  under the License.
+
+import pytest
+
+from selenium.common.exceptions import SeleniumManagerException
+from selenium.webdriver.common.selenium_manager import SeleniumManager
+
+
+def test_non_supported_browser_raises_sme():
+    msg = r"foo is not a valid browser.  Choose one of: \('chrome', 'firefox', 'edge', 'ie'\)"
+    with pytest.raises(SeleniumManagerException, match=msg):
+        _ = SeleniumManager().driver_location("foo")
+
+
+def test_stderr_is_propagated_to_exception_messages():
+    msg = "Selenium manager failed for.*Error: \"Invalid browser/driver name\""
+    with pytest.raises(SeleniumManagerException, match=msg):
+        manager = SeleniumManager()
+        binary = manager.get_binary()
+        _ = manager.run((str(binary), "--browser", "foo"))