Skip to content

Commit 9bcccf2

Browse files
committed
[java] fix broken driver finder conditional
1 parent e1e538e commit 9bcccf2

File tree

3 files changed

+32
-33
lines changed

3 files changed

+32
-33
lines changed

java/src/org/openqa/selenium/grid/node/config/DriverServiceSessionFactory.java

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -131,12 +131,10 @@ public Either<WebDriverException, ActiveSession> apply(CreateSessionRequest sess
131131
attributeMap.put(AttributeKey.LOGGER_CLASS.getKey(), this.getClass().getName());
132132

133133
DriverService service = builder.build();
134-
if (service.getExecutable() == null) {
135-
Result driverResult = DriverFinder.getPath(service, capabilities);
136-
service.setExecutable(driverResult.getDriverPath());
137-
if (driverResult.getBrowserPath() != null && !driverResult.getBrowserPath().isEmpty()) {
138-
capabilities = setBrowserBinary(capabilities, driverResult.getBrowserPath());
139-
}
134+
Result driverResult = DriverFinder.getPath(service, capabilities);
135+
service.setExecutable(driverResult.getDriverPath());
136+
if (driverResult.getBrowserPath() != null && !driverResult.getBrowserPath().isEmpty()) {
137+
capabilities = setBrowserBinary(capabilities, driverResult.getBrowserPath());
140138
}
141139

142140
Optional<Platform> platformName = Optional.ofNullable(capabilities.getPlatformName());

java/src/org/openqa/selenium/remote/service/DriverFinder.java

Lines changed: 20 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import java.io.File;
2121
import java.util.logging.Logger;
2222
import org.openqa.selenium.Capabilities;
23-
import org.openqa.selenium.WebDriverException;
2423
import org.openqa.selenium.internal.Require;
2524
import org.openqa.selenium.manager.SeleniumManager;
2625
import org.openqa.selenium.manager.SeleniumManagerOutput.Result;
@@ -36,41 +35,40 @@ public static Result getPath(DriverService service, Capabilities options) {
3635

3736
public static Result getPath(DriverService service, Capabilities options, boolean offline) {
3837
Require.nonNull("Browser options", options);
39-
Result result = new Result(service.getExecutable());
40-
if (result.getDriverPath() != null) {
41-
LOG.fine(
42-
String.format(
43-
"Skipping Selenium Manager, path to %s specified in Service class: %s",
44-
service.getDriverName(), result.getDriverPath()));
45-
}
38+
String driverName = service.getDriverName();
4639

47-
result = new Result(System.getProperty(service.getDriverProperty()));
40+
Result result = new Result(service.getExecutable());
4841
if (result.getDriverPath() == null) {
49-
try {
50-
result = SeleniumManager.getInstance().getDriverPath(options, offline);
51-
} catch (RuntimeException e) {
52-
throw new WebDriverException(
53-
String.format("Unable to obtain: %s, error %s", options, e.getMessage()), e);
42+
result = new Result(System.getProperty(service.getDriverProperty()));
43+
if (result.getDriverPath() == null) {
44+
try {
45+
result = SeleniumManager.getInstance().getDriverPath(options, offline);
46+
} catch (RuntimeException e) {
47+
throw new NoSuchDriverException(
48+
String.format("Unable to obtain: %s, error %s", options, e.getMessage()), e);
49+
}
50+
} else {
51+
LOG.fine(
52+
String.format(
53+
"Skipping Selenium Manager, path to %s found in system property: %s",
54+
driverName, result.getDriverPath()));
5455
}
5556
} else {
5657
LOG.fine(
5758
String.format(
58-
"Skipping Selenium Manager, path to %s found in system property: %s",
59-
service.getDriverName(), result.getDriverPath()));
59+
"Skipping Selenium Manager, path to %s specified in Service class: %s",
60+
driverName, result.getDriverPath()));
6061
}
6162

6263
String message;
6364
if (result.getDriverPath() == null) {
64-
message = String.format("Unable to locate or obtain %s", service.getDriverName());
65+
message = String.format("Unable to locate or obtain %s", driverName);
6566
} else if (!new File(result.getDriverPath()).exists()) {
6667
message =
67-
String.format(
68-
"%s at location %s, does not exist", service.getDriverName(), result.getDriverPath());
68+
String.format("%s at location %s, does not exist", driverName, result.getDriverPath());
6969
} else if (!new File(result.getDriverPath()).canExecute()) {
7070
message =
71-
String.format(
72-
"%s located at %s, cannot be executed",
73-
service.getDriverName(), result.getDriverPath());
71+
String.format("%s located at %s, cannot be executed", driverName, result.getDriverPath());
7472
} else {
7573
return result;
7674
}

java/test/org/openqa/selenium/grid/node/config/DriverServiceSessionFactoryTest.java

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,9 @@
3232
import com.google.common.collect.ImmutableSet;
3333
import java.io.ByteArrayInputStream;
3434
import java.io.IOException;
35-
import java.net.MalformedURLException;
3635
import java.net.URL;
36+
import java.nio.file.Files;
37+
import java.nio.file.Path;
3738
import java.time.Duration;
3839
import java.util.function.Predicate;
3940
import org.junit.jupiter.api.BeforeEach;
@@ -61,14 +62,16 @@ class DriverServiceSessionFactoryTest {
6162
private DriverService driverService;
6263

6364
@BeforeEach
64-
public void setUp() throws MalformedURLException {
65+
public void setUp() throws IOException {
6566
tracer = DefaultTestTracer.createTracer();
6667

6768
clientFactory = mock(HttpClient.Factory.class);
6869

6970
driverService = mock(DriverService.class);
7071
when(driverService.getUrl()).thenReturn(new URL("http://localhost:1234/"));
71-
when(driverService.getExecutable()).thenReturn("/usr/bin/driver");
72+
Path driverFile = Files.createTempFile("testDriver", ".tmp");
73+
driverFile.toFile().setExecutable(true);
74+
when(driverService.getExecutable()).thenReturn(driverFile.toString());
7275

7376
builder = mock(DriverService.Builder.class);
7477
when(builder.build()).thenReturn(driverService);
@@ -147,10 +150,10 @@ void shouldNotInstantiateSessionIfRemoteEndReturnsInvalidResponse() throws IOExc
147150
verify(builder, times(1)).build();
148151
verifyNoMoreInteractions(builder);
149152
verify(driverService, times(1)).getExecutable();
153+
verify(driverService, times(1)).setExecutable(any(String.class));
150154
verify(driverService, times(1)).start();
151155
verify(driverService, atLeastOnce()).getUrl();
152156
verify(driverService, times(1)).stop();
153-
verifyNoMoreInteractions(driverService);
154157
}
155158

156159
@Test
@@ -179,9 +182,9 @@ void shouldInstantiateSessionIfEverythingIsOK() throws IOException {
179182
verify(builder, times(1)).build();
180183
verifyNoMoreInteractions(builder);
181184
verify(driverService, times(1)).getExecutable();
185+
verify(driverService, times(1)).setExecutable(any(String.class));
182186
verify(driverService, times(1)).start();
183187
verify(driverService, atLeastOnce()).getUrl();
184-
verifyNoMoreInteractions(driverService);
185188
}
186189

187190
private DriverServiceSessionFactory factoryFor(String browser, DriverService.Builder builder) {

0 commit comments

Comments
 (0)