Skip to content
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

Use JVM opts in test called by code lens #239

Open
GuillaumeDesforges opened this issue Nov 2, 2021 · 11 comments
Open

Use JVM opts in test called by code lens #239

GuillaumeDesforges opened this issue Nov 2, 2021 · 11 comments

Comments

@GuillaumeDesforges
Copy link

GuillaumeDesforges commented Nov 2, 2021

I'd like to launch tests using the code lens given by metal, however I need options set in .jvmopts:

--add-exports=java.base/sun.nio.ch=ALL-UNNAMED

This applies when I run sbt test and test pass, but not when I use Metal's code lens.
How can I fix that?

@tgodzik
Copy link
Contributor

tgodzik commented Nov 2, 2021

Thanks for reporting! It's not possible to do it currently, best to open an issue here https://github.com/scalameta/metals-feature-requests/issues

We can also just move the issue there if you prefer.

@GuillaumeDesforges
Copy link
Author

Okay thanks, please move this issue if possible :)

@ckipp01 ckipp01 transferred this issue from scalameta/metals-languageclient Nov 2, 2021
@tgodzik
Copy link
Contributor

tgodzik commented Nov 2, 2021

First action would be to add an ability for BSP to specify jvm options for running test classes

@olafurpg
Copy link
Member

olafurpg commented Nov 3, 2021

Is this a Bloop issue? Does sbt BSP have the same problem?

@adpi2
Copy link
Member

adpi2 commented Nov 3, 2021

Bloop and sbt have the same problem. To run the tests, the current implementation in Bloop and sbt only accepts a list of test suites, as List[String], in the data field of the DebugRequest.

However I don't think we need to add anything to BSP. We already have ScalaTestParams which contains a jvmOptions field. We can pass it in the data field of the DebugRequest.

So we just need to adapt the implementation in Bloop, sbt and Metals.

@olafurpg
Copy link
Member

olafurpg commented Nov 4, 2021

Does sbt use those JVM options when running sbt test? If so, then I would expect the flags to be picked up by default when running tests via Metals together with sbt BSP server. This should be an implementation detail of the build server and Metals shouldn't need to guess that a .jvmoptions file should be forwarded to the jvmOptions field of BSP.

@adpi2
Copy link
Member

adpi2 commented Nov 4, 2021

Agreed that the .jvmOptions file is an implementation detail of the build server and that Metals should not forward them.

However I think that jvmOptions should be transmitted by the build server to Metals in the buildTarget/scalaTestClasses request. It already works like that in the buildTarget/scalaMainClasses request, whose response contains:

export interface ScalaMainClassesItem {
  /** The build target that contains the test classes. */
  target: BuildTargetIdentifier;

  /** The main class item. */
  classes: ScalaMainClass[];
}

export interface ScalaMainClass {
  /** The main class to run. */
  class: String;

  /** The user arguments to the main entrypoint. */
  arguments: String[];

  /** The jvm options for the application. */
  jvmOptions: String[];

  /** The environment variables for the application. */
  environmentVariables?: String[];
}

Unfortunately ScalaTestClassesItem is:

export interface ScalaTestClassesItem {
  /** The build target that contains the test classes. */
  target: BuildTargetIdentifier;

  /** The fully qualified names of the test classes in this target */
  classes: String[];
}

And I think it should be:

export interface ScalaTestClassesItem {
  /** The build target that contains the test classes. */
  target: BuildTargetIdentifier;

  /** The jvm options for the application. */
  jvmOptions: String[];

  /** The environment variables for the application. */
  environmentVariables?: String[];

  /** The fully qualified names of the test classes in this target */
  classes: String[];
}

Assuming Metals receives those jvmOptions should it systematically forward them back? Symetrically, when server receives some jvmOptions field in the debugSession/start request, what should it do?

  1. Add them to the pre-existing one. (That means Metals does not need to forward everything)
  2. Override the pre-existing one. (That means Metals should forward them most of the time)

Option 1 is easier to use in Metals but it won't be possible to override the existing options.
And option 2 is a bit cumbersome but more powerful.

I am slightly more in favor of 1 because in any case the user can change the jvmOptions in the build definition.

@GuillaumeDesforges
Copy link
Author

Until this feature is implemented, do you think there a workaround?

@adpi2
Copy link
Member

adpi2 commented Nov 10, 2021

Until this feature is implemented, do you think there a workaround?

Do you use sbt or Bloop as the build server? If sbt you can try to set Test / fork := true in your project.

@GuillaumeDesforges
Copy link
Author

GuillaumeDesforges commented Nov 15, 2021

Thanks, will try asap (blocked by scalameta/metals-vscode#758 atm...)

@GuillaumeDesforges
Copy link
Author

Test / fork := true did not solve my issue, the same error still appears.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants