[java] Selenium Manager generates output from argument list (#13385)

* [java] rename driver finder and selenium manager methods to getResult * [java] Selenium Manager generates output from argument list instead of capabilities instance * [java] allow driver finder to store state instead of static * [java] rename method to getBinaryPaths * [java] rename method again * [java] false and true are different * a getter method should have a return value --------- Co-authored-by: Diego Molina <[email protected]>
SeleniumHQ · Apr 14, 2024 · 4ecc103 · 4ecc103
1 parent 25fd49a
commit 4ecc103
Show file tree

Hide file tree

Showing 26 changed files with 370 additions and 317 deletions.
diff --git a/java/src/org/openqa/selenium/PersistentCapabilities.java b/java/src/org/openqa/selenium/PersistentCapabilities.java
@@ -50,7 +50,6 @@ private PersistentCapabilities(Capabilities previousValues, Capabilities newValu
 
   public PersistentCapabilities setCapability(String name, Object value) {
     Require.nonNull("Name", name);
-    Require.nonNull("Value", value);
 
     return new PersistentCapabilities(this, new ImmutableCapabilities(name, value));
   }

diff --git a/java/src/org/openqa/selenium/chrome/ChromeDriver.java b/java/src/org/openqa/selenium/chrome/ChromeDriver.java
@@ -26,7 +26,6 @@
 import org.openqa.selenium.chromium.ChromiumDriver;
 import org.openqa.selenium.chromium.ChromiumDriverCommandExecutor;
 import org.openqa.selenium.internal.Require;
-import org.openqa.selenium.manager.SeleniumManagerOutput.Result;
 import org.openqa.selenium.remote.CommandInfo;
 import org.openqa.selenium.remote.RemoteWebDriver;
 import org.openqa.selenium.remote.RemoteWebDriverBuilder;
@@ -96,10 +95,11 @@ private static ChromeDriverCommandExecutor generateExecutor(
     Require.nonNull("Driver service", service);
     Require.nonNull("Driver options", options);
     Require.nonNull("Driver clientConfig", clientConfig);
-    Result result = DriverFinder.getPath(service, options);
-    service.setExecutable(result.getDriverPath());
-    if (result.getBrowserPath() != null && !result.getBrowserPath().isEmpty()) {
-      options.setBinary(result.getBrowserPath());
+    DriverFinder finder = new DriverFinder(service, options);
+    service.setExecutable(finder.getDriverPath());
+    if (finder.hasBrowserPath()) {
+      options.setBinary(finder.getBrowserPath());
+      options.setCapability("browserVersion", (Object) null);
     }
     return new ChromeDriverCommandExecutor(service, clientConfig);
   }

diff --git a/java/src/org/openqa/selenium/chrome/ChromeDriverInfo.java b/java/src/org/openqa/selenium/chrome/ChromeDriverInfo.java
@@ -21,17 +21,14 @@
 
 import com.google.auto.service.AutoService;
 import java.util.Optional;
-import java.util.logging.Level;
 import java.util.logging.Logger;
 import org.openqa.selenium.Capabilities;
 import org.openqa.selenium.ImmutableCapabilities;
 import org.openqa.selenium.SessionNotCreatedException;
 import org.openqa.selenium.WebDriver;
-import org.openqa.selenium.WebDriverException;
 import org.openqa.selenium.WebDriverInfo;
 import org.openqa.selenium.chromium.ChromiumDriverInfo;
 import org.openqa.selenium.remote.CapabilityType;
-import org.openqa.selenium.remote.NoSuchDriverException;
 import org.openqa.selenium.remote.service.DriverFinder;
 
 @AutoService(WebDriverInfo.class)
@@ -65,29 +62,14 @@ public boolean isSupportingBiDi() {
 
   @Override
   public boolean isAvailable() {
-    try {
-      DriverFinder.getPath(ChromeDriverService.createDefaultService(), getCanonicalCapabilities());
-      return true;
-    } catch (NoSuchDriverException e) {
-      return false;
-    } catch (IllegalStateException | WebDriverException e) {
-      LOG.log(Level.WARNING, "failed to discover driver path", e);
-      return false;
-    }
+    return new DriverFinder(ChromeDriverService.createDefaultService(), getCanonicalCapabilities())
+        .isAvailable();
   }
 
   @Override
   public boolean isPresent() {
-    try {
-      DriverFinder.getPath(
-          ChromeDriverService.createDefaultService(), getCanonicalCapabilities(), true);
-      return true;
-    } catch (NoSuchDriverException e) {
-      return false;
-    } catch (IllegalStateException | WebDriverException e) {
-      LOG.log(Level.WARNING, "failed to discover driver path", e);
-      return false;
-    }
+    return new DriverFinder(ChromeDriverService.createDefaultService(), getCanonicalCapabilities())
+        .isPresent();
   }
 
   @Override

diff --git a/java/src/org/openqa/selenium/chrome/ChromeDriverService.java b/java/src/org/openqa/selenium/chrome/ChromeDriverService.java
@@ -128,7 +128,7 @@ public Capabilities getDefaultDriverOptions() {
   /**
    * Configures and returns a new {@link ChromeDriverService} using the default configuration. In
    * this configuration, the service will use the ChromeDriver executable identified by {@link
-   * org.openqa.selenium.remote.service.DriverFinder#getPath(DriverService, Capabilities)}. Each
+   * org.openqa.selenium.remote.service.DriverFinder#getResult(DriverService, Capabilities)}. Each
    * service created by this method will be configured to use a free port on the current system.
    *
    * @return A new ChromeDriverService using the default configuration.

diff --git a/java/src/org/openqa/selenium/edge/EdgeDriver.java b/java/src/org/openqa/selenium/edge/EdgeDriver.java
@@ -24,7 +24,6 @@
 import org.openqa.selenium.chromium.ChromiumDriver;
 import org.openqa.selenium.chromium.ChromiumDriverCommandExecutor;
 import org.openqa.selenium.internal.Require;
-import org.openqa.selenium.manager.SeleniumManagerOutput.Result;
 import org.openqa.selenium.remote.CommandInfo;
 import org.openqa.selenium.remote.RemoteWebDriver;
 import org.openqa.selenium.remote.RemoteWebDriverBuilder;
@@ -68,10 +67,11 @@ private static EdgeDriverCommandExecutor generateExecutor(
     Require.nonNull("Driver service", service);
     Require.nonNull("Driver options", options);
     Require.nonNull("Driver clientConfig", clientConfig);
-    Result result = DriverFinder.getPath(service, options);
-    service.setExecutable(result.getDriverPath());
-    if (result.getBrowserPath() != null && !result.getBrowserPath().isEmpty()) {
-      options.setBinary(result.getBrowserPath());
+    DriverFinder finder = new DriverFinder(service, options);
+    service.setExecutable(finder.getDriverPath());
+    if (finder.hasBrowserPath()) {
+      options.setBinary(finder.getBrowserPath());
+      options.setCapability("browserVersion", (Object) null);
     }
     return new EdgeDriverCommandExecutor(service, clientConfig);
   }

diff --git a/java/src/org/openqa/selenium/edge/EdgeDriverInfo.java b/java/src/org/openqa/selenium/edge/EdgeDriverInfo.java
@@ -21,17 +21,14 @@
 
 import com.google.auto.service.AutoService;
 import java.util.Optional;
-import java.util.logging.Level;
 import java.util.logging.Logger;
 import org.openqa.selenium.Capabilities;
 import org.openqa.selenium.ImmutableCapabilities;
 import org.openqa.selenium.SessionNotCreatedException;
 import org.openqa.selenium.WebDriver;
-import org.openqa.selenium.WebDriverException;
 import org.openqa.selenium.WebDriverInfo;
 import org.openqa.selenium.chromium.ChromiumDriverInfo;
 import org.openqa.selenium.remote.CapabilityType;
-import org.openqa.selenium.remote.NoSuchDriverException;
 import org.openqa.selenium.remote.service.DriverFinder;
 
 @AutoService(WebDriverInfo.class)
@@ -68,29 +65,14 @@ public boolean isSupportingBiDi() {
 
   @Override
   public boolean isAvailable() {
-    try {
-      DriverFinder.getPath(EdgeDriverService.createDefaultService(), getCanonicalCapabilities());
-      return true;
-    } catch (NoSuchDriverException e) {
-      return false;
-    } catch (IllegalStateException | WebDriverException e) {
-      LOG.log(Level.WARNING, "failed to discover driver path", e);
-      return false;
-    }
+    return new DriverFinder(EdgeDriverService.createDefaultService(), getCanonicalCapabilities())
+        .isAvailable();
   }
 
   @Override
   public boolean isPresent() {
-    try {
-      DriverFinder.getPath(
-          EdgeDriverService.createDefaultService(), getCanonicalCapabilities(), true);
-      return true;
-    } catch (NoSuchDriverException e) {
-      return false;
-    } catch (IllegalStateException | WebDriverException e) {
-      LOG.log(Level.WARNING, "failed to discover driver path", e);
-      return false;
-    }
+    return new DriverFinder(EdgeDriverService.createDefaultService(), getCanonicalCapabilities())
+        .isPresent();
   }
 
   @Override

diff --git a/java/src/org/openqa/selenium/edge/EdgeDriverService.java b/java/src/org/openqa/selenium/edge/EdgeDriverService.java
@@ -117,7 +117,7 @@ public Capabilities getDefaultDriverOptions() {
   /**
    * Configures and returns a new {@link EdgeDriverService} using the default configuration. In this
    * configuration, the service will use the MSEdgeDriver executable identified by the {@link
-   * org.openqa.selenium.remote.service.DriverFinder#getPath(DriverService, Capabilities)}. Each
+   * org.openqa.selenium.remote.service.DriverFinder#getResult(DriverService, Capabilities)}. Each
    * service created by this method will be configured to use a free port on the current system.
    *
    * @return A new EdgeDriverService using the default configuration.

diff --git a/java/src/org/openqa/selenium/firefox/FirefoxDriver.java b/java/src/org/openqa/selenium/firefox/FirefoxDriver.java
@@ -51,7 +51,6 @@
 import org.openqa.selenium.html5.SessionStorage;
 import org.openqa.selenium.html5.WebStorage;
 import org.openqa.selenium.internal.Require;
-import org.openqa.selenium.manager.SeleniumManagerOutput.Result;
 import org.openqa.selenium.remote.CommandInfo;
 import org.openqa.selenium.remote.FileDetector;
 import org.openqa.selenium.remote.RemoteWebDriver;
@@ -138,10 +137,11 @@ private static FirefoxDriverCommandExecutor generateExecutor(
     Require.nonNull("Driver service", service);
     Require.nonNull("Driver options", options);
     Require.nonNull("Driver clientConfig", clientConfig);
-    Result result = DriverFinder.getPath(service, options);
-    service.setExecutable(result.getDriverPath());
-    if (result.getBrowserPath() != null && !result.getBrowserPath().isEmpty()) {
-      options.setBinary(result.getBrowserPath());
+    DriverFinder finder = new DriverFinder(service, options);
+    service.setExecutable(finder.getDriverPath());
+    if (finder.hasBrowserPath()) {
+      options.setBinary(finder.getBrowserPath());
+      options.setCapability("browserVersion", (Object) null);
     }
     return new FirefoxDriverCommandExecutor(service, clientConfig);
   }

diff --git a/java/src/org/openqa/selenium/firefox/GeckoDriverInfo.java b/java/src/org/openqa/selenium/firefox/GeckoDriverInfo.java
@@ -22,15 +22,12 @@
 
 import com.google.auto.service.AutoService;
 import java.util.Optional;
-import java.util.logging.Level;
 import java.util.logging.Logger;
 import org.openqa.selenium.Capabilities;
 import org.openqa.selenium.ImmutableCapabilities;
 import org.openqa.selenium.SessionNotCreatedException;
 import org.openqa.selenium.WebDriver;
-import org.openqa.selenium.WebDriverException;
 import org.openqa.selenium.WebDriverInfo;
-import org.openqa.selenium.remote.NoSuchDriverException;
 import org.openqa.selenium.remote.service.DriverFinder;
 
 @AutoService(WebDriverInfo.class)
@@ -68,29 +65,14 @@ public boolean isSupportingBiDi() {
 
   @Override
   public boolean isAvailable() {
-    try {
-      DriverFinder.getPath(GeckoDriverService.createDefaultService(), getCanonicalCapabilities());
-      return true;
-    } catch (NoSuchDriverException e) {
-      return false;
-    } catch (IllegalStateException | WebDriverException e) {
-      LOG.log(Level.WARNING, "failed to discover driver path", e);
-      return false;
-    }
+    return new DriverFinder(GeckoDriverService.createDefaultService(), getCanonicalCapabilities())
+        .isAvailable();
   }
 
   @Override
   public boolean isPresent() {
-    try {
-      DriverFinder.getPath(
-          GeckoDriverService.createDefaultService(), getCanonicalCapabilities(), true);
-      return true;
-    } catch (NoSuchDriverException e) {
-      return false;
-    } catch (IllegalStateException | WebDriverException e) {
-      LOG.log(Level.WARNING, "failed to discover driver path", e);
-      return false;
-    }
+    return new DriverFinder(GeckoDriverService.createDefaultService(), getCanonicalCapabilities())
+        .isPresent();
   }
 
   @Override

diff --git a/java/src/org/openqa/selenium/firefox/GeckoDriverService.java b/java/src/org/openqa/selenium/firefox/GeckoDriverService.java
@@ -110,7 +110,7 @@ public Capabilities getDefaultDriverOptions() {
   /**
    * Configures and returns a new {@link GeckoDriverService} using the default configuration. In
    * this configuration, the service will use the GeckoDriver executable identified by the {@link
-   * org.openqa.selenium.remote.service.DriverFinder#getPath(DriverService, Capabilities)}. Each
+   * org.openqa.selenium.remote.service.DriverFinder#getResult(DriverService, Capabilities)}. Each
    * service created by this method will be configured to use a free port on the current system.
    *
    * @return A new GeckoDriverService using the default configuration.

diff --git a/java/src/org/openqa/selenium/grid/node/config/DriverServiceSessionFactory.java b/java/src/org/openqa/selenium/grid/node/config/DriverServiceSessionFactory.java
@@ -49,7 +49,6 @@
 import org.openqa.selenium.grid.node.SessionFactory;
 import org.openqa.selenium.internal.Either;
 import org.openqa.selenium.internal.Require;
-import org.openqa.selenium.manager.SeleniumManagerOutput.Result;
 import org.openqa.selenium.net.HostIdentifier;
 import org.openqa.selenium.net.NetworkUtils;
 import org.openqa.selenium.remote.Command;
@@ -130,10 +129,10 @@ public Either<WebDriverException, ActiveSession> apply(CreateSessionRequest sess
       attributeMap.put(AttributeKey.LOGGER_CLASS.getKey(), this.getClass().getName());
 
       DriverService service = builder.build();
-      Result driverResult = DriverFinder.getPath(service, capabilities);
-      service.setExecutable(driverResult.getDriverPath());
-      if (driverResult.getBrowserPath() != null && !driverResult.getBrowserPath().isEmpty()) {
-        capabilities = setBrowserBinary(capabilities, driverResult.getBrowserPath());
+      DriverFinder finder = new DriverFinder(service, capabilities);
+      service.setExecutable(finder.getDriverPath());
+      if (finder.hasBrowserPath()) {
+        capabilities = setBrowserBinary(capabilities, finder.getBrowserPath());
       }
 
       Optional<Platform> platformName = Optional.ofNullable(capabilities.getPlatformName());
@@ -327,7 +326,8 @@ private Capabilities setBrowserBinary(Capabilities options, String browserPath)
               (Map<String, Object>) options.getCapability(vendorOptionsCapability);
           vendorOptions.put("binary", browserPath);
           return new PersistentCapabilities(options)
-              .setCapability(vendorOptionsCapability, vendorOptions);
+              .setCapability(vendorOptionsCapability, vendorOptions)
+              .setCapability("browserVersion", null);
         } catch (Exception e) {
           LOG.warning(
               String.format(

diff --git a/java/src/org/openqa/selenium/ie/InternetExplorerDriver.java b/java/src/org/openqa/selenium/ie/InternetExplorerDriver.java
@@ -21,7 +21,6 @@
 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;
@@ -125,8 +124,7 @@ public InternetExplorerDriver(
     if (service == null) {
       service = InternetExplorerDriverService.createDefaultService();
     }
-    Result result = DriverFinder.getPath(service, options);
-    service.setExecutable(result.getDriverPath());
+    service.setExecutable(new DriverFinder(service, options).getDriverPath());
     if (clientConfig == null) {
       clientConfig = ClientConfig.defaultConfig();
     }

diff --git a/java/src/org/openqa/selenium/ie/InternetExplorerDriverInfo.java b/java/src/org/openqa/selenium/ie/InternetExplorerDriverInfo.java
@@ -22,16 +22,13 @@
 
 import com.google.auto.service.AutoService;
 import java.util.Optional;
-import java.util.logging.Level;
 import java.util.logging.Logger;
 import org.openqa.selenium.Capabilities;
 import org.openqa.selenium.ImmutableCapabilities;
 import org.openqa.selenium.Platform;
 import org.openqa.selenium.SessionNotCreatedException;
 import org.openqa.selenium.WebDriver;
-import org.openqa.selenium.WebDriverException;
 import org.openqa.selenium.WebDriverInfo;
-import org.openqa.selenium.remote.NoSuchDriverException;
 import org.openqa.selenium.remote.service.DriverFinder;
 
 @AutoService(WebDriverInfo.class)
@@ -65,36 +62,18 @@ public boolean isSupportingBiDi() {
 
   @Override
   public boolean isAvailable() {
-    try {
-      if (Platform.getCurrent().is(Platform.WINDOWS)) {
-        DriverFinder.getPath(
-            InternetExplorerDriverService.createDefaultService(), getCanonicalCapabilities());
-        return true;
-      }
-      return false;
-    } catch (NoSuchDriverException e) {
-      return false;
-    } catch (IllegalStateException | WebDriverException e) {
-      LOG.log(Level.WARNING, "failed to discover driver path", e);
-      return false;
-    }
+    return Platform.getCurrent().is(Platform.WINDOWS)
+        && new DriverFinder(
+                InternetExplorerDriverService.createDefaultService(), getCanonicalCapabilities())
+            .isAvailable();
   }
 
   @Override
   public boolean isPresent() {
-    try {
-      if (Platform.getCurrent().is(Platform.WINDOWS)) {
-        DriverFinder.getPath(
-            InternetExplorerDriverService.createDefaultService(), getCanonicalCapabilities(), true);
-        return true;
-      }
-      return false;
-    } catch (NoSuchDriverException e) {
-      return false;
-    } catch (IllegalStateException | WebDriverException e) {
-      LOG.log(Level.WARNING, "failed to discover driver path", e);
-      return false;
-    }
+    return Platform.getCurrent().is(Platform.WINDOWS)
+        && new DriverFinder(
+                InternetExplorerDriverService.createDefaultService(), getCanonicalCapabilities())
+            .isPresent();
   }
 
   @Override

diff --git a/java/src/org/openqa/selenium/ie/InternetExplorerDriverService.java b/java/src/org/openqa/selenium/ie/InternetExplorerDriverService.java
@@ -104,9 +104,9 @@ public Capabilities getDefaultDriverOptions() {
   /**
    * Configures and returns a new {@link InternetExplorerDriverService} using the default
    * configuration. In this configuration, the service will use the IEDriverServer executable
-   * identified by the {@link org.openqa.selenium.remote.service.DriverFinder#getPath(DriverService,
-   * Capabilities)}. Each service created by this method will be configured to use a free port on
-   * the current system.
+   * identified by the {@link
+   * org.openqa.selenium.remote.service.DriverFinder#getResult(DriverService, Capabilities)}. Each
+   * service created by this method will be configured to use a free port on the current system.
    *
    * @return A new InternetExplorerDriverService using the default configuration.
    */