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

Fixed providing jvm options in DAP #1245

Merged
merged 2 commits into from Apr 15, 2020

Conversation

alekseiAlefirov
Copy link
Contributor

@alekseiAlefirov alekseiAlefirov commented Apr 15, 2020

Issue: jvmOptions in ScalaMainClass have been ignored while running a debug session.

Note: not really sure, if the testing is done elegant enough, or can even be put into already existing test.

Copy link
Contributor

@jvican jvican left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great @alekseiAlefirov Seems like a good catch 👍 I LGTM it but I'm curious, could you try undoing the fix and making sure the test fails?

frontend/src/test/scala/bloop/dap/DebugServerSpec.scala Outdated Show resolved Hide resolved
@@ -46,7 +46,7 @@ private final class MainClassDebugAdapter(
env,
workingDir,
mainClass.`class`,
mainClass.arguments.toArray,
(mainClass.arguments ++ mainClass.jvmOptions).toArray,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, interesting. I thought we were passing the JVM options. Did you double-check that the test passes without this fix?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, it fails without it.

@jvican jvican added bug A defect or misbehaviour. task / debug labels Apr 15, 2020
Co-Authored-By: Jorge <jorge@vican.me>
Copy link
Contributor

@tgodzik tgodzik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking a look @alekseiAlefirov ! I can confirm it wasn't working before. This came up when working on the launch.json for VS Code

@jvican jvican merged commit ae96534 into scalacenter:master Apr 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A defect or misbehaviour. task / debug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants