Forked process don't respect JAVA_HOME when calling java

[DaniRey]

With forkMode set to once or always new processes are forked to execute the tests. They start java using the command java, but in the maven ecosystem one would expect the forked processes to use $JAVA_HOME/bin/java instead. This would ensure that the same java version is used as for the build as for the test execution.

You can see the following line in your maven output when run with -X option (values in ??? are replaced with what they represent)
[14:07:24][???groupdId:artifactId???] [DEBUG] Forking ScalaTest via: /bin/sh -c cd ???basedir??? && java -Dbasedir=???basedir??? org.scalatest.tools.Runner -R '???basedir???/target/classes ???basedir???/target/test-classes' -o -u ???basedir???/target/surefire-reports/

but would expect the following

[14:07:24][???groupdId:artifactId???] [DEBUG] Forking ScalaTest via: /bin/sh -c cd ???basedir??? && $JAVA_HOME/bin/java -Dbasedir=???basedir??? org.scalatest.tools.Runner -R '???basedir???/target/classes ???basedir???/target/test-classes' -o -u ???basedir???/target/surefire-reports/

A typical symptom of this is the following error message:
Unsupported major.minor version 52.0

This happens if your java code is compiled with Java 8 and then executed with Java <8. Other combinations of Java versions will lead to slightly different error messages.

[cstroe]

@DaniRey It seems you're referring to how the java process is called here: https://github.com/scalatest/scalatest-maven-plugin/blob/master/src/main/java/org/scalatest/tools/maven/AbstractScalaTestMojo.java#L273

We should probably check if $JAVA_HOME is set before automatically using the java on the path.

[DaniRey]

@cstroe indeed, that's what I tried to refer to. Checking for $JAVA_HOME would be perfect.

[DaniRey]

@cstroe If I understand the change in #43 correctly, it currently has a bug.

if (isEmpty(jvmToUse)) will always return false and should be replaced with if(isEmpty(System.getProperty("java.home"))) or similar.

[cstroe]

@DaniRey The line before that if check is:

String jvmToUse = System.getProperty( "java.home" ) + File.separator + "bin" + File.separator + "java";

Does that clarify the way that variable is used?

[cstroe]

Also, I've made sure that the integration tests pass (we should make them run automatically via Travis-CI, see #39)

[cstroe]

Also, about checking $JAVA_HOME, that environment variable gets respected by the JVM, which is set as a system property. So it should be enough to check the java.home system property.

See: https://docs.oracle.com/javase/tutorial/essential/environment/sysprop.html

[DaniRey]

@cstroe Sorry, I still don't get it. I just wrote the following test

import org.junit.Test;

import java.io.File;

import static org.assertj.core.api.Assertions.assertThat;
import static org.apache.commons.lang3.StringUtils.isEmpty;

public class SystemPropertyTest {

    @Test
    public void unavailableTest() {
       String jvmToUse = System.getProperty( "nonexisting" ) + File.separator + "bin" + File.separator + "java";
       System.out.println(jvmToUse);
       assertThat(isEmpty(jvmToUse)).isTrue();
    }
}

and it fails with

/usr/lib/jvm/java-1.8.0-openjdk-amd64/bin/java ...
null/bin/java

org.junit.ComparisonFailure:
Expected :true
Actual :false

Am I missing something, or might the behavior not yet be as intended?

[cstroe]

@DaniRey I understand now. Since I'm appending to the "java.home" system property string, it will never be empty.

I fixed the logic to check the system property before appending and added tests around it. See the updated PR #43 .

[DaniRey]

@cstroe Looks good to me. Thank you.