[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #289: SUREFIRE-1784 Fork JVM defined by jvm param should not inherit JAVA_HOME from Maven process

[GitBox <gi...@apache.org>]

Tibor17 commented on a change in pull request #289:
URL: https://github.com/apache/maven-surefire/pull/289#discussion_r416446480



##########
File path: maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoToolchainsTest.java
##########
@@ -154,6 +157,36 @@ public void shouldChangeJavaHomeFromToolchain() throws Exception
         assertThat( mojo.getEnvironmentVariables() ).includes( MapAssert.entry( "JAVA_HOME", "/some/path" ) );
     }
 
+    /**
+     * 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();
+        String fakeJava = "/some/path/to/jdk/bin/java";
+        mojo.setJvm( fakeJava );
+
+        //Set up a mocked File-object
+        File mockedFile = mock( File.class );
+        when( mockedFile.getAbsoluteFile() ).thenReturn( mockedFile );
+        when( mockedFile.getPath() ).thenReturn( fakeJava );
+        when( mockedFile.getName() ).thenReturn( "java" );
+        when( mockedFile.isFile() ).thenReturn( true );
+
+        //Trap constructor calls to return the mocked File-object
+        whenNew( File.class ).
+            withParameterTypes( String.class ).
+            withArguments( anyString() ).
+            thenReturn( mockedFile );
+
+        assertThat( mojo.getEnvironmentVariables() ).isEmpty();
+        invokeMethod( mojo, "getEffectiveJvm" );
+        assertThat( mojo.getEnvironmentVariables() ).includes( MapAssert.entry( "JAVA_HOME", "/some/path/to/jdk" ) );

Review comment:
       The Windows fails with 
   `java.lang.AssertionError: the map:<{'JAVA_HOME'='D:\some\path\to\jdk'}> does not contain the entry:<['JAVA_HOME'='/some/path/to/jdk']>`
   What about to do it a bit of easier and use the real path taken from system property `java.home` which is JRE. It would be the value for `String fakeJava`. Then we would not have to mock the `File`.
   It would be worth to write the second test where the `environmentVariables` has already some other path and the assertion statement would verify that the `environmentVariables` is intact.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org