From dd5164dc9de7f060bb0dab79820cb43ac434d703 Mon Sep 17 00:00:00 2001
From: Igor Bernstein <igorbernstein@google.com>
Date: Mon, 13 Sep 2021 13:26:33 -0400
Subject: [PATCH] fix: parallel execution and verbose grpc logs (#1004)

* chore: fix parallel execution and verbose grpc logs

Mucking around with java util logging fails when tests are executed in parallel. So make the 2 features exclusive: by default tests are executed in parallel, if verbose logs are needed they can be activated by a profile that will disable parallel execution.

Also switch parallel tests to run in threads instead of processes and enable parallel execution for unit tests

* fix sys prop name
---
 google-cloud-bigtable/pom.xml                 | 33 +++++++++++++++++--
 .../test_helpers/env/TestEnvRule.java         | 10 +++++-
 2 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/google-cloud-bigtable/pom.xml b/google-cloud-bigtable/pom.xml
index c9d3959d7..8c21a2cbb 100644
--- a/google-cloud-bigtable/pom.xml
+++ b/google-cloud-bigtable/pom.xml
@@ -279,6 +279,28 @@
   </dependencies>
 
   <profiles>
+    <profile>
+      <!-- Enable per-test verbose grpc logs
+      However, parallel test execution must be disabled to allow TestEnvRule to add/remove the
+      appender -->
+      <id>enable-verbose-grpc-logs</id>
+      <properties>
+        <bigtable.enable-grpc-logs>true</bigtable.enable-grpc-logs>
+        <!-- NOTE: bigtable.grpc-log-dir is configured separately for each execution -->
+      </properties>
+      <build>
+        <plugins>
+          <plugin>
+            <groupId>org.apache.maven.plugins</groupId>
+            <artifactId>maven-failsafe-plugin</artifactId>
+            <configuration>
+              <forkCount>1</forkCount>
+              <parallel>none</parallel>
+            </configuration>
+          </plugin>
+        </plugins>
+      </build>
+    </profile>
     <profile>
       <id>bigtable-emulator-it</id>
       <activation>
@@ -587,11 +609,11 @@
           <!-- enabled by profiles -->
           <skip>true</skip>
 
+          <!-- Enable concurrent test execution by default -->
           <parallel>classes</parallel>
-          <forkCount>2C</forkCount>
-          <threadCount>1</threadCount>
-          <reuseForks>true</reuseForks>
+          <threadCount>10</threadCount>
 
+          <!-- print full stacktraces by default -->
           <trimStackTrace>false</trimStackTrace>
         </configuration>
       </plugin>
@@ -602,6 +624,11 @@
         <configuration>
           <!-- enable the ability to skip unit tests, while running integration tests -->
           <skipTests>${skipUnitTests}</skipTests>
+
+          <!-- Enable concurrent test execution by default -->
+          <parallel>classes</parallel>
+          <threadCount>10</threadCount>
+
           <trimStackTrace>false</trimStackTrace>
         </configuration>
       </plugin>
diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/test_helpers/env/TestEnvRule.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/test_helpers/env/TestEnvRule.java
index 23b2c302c..d4470637a 100644
--- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/test_helpers/env/TestEnvRule.java
+++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/test_helpers/env/TestEnvRule.java
@@ -25,6 +25,7 @@
 import com.google.cloud.bigtable.admin.v2.models.Cluster;
 import com.google.cloud.bigtable.admin.v2.models.Instance;
 import com.google.common.base.Joiner;
+import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableSet;
 import java.io.IOException;
@@ -69,6 +70,8 @@
 public class TestEnvRule implements TestRule {
 
   private static final Logger LOGGER = Logger.getLogger(TestEnvRule.class.getName());
+  private static final Boolean BIGTABLE_ENABLE_VERBOSE_GRPC_LOGS =
+      Boolean.getBoolean("bigtable.enable-grpc-logs");
   private static final String BIGTABLE_GRPC_LOG_DIR = System.getProperty("bigtable.grpc-log-dir");
   private static final String BIGTABLE_EMULATOR_HOST_ENV_VAR = "BIGTABLE_EMULATOR_HOST";
   private static final String ENV_PROPERTY = "bigtable.env";
@@ -122,9 +125,14 @@ protected void before(Description description) throws Throwable {
   }
 
   private void configureLogging(Description description) throws IOException {
-    if (Strings.isNullOrEmpty(BIGTABLE_GRPC_LOG_DIR)) {
+    if (!BIGTABLE_ENABLE_VERBOSE_GRPC_LOGS) {
       return;
     }
+    Preconditions.checkState(
+        !Strings.isNullOrEmpty(BIGTABLE_GRPC_LOG_DIR),
+        "The property "
+            + BIGTABLE_GRPC_LOG_DIR
+            + " must be set when verbose grpc logs are enabled");
 
     Files.createDirectories(Paths.get(BIGTABLE_GRPC_LOG_DIR));