Tibor17 commented on a change in pull request #289:
URL: https://github.com/apache/maven-surefire/pull/289#discussion_r416448777
##########
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" );
Review comment:
It would be nice to add an assertion statement for returned value and
check the value in `JdkAttributes#getJvmExecutable()`. The point of the test is
to be very sensitive to a change and fail, that's why more and more assertions
are employed.
----------------------------------------------------------------
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:
us...@infra.apache.org
