From eea4eb00dca13318b18345b0d928cc02fa66e9f0 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Wed, 12 Jun 2024 11:59:15 -0400 Subject: [PATCH] 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 --- google-cloud-bigtable/pom.xml | 17 +-- .../v2/stub/EnhancedBigtableStubTest.java | 44 +++--- .../metrics/ErrorCountPerConnectionTest.java | 129 +++++++++--------- .../src/test/resources/logging.properties | 17 +++ pom.xml | 3 +- 5 files changed, 111 insertions(+), 99 deletions(-) create mode 100644 google-cloud-bigtable/src/test/resources/logging.properties diff --git a/google-cloud-bigtable/pom.xml b/google-cloud-bigtable/pom.xml index f55d736d88..6dd468e778 100644 --- a/google-cloud-bigtable/pom.xml +++ b/google-cloud-bigtable/pom.xml @@ -121,16 +121,6 @@ com.google.guava guava - - com.google.http-client - google-http-client - runtime - - - com.google.http-client - google-http-client-gson - runtime - com.google.protobuf protobuf-java @@ -151,7 +141,8 @@ org.checkerframework checker-qual - + + com.google.http-client google-http-client @@ -749,6 +740,10 @@ 10 false + + + src/test/resources/logging.properties + diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubTest.java index 2eb0700488..e2e44b0b83 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubTest.java +++ b/google-cloud-bigtable/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 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 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 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 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/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java index 8d05e13119..b34d21da17 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java +++ b/google-cloud-bigtable/src/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,6 +47,7 @@ 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; @@ -53,6 +55,7 @@ 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>) 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 new file mode 100644 index 0000000000..9da2fe4900 --- /dev/null +++ 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: +# : [] +# + +#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 index 36aa23e89d..11d43a0eeb 100644 --- a/pom.xml +++ b/pom.xml @@ -226,6 +226,7 @@ org.apache.maven.plugins maven-javadoc-plugin + 3.7.0 aggregate @@ -321,7 +322,7 @@ ${docletPath} -outputpath ${project.build.directory}/docfx-yml - -projectname ${artifactId} + -projectname ${project.artifactId}