[java] fix broken driver finder conditional

NDViet · Jan 8, 2024 · 9bcccf2 · 9bcccf2
1 parent e1e538e
commit 9bcccf2
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 33 deletions.
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 driverResult = DriverFinder.getPath(service, capabilities);
-        service.setExecutable(driverResult.getDriverPath());
-        if (driverResult.getBrowserPath() != null && !driverResult.getBrowserPath().isEmpty()) {
-          capabilities = setBrowserBinary(capabilities, driverResult.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/remote/service/DriverFinder.java b/java/src/org/openqa/selenium/remote/service/DriverFinder.java
@@ -20,7 +20,6 @@
 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;
 import org.openqa.selenium.manager.SeleniumManager;
 import org.openqa.selenium.manager.SeleniumManagerOutput.Result;
@@ -36,41 +35,40 @@ public static Result getPath(DriverService service, Capabilities options) {
 
   public static Result getPath(DriverService service, Capabilities options, boolean offline) {
     Require.nonNull("Browser options", options);
-    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()));
-    }
+    String driverName = service.getDriverName();
 
-    result = new Result(System.getProperty(service.getDriverProperty()));
+    Result result = new Result(service.getExecutable());
     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);
+      result = new Result(System.getProperty(service.getDriverProperty()));
+      if (result.getDriverPath() == null) {
+        try {
+          result = SeleniumManager.getInstance().getDriverPath(options, offline);
+        } catch (RuntimeException e) {
+          throw new NoSuchDriverException(
+              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",
+                driverName, result.getDriverPath()));
       }
     } else {
       LOG.fine(
           String.format(
-              "Skipping Selenium Manager, path to %s found in system property: %s",
-              service.getDriverName(), result.getDriverPath()));
+              "Skipping Selenium Manager, path to %s specified in Service class: %s",
+              driverName, result.getDriverPath()));
     }
 
     String message;
     if (result.getDriverPath() == null) {
-      message = String.format("Unable to locate or obtain %s", service.getDriverName());
+      message = String.format("Unable to locate or obtain %s", driverName);
     } else if (!new File(result.getDriverPath()).exists()) {
       message =
-          String.format(
-              "%s at location %s, does not exist", service.getDriverName(), result.getDriverPath());
+          String.format("%s at location %s, does not exist", driverName, result.getDriverPath());
     } else if (!new File(result.getDriverPath()).canExecute()) {
       message =
-          String.format(
-              "%s located at %s, cannot be executed",
-              service.getDriverName(), result.getDriverPath());
+          String.format("%s located at %s, cannot be executed", driverName, result.getDriverPath());
     } else {
       return result;
     }

diff --git a/java/test/org/openqa/selenium/grid/node/config/DriverServiceSessionFactoryTest.java b/java/test/org/openqa/selenium/grid/node/config/DriverServiceSessionFactoryTest.java
@@ -32,8 +32,9 @@
 import com.google.common.collect.ImmutableSet;
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
-import java.net.MalformedURLException;
 import java.net.URL;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.time.Duration;
 import java.util.function.Predicate;
 import org.junit.jupiter.api.BeforeEach;
@@ -61,14 +62,16 @@ class DriverServiceSessionFactoryTest {
   private DriverService driverService;
 
   @BeforeEach
-  public void setUp() throws MalformedURLException {
+  public void setUp() throws IOException {
     tracer = DefaultTestTracer.createTracer();
 
     clientFactory = mock(HttpClient.Factory.class);
 
     driverService = mock(DriverService.class);
     when(driverService.getUrl()).thenReturn(new URL("http://localhost:1234/"));
-    when(driverService.getExecutable()).thenReturn("/usr/bin/driver");
+    Path driverFile = Files.createTempFile("testDriver", ".tmp");
+    driverFile.toFile().setExecutable(true);
+    when(driverService.getExecutable()).thenReturn(driverFile.toString());
 
     builder = mock(DriverService.Builder.class);
     when(builder.build()).thenReturn(driverService);
@@ -147,10 +150,10 @@ void shouldNotInstantiateSessionIfRemoteEndReturnsInvalidResponse() throws IOExc
     verify(builder, times(1)).build();
     verifyNoMoreInteractions(builder);
     verify(driverService, times(1)).getExecutable();
+    verify(driverService, times(1)).setExecutable(any(String.class));
     verify(driverService, times(1)).start();
     verify(driverService, atLeastOnce()).getUrl();
     verify(driverService, times(1)).stop();
-    verifyNoMoreInteractions(driverService);
   }
 
   @Test
@@ -179,9 +182,9 @@ void shouldInstantiateSessionIfEverythingIsOK() throws IOException {
     verify(builder, times(1)).build();
     verifyNoMoreInteractions(builder);
     verify(driverService, times(1)).getExecutable();
+    verify(driverService, times(1)).setExecutable(any(String.class));
     verify(driverService, times(1)).start();
     verify(driverService, atLeastOnce()).getUrl();
-    verifyNoMoreInteractions(driverService);
   }
 
   private DriverServiceSessionFactory factoryFor(String browser, DriverService.Builder builder) {