You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by ti...@apache.org on 2020/04/28 20:46:48 UTC

[maven-surefire] branch master updated: [SUREFIRE-1784] Fork JVM defined by jvm parameter should not inherit JAVA_HOME from Maven process

This is an automated email from the ASF dual-hosted git repository.

tibordigana pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven-surefire.git


The following commit(s) were added to refs/heads/master by this push:
     new d8fe77d  [SUREFIRE-1784] Fork JVM defined by jvm parameter should not inherit JAVA_HOME from Maven process
d8fe77d is described below

commit d8fe77d163a1da78b6eaaaadede1ad9d62eb5b31
Author: akomakom <ak...@users.noreply.github.com>
AuthorDate: Tue Apr 28 16:46:38 2020 -0400

    [SUREFIRE-1784] Fork JVM defined by jvm parameter should not inherit JAVA_HOME from Maven process
---
 .../plugin/surefire/AbstractSurefireMojo.java      |  4 ++
 .../plugin/surefire/AbstractSurefireMojoTest.java  | 11 ++--
 .../AbstractSurefireMojoToolchainsTest.java        | 69 +++++++++++++++++++++-
 3 files changed, 78 insertions(+), 6 deletions(-)

diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java
index ee3106a..6fd5df4 100644
--- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java
+++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java
@@ -2537,6 +2537,10 @@ public abstract class AbstractSurefireMojo
             }
 
             File jdkHome = toJdkHomeFromJvmExec( pathToJava.getPath() );
+            if ( !environmentVariables.containsKey( "JAVA_HOME" ) )
+            {
+                environmentVariables.put( "JAVA_HOME", jdkHome.getAbsolutePath() );
+            }
             BigDecimal version = jdkHome == null ? null : toJdkVersionFromReleaseFile( jdkHome );
             boolean javaVersion9 = version == null ? isJava9AtLeast( pathToJava.getPath() ) : isJava9AtLeast( version );
             return new JdkAttributes( pathToJava.getPath(), javaVersion9 );
diff --git a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoTest.java b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoTest.java
index fc9a6fc..a2a8795 100644
--- a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoTest.java
+++ b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoTest.java
@@ -62,7 +62,6 @@ import org.powermock.modules.junit4.PowerMockRunner;
 
 import java.io.File;
 import java.io.IOException;
-import java.lang.reflect.Field;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -97,6 +96,7 @@ import static org.powermock.api.mockito.PowerMockito.mock;
 import static org.powermock.api.mockito.PowerMockito.spy;
 import static org.powermock.api.mockito.PowerMockito.verifyPrivate;
 import static org.powermock.reflect.Whitebox.invokeMethod;
+import static org.powermock.reflect.Whitebox.setInternalState;
 
 /**
  * Test for {@link AbstractSurefireMojo}.
@@ -2179,9 +2179,12 @@ public class AbstractSurefireMojoTest
 
         public void setToolchain( Toolchain toolchain ) throws Exception
         {
-            Field toolchainField = AbstractSurefireMojo.class.getDeclaredField( "toolchain" );
-            toolchainField.setAccessible( true );
-            toolchainField.set( this, toolchain );
+            setInternalState( this, "toolchain", toolchain );
+        }
+
+        public void setJvm( String jvm ) throws Exception
+        {
+            setInternalState( this, "jvm", jvm );
         }
     }
 
diff --git a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoToolchainsTest.java b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoToolchainsTest.java
index 5fd8101..64437fb 100644
--- a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoToolchainsTest.java
+++ b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoToolchainsTest.java
@@ -21,6 +21,7 @@ package org.apache.maven.plugin.surefire;
 
 import org.apache.maven.execution.MavenSession;
 import org.apache.maven.plugin.MojoFailureException;
+import org.apache.maven.surefire.shared.io.FilenameUtils;
 import org.apache.maven.toolchain.Toolchain;
 import org.apache.maven.toolchain.ToolchainManager;
 import org.apache.maven.toolchain.java.DefaultJavaToolChain;
@@ -31,6 +32,7 @@ import org.powermock.core.classloader.annotations.PowerMockIgnore;
 import org.powermock.core.classloader.annotations.PrepareForTest;
 import org.powermock.modules.junit4.PowerMockRunner;
 
+import java.io.File;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -39,6 +41,7 @@ import static java.util.Collections.emptyMap;
 import static java.util.Collections.singletonList;
 import static java.util.Collections.singletonMap;
 import static junit.framework.TestCase.assertNull;
+import static org.apache.maven.surefire.booter.SystemUtils.toJdkHomeFromJre;
 import static org.fest.assertions.Assertions.assertThat;
 import static org.powermock.api.mockito.PowerMockito.mock;
 import static org.powermock.api.mockito.PowerMockito.mockStatic;
@@ -145,15 +148,77 @@ public class AbstractSurefireMojoToolchainsTest
     {
         AbstractSurefireMojoTest.Mojo mojo = new AbstractSurefireMojoTest.Mojo();
         DefaultJavaToolChain toolchain = mock( DefaultJavaToolChain.class );
-        when( toolchain.findTool( "java" ) ).thenReturn( "/some/path/bin/java" );
+        when( toolchain.findTool( "java" ) ).thenReturn( "/path/from/toolchain" );
         when( toolchain.getJavaHome() ).thenReturn( "/some/path" );
         mojo.setToolchain( toolchain );
 
         assertThat( mojo.getEnvironmentVariables() ).isEmpty();
-        invokeMethod( mojo, "getEffectiveJvm" );
+        JdkAttributes effectiveJvm = invokeMethod( mojo, "getEffectiveJvm" );
         assertThat( mojo.getEnvironmentVariables() ).includes( MapAssert.entry( "JAVA_HOME", "/some/path" ) );
+        assertThat( effectiveJvm.getJvmExecutable() ).contains( "/path/from/toolchain" );
     }
 
+    @Test
+    public void shouldNotChangeJavaHomeFromToolchainIfAlreadySet() throws Exception
+    {
+        AbstractSurefireMojoTest.Mojo mojo = new AbstractSurefireMojoTest.Mojo();
+        mojo.setEnvironmentVariables( singletonMap( "JAVA_HOME", "/already/set/path" ) );
+
+        DefaultJavaToolChain toolchain = mock( DefaultJavaToolChain.class );
+        when( toolchain.findTool( "java" ) ).thenReturn( "/path/from/toolchain" );
+        when( toolchain.getJavaHome() ).thenReturn( "/some/path" );
+        mojo.setToolchain( toolchain );
+
+        JdkAttributes effectiveJvm = invokeMethod( mojo, "getEffectiveJvm" );
+        assertThat( mojo.getEnvironmentVariables() ).includes( MapAssert.entry( "JAVA_HOME", "/already/set/path" ) );
+        assertThat( effectiveJvm.getJvmExecutable() ).contains( "/path/from/toolchain" );
+    }
+
+    /**
+     * Ensures that the environmentVariables map for launching a test jvm
+     * contains a jvm parameter-driven entry when jvm is set.
+     */
+    @Test
+    public void shouldChangeJavaHomeFromJvm() throws Exception
+    {
+        AbstractSurefireMojoTest.Mojo mojo = new AbstractSurefireMojoTest.Mojo();
+
+        File currentJdkHome = toJdkHomeFromJre();
+        String javaExecutablePath = FilenameUtils.concat(
+            currentJdkHome.getAbsolutePath(), "bin/java" );
+
+        mojo.setJvm( javaExecutablePath );
+
+        assertThat( mojo.getEnvironmentVariables() ).isEmpty();
+        JdkAttributes effectiveJvm = invokeMethod( mojo, "getEffectiveJvm" );
+        assertThat( mojo.getEnvironmentVariables() ).
+            includes( MapAssert.entry( "JAVA_HOME", currentJdkHome.getAbsolutePath() ) );
+        assertThat( effectiveJvm.getJvmExecutable() ).contains( javaExecutablePath );
+    }
+
+    /**
+     * Ensures that users can manually configure a value for JAVA_HOME
+     * and we will not override it
+     */
+    @Test
+    public void shouldNotChangeJavaHomeFromJvmIfAlreadySet() throws Exception
+    {
+        AbstractSurefireMojoTest.Mojo mojo = new AbstractSurefireMojoTest.Mojo();
+        mojo.setEnvironmentVariables( singletonMap( "JAVA_HOME", "/already/set/path" ) );
+
+        File currentJdkHome = toJdkHomeFromJre();
+        String javaExecutablePath = FilenameUtils.concat(
+            currentJdkHome.getAbsolutePath(), "bin/java" );
+
+        mojo.setJvm( javaExecutablePath );
+
+        JdkAttributes effectiveJvm = invokeMethod( mojo, "getEffectiveJvm" );
+        assertThat( mojo.getEnvironmentVariables() ).
+            includes( MapAssert.entry( "JAVA_HOME", "/already/set/path" ) );
+        assertThat( effectiveJvm.getJvmExecutable() ).contains( javaExecutablePath );
+    }
+
+
 
     /**
      * Mocks a ToolchainManager