[java] fix chromedriver log level logic

titusfortner · titusfortner · commit 6cbfbf6174c8 · 2022-12-10T14:00:38.000-06:00
diff --git a/java/src/org/openqa/selenium/chrome/ChromeDriverService.java b/java/src/org/openqa/selenium/chrome/ChromeDriverService.java
@@ -190,7 +190,10 @@ public Builder withAppendLog(boolean appendLog) {
      */
     @SuppressWarnings("UnusedReturnValue")
     public Builder withVerbose(boolean verbose) {
-      this.verbose = verbose;
+      if (verbose) {
+        this.logLevel = ChromeDriverLogLevel.ALL;
+      }
+      this.verbose = false;
       return this;
     }
 
@@ -201,6 +204,8 @@ public Builder withVerbose(boolean verbose) {
      * @return A self reference.
      */
     public Builder withLogLevel(ChromeDriverLogLevel logLevel) {
+      this.verbose = false;
+      this.silent = false;
       this.logLevel = logLevel;
       return this;
     }
@@ -212,7 +217,10 @@ public Builder withLogLevel(ChromeDriverLogLevel logLevel) {
      * @return A self reference.
      */
     public Builder withSilent(boolean silent) {
-      this.silent = silent;
+      if (silent) {
+        this.logLevel = ChromeDriverLogLevel.OFF;
+      }
+      this.silent = false;
       return this;
     }
 
@@ -245,12 +253,12 @@ protected List<String> createArgs() {
         }
       }
 
-      if (logLevel != null) {
-        withLogLevel(logLevel);
-        withVerbose(false);
-      }
+      // If set in properties and not overwritten by method
       if (verbose) {
-        withLogLevel(ChromeDriverLogLevel.ALL);
+        withVerbose(true);
+      }
+      if (silent) {
+        withSilent(true);
       }
 
       List<String> args = new ArrayList<>();
@@ -265,9 +273,6 @@ protected List<String> createArgs() {
       if (logLevel != null) {
         args.add(String.format("--log-level=%s", logLevel.toString().toUpperCase()));
       }
-      if (silent) {
-        args.add("--silent");
-      }
       if (whitelistedIps != null) {
         args.add(String.format("--whitelisted-ips=%s", whitelistedIps));
       }
diff --git a/java/test/org/openqa/selenium/chrome/ChromeDriverServiceTest.java b/java/test/org/openqa/selenium/chrome/ChromeDriverServiceTest.java
@@ -29,6 +29,8 @@
 
 import java.io.File;
 import java.time.Duration;
+import java.util.Arrays;
+import java.util.List;
 
 @Tag("UnitTests")
 class ChromeDriverServiceTest {
@@ -49,4 +51,42 @@ void builderPassesTimeoutToDriverService() {
     builderMock.build();
     verify(builderMock).createDriverService(any(), anyInt(), eq(customTimeout), any(), any());
   }
+
+  // Alternate behavior is throwing an error, but have to at least be consistent
+  @Test
+  void logLevelLastWins() {
+    File exe = new File("someFile");
+
+    ChromeDriverService.Builder builderMock = spy(ChromeDriverService.Builder.class);
+    doReturn(exe).when(builderMock).findDefaultExecutable();
+
+    List<String> silentLast = Arrays.asList("--port=1", "--log-level=OFF");
+    builderMock.withLogLevel(ChromeDriverLogLevel.ALL).usingPort(1).withSilent(true).build();
+    verify(builderMock).createDriverService(any(), anyInt(), any(), eq(silentLast), any());
+
+    List<String> silentFirst = Arrays.asList("--port=1", "--log-level=DEBUG");
+    builderMock.withSilent(true).withLogLevel(ChromeDriverLogLevel.DEBUG).usingPort(1).build();
+    verify(builderMock).createDriverService(any(), anyInt(), any(), eq(silentFirst), any());
+
+    List<String> verboseLast = Arrays.asList("--port=1", "--log-level=ALL");
+    builderMock.withLogLevel(ChromeDriverLogLevel.OFF).usingPort(1).withVerbose(true).build();
+    verify(builderMock).createDriverService(any(), anyInt(), any(), eq(verboseLast), any());
+
+    List<String> verboseFirst = Arrays.asList("--port=1", "--log-level=INFO");
+    builderMock.withVerbose(true).withLogLevel(ChromeDriverLogLevel.INFO).usingPort(1).build();
+    verify(builderMock).createDriverService(any(), anyInt(), any(), eq(verboseFirst), any());
+  }
+
+  // Setting these to false makes no sense; we're just going to ignore it.
+  @Test
+  void ignoreFalseLogging() {
+    File exe = new File("someFile");
+
+    ChromeDriverService.Builder builderMock = spy(ChromeDriverService.Builder.class);
+    doReturn(exe).when(builderMock).findDefaultExecutable();
+
+    List<String> falseSilent = Arrays.asList("--port=1", "--log-level=DEBUG");
+    builderMock.withLogLevel(ChromeDriverLogLevel.DEBUG).usingPort(1).withSilent(false).build();
+    verify(builderMock).createDriverService(any(), anyInt(), any(), eq(falseSilent), any());
+  }
 }
