Skip to content

Commit

Permalink
Make Driver Services consistent (#11973)
Browse files Browse the repository at this point in the history 
* [java] deprecate the old driver service constructors that hard coded the timeout value

Users should be using the builder

* [java] make javadoc and formatting consistent across driver services

* [java] deprecate unused and redundant geckodriver service methods

* [java] add support for safaridriver logging with service class

* [java] implement additional geckodriver service features

Allow log level to be set in service class, not just in options class
Allow Toggling truncation of log lines
Allow users to specify a different location for profile creation

* [java] implement driver service logging with sendOutputTo for all drivers

This allows completely silencing all logging output or directing everything from the process to a specified file

* [java] process system properties at run time not compile time

* [java] fix formatting

* Addressing PR comments and making it compile

* [Java] Conserving behaviour for setting log level

---------

Co-authored-by: Diego Molina <diemol@gmail.com>
Co-authored-by: Diego Molina <diemol@users.noreply.github.com>
  • Loading branch information
@titusfortner @diemol
3 people committed May 4, 2023
1 parent d530584 commit 66e51be
Show file tree
Hide file tree
Showing 10 changed files with 646 additions and 304 deletions.
186 changes: 110 additions & 76 deletions java/src/org/openqa/selenium/chrome/ChromeDriverService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,13 @@ public class ChromeDriverService extends DriverService {
public static final String CHROME_DRIVER_READABLE_TIMESTAMP = "webdriver.chrome.readableTimestamp";

/**
* System property that defines the default location where ChromeDriver output is logged.
* System property that defines the location of the file where ChromeDriver should write log
* messages to.
*/
public static final String CHROME_DRIVER_LOG_PROPERTY = "webdriver.chrome.logfile";

/**
* System property that defines the log level when ChromeDriver output is logged.
* System property that defines the {@link ChromiumDriverLogLevel} for ChromeDriver logs.
*/
public static final String CHROME_DRIVER_LOG_LEVEL_PROPERTY = "webdriver.chrome.loglevel";

Expand All @@ -70,34 +71,35 @@ public class ChromeDriverService extends DriverService {
public static final String CHROME_DRIVER_APPEND_LOG_PROPERTY = "webdriver.chrome.appendLog";

/**
* Boolean system property that defines whether the ChromeDriver executable should be started
* with verbose logging.
* Boolean system property that defines whether the ChromeDriver executable should be started with
* verbose logging.
*/
public static final String CHROME_DRIVER_VERBOSE_LOG_PROPERTY = "webdriver.chrome.verboseLogging";

/**
* Boolean system property that defines whether the ChromeDriver executable should be started
* in silent mode.
* Boolean system property that defines whether the ChromeDriver executable should be started in
* silent mode.
*/
public static final String CHROME_DRIVER_SILENT_OUTPUT_PROPERTY = "webdriver.chrome.silentOutput";

/**
* System property that defines comma-separated list of remote IPv4 addresses which are
* allowed to connect to ChromeDriver.
* System property that defines comma-separated list of remote IPv4 addresses which are allowed to
* connect to ChromeDriver.
*/
public static final String CHROME_DRIVER_ALLOWED_IPS_PROPERTY = "webdriver.chrome.withAllowedIps";

/**
* System property that defines comma-separated list of remote IPv4 addresses which are
* allowed to connect to ChromeDriver.
* System property that defines comma-separated list of remote IPv4 addresses which are allowed to
* connect to ChromeDriver.
*
* @deprecated use {@link #CHROME_DRIVER_ALLOWED_IPS_PROPERTY}
*/
@Deprecated
public static final String CHROME_DRIVER_WHITELISTED_IPS_PROPERTY = "webdriver.chrome.whitelistedIps";

/**
* System property that defines whether the ChromeDriver executable should check for build
* version compatibility between ChromeDriver and the browser.
* System property that defines whether the ChromeDriver executable should check for build version
* compatibility between ChromeDriver and the browser.
*/
public static final String CHROME_DRIVER_DISABLE_BUILD_CHECK = "webdriver.chrome.disableBuildCheck";

Expand All @@ -107,13 +109,15 @@ public class ChromeDriverService extends DriverService {
* @param args The arguments to the launched server.
* @param environment The environment for the launched server.
* @throws IOException If an I/O error occurs.
* @deprecated use {@link ChromeDriverService#ChromeDriverService(File, int, Duration, List, Map)}
*/
@Deprecated
public ChromeDriverService(
File executable,
int port,
List<String> args,
Map<String, String> environment) throws IOException {
super(executable, port, DEFAULT_TIMEOUT,
this(executable, port, DEFAULT_TIMEOUT,
unmodifiableList(new ArrayList<>(args)),
unmodifiableMap(new HashMap<>(environment)));
}
Expand All @@ -127,11 +131,11 @@ public ChromeDriverService(
* @throws IOException If an I/O error occurs.
*/
public ChromeDriverService(
File executable,
int port,
Duration timeout,
List<String> args,
Map<String, String> environment) throws IOException {
File executable,
int port,
Duration timeout,
List<String> args,
Map<String, String> environment) throws IOException {
super(executable, port, timeout,
unmodifiableList(new ArrayList<>(args)),
unmodifiableMap(new HashMap<>(environment)));
Expand All @@ -154,7 +158,8 @@ 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 service created by this method will be configured to use a free port on the current system.
* 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.
*/
Expand All @@ -166,7 +171,8 @@ public static ChromeDriverService createDefaultService() {
* Configures and returns a new {@link ChromeDriverService} using the supplied configuration. In
* this configuration, the service will use the ChromeDriver executable identified by
* {@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.
* Each service created by this method will be configured to use a free port on the current
* system.
*
* @return A new ChromeDriverService using the supplied configuration from {@link ChromeOptions}.
* @deprecated Use {@link Builder#withLogLevel(ChromiumDriverLogLevel)} }
Expand All @@ -175,15 +181,15 @@ public static ChromeDriverService createDefaultService() {
public static ChromeDriverService createServiceWithConfig(ChromeOptions options) {
ChromeDriverLogLevel oldLevel = options.getLogLevel();
ChromiumDriverLogLevel level = (oldLevel == null) ? null :
ChromiumDriverLogLevel.fromString(oldLevel.toString());
ChromiumDriverLogLevel.fromString(oldLevel.toString());
return new Builder()
.withLogLevel(level)
.build();
}

/**
* Checks if the browser driver binary is already present. Grid uses this method to show
* the available browsers and drivers, hence its visibility.
* Checks if the ChromeDriver binary is already present. Grid uses this method to show the
* available browsers and drivers, hence its visibility.
*
* @return Whether the browser driver path was found.
*/
Expand All @@ -198,16 +204,13 @@ static boolean isPresent() {
public static class Builder extends DriverService.Builder<
ChromeDriverService, ChromeDriverService.Builder> {

private boolean disableBuildCheck = Boolean.getBoolean(CHROME_DRIVER_DISABLE_BUILD_CHECK);
private boolean readableTimestamp = Boolean.getBoolean(CHROME_DRIVER_READABLE_TIMESTAMP);
private boolean appendLog = Boolean.getBoolean(CHROME_DRIVER_APPEND_LOG_PROPERTY);
private boolean verbose = Boolean.getBoolean(CHROME_DRIVER_VERBOSE_LOG_PROPERTY);
private boolean silent = Boolean.getBoolean(CHROME_DRIVER_SILENT_OUTPUT_PROPERTY);
private String allowedListIps = System.getProperty(CHROME_DRIVER_ALLOWED_IPS_PROPERTY,
System.getProperty(
CHROME_DRIVER_WHITELISTED_IPS_PROPERTY));
private ChromiumDriverLogLevel logLevel = ChromiumDriverLogLevel
.fromString(System.getProperty(CHROME_DRIVER_LOG_LEVEL_PROPERTY));
private Boolean disableBuildCheck;
private Boolean readableTimestamp;
private Boolean appendLog;
private Boolean verbose;
private Boolean silent;
private String allowedListIps;
private ChromiumDriverLogLevel logLevel;

@Override
public int score(Capabilities capabilities) {
Expand Down Expand Up @@ -246,21 +249,6 @@ public Builder withBuildCheckDisabled(boolean noBuildCheck) {
return this;
}

/**
* Configures the driver server verbosity.
*
* @param verbose Log all output for true, no changes made if false.
* @return A self reference.
*/
@SuppressWarnings("UnusedReturnValue")
public Builder withVerbose(boolean verbose) {
if (verbose) {
this.logLevel = ChromiumDriverLogLevel.ALL;
}
this.verbose = false;
return this;
}

/**
* Configures the driver server verbosity.
*
Expand All @@ -270,20 +258,22 @@ public Builder withVerbose(boolean verbose) {
*/
@Deprecated
public Builder withLogLevel(ChromeDriverLogLevel logLevel) {
this.verbose = false;
this.silent = false;
this.logLevel = ChromiumDriverLogLevel.fromString(logLevel.toString());
this.silent = false;
this.verbose = false;
return this;
}

/**
* Configures the driver server verbosity.
* Configures the driver server log level.
*
* @param logLevel {@link ChromiumDriverLogLevel} for desired log level output.
* @return A self reference.
*/
public Builder withLogLevel(ChromiumDriverLogLevel logLevel) {
this.logLevel = logLevel;
this.silent = false;
this.verbose = false;
return this;
}

Expand All @@ -302,8 +292,22 @@ public Builder withSilent(boolean silent) {
}

/**
* Configures the comma-separated list of remote IPv4 addresses which are allowed to connect
* to the driver server.
* Configures the driver server verbosity.
*
* @param verbose Log all output for true, no changes made if false.
* @return A self reference.
*/
public Builder withVerbose(boolean verbose) {
if (verbose) {
this.logLevel = ChromiumDriverLogLevel.ALL;
}
this.verbose = false;
return this;
}

/**
* Configures the comma-separated list of remote IPv4 addresses which are allowed to connect to
* the driver server.
*
* @param allowedListIps Comma-separated list of remote IPv4 addresses.
* @return A self reference.
Expand All @@ -316,8 +320,8 @@ public Builder withWhitelistedIps(String allowedListIps) {
}

/**
* Configures the comma-separated list of remote IPv4 addresses which are allowed to connect
* to the driver server.
* Configures the comma-separated list of remote IPv4 addresses which are allowed to connect to
* the driver server.
*
* @param allowedListIps Comma-separated list of remote IPv4 addresses.
* @return A self reference.
Expand All @@ -339,42 +343,69 @@ public Builder withReadableTimestamp(Boolean readableTimestamp) {
}

@Override
protected List<String> createArgs() {
protected void loadSystemProperties() {
if (getLogFile() == null) {
String logFilePath = System.getProperty(CHROME_DRIVER_LOG_PROPERTY);
if (logFilePath != null) {
withLogFile(new File(logFilePath));
}
}

// If set in properties and not overwritten by method
if (verbose) {
withVerbose(true);
if (disableBuildCheck == null) {
this.disableBuildCheck = Boolean.getBoolean(CHROME_DRIVER_DISABLE_BUILD_CHECK);
}
if (silent) {
withSilent(true);
if (readableTimestamp == null) {
this.readableTimestamp = Boolean.getBoolean(CHROME_DRIVER_READABLE_TIMESTAMP);
}
if (appendLog == null) {
this.appendLog = Boolean.getBoolean(CHROME_DRIVER_APPEND_LOG_PROPERTY);
}
if (verbose == null && Boolean.getBoolean(CHROME_DRIVER_VERBOSE_LOG_PROPERTY)) {
withVerbose(Boolean.getBoolean(CHROME_DRIVER_VERBOSE_LOG_PROPERTY));
}
if (silent == null && Boolean.getBoolean(CHROME_DRIVER_SILENT_OUTPUT_PROPERTY)) {
withSilent(Boolean.getBoolean(CHROME_DRIVER_SILENT_OUTPUT_PROPERTY));
}
if (allowedListIps == null) {
this.allowedListIps = System.getProperty(CHROME_DRIVER_ALLOWED_IPS_PROPERTY,
System.getProperty(CHROME_DRIVER_WHITELISTED_IPS_PROPERTY));
}
if (logLevel == null && System.getProperty(CHROME_DRIVER_LOG_LEVEL_PROPERTY) != null) {
String level = System.getProperty(CHROME_DRIVER_LOG_LEVEL_PROPERTY);
withLogLevel(ChromiumDriverLogLevel.fromString(level));
}
}

@Override
protected List<String> createArgs() {
List<String> args = new ArrayList<>();

args.add(String.format("--port=%d", getPort()));
if (getLogFile() != null) {

// Readable timestamp and append logs only work if a file is specified
// Can only get readable logs via arguments; otherwise send service output as directed
if (getLogFile() != null && (readableTimestamp || appendLog)) {
args.add(String.format("--log-path=%s", getLogFile().getAbsolutePath()));
// This flag only works when logged to file
if (readableTimestamp) {
if (readableTimestamp != null && readableTimestamp.equals(Boolean.TRUE)) {
args.add("--readable-timestamp");
}
if (appendLog != null && appendLog.equals(Boolean.TRUE)) {
args.add("--append-log");
}
withLogFile(null); // Do not overwrite in sendOutputTo()
}
if (appendLog) {
args.add("--append-log");
}

if (logLevel != null) {
args.add(String.format("--log-level=%s", logLevel.toString().toUpperCase()));
}
// if (silent != null && silent.equals(Boolean.TRUE)) {
// args.add("--silent");
// }
// if (verbose != null && verbose.equals(Boolean.TRUE)) {
// args.add("--verbose");
// }
if (allowedListIps != null) {
args.add(String.format("--allowed-ips=%s", allowedListIps));
}
if (disableBuildCheck) {
if (disableBuildCheck != null && disableBuildCheck.equals(Boolean.TRUE)) {
args.add("--disable-build-check");
}

Expand All @@ -383,13 +414,16 @@ protected List<String> createArgs() {

@Override
protected ChromeDriverService createDriverService(
File exe,
int port,
Duration timeout,
List<String> args,
Map<String, String> environment) {
File exe,
int port,
Duration timeout,
List<String> args,
Map<String, String> environment) {
try {
return new ChromeDriverService(exe, port, timeout, args, environment);
ChromeDriverService service = new ChromeDriverService(exe, port, timeout, args,
environment);
service.sendOutputTo(getLogOutput(CHROME_DRIVER_LOG_PROPERTY));
return service;
} catch (IOException e) {
throw new WebDriverException(e);
}
Expand Down
Loading

0 comments on commit 66e51be

Please sign in to comment.