chore: add junit5 extension to setup environment variable (#2799)

JoeWang1127 · lqiu96 · commit 7b494c6fc252 · 2024-05-23T16:12:40.000-04:00
Fix: #2785.
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
@@ -22,14 +22,6 @@ jobs:
         run: |
           mvn test --batch-mode --no-transfer-progress -Dcheckstyle.skip \
               -Dfmt.skip -DenableTestCoverage
-      # The `envVarTest` profile runs tests that require an environment variable
-      - name: Env Var Tests
-        run: |
-          mvn test --batch-mode --no-transfer-progress -Dcheckstyle.skip \
-              -Dfmt.skip -DenableTestCoverage -PenvVarTest
-        # Set the Env Var for this step only
-        env:
-          GOOGLE_CLOUD_UNIVERSE_DOMAIN: random.com
       - run: bazelisk version
       - name: Install Maven modules
         run: |
@@ -71,15 +63,7 @@ jobs:
       - name: Unit Tests
         run: |
           mvn test --batch-mode --no-transfer-progress -Dcheckstyle.skip \
-              -Dfmt.skip -DenableTestCoverage
-      # The `envVarTest` profile runs tests that require an environment variable
-      - name: Env Var Tests
-        run: |
-          mvn test --batch-mode --no-transfer-progress -Dcheckstyle.skip \
-              -Dfmt.skip -DenableTestCoverage -PenvVarTest
-        # Set the Env Var for this step only
-        env:
-          GOOGLE_CLOUD_UNIVERSE_DOMAIN: random.com
+
       - run: bazelisk version
       - name: Install Maven modules
         run: |
@@ -111,25 +95,14 @@ jobs:
           export PATH=${JAVA_HOME}/bin:$PATH
           # Maven surefire plugin lets us to specify the JVM when running tests via
           # the "jvm" system property.
-          mvn verify --batch-mode --no-transfer-progress -Dcheckstyle.skip \
+          mvn org.apache.maven.plugins:maven-surefire-plugin:test \
+              verify \
+              --batch-mode \
+              --no-transfer-progress \
+              -Dcheckstyle.skip \
               -Dfmt.skip \
-              -Djvm="${JAVA8_HOME}/bin/java"
-      # The `envVarTest` profile runs tests that require an environment variable
-      - name: Compile with Java 17 and run tests with Java 8 (Env Var Tests)
-        shell: bash
-        run: |
-          set -x
-          export JAVA_HOME=$JAVA_HOME
-          export PATH=${JAVA_HOME}/bin:$PATH
-          # Maven surefire plugin lets us to specify the JVM when running tests via
-          # the "jvm" system property.
-          export GOOGLE_CLOUD_UNIVERSE_DOMAIN=random.com
-          mvn test --batch-mode --no-transfer-progress -Dcheckstyle.skip \
-              -Dfmt.skip -DenableTestCoverage -Dsurefire.failIfNoSpecifiedTests=false \
-              -PenvVarTest
-        # Set the Env Var for this step only
-        env:
-          GOOGLE_CLOUD_UNIVERSE_DOMAIN: random.com
+              -Djvm="${JAVA8_HOME}/bin/java" \
+              -DargLine="-Djava.util.logging.SimpleFormatter.format='%1$tY %1$tl:%1$tM:%1$tS.%1$tL %2$s %4$s: %5$s%6$s%n'"
 
   build-java8-gapic-generator-java:
     name: "build(8) for gapic-generator-java"
diff --git a/.github/workflows/sonar.yaml b/.github/workflows/sonar.yaml
@@ -47,10 +47,6 @@ jobs:
             tar -xf showcase-*
             ./gapic-showcase run &
             cd -
-      # Intentionally do not run the Env Var Tests (no -PenvVarTests) as setting the Env Var
-      # may alter the results for other tests that use Env Var in the logic. Adding a Sonar
-      # step for a few tests (env var tests) may be overkill and should be better covered
-      # when we can upgrade to JUnit 5 (https://github.com/googleapis/sdk-platform-java/issues/1611#issuecomment-1970079325)
       - name: Build and analyze for full test coverage
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}  # Needed to get PR information, if any
diff --git a/gapic-generator-java-pom-parent/pom.xml b/gapic-generator-java-pom-parent/pom.xml
@@ -182,23 +182,6 @@
         </plugins>
       </build>
     </profile>
-    <profile>
-      <id>envVarTest</id>
-      <build>
-        <plugins>
-          <plugin>
-            <groupId>org.apache.maven.plugins</groupId>
-            <artifactId>maven-surefire-plugin</artifactId>
-            <configuration>
-              <!-- Unless overriden in the submodules, this profile run no tests in a submodule -->
-              <excludes>
-                <exclude>**/*.java</exclude>
-              </excludes>
-            </configuration>
-          </plugin>
-        </plugins>
-      </build>
-    </profile>
   </profiles>
   <repositories>
     <repository>
diff --git a/gax-java/dependencies.properties b/gax-java/dependencies.properties
@@ -88,3 +88,4 @@ maven.net_bytebuddy_byte_buddy=net.bytebuddy:byte-buddy:1.14.16
 maven.org_objenesis_objenesis=org.objenesis:objenesis:2.6
 maven.org_junit_jupiter_junit_jupiter_api=org.junit.jupiter:junit-jupiter-api:5.10.2
 maven.org_junit_jupiter_junit_jupiter_params=org.junit.jupiter:junit-jupiter-params:5.10.2
+maven.org_junit_pioneer_junit_pioneer=org.junit-pioneer:junit-pioneer:2.2.0
diff --git a/gax-java/gax/BUILD.bazel b/gax-java/gax/BUILD.bazel
@@ -41,6 +41,7 @@ _TEST_COMPILE_DEPS = [
     "@net_bytebuddy_byte_buddy//jar",
     "@org_objenesis_objenesis//jar",
     "@com_googlecode_java_diff_utils_diffutils//jar",
+    "@org_junit_pioneer_junit_pioneer//jar",
 ]
 
 java_library(
diff --git a/gax-java/gax/pom.xml b/gax-java/gax/pom.xml
@@ -99,32 +99,18 @@
         </configuration>
       </plugin>
       <plugin>
-        <!-- Troubleshooting a flaky test in https://github.com/googleapis/sdk-platform-java/issues/1931 -->
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-surefire-plugin</artifactId>
         <configuration>
-          <argLine>-Djava.util.logging.SimpleFormatter.format="%1$tY %1$tl:%1$tM:%1$tS.%1$tL %2$s %4$s: %5$s%6$s%n"</argLine>
-          <!-- These tests require an Env Var to be set. Use -PenvVarTest to ONLY run these tests -->
-          <test>!EndpointContextTest#endpointContextBuild_universeDomainEnvVarSet+endpointContextBuild_multipleUniverseDomainConfigurations_clientSettingsHasPriority</test>
+          <argLine>
+            -Djava.util.logging.SimpleFormatter.format="%1$tY %1$tl:%1$tM:%1$tS.%1$tL %2$s %4$s: %5$s%6$s%n"
+            <!-- workaround setup for using junit-pioneer with Java 17 or above,
+            see https://junit-pioneer.org/docs/environment-variables/#warnings-for-reflective-access -->
+            --add-opens java.base/java.util=ALL-UNNAMED
+            --add-opens java.base/java.lang=ALL-UNNAMED
+          </argLine>
         </configuration>
       </plugin>
     </plugins>
   </build>
-
-  <profiles>
-    <profile>
-      <id>envVarTest</id>
-      <build>
-        <plugins>
-          <plugin>
-            <groupId>org.apache.maven.plugins</groupId>
-            <artifactId>maven-surefire-plugin</artifactId>
-            <configuration>
-              <test>EndpointContextTest#endpointContextBuild_universeDomainEnvVarSet+endpointContextBuild_multipleUniverseDomainConfigurations_clientSettingsHasPriority</test>
-            </configuration>
-          </plugin>
-        </plugins>
-      </build>
-    </profile>
-  </profiles>
-</project>
+</project>
diff --git a/gax-java/gax/src/test/java/com/google/api/gax/rpc/EndpointContextTest.java b/gax-java/gax/src/test/java/com/google/api/gax/rpc/EndpointContextTest.java
@@ -40,6 +40,7 @@
 import java.io.IOException;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junitpioneer.jupiter.SetEnvironmentVariable;
 import org.mockito.Mockito;
 
 class EndpointContextTest {
@@ -337,8 +338,8 @@ void endpointContextBuild_gdchFlow_noUniverseDomain_customEndpoint() throws IOEx
 
   // This Universe Domain should match the `GOOGLE_CLOUD_UNIVERSE_DOMAIN` Env Var
   // For this test running locally or in CI, check that the Env Var is set properly.
-  // This test should only run when the maven profile `EnvVarTest` is enabled.
   @Test
+  @SetEnvironmentVariable(key = EndpointContext.GOOGLE_CLOUD_UNIVERSE_DOMAIN, value = "random.com")
   void endpointContextBuild_universeDomainEnvVarSet() throws IOException {
     String envVarUniverseDomain = "random.com";
     EndpointContext endpointContext =
@@ -352,11 +353,10 @@ void endpointContextBuild_universeDomainEnvVarSet() throws IOException {
 
   // This Universe Domain should match the `GOOGLE_CLOUD_UNIVERSE_DOMAIN` Env Var
   // For this test running locally or in CI, check that the Env Var is set properly.
-  // This test should only run when the maven profile `EnvVarTest` is enabled.
   @Test
+  @SetEnvironmentVariable(key = EndpointContext.GOOGLE_CLOUD_UNIVERSE_DOMAIN, value = "random.com")
   void endpointContextBuild_multipleUniverseDomainConfigurations_clientSettingsHasPriority()
       throws IOException {
-    // This test has `GOOGLE_CLOUD_UNIVERSE_DOMAIN` = `random.com`
     String clientSettingsUniverseDomain = "clientSettingsUniverseDomain.com";
     EndpointContext endpointContext =
         defaultEndpointContextBuilder
diff --git a/gax-java/pom.xml b/gax-java/pom.xml
@@ -211,6 +211,12 @@
       </exclusions>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.junit-pioneer</groupId>
+      <artifactId>junit-pioneer</artifactId>
+      <version>2.2.0</version>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
 
   <build>

Original file line number	Diff line number	Diff line change
`@@ -41,6 +41,7 @@ _TEST_COMPILE_DEPS = [`
`41`	`41`	`"@net_bytebuddy_byte_buddy//jar",`
`42`	`42`	`"@org_objenesis_objenesis//jar",`
`43`	`43`	`"@com_googlecode_java_diff_utils_diffutils//jar",`
	`44`	`+ "@org_junit_pioneer_junit_pioneer//jar",`
`44`	`45`	`]`
`45`	`46`
`46`	`47`	`java_library(`