[java] update logging and errors for driver management

SeleniumHQ · Jan 7, 2024 · f75ea68 · f75ea68
1 parent 34adf3e
commit f75ea68
Show file tree

Hide file tree

Showing 7 changed files with 38 additions and 30 deletions.
diff --git a/java/src/org/openqa/selenium/chrome/ChromeDriver.java b/java/src/org/openqa/selenium/chrome/ChromeDriver.java
@@ -96,12 +96,10 @@ private static ChromeDriverCommandExecutor generateExecutor(
     Require.nonNull("Driver service", service);
     Require.nonNull("Driver options", options);
     Require.nonNull("Driver clientConfig", clientConfig);
-    if (service.getExecutable() == null) {
-      Result result = DriverFinder.getPath(service, options);
-      service.setExecutable(result.getDriverPath());
-      if (result.getBrowserPath() != null && !result.getBrowserPath().isEmpty()) {
-        options.setBinary(result.getBrowserPath());
-      }
+    Result result = DriverFinder.getPath(service, options);
+    service.setExecutable(result.getDriverPath());
+    if (result.getBrowserPath() != null && !result.getBrowserPath().isEmpty()) {
+      options.setBinary(result.getBrowserPath());
     }
     return new ChromeDriverCommandExecutor(service, clientConfig);
   }

diff --git a/java/src/org/openqa/selenium/edge/EdgeDriver.java b/java/src/org/openqa/selenium/edge/EdgeDriver.java
@@ -68,12 +68,10 @@ private static EdgeDriverCommandExecutor generateExecutor(
     Require.nonNull("Driver service", service);
     Require.nonNull("Driver options", options);
     Require.nonNull("Driver clientConfig", clientConfig);
-    if (service.getExecutable() == null) {
-      Result result = DriverFinder.getPath(service, options);
-      service.setExecutable(result.getDriverPath());
-      if (result.getBrowserPath() != null && !result.getBrowserPath().isEmpty()) {
-        options.setBinary(result.getBrowserPath());
-      }
+    Result result = DriverFinder.getPath(service, options);
+    service.setExecutable(result.getDriverPath());
+    if (result.getBrowserPath() != null && !result.getBrowserPath().isEmpty()) {
+      options.setBinary(result.getBrowserPath());
     }
     return new EdgeDriverCommandExecutor(service, clientConfig);
   }

diff --git a/java/src/org/openqa/selenium/firefox/FirefoxDriver.java b/java/src/org/openqa/selenium/firefox/FirefoxDriver.java
@@ -138,12 +138,10 @@ private static FirefoxDriverCommandExecutor generateExecutor(
     Require.nonNull("Driver service", service);
     Require.nonNull("Driver options", options);
     Require.nonNull("Driver clientConfig", clientConfig);
-    if (service.getExecutable() == null) {
-      Result result = DriverFinder.getPath(service, options);
-      service.setExecutable(result.getDriverPath());
-      if (result.getBrowserPath() != null && !result.getBrowserPath().isEmpty()) {
-        options.setBinary(result.getBrowserPath());
-      }
+    Result result = DriverFinder.getPath(service, options);
+    service.setExecutable(result.getDriverPath());
+    if (result.getBrowserPath() != null && !result.getBrowserPath().isEmpty()) {
+      options.setBinary(result.getBrowserPath());
     }
     return new FirefoxDriverCommandExecutor(service, clientConfig);
   }

diff --git a/java/src/org/openqa/selenium/grid/node/config/DriverServiceSessionFactory.java b/java/src/org/openqa/selenium/grid/node/config/DriverServiceSessionFactory.java
@@ -131,12 +131,10 @@ public Either<WebDriverException, ActiveSession> apply(CreateSessionRequest sess
       attributeMap.put(AttributeKey.LOGGER_CLASS.getKey(), this.getClass().getName());
 
       DriverService service = builder.build();
-      if (service.getExecutable() == null) {
-        Result result = DriverFinder.getPath(service, capabilities);
-        service.setExecutable(result.getDriverPath());
-        if (result.getBrowserPath() != null && !result.getBrowserPath().isEmpty()) {
-          capabilities = setBrowserBinary(capabilities, result.getBrowserPath());
-        }
+      Result driverResult = DriverFinder.getPath(service, capabilities);
+      service.setExecutable(driverResult.getDriverPath());
+      if (driverResult.getBrowserPath() != null && !driverResult.getBrowserPath().isEmpty()) {
+        capabilities = setBrowserBinary(capabilities, driverResult.getBrowserPath());
       }
 
       Optional<Platform> platformName = Optional.ofNullable(capabilities.getPlatformName());

diff --git a/java/src/org/openqa/selenium/ie/InternetExplorerDriver.java b/java/src/org/openqa/selenium/ie/InternetExplorerDriver.java
@@ -21,6 +21,7 @@
 import org.openqa.selenium.Capabilities;
 import org.openqa.selenium.Platform;
 import org.openqa.selenium.WebDriverException;
+import org.openqa.selenium.manager.SeleniumManagerOutput.Result;
 import org.openqa.selenium.remote.FileDetector;
 import org.openqa.selenium.remote.RemoteWebDriver;
 import org.openqa.selenium.remote.RemoteWebDriverBuilder;
@@ -124,10 +125,8 @@ public InternetExplorerDriver(
     if (service == null) {
       service = InternetExplorerDriverService.createDefaultService();
     }
-    if (service.getExecutable() == null) {
-      String path = DriverFinder.getPath(service, options).getDriverPath();
-      service.setExecutable(path);
-    }
+    Result result = DriverFinder.getPath(service, options);
+    service.setExecutable(result.getDriverPath());
     if (clientConfig == null) {
       clientConfig = ClientConfig.defaultConfig();
     }

diff --git a/java/src/org/openqa/selenium/manager/SeleniumManager.java b/java/src/org/openqa/selenium/manager/SeleniumManager.java
@@ -97,6 +97,8 @@ private SeleniumManager() {
                       }
                     }
                   }));
+    } else {
+      LOG.fine(String.format("Selenium Manager set by env 'SE_MANAGER_PATH': %s", managerPath));
     }
   }
 

diff --git a/java/src/org/openqa/selenium/remote/service/DriverFinder.java b/java/src/org/openqa/selenium/remote/service/DriverFinder.java
@@ -18,6 +18,7 @@
 package org.openqa.selenium.remote.service;
 
 import java.io.File;
+import java.util.logging.Logger;
 import org.openqa.selenium.Capabilities;
 import org.openqa.selenium.WebDriverException;
 import org.openqa.selenium.internal.Require;
@@ -27,21 +28,35 @@
 
 public class DriverFinder {
 
+  private static final Logger LOG = Logger.getLogger(DriverFinder.class.getName());
+
   public static Result getPath(DriverService service, Capabilities options) {
     return getPath(service, options, false);
   }
 
   public static Result getPath(DriverService service, Capabilities options, boolean offline) {
     Require.nonNull("Browser options", options);
-    Result result = new Result(System.getProperty(service.getDriverProperty()));
+    Result result = new Result(service.getExecutable());
+    if (result.getDriverPath() != null) {
+      LOG.fine(
+          String.format(
+              "Skipping Selenium Manager, path to %s specified in Service class: %s",
+              service.getDriverName(), result.getDriverPath()));
+    }
 
+    result = new Result(System.getProperty(service.getDriverProperty()));
     if (result.getDriverPath() == null) {
       try {
         result = SeleniumManager.getInstance().getDriverPath(options, offline);
       } catch (RuntimeException e) {
         throw new WebDriverException(
             String.format("Unable to obtain: %s, error %s", options, e.getMessage()), e);
       }
+    } else {
+      LOG.fine(
+          String.format(
+              "Skipping Selenium Manager, path to %s found in system property: %s",
+              service.getDriverName(), result.getDriverPath()));
     }
 
     String message;
@@ -50,7 +65,7 @@ public static Result getPath(DriverService service, Capabilities options, boolea
     } else if (!new File(result.getDriverPath()).exists()) {
       message =
           String.format(
-              "%s located at %s, but invalid", service.getDriverName(), result.getDriverPath());
+              "%s at location %s, does not exist", service.getDriverName(), result.getDriverPath());
     } else if (!new File(result.getDriverPath()).canExecute()) {
       message =
           String.format(