stop using com.github.stefanbirkner.system-rules to mock the system environment variables, instead just allow the callback to set them - as a result no illegal access is needed anymore (YAY)

Author: TheSnoozer

.github/workflows/default-tests.yml

@@ -31,18 +31,7 @@ jobs:
     needs: checkstyle
     strategy:
       matrix:
-        java_allow_illegal_access: [ false ]
-        java_version: [ '11', '12', '13', '14', '15' ]
-        include:
-          - java_version: '16'
-            java_allow_illegal_access: true
-          - java_version: '17'
-            java_allow_illegal_access: true
-          - java_version: '18'
-            java_allow_illegal_access: true
-          - java_version: '19'
-            java_allow_illegal_access: true
-
+        java_version: [ '11', '12', '13', '14', '15', '16', '17', '18', '19' ]
     steps:
       - uses: actions/checkout@v3
         with:
@@ -59,12 +48,8 @@ jobs:
           path: ~/.m2
           key: ${{ runner.os }}-m2-${{ hashFiles('**/pom.xml') }}
           restore-keys: ${{ runner.os }}-m2
-      - name: Build with Maven (no illegal access allowed)
-        if: ${{ ! matrix.java_allow_illegal_access }}
+      - name: Build with Maven
         run: mvn clean verify javadoc:javadoc -B
-      - name: Build with Maven (illegal access allowed)
-        if: ${{ matrix.java_allow_illegal_access }}
-        run: mvn clean verify javadoc:javadoc -Pjava-allow-illegal-access -B
 
   coveralls:
     name: Run coveralls

pom.xml

@@ -238,12 +238,6 @@
             <version>1.1.1</version>
             <scope>test</scope>
         </dependency>
-        <dependency>
-            <groupId>com.github.stefanbirkner</groupId>
-            <artifactId>system-rules</artifactId>
-            <version>1.19.0</version>
-            <scope>test</scope>
-        </dependency>
         <!-- slf4j is a dependency of jgit, this suppresses slf4j's warning during testing when we mock jgit stuff -->
         <dependency>
             <groupId>org.slf4j</groupId>

src/main/java/pl/project13/core/GitCommitIdPlugin.java

@@ -33,6 +33,16 @@
 public class GitCommitIdPlugin {
   public interface Callback {
 
+    /**
+     * The system environment variables.
+     * Implementations usually do not need to set or overwrite this,
+     * this is mainly meant for testing purposes...
+     * @return unmodifiable string map view of the current system environment {@link System#getenv}.
+     */
+    default Map<String, String> getSystemEnv() {
+      return System.getenv();
+    }
+
     /**
      * @return Supplier that provides the version of the project that is currently evaluated.
      *         Used to determine {@link GitCommitPropertyConstant#BUILD_VERSION}.
@@ -312,7 +322,7 @@ protected static void loadBuildData(@Nonnull Callback cb, @Nonnull Properties pr
     Map<String, Supplier<String>> additionalProperties = Collections.singletonMap(
             GitCommitPropertyConstant.BUILD_VERSION, cb.supplyProjectVersion());
     BuildServerDataProvider buildServerDataProvider = BuildServerDataProvider.getBuildServerProvider(
-            System.getenv(), cb.getLogInterface());
+            cb.getSystemEnv(), cb.getLogInterface());
     buildServerDataProvider
             .setDateFormat(cb.getDateFormat())
             .setDateFormatTimeZone(cb.getDateFormatTimeZone())
@@ -345,7 +355,7 @@ private static void loadGitDataWithNativeGit(@Nonnull Callback cb, @Nonnull Prop
             .setIncludeOnlyProperties(cb.getIncludeOnlyProperties())
             .setOffline(cb.isOffline());
 
-    nativeGitProvider.loadGitData(cb.getEvaluateOnCommit(), properties);
+    nativeGitProvider.loadGitData(cb.getEvaluateOnCommit(), cb.getSystemEnv(), properties);
   }
 
   private static void loadGitDataWithJGit(@Nonnull Callback cb, @Nonnull Properties properties) throws GitCommitIdExecutionException {
@@ -362,6 +372,6 @@ private static void loadGitDataWithJGit(@Nonnull Callback cb, @Nonnull Propertie
             .setIncludeOnlyProperties(cb.getIncludeOnlyProperties())
             .setOffline(cb.isOffline());
 
-    jGitProvider.loadGitData(cb.getEvaluateOnCommit(), properties);
+    jGitProvider.loadGitData(cb.getEvaluateOnCommit(), cb.getSystemEnv(), properties);
   }
 }

src/main/java/pl/project13/core/GitDataProvider.java

@@ -247,10 +247,11 @@ public GitDataProvider setOffline(boolean offline) {
    *
    * @param evaluateOnCommit The commit that should be used as reference to generate the properties from.
    *                         Defaults to {@code HEAD}.
+   * @param env unmodifiable string map view of the current system environment {@link System#getenv}.
    * @param properties The Properties-Set that should be enriched by the generated one.
    * @throws GitCommitIdExecutionException In case any problem occurred during loading of the properties from the git repository.
    */
-  public void loadGitData(@Nonnull String evaluateOnCommit, @Nonnull Properties properties) throws GitCommitIdExecutionException {
+  protected void loadGitData(@Nonnull String evaluateOnCommit, @Nonnull Map<String,String> env, @Nonnull Properties properties) throws GitCommitIdExecutionException {
     this.evaluateOnCommit = evaluateOnCommit;
     init();
     // git.user.name
@@ -263,7 +264,7 @@ public void loadGitData(@Nonnull String evaluateOnCommit, @Nonnull Properties pr
       validateAbbrevLength(abbrevLength);
 
       // git.branch
-      maybePut(properties, GitCommitPropertyConstant.BRANCH, () -> determineBranchName(System.getenv()));
+      maybePut(properties, GitCommitPropertyConstant.BRANCH, () -> determineBranchName(env));
       // git.commit.id.describe
       maybePutGitDescribe(properties);
       loadShortDescribe(properties);
@@ -359,7 +360,7 @@ void validateAbbrevLength(int abbrevLength) throws GitCommitIdExecutionException
    * If running within Jenkins/Hudson, honor the branch name passed via GIT_BRANCH env var.
    * This is necessary because Jenkins/Hudson always invoke build in a detached head state.
    *
-   * @param env environment settings
+   * @param env unmodifiable string map view of the current system environment {@link System#getenv}.
    * @return results of getBranchName() or, if in Jenkins/Hudson, value of GIT_BRANCH
    * @throws GitCommitIdExecutionException the branch name could not be determined
    */

src/test/java/pl/project13/core/GitCommitIdPluginIntegrationTest.java

@@ -23,7 +23,6 @@
 import org.eclipse.jgit.api.Git;
 import org.eclipse.jgit.api.ResetCommand;
 import org.junit.*;
-import org.junit.contrib.java.lang.system.EnvironmentVariables;
 import org.junit.runner.RunWith;
 import pl.project13.core.git.GitDescribeConfig;
 import pl.project13.core.util.JsonManager;
@@ -43,10 +42,6 @@
 
 @RunWith(JUnitParamsRunner.class)
 public class GitCommitIdPluginIntegrationTest {
-
-  @Rule
-  public final EnvironmentVariables environmentVariablesMock = new EnvironmentVariables();
-
   private static final boolean UseJGit = false;
   private static final boolean UseNativeGit = true;
 
@@ -282,16 +277,6 @@ public void shouldNotUseBuildEnvironmentBranchInfoWhenParameterSet(boolean useNa
     env.put("JENKINS_URL", "http://myciserver.com");
     env.put("GIT_BRANCH", "mybranch");
     env.put("GIT_LOCAL_BRANCH", "localbranch");
-      
-    // remove all keys from System.getenv()
-    List<String> keySet = new ArrayList<>(System.getenv().keySet());
-    keySet.stream().forEach(key -> environmentVariablesMock.set(key, null));
-
-    // set System.getenv() to be equal to given parameter env
-    env.entrySet().stream().forEach(entry -> environmentVariablesMock.set(entry.getKey(), entry.getValue()));
-      
-    // verify that System.getenv() is actually equal
-    Assert.assertEquals(env, System.getenv());
 
     // reset repo and force detached HEAD
     try (final Git git = git(dotGitDirectory)) {
@@ -364,25 +349,13 @@ private void shouldUseJenkinsBranchInfoWhenAvailableHelperAndAssertBranch(boolea
             new GitCommitIdTestCallback()
                     .setDotGitDirectory(dotGitDirectory)
                     .setUseNativeGit(useNativeGit)
+                    .setSystemEnv(env)
                     .setUseBranchNameFromBuildEnvironment(true)
                     .build();
     Properties properties = new Properties();
 
     // given
 
-    // remove all keys from System.getenv()
-    List<String> keySet = new ArrayList<>(System.getenv().keySet());
-    for (String key: keySet) {
-      environmentVariablesMock.set(key, null);
-    }
-    // set System.getenv() to be equal to given parameter env
-    for (Map.Entry<String, String> entry: env.entrySet()) {
-      environmentVariablesMock.set(entry.getKey(), entry.getValue());
-    }
-
-    // verify that System.getenv() is actually equal
-    Assert.assertEquals(env, System.getenv());
-
     // reset repo and force detached HEAD
     try (final Git git = git(dotGitDirectory)) {
       git.reset().setMode(ResetCommand.ResetType.HARD).setRef("b6a73ed").call();

src/test/java/pl/project13/core/GitCommitIdTestCallback.java

@@ -32,6 +32,7 @@
 import java.util.function.Supplier;
 
 public class GitCommitIdTestCallback {
+  private Map<String, String> systemEnv = System.getenv();
   private String projectVersion = "dummy-version";
   private LogInterface logInterface = createDummyLogInterface();
   private String dateFormat = "yyyy-MM-dd'T'HH:mm:ssZ";
@@ -65,6 +66,11 @@ public GitCommitIdTestCallback() {
     }
   }
 
+  public GitCommitIdTestCallback setSystemEnv(Map<String, String> systemEnv) {
+    this.systemEnv = systemEnv;
+    return this;
+  }
+
   public GitCommitIdTestCallback setProjectVersion(String projectVersion) {
     this.projectVersion = projectVersion;
     return this;
@@ -187,6 +193,11 @@ public GitCommitIdTestCallback setShouldPropertiesEscapeUnicode(boolean shouldPr
 
   public GitCommitIdPlugin.Callback build() {
     return new GitCommitIdPlugin.Callback() {
+      @Override
+      public Map<String, String> getSystemEnv() {
+        return systemEnv;
+      }
+
       @Override
       public Supplier<String> supplyProjectVersion() {
         return () -> projectVersion;