Skip to content

Commit

Permalink
[java] parse log output to support streams and file location in syste…
Browse files Browse the repository at this point in the history 
…m properties (#12674)

Co-authored-by: Diego Molina <[email protected]>
  • Loading branch information
@titusfortner @diemol
titusfortner and diemol authored Sep 4, 2023
1 parent a6173b0 commit 839a598
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 77 deletions.
26 changes: 10 additions & 16 deletions java/src/org/openqa/selenium/chrome/ChromeDriverService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import com.google.common.io.ByteStreams;
import org.openqa.selenium.Capabilities;
import org.openqa.selenium.WebDriverException;
import org.openqa.selenium.chromium.ChromiumDriverLogLevel;
Expand Down Expand Up @@ -287,12 +289,7 @@ public Builder withReadableTimestamp(Boolean readableTimestamp) {

@Override
protected void loadSystemProperties() {
if (getLogFile() == null) {
String logFilePath = System.getProperty(CHROME_DRIVER_LOG_PROPERTY);
if (logFilePath != null) {
withLogFile(new File(logFilePath));
}
}
parseLogOutput(CHROME_DRIVER_LOG_PROPERTY);
if (disableBuildCheck == null) {
this.disableBuildCheck = Boolean.getBoolean(CHROME_DRIVER_DISABLE_BUILD_CHECK);
}
Expand Down Expand Up @@ -322,17 +319,17 @@ protected List<String> createArgs() {
List<String> args = new ArrayList<>();
args.add(String.format("--port=%d", getPort()));

// 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
// Readable timestamp and append logs only work if log path is specified in args
// Cannot use logOutput because goog:loggingPrefs requires --log-path get sent
if (getLogFile() != null) {
args.add(String.format("--log-path=%s", getLogFile().getAbsolutePath()));
if (readableTimestamp != null && readableTimestamp.equals(Boolean.TRUE)) {
if (Boolean.TRUE.equals(readableTimestamp)) {
args.add("--readable-timestamp");
}
if (appendLog != null && appendLog.equals(Boolean.TRUE)) {
if (Boolean.TRUE.equals(appendLog)) {
args.add("--append-log");
}
withLogFile(null); // Do not overwrite in sendOutputTo()
withLogOutput(ByteStreams.nullOutputStream()); // Do not overwrite log file in getLogOutput()
}

if (logLevel != null) {
Expand All @@ -341,7 +338,7 @@ protected List<String> createArgs() {
if (allowedListIps != null) {
args.add(String.format("--allowed-ips=%s", allowedListIps));
}
if (disableBuildCheck != null && disableBuildCheck.equals(Boolean.TRUE)) {
if (Boolean.TRUE.equals(disableBuildCheck)) {
args.add("--disable-build-check");
}

Expand All @@ -352,10 +349,7 @@ protected List<String> createArgs() {
protected ChromeDriverService createDriverService(
File exe, int port, Duration timeout, List<String> args, Map<String, String> environment) {
try {
ChromeDriverService service =
new ChromeDriverService(exe, port, timeout, args, environment);
service.sendOutputTo(getLogOutput(CHROME_DRIVER_LOG_PROPERTY));
return service;
return new ChromeDriverService(exe, port, timeout, args, environment);
} catch (IOException e) {
throw new WebDriverException(e);
}
Expand Down
29 changes: 12 additions & 17 deletions java/src/org/openqa/selenium/edge/EdgeDriverService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import com.google.common.io.ByteStreams;
import org.openqa.selenium.Capabilities;
import org.openqa.selenium.WebDriverException;
import org.openqa.selenium.chromium.ChromiumDriverLogLevel;
Expand Down Expand Up @@ -255,12 +257,7 @@ public Builder withReadableTimestamp(Boolean readableTimestamp) {

@Override
protected void loadSystemProperties() {
if (getLogFile() == null) {
String logFilePath = System.getProperty(EDGE_DRIVER_LOG_PROPERTY);
if (logFilePath != null) {
withLogFile(new File(logFilePath));
}
}
parseLogOutput(EDGE_DRIVER_LOG_PROPERTY);
if (disableBuildCheck == null) {
this.disableBuildCheck = Boolean.getBoolean(EDGE_DRIVER_DISABLE_BUILD_CHECK);
}
Expand Down Expand Up @@ -290,32 +287,32 @@ protected List<String> createArgs() {
List<String> args = new ArrayList<>();
args.add(String.format("--port=%d", getPort()));

// 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
// Readable timestamp and append logs only work if log path is specified in args
// Cannot use logOutput because goog:loggingPrefs requires --log-path get sent
if (getLogFile() != null) {
args.add(String.format("--log-path=%s", getLogFile().getAbsolutePath()));
if (readableTimestamp != null && readableTimestamp.equals(Boolean.TRUE)) {
if (Boolean.TRUE.equals(readableTimestamp)) {
args.add("--readable-timestamp");
}
if (appendLog != null && appendLog.equals(Boolean.TRUE)) {
if (Boolean.TRUE.equals(appendLog)) {
args.add("--append-log");
}
withLogFile(null); // Do not overwrite in sendOutputTo()
withLogOutput(ByteStreams.nullOutputStream()); // Do not overwrite log file in getLogOutput()
}

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

Expand All @@ -326,9 +323,7 @@ protected List<String> createArgs() {
protected EdgeDriverService createDriverService(
File exe, int port, Duration timeout, List<String> args, Map<String, String> environment) {
try {
EdgeDriverService service = new EdgeDriverService(exe, port, timeout, args, environment);
service.sendOutputTo(getLogOutput(EDGE_DRIVER_LOG_PROPERTY));
return service;
return new EdgeDriverService(exe, port, timeout, args, environment);
} catch (IOException e) {
throw new WebDriverException(e);
}
Expand Down
5 changes: 2 additions & 3 deletions java/src/org/openqa/selenium/firefox/GeckoDriverService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ public GeckoDriverService.Builder withProfileRoot(File root) {

@Override
protected void loadSystemProperties() {
parseLogOutput(GECKO_DRIVER_LOG_PROPERTY);
if (logLevel == null) {
String logFilePath = System.getProperty(GECKO_DRIVER_LOG_LEVEL_PROPERTY);
if (logFilePath != null) {
Expand Down Expand Up @@ -304,9 +305,7 @@ protected List<String> createArgs() {
protected GeckoDriverService createDriverService(
File exe, int port, Duration timeout, List<String> args, Map<String, String> environment) {
try {
GeckoDriverService service = new GeckoDriverService(exe, port, timeout, args, environment);
service.sendOutputTo(getLogOutput(GECKO_DRIVER_LOG_PROPERTY));
return service;
return new GeckoDriverService(exe, port, timeout, args, environment);
} catch (IOException e) {
throw new WebDriverException(e);
}
Expand Down
14 changes: 3 additions & 11 deletions java/src/org/openqa/selenium/ie/InternetExplorerDriverService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,12 +187,7 @@ public Builder withSilent(Boolean silent) {

@Override
protected void loadSystemProperties() {
if (getLogFile() == null) {
String logFilePath = System.getProperty(IE_DRIVER_LOGFILE_PROPERTY);
if (logFilePath != null) {
withLogFile(new File(logFilePath));
}
}
parseLogOutput(IE_DRIVER_LOGFILE_PROPERTY);
if (logLevel == null) {
String level = System.getProperty(IE_DRIVER_LOGLEVEL_PROPERTY);
if (level != null) {
Expand Down Expand Up @@ -233,7 +228,7 @@ protected List<String> createArgs() {
if (extractPath != null) {
args.add(String.format("--extract-path=\"%s\"", extractPath.getAbsolutePath()));
}
if (silent != null && silent.equals(Boolean.TRUE)) {
if (Boolean.TRUE.equals(silent)) {
args.add("--silent");
}

Expand All @@ -244,10 +239,7 @@ protected List<String> createArgs() {
protected InternetExplorerDriverService createDriverService(
File exe, int port, Duration timeout, List<String> args, Map<String, String> environment) {
try {
InternetExplorerDriverService service =
new InternetExplorerDriverService(exe, port, timeout, args, environment);
service.sendOutputTo(getLogOutput(IE_DRIVER_LOGFILE_PROPERTY));
return service;
return new InternetExplorerDriverService(exe, port, timeout, args, environment);
} catch (IOException e) {
throw new WebDriverException(e);
}
Expand Down
54 changes: 26 additions & 28 deletions java/src/org/openqa/selenium/remote/service/DriverService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -437,40 +437,36 @@ protected Duration getDefaultTimeout() {
return DEFAULT_TIMEOUT;
}

protected OutputStream getLogOutput(String logProperty) {
if (logOutputStream != null) {
return logOutputStream;
}

protected OutputStream getLogOutput() {
try {
File logFileLocation = getLogFile();
String logLocation;

if (logFileLocation == null) {
logLocation = System.getProperty(logProperty);
} else {
logLocation = logFileLocation.getAbsolutePath();
}

if (logLocation == null) {
return ByteStreams.nullOutputStream();
}

switch (logLocation) {
case LOG_STDOUT:
return System.out;
case LOG_STDERR:
return System.err;
case LOG_NULL:
return ByteStreams.nullOutputStream();
default:
return new FileOutputStream(logLocation);
}
return logOutputStream != null ? logOutputStream : new FileOutputStream(logFile);

This comment has been minimized.

Sign in to view

Copy link
@vnkunta

vnkunta Sep 6, 2023

Not sure if this is the right forum , but this is impacting the existing AppiumDriverLocalService. The following code throws null pointer excpetion.
AppiumDriverLocalService.buildDefaultService();
It forces us to pass output stream or logfile .
thanks,
naveen

This comment has been minimized.

Sign in to view

Copy link
@vnkunta

vnkunta Sep 6, 2023

Raised an issue in appium as well appium/java-client#2004
} catch (FileNotFoundException e) {
throw new RuntimeException(e);
}
}

protected void parseLogOutput(String logProperty) {
if (getLogFile() != null) {
return;
}

String logLocation = System.getProperty(logProperty, LOG_NULL);
switch (logLocation) {
case LOG_STDOUT:
withLogOutput(System.out);
break;
case LOG_STDERR:
withLogOutput(System.err);
break;
case LOG_NULL:
withLogOutput(ByteStreams.nullOutputStream());
break;
default:
withLogFile(new File(logLocation));
break;
}
}

/**
* Creates a new service to manage the driver server. Before creating a new service, the builder
* will find a port for the server to listen to.
Expand All @@ -490,6 +486,8 @@ public DS build() {
List<String> args = createArgs();

DS service = createDriverService(exe, port, timeout, args, environment);
service.sendOutputTo(getLogOutput());

port = 0; // reset port to allow reusing this builder

return service;
Expand Down
5 changes: 4 additions & 1 deletion java/src/org/openqa/selenium/safari/SafariDriverService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import com.google.common.io.ByteStreams;
import org.openqa.selenium.Capabilities;
import org.openqa.selenium.WebDriverException;
import org.openqa.selenium.net.PortProber;
Expand Down Expand Up @@ -168,7 +170,7 @@ protected void loadSystemProperties() {
@Override
protected List<String> createArgs() {
List<String> args = new ArrayList<>(Arrays.asList("--port", String.valueOf(getPort())));
if (diagnose != null && diagnose.equals(Boolean.TRUE)) {
if (Boolean.TRUE.equals(diagnose)) {
args.add("--diagnose");
}
return args;
Expand All @@ -178,6 +180,7 @@ protected List<String> createArgs() {
protected SafariDriverService createDriverService(
File exe, int port, Duration timeout, List<String> args, Map<String, String> environment) {
try {
withLogOutput(ByteStreams.nullOutputStream());
return new SafariDriverService(exe, port, timeout, args, environment);
} catch (IOException e) {
throw new WebDriverException(e);
Expand Down
5 changes: 4 additions & 1 deletion java/src/org/openqa/selenium/safari/SafariTechPreviewDriverService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import com.google.common.io.ByteStreams;
import org.openqa.selenium.Capabilities;
import org.openqa.selenium.WebDriverException;
import org.openqa.selenium.net.PortProber;
Expand Down Expand Up @@ -172,7 +174,7 @@ protected void loadSystemProperties() {
@Override
protected List<String> createArgs() {
List<String> args = new ArrayList<>(Arrays.asList("--port", String.valueOf(getPort())));
if (this.diagnose) {
if (Boolean.TRUE.equals(diagnose)) {
args.add("--diagnose");
}
return args;
Expand All @@ -182,6 +184,7 @@ protected List<String> createArgs() {
protected SafariTechPreviewDriverService createDriverService(
File exe, int port, Duration timeout, List<String> args, Map<String, String> environment) {
try {
withLogOutput(ByteStreams.nullOutputStream());
return new SafariTechPreviewDriverService(exe, port, timeout, args, environment);
} catch (IOException e) {
throw new WebDriverException(e);
Expand Down

0 comments on commit 839a598

Please sign in to comment.