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 <zabe...@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