Skip to content

Commit 1c70137

Browse files
titusfortnerdiemol
andauthored
[java] implement DriverFinder completely independent of Service classes (#11491)
* [java] selenium manager fallback can result in multiple lines * [java] implement DriverFinder moves driver location out of the Service classes * [java] keep service constants with DriverServiceInfo interface * [grid] need to use DriverFinder to check if drivers are available * Making tests compile * Fixing DriverFinder * Setting the path when using Grid. * Setting executable when service is about to start if not set. * Using static vars --------- Co-authored-by: Diego Molina <diemol@gmail.com> Co-authored-by: Diego Molina <diemol@users.noreply.github.com>
1 parent ecea48d commit 1c70137

32 files changed

+292
-222
lines changed

java/src/org/openqa/selenium/chrome/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ java_export(
1717
"//java/src/org/openqa/selenium/chromium",
1818
"//java/src/org/openqa/selenium/json",
1919
"//java/src/org/openqa/selenium/remote",
20+
"//java/src/org/openqa/selenium/manager:manager",
2021
artifact("com.google.guava:guava"),
2122
],
2223
)

java/src/org/openqa/selenium/chrome/ChromeDriver.java

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.openqa.selenium.remote.CommandInfo;
2929
import org.openqa.selenium.remote.RemoteWebDriver;
3030
import org.openqa.selenium.remote.RemoteWebDriverBuilder;
31+
import org.openqa.selenium.remote.service.DriverFinder;
3132
import org.openqa.selenium.remote.service.DriverService;
3233

3334
import java.util.Map;
@@ -79,11 +80,21 @@ public ChromeDriver(ChromeOptions options) {
7980
* @param options The options required from ChromeDriver.
8081
*/
8182
public ChromeDriver(ChromeDriverService service, ChromeOptions options) {
82-
super(new ChromeDriverCommandExecutor(service), Require.nonNull("Driver options", options), ChromeOptions.CAPABILITY);
83+
super(generateExecutor(service, options), options, ChromeOptions.CAPABILITY);
8384
casting = new AddHasCasting().getImplementation(getCapabilities(), getExecuteMethod());
8485
cdp = new AddHasCdp().getImplementation(getCapabilities(), getExecuteMethod());
8586
}
8687

88+
private static ChromeDriverCommandExecutor generateExecutor(ChromeDriverService service, ChromeOptions options) {
89+
Require.nonNull("Driver service", service);
90+
Require.nonNull("Driver options", options);
91+
if (service.getExecutable() == null) {
92+
String path = DriverFinder.getPath(service, options);
93+
service.setExecutable(path);
94+
}
95+
return new ChromeDriverCommandExecutor(service);
96+
}
97+
8798
@Beta
8899
public static RemoteWebDriverBuilder builder() {
89100
return RemoteWebDriver.builder().oneOf(new ChromeOptions());

java/src/org/openqa/selenium/chrome/ChromeDriverInfo.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.openqa.selenium.WebDriverInfo;
2828
import org.openqa.selenium.chromium.ChromiumDriverInfo;
2929
import org.openqa.selenium.remote.CapabilityType;
30+
import org.openqa.selenium.remote.service.DriverFinder;
3031

3132
import java.util.Optional;
3233

@@ -64,7 +65,7 @@ public boolean isSupportingBiDi() {
6465
@Override
6566
public boolean isAvailable() {
6667
try {
67-
ChromeDriverService.createDefaultService();
68+
DriverFinder.getPath(ChromeDriverService.createDefaultService());
6869
return true;
6970
} catch (IllegalStateException | WebDriverException e) {
7071
return false;

java/src/org/openqa/selenium/chrome/ChromeDriverService.java

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.openqa.selenium.WebDriverException;
2424
import org.openqa.selenium.chromium.ChromiumDriverLogLevel;
2525
import org.openqa.selenium.remote.service.DriverService;
26+
import org.openqa.selenium.remote.service.DriverServiceInfo;
2627

2728
import java.io.File;
2829
import java.io.IOException;
@@ -41,6 +42,8 @@
4142
*/
4243
public class ChromeDriverService extends DriverService {
4344

45+
public static final String CHROME_DRIVER_NAME = "chromedriver";
46+
4447
/**
4548
* System property that defines the location of the ChromeDriver executable that will be used by
4649
* the {@link #createDefaultService() default service}.
@@ -135,11 +138,19 @@ public ChromeDriverService(
135138
unmodifiableMap(new HashMap<>(environment)));
136139
}
137140

141+
public String getDriverName() {
142+
return CHROME_DRIVER_NAME;
143+
}
144+
145+
public String getDriverProperty() {
146+
return CHROME_DRIVER_EXE_PROPERTY;
147+
}
148+
138149
/**
139150
* Configures and returns a new {@link ChromeDriverService} using the default configuration. In
140-
* this configuration, the service will use the ChromeDriver executable identified by the
141-
* {@link #CHROME_DRIVER_EXE_PROPERTY} system property. Each service created by this method will
142-
* be configured to use a free port on the current system.
151+
* this configuration, the service will use the ChromeDriver executable identified by
152+
* {@link org.openqa.selenium.remote.service.DriverFinder#getPath(DriverServiceInfo, Capabilities)}.
153+
* Each service created by this method will be configured to use a free port on the current system.
143154
*
144155
* @return A new ChromeDriverService using the default configuration.
145156
*/
@@ -149,9 +160,9 @@ public static ChromeDriverService createDefaultService() {
149160

150161
/**
151162
* Configures and returns a new {@link ChromeDriverService} using the supplied configuration. In
152-
* this configuration, the service will use the ChromeDriver executable identified by the
153-
* {@link #CHROME_DRIVER_EXE_PROPERTY} system property. Each service created by this method will
154-
* be configured to use a free port on the current system.
163+
* this configuration, the service will use the ChromeDriver executable identified by
164+
* {@link org.openqa.selenium.remote.service.DriverFinder#getPath(DriverServiceInfo, Capabilities)}.
165+
* Each service created by this method will be configured to use a free port on the current system.
155166
*
156167
* @return A new ChromeDriverService using the supplied configuration from {@link ChromeOptions}.
157168
* @deprecated Use {@link Builder#withLogLevel(ChromiumDriverLogLevel)} }
@@ -173,7 +184,7 @@ public static ChromeDriverService createServiceWithConfig(ChromeOptions options)
173184
* @return Whether the browser driver path was found.
174185
*/
175186
static boolean isPresent() {
176-
return findExePath("chromedriver", CHROME_DRIVER_EXE_PROPERTY) != null;
187+
return findExePath(CHROME_DRIVER_NAME, CHROME_DRIVER_EXE_PROPERTY) != null;
177188
}
178189

179190
/**
@@ -323,14 +334,6 @@ public Builder withReadableTimestamp(Boolean readableTimestamp) {
323334
return this;
324335
}
325336

326-
@Override
327-
protected File findDefaultExecutable() {
328-
return findExecutable(
329-
"chromedriver", CHROME_DRIVER_EXE_PROPERTY,
330-
"https://chromedriver.chromium.org/",
331-
"https://chromedriver.chromium.org/downloads");
332-
}
333-
334337
@Override
335338
protected List<String> createArgs() {
336339
if (getLogFile() == null) {

java/src/org/openqa/selenium/edge/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ java_export(
1818
"//java/src/org/openqa/selenium:core",
1919
"//java/src/org/openqa/selenium/chromium",
2020
"//java/src/org/openqa/selenium/remote",
21+
"//java/src/org/openqa/selenium/manager:manager",
2122
artifact("com.google.guava:guava"),
2223
],
2324
)

java/src/org/openqa/selenium/edge/EdgeDriver.java

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.openqa.selenium.remote.CommandInfo;
2727
import org.openqa.selenium.remote.RemoteWebDriver;
2828
import org.openqa.selenium.remote.RemoteWebDriverBuilder;
29+
import org.openqa.selenium.remote.service.DriverFinder;
2930
import org.openqa.selenium.remote.service.DriverService;
3031

3132
import java.util.Map;
@@ -51,11 +52,21 @@ public EdgeDriver(EdgeDriverService service) {
5152
}
5253

5354
public EdgeDriver(EdgeDriverService service, EdgeOptions options) {
54-
super(new EdgeDriverCommandExecutor(service), Require.nonNull("Driver options", options), EdgeOptions.CAPABILITY);
55+
super(generateExecutor(service, options), options, EdgeOptions.CAPABILITY);
5556
casting = new AddHasCasting().getImplementation(getCapabilities(), getExecuteMethod());
5657
cdp = new AddHasCdp().getImplementation(getCapabilities(), getExecuteMethod());
5758
}
5859

60+
private static EdgeDriverCommandExecutor generateExecutor(EdgeDriverService service, EdgeOptions options) {
61+
Require.nonNull("Driver service", service);
62+
Require.nonNull("Driver options", options);
63+
if (service.getExecutable() == null) {
64+
String path = DriverFinder.getPath(service, options);
65+
service.setExecutable(path);
66+
}
67+
return new EdgeDriverCommandExecutor(service);
68+
}
69+
5970
@Beta
6071
public static RemoteWebDriverBuilder builder() {
6172
return RemoteWebDriver.builder().oneOf(new EdgeOptions());

java/src/org/openqa/selenium/edge/EdgeDriverInfo.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.openqa.selenium.WebDriverException;
2626
import org.openqa.selenium.WebDriverInfo;
2727
import org.openqa.selenium.chromium.ChromiumDriverInfo;
28+
import org.openqa.selenium.remote.service.DriverFinder;
2829

2930
import java.util.Optional;
3031

@@ -65,7 +66,7 @@ public boolean isSupportingBiDi() {
6566
@Override
6667
public boolean isAvailable() {
6768
try {
68-
EdgeDriverService.createDefaultService();
69+
DriverFinder.getPath(EdgeDriverService.createDefaultService());
6970
return true;
7071
} catch (IllegalStateException | WebDriverException e) {
7172
return false;

java/src/org/openqa/selenium/edge/EdgeDriverService.java

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.openqa.selenium.WebDriverException;
2424
import org.openqa.selenium.chromium.ChromiumDriverLogLevel;
2525
import org.openqa.selenium.remote.service.DriverService;
26+
import org.openqa.selenium.remote.service.DriverServiceInfo;
2627

2728
import java.io.File;
2829
import java.io.IOException;
@@ -41,6 +42,8 @@
4142
*/
4243
public class EdgeDriverService extends DriverService {
4344

45+
public static final String EDGE_DRIVER_NAME = "msedgedriver";
46+
4447
/**
4548
* System property that defines the location of the MSEdgeDriver executable that will be used by
4649
* the default service.
@@ -110,11 +113,19 @@ public EdgeDriverService(
110113
unmodifiableMap(new HashMap<>(environment)));
111114
}
112115

116+
public String getDriverName() {
117+
return EDGE_DRIVER_NAME;
118+
}
119+
120+
public String getDriverProperty() {
121+
return EDGE_DRIVER_EXE_PROPERTY;
122+
}
123+
113124
/**
114125
* Configures and returns a new {@link EdgeDriverService} using the default configuration. In
115126
* this configuration, the service will use the MSEdgeDriver executable identified by the
116-
* {@link #EDGE_DRIVER_EXE_PROPERTY} system property. Each service created by this method will
117-
* be configured to use a free port on the current system.
127+
* {@link org.openqa.selenium.remote.service.DriverFinder#getPath(DriverServiceInfo, Capabilities)}.
128+
* Each service created by this method will be configured to use a free port on the current system.
118129
*
119130
* @return A new ChromiumEdgeDriverService using the default configuration.
120131
*/
@@ -129,7 +140,7 @@ public static EdgeDriverService createDefaultService() {
129140
* @return Whether the browser driver path was found.
130141
*/
131142
static boolean isPresent() {
132-
return findExePath("msedgedriver", EDGE_DRIVER_EXE_PROPERTY) != null;
143+
return findExePath(EDGE_DRIVER_NAME, EDGE_DRIVER_EXE_PROPERTY) != null;
133144
}
134145

135146

@@ -263,14 +274,6 @@ public Builder withReadableTimestamp(Boolean readableTimestamp) {
263274
return this;
264275
}
265276

266-
@Override
267-
protected File findDefaultExecutable() {
268-
return findExecutable(
269-
"msedgedriver", EDGE_DRIVER_EXE_PROPERTY,
270-
"https://docs.microsoft.com/en-us/microsoft-edge/webdriver-chromium/",
271-
"https://developer.microsoft.com/en-us/microsoft-edge/tools/webdriver/");
272-
}
273-
274277
@Override
275278
protected List<String> createArgs() {
276279
if (getLogFile() == null) {

java/src/org/openqa/selenium/firefox/BUILD.bazel

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ java_export(
1414
"//java/src/org/openqa/selenium/devtools/v85",
1515
"//java/src/org/openqa/selenium/json",
1616
"//java/src/org/openqa/selenium/remote",
17-
artifact("com.google.guava:guava"),
17+
"//java/src/org/openqa/selenium/manager:manager",
18+
artifact("com.google.guava:guava"),
1819
],
1920
)

java/src/org/openqa/selenium/firefox/FirefoxDriver.java

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
import org.openqa.selenium.remote.http.ClientConfig;
5353
import org.openqa.selenium.remote.http.HttpClient;
5454
import org.openqa.selenium.remote.service.DriverCommandExecutor;
55+
import org.openqa.selenium.remote.service.DriverFinder;
5556
import org.openqa.selenium.remote.service.DriverService;
5657

5758
import java.net.URI;
@@ -108,7 +109,7 @@ public FirefoxDriver() {
108109
* @see #FirefoxDriver(FirefoxDriverService, FirefoxOptions)
109110
*/
110111
public FirefoxDriver(FirefoxOptions options) {
111-
this(new FirefoxDriverCommandExecutor(GeckoDriverService.createDefaultService()), options);
112+
this(GeckoDriverService.createDefaultService(), options);
112113
}
113114

114115
/**
@@ -123,7 +124,17 @@ public FirefoxDriver(FirefoxDriverService service) {
123124
}
124125

125126
public FirefoxDriver(FirefoxDriverService service, FirefoxOptions options) {
126-
this(new FirefoxDriverCommandExecutor(service), Require.nonNull("Driver options", options));
127+
this(generateExecutor(service, options), options);
128+
}
129+
130+
private static FirefoxDriverCommandExecutor generateExecutor(FirefoxDriverService service, FirefoxOptions options) {
131+
Require.nonNull("Driver service", service);
132+
Require.nonNull("Driver options", options);
133+
if (service.getExecutable() == null) {
134+
String path = DriverFinder.getPath(service, options);
135+
service.setExecutable(path);
136+
}
137+
return new FirefoxDriverCommandExecutor(service);
127138
}
128139

129140
private FirefoxDriver(FirefoxDriverCommandExecutor executor, FirefoxOptions options) {

0 commit comments

Comments
 (0)