Skip to content

Commit 839a598

Browse files
titusfortnerdiemol
titusfortner
and
diemol
authored
[java] parse log output to support streams and file location in system properties (#12674)
Co-authored-by: Diego Molina <[email protected]>
1 parent a6173b0 commit 839a598

File tree

7 files changed

+61
-77
lines changed

7 files changed

+61
-77
lines changed

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

+10-16
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
import java.util.HashMap;
3030
import java.util.List;
3131
import java.util.Map;
32+
33+
import com.google.common.io.ByteStreams;
3234
import org.openqa.selenium.Capabilities;
3335
import org.openqa.selenium.WebDriverException;
3436
import org.openqa.selenium.chromium.ChromiumDriverLogLevel;
@@ -287,12 +289,7 @@ public Builder withReadableTimestamp(Boolean readableTimestamp) {
287289

288290
@Override
289291
protected void loadSystemProperties() {
290-
if (getLogFile() == null) {
291-
String logFilePath = System.getProperty(CHROME_DRIVER_LOG_PROPERTY);
292-
if (logFilePath != null) {
293-
withLogFile(new File(logFilePath));
294-
}
295-
}
292+
parseLogOutput(CHROME_DRIVER_LOG_PROPERTY);
296293
if (disableBuildCheck == null) {
297294
this.disableBuildCheck = Boolean.getBoolean(CHROME_DRIVER_DISABLE_BUILD_CHECK);
298295
}
@@ -322,17 +319,17 @@ protected List<String> createArgs() {
322319
List<String> args = new ArrayList<>();
323320
args.add(String.format("--port=%d", getPort()));
324321

325-
// Readable timestamp and append logs only work if a file is specified
326-
// Can only get readable logs via arguments; otherwise send service output as directed
322+
// Readable timestamp and append logs only work if log path is specified in args
323+
// Cannot use logOutput because goog:loggingPrefs requires --log-path get sent
327324
if (getLogFile() != null) {
328325
args.add(String.format("--log-path=%s", getLogFile().getAbsolutePath()));
329-
if (readableTimestamp != null && readableTimestamp.equals(Boolean.TRUE)) {
326+
if (Boolean.TRUE.equals(readableTimestamp)) {
330327
args.add("--readable-timestamp");
331328
}
332-
if (appendLog != null && appendLog.equals(Boolean.TRUE)) {
329+
if (Boolean.TRUE.equals(appendLog)) {
333330
args.add("--append-log");
334331
}
335-
withLogFile(null); // Do not overwrite in sendOutputTo()
332+
withLogOutput(ByteStreams.nullOutputStream()); // Do not overwrite log file in getLogOutput()
336333
}
337334

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

@@ -352,10 +349,7 @@ protected List<String> createArgs() {
352349
protected ChromeDriverService createDriverService(
353350
File exe, int port, Duration timeout, List<String> args, Map<String, String> environment) {
354351
try {
355-
ChromeDriverService service =
356-
new ChromeDriverService(exe, port, timeout, args, environment);
357-
service.sendOutputTo(getLogOutput(CHROME_DRIVER_LOG_PROPERTY));
358-
return service;
352+
return new ChromeDriverService(exe, port, timeout, args, environment);
359353
} catch (IOException e) {
360354
throw new WebDriverException(e);
361355
}

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

+12-17
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@
3030
import java.util.HashMap;
3131
import java.util.List;
3232
import java.util.Map;
33+
34+
import com.google.common.io.ByteStreams;
3335
import org.openqa.selenium.Capabilities;
3436
import org.openqa.selenium.WebDriverException;
3537
import org.openqa.selenium.chromium.ChromiumDriverLogLevel;
@@ -255,12 +257,7 @@ public Builder withReadableTimestamp(Boolean readableTimestamp) {
255257

256258
@Override
257259
protected void loadSystemProperties() {
258-
if (getLogFile() == null) {
259-
String logFilePath = System.getProperty(EDGE_DRIVER_LOG_PROPERTY);
260-
if (logFilePath != null) {
261-
withLogFile(new File(logFilePath));
262-
}
263-
}
260+
parseLogOutput(EDGE_DRIVER_LOG_PROPERTY);
264261
if (disableBuildCheck == null) {
265262
this.disableBuildCheck = Boolean.getBoolean(EDGE_DRIVER_DISABLE_BUILD_CHECK);
266263
}
@@ -290,32 +287,32 @@ protected List<String> createArgs() {
290287
List<String> args = new ArrayList<>();
291288
args.add(String.format("--port=%d", getPort()));
292289

293-
// Readable timestamp and append logs only work if a file is specified
294-
// Can only get readable logs via arguments; otherwise send service output as directed
290+
// Readable timestamp and append logs only work if log path is specified in args
291+
// Cannot use logOutput because goog:loggingPrefs requires --log-path get sent
295292
if (getLogFile() != null) {
296293
args.add(String.format("--log-path=%s", getLogFile().getAbsolutePath()));
297-
if (readableTimestamp != null && readableTimestamp.equals(Boolean.TRUE)) {
294+
if (Boolean.TRUE.equals(readableTimestamp)) {
298295
args.add("--readable-timestamp");
299296
}
300-
if (appendLog != null && appendLog.equals(Boolean.TRUE)) {
297+
if (Boolean.TRUE.equals(appendLog)) {
301298
args.add("--append-log");
302299
}
303-
withLogFile(null); // Do not overwrite in sendOutputTo()
300+
withLogOutput(ByteStreams.nullOutputStream()); // Do not overwrite log file in getLogOutput()
304301
}
305302

306303
if (logLevel != null) {
307304
args.add(String.format("--log-level=%s", logLevel.toString().toUpperCase()));
308305
}
309-
if (silent != null && silent.equals(Boolean.TRUE)) {
306+
if (Boolean.TRUE.equals(silent)) {
310307
args.add("--silent");
311308
}
312-
if (verbose != null && verbose.equals(Boolean.TRUE)) {
309+
if (Boolean.TRUE.equals(verbose)) {
313310
args.add("--verbose");
314311
}
315312
if (allowedListIps != null) {
316313
args.add(String.format("--allowed-ips=%s", allowedListIps));
317314
}
318-
if (disableBuildCheck != null && disableBuildCheck.equals(Boolean.TRUE)) {
315+
if (Boolean.TRUE.equals(disableBuildCheck)) {
319316
args.add("--disable-build-check");
320317
}
321318

@@ -326,9 +323,7 @@ protected List<String> createArgs() {
326323
protected EdgeDriverService createDriverService(
327324
File exe, int port, Duration timeout, List<String> args, Map<String, String> environment) {
328325
try {
329-
EdgeDriverService service = new EdgeDriverService(exe, port, timeout, args, environment);
330-
service.sendOutputTo(getLogOutput(EDGE_DRIVER_LOG_PROPERTY));
331-
return service;
326+
return new EdgeDriverService(exe, port, timeout, args, environment);
332327
} catch (IOException e) {
333328
throw new WebDriverException(e);
334329
}

‎java/src/org/openqa/selenium/firefox/GeckoDriverService.java

+2-3
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,7 @@ public GeckoDriverService.Builder withProfileRoot(File root) {
245245

246246
@Override
247247
protected void loadSystemProperties() {
248+
parseLogOutput(GECKO_DRIVER_LOG_PROPERTY);
248249
if (logLevel == null) {
249250
String logFilePath = System.getProperty(GECKO_DRIVER_LOG_LEVEL_PROPERTY);
250251
if (logFilePath != null) {
@@ -304,9 +305,7 @@ protected List<String> createArgs() {
304305
protected GeckoDriverService createDriverService(
305306
File exe, int port, Duration timeout, List<String> args, Map<String, String> environment) {
306307
try {
307-
GeckoDriverService service = new GeckoDriverService(exe, port, timeout, args, environment);
308-
service.sendOutputTo(getLogOutput(GECKO_DRIVER_LOG_PROPERTY));
309-
return service;
308+
return new GeckoDriverService(exe, port, timeout, args, environment);
310309
} catch (IOException e) {
311310
throw new WebDriverException(e);
312311
}

‎java/src/org/openqa/selenium/ie/InternetExplorerDriverService.java

+3-11
Original file line numberDiff line numberDiff line change
@@ -187,12 +187,7 @@ public Builder withSilent(Boolean silent) {
187187

188188
@Override
189189
protected void loadSystemProperties() {
190-
if (getLogFile() == null) {
191-
String logFilePath = System.getProperty(IE_DRIVER_LOGFILE_PROPERTY);
192-
if (logFilePath != null) {
193-
withLogFile(new File(logFilePath));
194-
}
195-
}
190+
parseLogOutput(IE_DRIVER_LOGFILE_PROPERTY);
196191
if (logLevel == null) {
197192
String level = System.getProperty(IE_DRIVER_LOGLEVEL_PROPERTY);
198193
if (level != null) {
@@ -233,7 +228,7 @@ protected List<String> createArgs() {
233228
if (extractPath != null) {
234229
args.add(String.format("--extract-path=\"%s\"", extractPath.getAbsolutePath()));
235230
}
236-
if (silent != null && silent.equals(Boolean.TRUE)) {
231+
if (Boolean.TRUE.equals(silent)) {
237232
args.add("--silent");
238233
}
239234

@@ -244,10 +239,7 @@ protected List<String> createArgs() {
244239
protected InternetExplorerDriverService createDriverService(
245240
File exe, int port, Duration timeout, List<String> args, Map<String, String> environment) {
246241
try {
247-
InternetExplorerDriverService service =
248-
new InternetExplorerDriverService(exe, port, timeout, args, environment);
249-
service.sendOutputTo(getLogOutput(IE_DRIVER_LOGFILE_PROPERTY));
250-
return service;
242+
return new InternetExplorerDriverService(exe, port, timeout, args, environment);
251243
} catch (IOException e) {
252244
throw new WebDriverException(e);
253245
}

‎java/src/org/openqa/selenium/remote/service/DriverService.java

+26-28
Original file line numberDiff line numberDiff line change
@@ -437,40 +437,36 @@ protected Duration getDefaultTimeout() {
437437
return DEFAULT_TIMEOUT;
438438
}
439439

440-
protected OutputStream getLogOutput(String logProperty) {
441-
if (logOutputStream != null) {
442-
return logOutputStream;
443-
}
444-
440+
protected OutputStream getLogOutput() {
445441
try {
446-
File logFileLocation = getLogFile();
447-
String logLocation;
448-
449-
if (logFileLocation == null) {
450-
logLocation = System.getProperty(logProperty);
451-
} else {
452-
logLocation = logFileLocation.getAbsolutePath();
453-
}
454-
455-
if (logLocation == null) {
456-
return ByteStreams.nullOutputStream();
457-
}
458-
459-
switch (logLocation) {
460-
case LOG_STDOUT:
461-
return System.out;
462-
case LOG_STDERR:
463-
return System.err;
464-
case LOG_NULL:
465-
return ByteStreams.nullOutputStream();
466-
default:
467-
return new FileOutputStream(logLocation);
468-
}
442+
return logOutputStream != null ? logOutputStream : new FileOutputStream(logFile);
469443
} catch (FileNotFoundException e) {
470444
throw new RuntimeException(e);
471445
}
472446
}
473447

448+
protected void parseLogOutput(String logProperty) {
449+
if (getLogFile() != null) {
450+
return;
451+
}
452+
453+
String logLocation = System.getProperty(logProperty, LOG_NULL);
454+
switch (logLocation) {
455+
case LOG_STDOUT:
456+
withLogOutput(System.out);
457+
break;
458+
case LOG_STDERR:
459+
withLogOutput(System.err);
460+
break;
461+
case LOG_NULL:
462+
withLogOutput(ByteStreams.nullOutputStream());
463+
break;
464+
default:
465+
withLogFile(new File(logLocation));
466+
break;
467+
}
468+
}
469+
474470
/**
475471
* Creates a new service to manage the driver server. Before creating a new service, the builder
476472
* will find a port for the server to listen to.
@@ -490,6 +486,8 @@ public DS build() {
490486
List<String> args = createArgs();
491487

492488
DS service = createDriverService(exe, port, timeout, args, environment);
489+
service.sendOutputTo(getLogOutput());
490+
493491
port = 0; // reset port to allow reusing this builder
494492

495493
return service;

‎java/src/org/openqa/selenium/safari/SafariDriverService.java

+4-1
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@
3131
import java.util.HashMap;
3232
import java.util.List;
3333
import java.util.Map;
34+
35+
import com.google.common.io.ByteStreams;
3436
import org.openqa.selenium.Capabilities;
3537
import org.openqa.selenium.WebDriverException;
3638
import org.openqa.selenium.net.PortProber;
@@ -168,7 +170,7 @@ protected void loadSystemProperties() {
168170
@Override
169171
protected List<String> createArgs() {
170172
List<String> args = new ArrayList<>(Arrays.asList("--port", String.valueOf(getPort())));
171-
if (diagnose != null && diagnose.equals(Boolean.TRUE)) {
173+
if (Boolean.TRUE.equals(diagnose)) {
172174
args.add("--diagnose");
173175
}
174176
return args;
@@ -178,6 +180,7 @@ protected List<String> createArgs() {
178180
protected SafariDriverService createDriverService(
179181
File exe, int port, Duration timeout, List<String> args, Map<String, String> environment) {
180182
try {
183+
withLogOutput(ByteStreams.nullOutputStream());
181184
return new SafariDriverService(exe, port, timeout, args, environment);
182185
} catch (IOException e) {
183186
throw new WebDriverException(e);

‎java/src/org/openqa/selenium/safari/SafariTechPreviewDriverService.java

+4-1
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@
3131
import java.util.HashMap;
3232
import java.util.List;
3333
import java.util.Map;
34+
35+
import com.google.common.io.ByteStreams;
3436
import org.openqa.selenium.Capabilities;
3537
import org.openqa.selenium.WebDriverException;
3638
import org.openqa.selenium.net.PortProber;
@@ -172,7 +174,7 @@ protected void loadSystemProperties() {
172174
@Override
173175
protected List<String> createArgs() {
174176
List<String> args = new ArrayList<>(Arrays.asList("--port", String.valueOf(getPort())));
175-
if (this.diagnose) {
177+
if (Boolean.TRUE.equals(diagnose)) {
176178
args.add("--diagnose");
177179
}
178180
return args;
@@ -182,6 +184,7 @@ protected List<String> createArgs() {
182184
protected SafariTechPreviewDriverService createDriverService(
183185
File exe, int port, Duration timeout, List<String> args, Map<String, String> environment) {
184186
try {
187+
withLogOutput(ByteStreams.nullOutputStream());
185188
return new SafariTechPreviewDriverService(exe, port, timeout, args, environment);
186189
} catch (IOException e) {
187190
throw new WebDriverException(e);

0 commit comments

Comments
 (0)