Skip to content

Allow to specify JVM path #95

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 19, 2022
Merged

Conversation

sdruzkin
Copy link
Contributor

This change adds a capability to explicitly specify the path to Java executable for the forked process. This option works similar to Surefire's <jvm> configuration field.

We have custom version of Spark which uses JNI. We have a custom wrapper around java executable that starts the actual java processes using ld.so and sets all LD_ environment variables. Unfortunately the existing way of setting a JVM path via JAVA_HOME added in #43 doesn't quite work for us, as well as it's not unified with the Surefire config.

@cla-bot
Copy link

cla-bot bot commented Jul 11, 2022

Hi @sdruzkin, we require contributors to sign our Contributor License Agreement, and we don't have yours on file. In order for us to review and merge your code, please access https://www.artima.com/cla/choose-type to sign our Contributor License Agreement. Your effort is highly appreciated. Thank you.

@artimasites
Copy link

@cla-bot[bot] check

@cla-bot cla-bot bot added the cla-signed label Jul 11, 2022
@cla-bot
Copy link

cla-bot bot commented Jul 11, 2022

The cla-bot has been summoned, and re-checked this pull request!

@cheeseng
Copy link
Contributor

@sdruzkin I was just about to submit a PR to add the @parameter in the javadoc, seems you added already. :)

@bvenners Fyi I tested this and it works.

Thanks.

@sdruzkin
Copy link
Contributor Author

@bvenners Fyi I tested this and it works.

Apologies about missing unit tests. I tried to fit it into existing cases in PluginTest, but they only check the arguments passed to the CLI, not the executable. If needed I can try to expose some pieces of the CLI to make them more testable.

@sdruzkin
Copy link
Contributor Author

Up! Can someone please take a look and merge it?

@cheeseng cheeseng merged commit bf86733 into scalatest:master Jul 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants