test(showcase): migrate Thread.sleep calls to Awaitility (#13728)

Migrate Thread.sleep and loop constructs in java-showcase to Awaitility.

Author: blakeli0

.github/workflows/ci.yaml

@@ -51,8 +51,8 @@ jobs:
           workflows:
             - '.github/workflows/**'
           src:
-            - '!(google-auth-library-java|grpc-gcp-java|java-bigquery|java-bigquery-jdbc|java-bigquerystorage|java-bigtable|java-cloud-bom|java-datastore|java-firestore|java-logging|java-logging-logback|java-pubsub|java-shared-config|java-spanner|java-storage)/**/*.java'
-            - '!(google-auth-library-java|grpc-gcp-java|java-bigquery|java-bigquery-jdbc|java-bigquerystorage|java-bigtable|java-cloud-bom|java-datastore|java-firestore|java-logging|java-logging-logback|java-pubsub|java-shared-config|java-spanner|java-storage)/**/pom.xml'
+            - '!(google-auth-library-java|grpc-gcp-java|java-bigquery|java-bigquery-jdbc|java-bigquerystorage|java-bigtable|java-cloud-bom|java-datastore|java-firestore|java-logging|java-logging-logback|java-pubsub|java-shared-config|java-showcase|java-spanner|java-storage)/**/*.java'
+            - '!(google-auth-library-java|grpc-gcp-java|java-bigquery|java-bigquery-jdbc|java-bigquerystorage|java-bigtable|java-cloud-bom|java-datastore|java-firestore|java-logging|java-logging-logback|java-pubsub|java-shared-config|java-showcase|java-spanner|java-storage)/**/pom.xml'
             - 'pom.xml'
           ci:
             - '.github/workflows/ci.yaml'

java-showcase/gapic-showcase/pom.xml

@@ -257,6 +257,12 @@
       <scope>test</scope>
     </dependency>
 
+    <dependency>
+      <groupId>org.awaitility</groupId>
+      <artifactId>awaitility</artifactId>
+      <scope>test</scope>
+    </dependency>
+
   </dependencies>
   <profiles>
     <profile>

java-showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITClientShutdown.java

@@ -24,6 +24,7 @@
 import com.google.showcase.v1beta1.EchoClient;
 import com.google.showcase.v1beta1.EchoRequest;
 import com.google.showcase.v1beta1.it.util.TestClientInitializer;
+import org.awaitility.Awaitility;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.Timeout;
 import org.threeten.bp.Duration;
@@ -143,12 +144,9 @@ private void assertClientTerminated(EchoClient echoClient) throws InterruptedExc
     // check that everything is properly terminated after close() is called.
     echoClient.close();
 
-    // Loop until the client has terminated successfully. For tests that use this,
-    // try to ensure there is a timeout associated, otherwise this may run forever.
-    // Future enhancement: Use awaitility instead of busy waiting
-    while (!echoClient.isTerminated()) {
-      Thread.sleep(500L);
-    }
+    Awaitility.await()
+        .atMost(java.time.Duration.ofMillis(DEFAULT_CLIENT_TERMINATION_MS))
+        .until(echoClient::isTerminated);
     // The busy-wait time won't be accurate, so account for a bit of buffer
     long end = System.currentTimeMillis();
 

java-showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITCompositeTracer.java

@@ -49,9 +49,11 @@
 import io.opentelemetry.sdk.trace.SdkTracerProvider;
 import io.opentelemetry.sdk.trace.data.SpanData;
 import io.opentelemetry.sdk.trace.export.SimpleSpanProcessor;
+import java.time.Duration;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
+import org.awaitility.Awaitility;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -124,13 +126,11 @@ void testCompositeTracer() throws Exception {
                   .get(AttributeKey.stringKey(ObservabilityAttributes.SERVER_ADDRESS_ATTRIBUTE)))
           .isEqualTo(SHOWCASE_SERVER_ADDRESS);
 
-      Thread.sleep(100);
       // Verify metric name and one basic attribute server.address
+      Awaitility.await()
+          .atMost(Duration.ofSeconds(10))
+          .until(() -> !metricReader.collectAllMetrics().isEmpty());
       Collection<MetricData> actualMetrics = metricReader.collectAllMetrics();
-      for (int i = 0; i < 10 && actualMetrics.isEmpty(); i++) {
-        Thread.sleep(1000L);
-        actualMetrics = metricReader.collectAllMetrics();
-      }
 
       assertThat(actualMetrics).isNotEmpty();
       MetricData metricData =

java-showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelGoldenMetrics.java

@@ -63,6 +63,7 @@
 import java.io.InputStream;
 import java.time.Duration;
 import java.util.Collection;
+import org.awaitility.Awaitility;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -106,8 +107,7 @@ void testMetrics_successfulEcho_grpc() throws Exception {
       // This is implemented by adding a TraceFinisher to ApiFuture as a callback in
       // TracedUnaryCallable,
       // which could be executed in a different thread.
-      Thread.sleep(100);
-      Collection<MetricData> metrics = metricReader.collectAllMetrics();
+      Collection<MetricData> metrics = waitAndCollectMetrics();
       assertThat(metrics).isNotEmpty();
 
       MetricData durationMetric =
@@ -192,8 +192,7 @@ public void sendMessage(ReqT message) {}
           UnavailableException.class,
           () -> client.echo(EchoRequest.newBuilder().setContent("metrics-test").build()));
 
-      Thread.sleep(100);
-      Collection<MetricData> metrics = metricReader.collectAllMetrics();
+      Collection<MetricData> metrics = waitAndCollectMetrics();
       assertThat(metrics).isNotEmpty();
 
       MetricData durationMetric =
@@ -224,8 +223,7 @@ void testMetrics_successfulEcho_httpjson() throws Exception {
 
       client.echo(EchoRequest.newBuilder().setContent("metrics-test").build());
 
-      Thread.sleep(100);
-      Collection<MetricData> metrics = metricReader.collectAllMetrics();
+      Collection<MetricData> metrics = waitAndCollectMetrics();
       assertThat(metrics).isNotEmpty();
 
       MetricData durationMetric =
@@ -366,8 +364,7 @@ public String getHeaderValue(int index) {
           UnavailableException.class,
           () -> client.echo(EchoRequest.newBuilder().setContent("metrics-test").build()));
 
-      Thread.sleep(100);
-      Collection<MetricData> metrics = metricReader.collectAllMetrics();
+      Collection<MetricData> metrics = waitAndCollectMetrics();
       assertThat(metrics).isNotEmpty();
 
       MetricData durationMetric =
@@ -415,8 +412,7 @@ void testMetrics_clientTimeout_grpc() throws Exception {
           Exception.class,
           () -> client.echo(EchoRequest.newBuilder().setContent("metrics-test").build()));
 
-      Thread.sleep(100);
-      Collection<MetricData> metrics = metricReader.collectAllMetrics();
+      Collection<MetricData> metrics = waitAndCollectMetrics();
       assertThat(metrics).isNotEmpty();
 
       MetricData durationMetric =
@@ -458,8 +454,7 @@ void testMetrics_clientTimeout_httpjson() throws Exception {
           Exception.class,
           () -> client.echo(EchoRequest.newBuilder().setContent("metrics-test").build()));
 
-      Thread.sleep(100);
-      Collection<MetricData> metrics = metricReader.collectAllMetrics();
+      Collection<MetricData> metrics = waitAndCollectMetrics();
       assertThat(metrics).isNotEmpty();
 
       MetricData durationMetric =
@@ -538,8 +533,7 @@ public void sendMessage(ReqT message) {}
 
       assertThat(attemptCount.get()).isEqualTo(3);
 
-      Thread.sleep(100);
-      Collection<MetricData> metrics = metricReader.collectAllMetrics();
+      Collection<MetricData> metrics = waitAndCollectMetrics();
       assertThat(metrics).hasSize(1);
 
       MetricData durationMetric =
@@ -709,8 +703,7 @@ public String getHeaderValue(int index) {
 
       assertThat(requestCount.get()).isEqualTo(3);
 
-      Thread.sleep(100);
-      Collection<MetricData> metrics = metricReader.collectAllMetrics();
+      Collection<MetricData> metrics = waitAndCollectMetrics();
       assertThat(metrics).hasSize(1);
 
       MetricData durationMetric =
@@ -730,4 +723,11 @@ public String getHeaderValue(int index) {
           .isEqualTo("OK");
     }
   }
+
+  private Collection<MetricData> waitAndCollectMetrics() {
+    Awaitility.await()
+        .atMost(Duration.ofSeconds(5))
+        .until(() -> !metricReader.collectAllMetrics().isEmpty());
+    return metricReader.collectAllMetrics();
+  }
 }

java-showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java

@@ -83,6 +83,8 @@
 import java.util.concurrent.TimeUnit;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
+import org.awaitility.Awaitility;
+import org.awaitility.core.ConditionTimeoutException;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
@@ -290,24 +292,29 @@ private List<MetricData> getMetricDataList() throws InterruptedException {
   }
 
   /**
-   * Attempts to retrieve the metrics from a custom InMemoryMetricsReader. Sleep every second for at
-   * most 10s to try and retrieve all the metrics available. If it is unable to retrieve all the
-   * metrics, fail the test.
+   * Attempts to retrieve the metrics from a custom InMemoryMetricsReader. Polls for at most 10s to
+   * try and retrieve all the metrics available. If it is unable to retrieve all the metrics, fail
+   * the test.
    */
   private List<MetricData> getMetricDataList(InMemoryMetricReader metricReader)
       throws InterruptedException {
-    for (int i = 0; i < NUM_DEFAULT_FLUSH_ATTEMPTS; i++) {
-      Thread.sleep(1000L);
-      List<MetricData> metricData = new ArrayList<>(metricReader.collectAllMetrics());
-      // Depending on the OpenTelemetry instance (i.e. OpenTelemetry, GrpcOpenTelemetry, etc.)
-      // there may be additional metrics recorded. Only check to ensure the Gax Metrics
-      // are recorded properly. Any additional metrics are fine to be passed.
-      if (metricData.size() >= NUM_GAX_OTEL_METRICS && areAllGaxMetricsRecorded(metricData)) {
-        return metricData;
-      }
+    try {
+      Awaitility.await()
+          .atMost(java.time.Duration.ofSeconds(NUM_DEFAULT_FLUSH_ATTEMPTS))
+          .until(
+              () -> {
+                List<MetricData> metricData = new ArrayList<>(metricReader.collectAllMetrics());
+                // Depending on the OpenTelemetry instance (i.e. OpenTelemetry, GrpcOpenTelemetry,
+                // etc.)
+                // there may be additional metrics recorded. Only check to ensure the Gax Metrics
+                // are recorded properly. Any additional metrics are fine to be passed.
+                return metricData.size() >= NUM_GAX_OTEL_METRICS
+                    && areAllGaxMetricsRecorded(metricData);
+              });
+    } catch (ConditionTimeoutException e) {
+      Assertions.fail("Unable to collect all the GAX metrics required for the test");
     }
-    Assertions.fail("Unable to collect all the GAX metrics required for the test");
-    return new ArrayList<>();
+    return new ArrayList<>(metricReader.collectAllMetrics());
   }
 
   private boolean areAllGaxMetricsRecorded(List<MetricData> metricData) {
@@ -370,7 +377,7 @@ void testGrpc_operationCancelled_recordsMetrics() throws Exception {
     UnaryCallable<BlockRequest, BlockResponse> blockCallable = grpcClient.blockCallable();
     ApiFuture<BlockResponse> blockResponseApiFuture = blockCallable.futureCall(blockRequest);
     // Sleep 1s before cancelling to let the request go through
-    Thread.sleep(1000);
+    Awaitility.await().pollDelay(java.time.Duration.ofSeconds(1)).until(() -> true);
     blockResponseApiFuture.cancel(true);
 
     List<MetricData> actualMetricDataList = getMetricDataList();
@@ -397,7 +404,7 @@ void testHttpJson_operationCancelled_recordsMetrics() throws Exception {
     UnaryCallable<BlockRequest, BlockResponse> blockCallable = httpClient.blockCallable();
     ApiFuture<BlockResponse> blockResponseApiFuture = blockCallable.futureCall(blockRequest);
     // Sleep 1s before cancelling to let the request go through
-    Thread.sleep(1000);
+    Awaitility.await().pollDelay(java.time.Duration.ofSeconds(1)).until(() -> true);
     blockResponseApiFuture.cancel(true);
 
     List<MetricData> actualMetricDataList = getMetricDataList();