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.
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 @@
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 @@
/**
* 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 @@
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.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 @@
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 @@
// 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
