You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by za...@apache.org on 2021/12/14 13:06:14 UTC

[calcite-avatica] branch master updated: Silence standard out messages in tests

This is an automated email from the ASF dual-hosted git repository.

zabetak pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite-avatica.git


The following commit(s) were added to refs/heads/master by this push:
     new c6a819a  Silence standard out messages in tests
c6a819a is described below

commit c6a819aa6be211331ca726b88077e761d7853df1
Author: Stamatis Zampetakis <za...@gmail.com>
AuthorDate: Mon Dec 13 17:22:06 2021 +0100

    Silence standard out messages in tests
    
    Before this change running the tests prints a lot of messages to
    standard out cluttering useful output (testname, success, failure, etc.)
    and slowing down the build.
    
    1. Remove direct calls to System.out in tests; it is considered bad
    practice in general.
    2. Remove SPNEGO debug information by unsetting System properties; when
    necessary the developer can set them explicitly. Debug info shouldn't
    be always on especially on standard out.
    3. Use loggers instead of System.out to print useful info in production
    code.
---
 .../calcite/avatica/remote/CommonsHttpClientPoolCache.java |  4 ++--
 .../java/org/apache/calcite/avatica/AvaticaSpnegoTest.java | 14 ++++++++------
 .../java/org/apache/calcite/avatica/RemoteDriverTest.java  |  8 +++++---
 .../avatica/server/HttpServerSpnegoWithoutJaasTest.java    |  2 --
 4 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/avatica/remote/CommonsHttpClientPoolCache.java b/core/src/main/java/org/apache/calcite/avatica/remote/CommonsHttpClientPoolCache.java
index c032738..504bebb 100644
--- a/core/src/main/java/org/apache/calcite/avatica/remote/CommonsHttpClientPoolCache.java
+++ b/core/src/main/java/org/apache/calcite/avatica/remote/CommonsHttpClientPoolCache.java
@@ -126,8 +126,8 @@ public class CommonsHttpClientPoolCache {
       throws Exception {
     sslContextBuilder.loadTrustMaterial(config.truststore(),
         config.truststorePassword().toCharArray());
-    System.out.println("truststore loaded. truststore:" + config.truststore()
-        + "pw:" + config.truststorePassword());
+    // Avoid printing sensitive information such as passwords in the logs
+    LOG.info("Trustore loaded from: {}", config.truststore());
   }
 
   private static void configureHttpRegistry(
diff --git a/server/src/test/java/org/apache/calcite/avatica/AvaticaSpnegoTest.java b/server/src/test/java/org/apache/calcite/avatica/AvaticaSpnegoTest.java
index 66821c8..aeb63a1 100644
--- a/server/src/test/java/org/apache/calcite/avatica/AvaticaSpnegoTest.java
+++ b/server/src/test/java/org/apache/calcite/avatica/AvaticaSpnegoTest.java
@@ -49,6 +49,14 @@ import static org.junit.Assert.assertTrue;
 
 /**
  * End to end test case for SPNEGO with Avatica.
+ *
+ * <p>The following system properties are useful for debugging problems around SPNEGO.</p>
+ * <ul>
+ *   <li>sun.security.krb5.debug</li>
+ *   <li>sun.security.jgss.debug</li>
+ *   <li>sun.security.spnego.debug</li>
+ *   <li>java.security.debug</li>
+ * </ul>
  */
 @RunWith(Parameterized.class)
 public class AvaticaSpnegoTest extends HttpBaseTest {
@@ -65,12 +73,6 @@ public class AvaticaSpnegoTest extends HttpBaseTest {
   private static boolean isKdcStarted = false;
 
   private static void setupKdc() throws Exception {
-    System.setProperty("sun.security.krb5.debug", "true");
-    System.setProperty("sun.security.jgss.debug", "true");
-    System.setProperty("sun.security.spnego.debug", "true");
-    System.setProperty("java.security.debug", "all");
-    System.setProperty("org.slf4j.simpleLogger.defaultLogLevel", "debug");
-
     if (isKdcStarted) {
       return;
     }
diff --git a/server/src/test/java/org/apache/calcite/avatica/RemoteDriverTest.java b/server/src/test/java/org/apache/calcite/avatica/RemoteDriverTest.java
index a97d775..525a1e7 100644
--- a/server/src/test/java/org/apache/calcite/avatica/RemoteDriverTest.java
+++ b/server/src/test/java/org/apache/calcite/avatica/RemoteDriverTest.java
@@ -66,6 +66,7 @@ import java.util.Properties;
 import java.util.TimeZone;
 import java.util.UUID;
 import java.util.concurrent.Callable;
+import java.util.stream.Stream;
 
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.instanceOf;
@@ -891,9 +892,10 @@ public class RemoteDriverTest {
       getRequestInspection().getRequestLogger().enableAndClear();
       checkPrepareBindExecuteFetch(getLocalConnection());
       List<String[]> x = getRequestInspection().getRequestLogger().getAndDisable();
-      for (String[] pair : x) {
-        System.out.println(pair[0] + "=" + pair[1]);
-      }
+      // Counting the number of elements is not the best way to prevent regressions
+      // but it's better than just printing elements to standard out as it was before.
+      // Feel free to improve the assertion if you understand the original intention.
+      assertEquals(18, x.stream().flatMap(Stream::of).count());
     } finally {
       ConnectionSpec.getDatabaseLock().unlock();
     }
diff --git a/server/src/test/java/org/apache/calcite/avatica/server/HttpServerSpnegoWithoutJaasTest.java b/server/src/test/java/org/apache/calcite/avatica/server/HttpServerSpnegoWithoutJaasTest.java
index 318af48..2f01f6b 100644
--- a/server/src/test/java/org/apache/calcite/avatica/server/HttpServerSpnegoWithoutJaasTest.java
+++ b/server/src/test/java/org/apache/calcite/avatica/server/HttpServerSpnegoWithoutJaasTest.java
@@ -129,8 +129,6 @@ public class HttpServerSpnegoWithoutJaasTest {
     // Kerby sets "java.security.krb5.conf" for us!
     System.clearProperty("java.security.auth.login.config");
     System.setProperty("javax.security.auth.useSubjectCredsOnly", "false");
-    System.setProperty("sun.security.spnego.debug", "true");
-    System.setProperty("sun.security.krb5.debug", "true");
 
     // Create and start an HTTP server configured only to allow SPNEGO requests
     // We use `withAutomaticLogin(File)` here which should invalidate the need to do JAAS config