Revert "chore: add junit5 extension to setup environment variable" (#…

…2817) Reverts #2799 since `junit-pioneer` is not compatible with Java 8.
googleapis · May 24, 2024 · 3ba43c3 · 3ba43c3
1 parent 9f3a6f2
commit 3ba43c3
Show file tree

Hide file tree

Showing 8 changed files with 81 additions and 27 deletions.
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
@@ -22,6 +22,14 @@ 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: |
@@ -63,7 +71,15 @@ 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: |
@@ -95,14 +111,25 @@ 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 org.apache.maven.plugins:maven-surefire-plugin:test \
-              verify \
-              --batch-mode \
-              --no-transfer-progress \
-              -Dcheckstyle.skip \
+          mvn verify --batch-mode --no-transfer-progress -Dcheckstyle.skip \
               -Dfmt.skip \
-              -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'"
+              -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
 
   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,6 +47,10 @@ 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,6 +182,23 @@
         </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,4 +88,3 @@ 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,7 +41,6 @@ _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,18 +99,32 @@
         </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"
-            <!-- 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>
+          <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>
         </configuration>
       </plugin>
     </plugins>
   </build>
-</project>
+
+  <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>
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,7 +40,6 @@
 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 {
@@ -338,8 +337,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 =
@@ -353,10 +352,11 @@ 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
@@ -205,12 +205,6 @@
       </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>