Skip to content

Commit 755ac50

Browse files
diemoldiemol
committed
[Java] Conserving behaviour for setting log level
1 parent 635b5ce commit 755ac50

4 files changed

Lines changed: 90 additions & 35 deletions

File tree

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

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -259,17 +259,21 @@ public Builder withBuildCheckDisabled(boolean noBuildCheck) {
259259
@Deprecated
260260
public Builder withLogLevel(ChromeDriverLogLevel logLevel) {
261261
this.logLevel = ChromiumDriverLogLevel.fromString(logLevel.toString());
262+
this.silent = false;
263+
this.verbose = false;
262264
return this;
263265
}
264266

265267
/**
266-
* Configures the driver server verbosity.
268+
* Configures the driver server log level.
267269
*
268270
* @param logLevel {@link ChromiumDriverLogLevel} for desired log level output.
269271
* @return A self reference.
270272
*/
271273
public Builder withLogLevel(ChromiumDriverLogLevel logLevel) {
272274
this.logLevel = logLevel;
275+
this.silent = false;
276+
this.verbose = false;
273277
return this;
274278
}
275279

@@ -280,7 +284,10 @@ public Builder withLogLevel(ChromiumDriverLogLevel logLevel) {
280284
* @return A self reference.
281285
*/
282286
public Builder withSilent(boolean silent) {
283-
this.silent = silent;
287+
if (silent) {
288+
this.logLevel = ChromiumDriverLogLevel.OFF;
289+
}
290+
this.silent = false;
284291
return this;
285292
}
286293

@@ -291,7 +298,10 @@ public Builder withSilent(boolean silent) {
291298
* @return A self reference.
292299
*/
293300
public Builder withVerbose(boolean verbose) {
294-
this.verbose = verbose;
301+
if (verbose) {
302+
this.logLevel = ChromiumDriverLogLevel.ALL;
303+
}
304+
this.verbose = false;
295305
return this;
296306
}
297307

@@ -349,21 +359,19 @@ protected void loadSystemProperties() {
349359
if (appendLog == null) {
350360
this.appendLog = Boolean.getBoolean(CHROME_DRIVER_APPEND_LOG_PROPERTY);
351361
}
352-
if (verbose == null) {
353-
this.verbose = Boolean.getBoolean(CHROME_DRIVER_VERBOSE_LOG_PROPERTY);
362+
if (verbose == null && Boolean.getBoolean(CHROME_DRIVER_VERBOSE_LOG_PROPERTY)) {
363+
withVerbose(Boolean.getBoolean(CHROME_DRIVER_VERBOSE_LOG_PROPERTY));
354364
}
355-
if (silent == null) {
356-
this.silent = Boolean.getBoolean(CHROME_DRIVER_SILENT_OUTPUT_PROPERTY);
365+
if (silent == null && Boolean.getBoolean(CHROME_DRIVER_SILENT_OUTPUT_PROPERTY)) {
366+
withSilent(Boolean.getBoolean(CHROME_DRIVER_SILENT_OUTPUT_PROPERTY));
357367
}
358368
if (allowedListIps == null) {
359369
this.allowedListIps = System.getProperty(CHROME_DRIVER_ALLOWED_IPS_PROPERTY,
360370
System.getProperty(CHROME_DRIVER_WHITELISTED_IPS_PROPERTY));
361371
}
362-
if (logLevel == null) {
372+
if (logLevel == null && System.getProperty(CHROME_DRIVER_LOG_LEVEL_PROPERTY) != null) {
363373
String level = System.getProperty(CHROME_DRIVER_LOG_LEVEL_PROPERTY);
364-
if (level != null) {
365-
this.logLevel = ChromiumDriverLogLevel.fromString(level);
366-
}
374+
withLogLevel(ChromiumDriverLogLevel.fromString(level));
367375
}
368376
}
369377

@@ -388,12 +396,12 @@ protected List<String> createArgs() {
388396
if (logLevel != null) {
389397
args.add(String.format("--log-level=%s", logLevel.toString().toUpperCase()));
390398
}
391-
if (silent != null && silent.equals(Boolean.TRUE)) {
392-
args.add("--silent");
393-
}
394-
if (verbose != null && verbose.equals(Boolean.TRUE)) {
395-
args.add("--verbose");
396-
}
399+
// if (silent != null && silent.equals(Boolean.TRUE)) {
400+
// args.add("--silent");
401+
// }
402+
// if (verbose != null && verbose.equals(Boolean.TRUE)) {
403+
// args.add("--verbose");
404+
// }
397405
if (allowedListIps != null) {
398406
args.add(String.format("--allowed-ips=%s", allowedListIps));
399407
}

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

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -213,36 +213,49 @@ public Builder withBuildCheckDisabled(boolean noBuildCheck) {
213213
@Deprecated
214214
public Builder withLoglevel(String logLevel) {
215215
this.logLevel = ChromiumDriverLogLevel.fromString(logLevel);
216+
this.silent = false;
217+
this.verbose = false;
216218
return this;
217219
}
218220

219221
/**
220222
* Configures the driver server log level.
223+
*
224+
* @param logLevel {@link ChromiumDriverLogLevel} for desired log level output.
225+
* @return A self reference.
221226
*/
222227
public Builder withLoglevel(ChromiumDriverLogLevel logLevel) {
223228
this.logLevel = logLevel;
229+
this.silent = false;
230+
this.verbose = false;
224231
return this;
225232
}
226233

227234
/**
228235
* Configures the driver server for silent output.
229236
*
230-
* @param silent whether silent output is used
237+
* @param silent Log no output for true, no changes made if false.
231238
* @return A self reference.
232239
*/
233240
public Builder withSilent(boolean silent) {
234-
this.silent = silent;
241+
if (silent) {
242+
this.logLevel = ChromiumDriverLogLevel.OFF;
243+
}
244+
this.silent = false;
235245
return this;
236246
}
237247

238248
/**
239249
* Configures the driver server verbosity.
240250
*
241-
* @param verbose whether verbose output is used
251+
* @param verbose Log all output for true, no changes made if false.
242252
* @return A self reference.
243253
*/
244254
public Builder withVerbose(boolean verbose) {
245-
this.verbose = verbose;
255+
if (verbose) {
256+
this.logLevel = ChromiumDriverLogLevel.ALL;
257+
}
258+
this.verbose = false;
246259
return this;
247260
}
248261

@@ -286,20 +299,18 @@ protected void loadSystemProperties() {
286299
if (appendLog == null) {
287300
this.appendLog = Boolean.getBoolean(EDGE_DRIVER_APPEND_LOG_PROPERTY);
288301
}
289-
if (verbose == null) {
290-
this.verbose = Boolean.getBoolean(EDGE_DRIVER_VERBOSE_LOG_PROPERTY);
302+
if (verbose == null && Boolean.getBoolean(EDGE_DRIVER_VERBOSE_LOG_PROPERTY)) {
303+
withVerbose(Boolean.getBoolean(EDGE_DRIVER_VERBOSE_LOG_PROPERTY));
291304
}
292-
if (silent == null) {
293-
this.silent = Boolean.getBoolean(EDGE_DRIVER_SILENT_OUTPUT_PROPERTY);
305+
if (silent == null && Boolean.getBoolean(EDGE_DRIVER_SILENT_OUTPUT_PROPERTY)) {
306+
withSilent(Boolean.getBoolean(EDGE_DRIVER_SILENT_OUTPUT_PROPERTY));
294307
}
295308
if (allowedListIps == null) {
296309
this.allowedListIps = System.getProperty(EDGE_DRIVER_ALLOWED_IPS_PROPERTY);
297310
}
298-
if (logLevel == null) {
311+
if (logLevel == null && System.getProperty(EDGE_DRIVER_LOG_LEVEL_PROPERTY) != null) {
299312
String level = System.getProperty(EDGE_DRIVER_LOG_LEVEL_PROPERTY);
300-
if (level != null) {
301-
this.logLevel = ChromiumDriverLogLevel.fromString(level);
302-
}
313+
withLoglevel(ChromiumDriverLogLevel.fromString(level));
303314
}
304315
}
305316

‎java/test/org/openqa/selenium/chrome/ChromeDriverServiceTest.java‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@
1717

1818
package org.openqa.selenium.chrome;
1919

20+
import static org.assertj.core.api.Assertions.assertThat;
2021
import static org.mockito.ArgumentMatchers.any;
2122
import static org.mockito.ArgumentMatchers.anyInt;
2223
import static org.mockito.ArgumentMatchers.eq;
23-
import static org.mockito.Mockito.doReturn;
2424
import static org.mockito.Mockito.spy;
2525
import static org.mockito.Mockito.verify;
2626

@@ -52,11 +52,15 @@ void builderPassesTimeoutToDriverService() {
5252
verify(builderMock).createDriverService(any(), anyInt(), eq(customTimeout), any(), any());
5353
}
5454

55-
// Alternate behavior is throwing an error, but have to at least be consistent
5655
@Test
57-
void logLevelLastWins() {
58-
File exe = new File("someFile");
56+
void testScoring() {
57+
ChromeDriverService.Builder builder = new ChromeDriverService.Builder();
58+
assertThat(builder.score(new ChromeOptions())).isPositive();
59+
}
60+
5961

62+
@Test
63+
void logLevelLastWins() {
6064
ChromeDriverService.Builder builderMock = spy(ChromeDriverService.Builder.class);
6165

6266
List<String> silentLast = Arrays.asList("--port=1", "--log-level=OFF");
@@ -79,8 +83,6 @@ void logLevelLastWins() {
7983
// Setting these to false makes no sense; we're just going to ignore it.
8084
@Test
8185
void ignoreFalseLogging() {
82-
File exe = new File("someFile");
83-
8486
ChromeDriverService.Builder builderMock = spy(ChromeDriverService.Builder.class);
8587

8688
List<String> falseSilent = Arrays.asList("--port=1", "--log-level=DEBUG");

‎java/test/org/openqa/selenium/edge/EdgeDriverServiceTest.java‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,12 @@
1919

2020
import org.junit.jupiter.api.Tag;
2121
import org.junit.jupiter.api.Test;
22+
import org.openqa.selenium.chromium.ChromiumDriverLogLevel;
2223

2324
import java.io.File;
2425
import java.time.Duration;
26+
import java.util.Arrays;
27+
import java.util.List;
2528

2629
import static org.assertj.core.api.Assertions.assertThat;
2730
import static org.mockito.ArgumentMatchers.any;
@@ -54,4 +57,35 @@ void testScoring() {
5457
EdgeDriverService.Builder builder = new EdgeDriverService.Builder();
5558
assertThat(builder.score(new EdgeOptions())).isPositive();
5659
}
60+
61+
@Test
62+
void logLevelLastWins() {
63+
EdgeDriverService.Builder builderMock = spy(EdgeDriverService.Builder.class);
64+
65+
List<String> silentLast = Arrays.asList("--port=1", "--log-level=OFF");
66+
builderMock.withLoglevel(ChromiumDriverLogLevel.ALL).usingPort(1).withSilent(true).build();
67+
verify(builderMock).createDriverService(any(), anyInt(), any(), eq(silentLast), any());
68+
69+
List<String> silentFirst = Arrays.asList("--port=1", "--log-level=DEBUG");
70+
builderMock.withSilent(true).withLoglevel(ChromiumDriverLogLevel.DEBUG).usingPort(1).build();
71+
verify(builderMock).createDriverService(any(), anyInt(), any(), eq(silentFirst), any());
72+
73+
List<String> verboseLast = Arrays.asList("--port=1", "--log-level=ALL");
74+
builderMock.withLoglevel(ChromiumDriverLogLevel.OFF).usingPort(1).withVerbose(true).build();
75+
verify(builderMock).createDriverService(any(), anyInt(), any(), eq(verboseLast), any());
76+
77+
List<String> verboseFirst = Arrays.asList("--port=1", "--log-level=INFO");
78+
builderMock.withVerbose(true).withLoglevel(ChromiumDriverLogLevel.INFO).usingPort(1).build();
79+
verify(builderMock).createDriverService(any(), anyInt(), any(), eq(verboseFirst), any());
80+
}
81+
82+
// Setting these to false makes no sense; we're just going to ignore it.
83+
@Test
84+
void ignoreFalseLogging() {
85+
EdgeDriverService.Builder builderMock = spy(EdgeDriverService.Builder.class);
86+
87+
List<String> falseSilent = Arrays.asList("--port=1", "--log-level=DEBUG");
88+
builderMock.withLoglevel(ChromiumDriverLogLevel.DEBUG).usingPort(1).withSilent(false).build();
89+
verify(builderMock).createDriverService(any(), anyInt(), any(), eq(falseSilent), any());
90+
}
5791
}

0 commit comments

Comments
 (0)