test: reduce test noise (#2257)

* test: reduce noise by properly closing stubs Change-Id: I50f954be0d6a0c5b4db6377e3403c81f3b14a167 * test: fix some of the noise during builds and test runs - make sure to close stubs to avoid grpc warnings - add missing plugin versions - fix deprecated syntax in pom.xml - filter out useless "Connecting to the Bigtable emulator..." log lines - configure logs to be on a single line Change-Id: Iacbd41c953ef0f3f726ef041dde0093d8bc2c6e6 * cleanup Change-Id: I6cbd1c5d194112c7587f58337d7a810f81375ba7
googleapis · Jun 12, 2024 · eea4eb0 · eea4eb0
1 parent e62c969
commit eea4eb0
Show file tree

Hide file tree

Showing 5 changed files with 111 additions and 99 deletions.
diff --git a/google-cloud-bigtable/pom.xml b/google-cloud-bigtable/pom.xml
@@ -121,16 +121,6 @@
       <groupId>com.google.guava</groupId>
       <artifactId>guava</artifactId>
     </dependency>
-    <dependency>
-      <groupId>com.google.http-client</groupId>
-      <artifactId>google-http-client</artifactId>
-      <scope>runtime</scope>
-    </dependency>
-    <dependency>
-      <groupId>com.google.http-client</groupId>
-      <artifactId>google-http-client-gson</artifactId>
-      <scope>runtime</scope>
-    </dependency>
     <dependency>
       <groupId>com.google.protobuf</groupId>
       <artifactId>protobuf-java</artifactId>
@@ -151,7 +141,8 @@
     <dependency>
       <groupId>org.checkerframework</groupId>
       <artifactId>checker-qual</artifactId>
-    </dependency><!-- Runtime dependencies for credentials -->
+    </dependency>
+    <!-- Runtime dependencies for credentials -->
     <dependency>
       <groupId>com.google.http-client</groupId>
       <artifactId>google-http-client</artifactId>
@@ -749,6 +740,10 @@
           <threadCount>10</threadCount>
 
           <trimStackTrace>false</trimStackTrace>
+
+          <systemPropertyVariables>
+            <java.util.logging.config.file>src/test/resources/logging.properties</java.util.logging.config.file>
+          </systemPropertyVariables>
         </configuration>
       </plugin>
 

diff --git a/...gtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubTest.java b/...gtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubTest.java
@@ -33,6 +33,7 @@
 import com.google.api.gax.grpc.GrpcTransportChannel;
 import com.google.api.gax.rpc.FixedTransportChannelProvider;
 import com.google.api.gax.rpc.InstantiatingWatchdogProvider;
+import com.google.api.gax.rpc.ServerStream;
 import com.google.api.gax.rpc.ServerStreamingCallable;
 import com.google.api.gax.rpc.WatchdogTimeoutException;
 import com.google.auth.oauth2.ServiceAccountJwtAccessCredentials;
@@ -86,13 +87,11 @@
 import java.security.NoSuchAlgorithmException;
 import java.util.Base64;
 import java.util.Collection;
-import java.util.Iterator;
 import java.util.concurrent.ArrayBlockingQueue;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 import org.junit.After;
-import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -523,8 +522,9 @@ public void testBulkMutationFlowControlFeatureFlagIsSet() throws Exception {
     // Test the header is set when the feature is enabled
     EnhancedBigtableStubSettings.Builder settings = defaultSettings.toBuilder();
     settings.bulkMutateRowsSettings().setServerInitiatedFlowControl(true);
-    EnhancedBigtableStub stub = EnhancedBigtableStub.create(settings.build());
-    stub.bulkMutateRowsCallable().call(bulkMutation);
+    try (EnhancedBigtableStub stub = EnhancedBigtableStub.create(settings.build())) {
+      stub.bulkMutateRowsCallable().call(bulkMutation);
+    }
     assertThat(metadataInterceptor.headers).hasSize(1);
     Metadata metadata = metadataInterceptor.headers.take();
     String encodedFlags =
@@ -543,8 +543,9 @@ public void testBulkMutationFlowControlFeatureFlagIsNotSet() throws Exception {
 
     EnhancedBigtableStubSettings.Builder settings = defaultSettings.toBuilder();
     settings.bulkMutateRowsSettings().setServerInitiatedFlowControl(false);
-    EnhancedBigtableStub stub = EnhancedBigtableStub.create(settings.build());
-    stub.bulkMutateRowsCallable().call(bulkMutation);
+    try (EnhancedBigtableStub stub = EnhancedBigtableStub.create(settings.build())) {
+      stub.bulkMutateRowsCallable().call(bulkMutation);
+    }
     assertThat(metadataInterceptor.headers).hasSize(1);
     Metadata metadata = metadataInterceptor.headers.take();
     String encodedFlags =
@@ -553,7 +554,6 @@ public void testBulkMutationFlowControlFeatureFlagIsNotSet() throws Exception {
     FeatureFlags featureFlags = FeatureFlags.parseFrom(decodedFlags);
     assertThat(featureFlags.getMutateRowsRateLimit()).isFalse();
     assertThat(featureFlags.getMutateRowsRateLimit2()).isFalse();
-    stub.close();
   }
 
   @Test
@@ -564,14 +564,12 @@ public void testWaitTimeoutIsSet() throws Exception {
     settings.setStreamWatchdogProvider(
         InstantiatingWatchdogProvider.create().withCheckInterval(WATCHDOG_CHECK_DURATION));
 
-    EnhancedBigtableStub stub = EnhancedBigtableStub.create(settings.build());
-    Iterator<Row> iterator =
-        stub.readRowsCallable().call(Query.create(WAIT_TIME_TABLE_ID)).iterator();
-    try {
-      iterator.next();
-      Assert.fail("Should throw watchdog timeout exception");
-    } catch (WatchdogTimeoutException e) {
-      assertThat(e.getMessage()).contains("Canceled due to timeout waiting for next response");
+    try (EnhancedBigtableStub stub = EnhancedBigtableStub.create(settings.build())) {
+      ServerStream<Row> results = stub.readRowsCallable().call(Query.create(WAIT_TIME_TABLE_ID));
+      WatchdogTimeoutException ex =
+          assertThrows(WatchdogTimeoutException.class, () -> results.iterator().next());
+
+      assertThat(ex).hasMessageThat().contains("Canceled due to timeout waiting for next response");
     }
   }
 
@@ -583,16 +581,12 @@ public void testReadChangeStreamWaitTimeoutIsSet() throws Exception {
     settings.setStreamWatchdogProvider(
         InstantiatingWatchdogProvider.create().withCheckInterval(WATCHDOG_CHECK_DURATION));
 
-    EnhancedBigtableStub stub = EnhancedBigtableStub.create(settings.build());
-    Iterator<ChangeStreamRecord> iterator =
-        stub.readChangeStreamCallable()
-            .call(ReadChangeStreamQuery.create(WAIT_TIME_TABLE_ID))
-            .iterator();
-    try {
-      iterator.next();
-      Assert.fail("Should throw watchdog timeout exception");
-    } catch (WatchdogTimeoutException e) {
-      assertThat(e.getMessage()).contains("Canceled due to timeout waiting for next response");
+    try (EnhancedBigtableStub stub = EnhancedBigtableStub.create(settings.build())) {
+      ServerStream<ChangeStreamRecord> results =
+          stub.readChangeStreamCallable().call(ReadChangeStreamQuery.create(WAIT_TIME_TABLE_ID));
+      WatchdogTimeoutException ex =
+          assertThrows(WatchdogTimeoutException.class, () -> results.iterator().next());
+      assertThat(ex).hasMessageThat().contains("Canceled due to timeout waiting for next response");
     }
   }
 

diff --git a/...test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java b/...test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java
@@ -18,6 +18,7 @@
 import static com.google.common.truth.Truth.assertThat;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.Mockito.when;
 
 import com.google.api.gax.core.FixedExecutorProvider;
 import com.google.api.gax.grpc.ChannelPoolSettings;
@@ -46,13 +47,15 @@
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledFuture;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 import org.mockito.ArgumentCaptor;
 import org.mockito.Mockito;
+import org.mockito.stubbing.Answer;
 
 @RunWith(JUnit4.class)
 public class ErrorCountPerConnectionTest {
@@ -103,9 +106,8 @@ public void setup() throws Exception {
             .setMetricsProvider(CustomOpenTelemetryMetricsProvider.create(otel));
 
     runnableCaptor = ArgumentCaptor.forClass(Runnable.class);
-    Mockito.when(
-            executors.scheduleAtFixedRate(runnableCaptor.capture(), anyLong(), anyLong(), any()))
-        .thenReturn(null);
+    when(executors.scheduleAtFixedRate(runnableCaptor.capture(), anyLong(), anyLong(), any()))
+        .then((Answer<ScheduledFuture<?>>) invocation -> Mockito.mock(ScheduledFuture.class));
   }
 
   @After
@@ -117,21 +119,22 @@ public void tearDown() throws Exception {
 
   @Test
   public void readWithOneChannel() throws Exception {
-    EnhancedBigtableStub stub = EnhancedBigtableStub.create(builder.build());
     long errorCount = 0;
 
-    for (int i = 0; i < 20; i++) {
-      Query query;
-      if (i % 3 == 0) {
-        query = Query.create(ERROR_TABLE_NAME);
-        errorCount += 1;
-      } else {
-        query = Query.create(SUCCESS_TABLE_NAME);
-      }
-      try {
-        stub.readRowsCallable().call(query).iterator().hasNext();
-      } catch (Exception e) {
-        // noop
+    try (EnhancedBigtableStub stub = EnhancedBigtableStub.create(builder.build())) {
+      for (int i = 0; i < 20; i++) {
+        Query query;
+        if (i % 3 == 0) {
+          query = Query.create(ERROR_TABLE_NAME);
+          errorCount += 1;
+        } else {
+          query = Query.create(SUCCESS_TABLE_NAME);
+        }
+        try {
+          stub.readRowsCallable().call(query).iterator().hasNext();
+        } catch (Exception e) {
+          // noop
+        }
       }
     }
 
@@ -158,19 +161,19 @@ public void readWithTwoChannels() throws Exception {
                 .toBuilder()
                 .setChannelPoolSettings(ChannelPoolSettings.staticallySized(2))
                 .build());
-    EnhancedBigtableStub stub = EnhancedBigtableStub.create(builderWithTwoChannels.build());
     long totalErrorCount = 0;
-
-    for (int i = 0; i < 20; i++) {
-      try {
-        if (i < 10) {
-          totalErrorCount += 1;
-          stub.readRowsCallable().call(Query.create(ERROR_TABLE_NAME)).iterator().hasNext();
-        } else {
-          stub.readRowsCallable().call(Query.create(SUCCESS_TABLE_NAME)).iterator().hasNext();
+    try (EnhancedBigtableStub stub = EnhancedBigtableStub.create(builderWithTwoChannels.build())) {
+      for (int i = 0; i < 20; i++) {
+        try {
+          if (i < 10) {
+            totalErrorCount += 1;
+            stub.readRowsCallable().call(Query.create(ERROR_TABLE_NAME)).iterator().hasNext();
+          } else {
+            stub.readRowsCallable().call(Query.create(SUCCESS_TABLE_NAME)).iterator().hasNext();
+          }
+        } catch (Exception e) {
+          // noop
         }
-      } catch (Exception e) {
-        // noop
       }
     }
     runInterceptorTasksAndAssertCount();
@@ -193,39 +196,40 @@ public void readWithTwoChannels() throws Exception {
 
   @Test
   public void readOverTwoPeriods() throws Exception {
-    EnhancedBigtableStub stub = EnhancedBigtableStub.create(builder.build());
     long errorCount1 = 0;
+    long errorCount2 = 0;
+    try (EnhancedBigtableStub stub = EnhancedBigtableStub.create(builder.build())) {
 
-    for (int i = 0; i < 20; i++) {
-      Query query;
-      if (i % 3 == 0) {
-        query = Query.create(ERROR_TABLE_NAME);
-        errorCount1 += 1;
-      } else {
-        query = Query.create(SUCCESS_TABLE_NAME);
-      }
-      try {
-        stub.readRowsCallable().call(query).iterator().hasNext();
-      } catch (Exception e) {
-        // noop
+      for (int i = 0; i < 20; i++) {
+        Query query;
+        if (i % 3 == 0) {
+          query = Query.create(ERROR_TABLE_NAME);
+          errorCount1 += 1;
+        } else {
+          query = Query.create(SUCCESS_TABLE_NAME);
+        }
+        try {
+          stub.readRowsCallable().call(query).iterator().hasNext();
+        } catch (Exception e) {
+          // noop
+        }
       }
-    }
 
-    runInterceptorTasksAndAssertCount();
-    long errorCount2 = 0;
+      runInterceptorTasksAndAssertCount();
 
-    for (int i = 0; i < 20; i++) {
-      Query query;
-      if (i % 3 == 0) {
-        query = Query.create(SUCCESS_TABLE_NAME);
-      } else {
-        query = Query.create(ERROR_TABLE_NAME);
-        errorCount2 += 1;
-      }
-      try {
-        stub.readRowsCallable().call(query).iterator().hasNext();
-      } catch (Exception e) {
-        // noop
+      for (int i = 0; i < 20; i++) {
+        Query query;
+        if (i % 3 == 0) {
+          query = Query.create(SUCCESS_TABLE_NAME);
+        } else {
+          query = Query.create(ERROR_TABLE_NAME);
+          errorCount2 += 1;
+        }
+        try {
+          stub.readRowsCallable().call(query).iterator().hasNext();
+        } catch (Exception e) {
+          // noop
+        }
       }
     }
 
@@ -247,15 +251,16 @@ public void readOverTwoPeriods() throws Exception {
 
   @Test
   public void noFailedRequests() throws Exception {
-    EnhancedBigtableStub stub = EnhancedBigtableStub.create(builder.build());
-
-    for (int i = 0; i < 20; i++) {
-      try {
-        stub.readRowsCallable().call(Query.create(SUCCESS_TABLE_NAME)).iterator().hasNext();
-      } catch (Exception e) {
-        // noop
+    try (EnhancedBigtableStub stub = EnhancedBigtableStub.create(builder.build())) {
+      for (int i = 0; i < 20; i++) {
+        try {
+          stub.readRowsCallable().call(Query.create(SUCCESS_TABLE_NAME)).iterator().hasNext();
+        } catch (Exception e) {
+          // noop
+        }
       }
     }
+
     runInterceptorTasksAndAssertCount();
     MetricData metricData =
         BuiltinMetricsTestUtils.getMetricData(

diff --git a/google-cloud-bigtable/src/test/resources/logging.properties b/google-cloud-bigtable/src/test/resources/logging.properties
@@ -0,0 +1,17 @@
+handlers= java.util.logging.ConsoleHandler
+.level= INFO
+
+java.util.logging.ConsoleHandler.level = INFO
+java.util.logging.ConsoleHandler.formatter = java.util.logging.SimpleFormatter
+
+# Example to customize the SimpleFormatter output format
+# to print one-line log message like this:
+#     <level>: <log message> [<date/time>]
+#
+
+#java.util.logging.SimpleFormatter.format=%4$s: %5$s [%1$tc]%n
+# time [level] loggerName: message
+java.util.logging.SimpleFormatter.format=%1$tT [%4$-7s] %2$s: %5$s%n
+
+# hide "Connecting to the Bigtable emulator at localhost:XXXX" lines
+com.google.cloud.bigtable.data.v2.BigtableDataSettings.level=WARNING
diff --git a/pom.xml b/pom.xml
@@ -226,6 +226,7 @@
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-javadoc-plugin</artifactId>
+                <version>3.7.0</version>
                 <executions>
                     <execution>
                         <id>aggregate</id>
@@ -321,7 +322,7 @@
                             <docletPath>${docletPath}</docletPath>
                             <additionalOptions>
                                 -outputpath ${project.build.directory}/docfx-yml
-                                -projectname ${artifactId}
+                                -projectname ${project.artifactId}
                                 <!-- List of excluded packages as regex, separated by a colon-->
                                 <!-- Exclude generating javadocs for internal implementations for admin and data, which
                                  are under internal/ and .v2.stub/, and exclude all BaseClients and BaseSettings.