-
Notifications
You must be signed in to change notification settings - Fork 338
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
Move usage of Scala Debug Adapter to Metals #5928
Comments
What is the expected impact on detecting and executing tests in Metals? Currently, Metals is not detecting tests in Mill projects when mill-bsp is used. This is due to the fact, that Metals uses the |
We wouldn't detect tests in Metals, we would still need to use the buildTargetScalaTestClasses endpoint. With that, we could use the jvm test environment request to get all that we need to execute test via DAP. Both of those requests are implemented in mill sbt and Bloop. |
Great. |
How would test results be collated - is that currently done by the build server and passed back by BSP? |
That's done via DAP itself as far as I know, so no change would be needed. We would need to make sure that the results are properly forwarded in case I am mistaken |
@tgodzik What does the BSP server need to implement if the Scala Debug Adapter is moved to Metals? Does it no longer have to implement And then Metals/DAP passes the debugging options in the |
The plan is to just run the process inaide metals using the scala-debug-adapter library, which means we would need jvmEnvironment and mainClasses/testClasses endpoints. At least that's my hope currently. We could try to use run/test endpoints but that would be more complicated |
This should now work. For any server that doesn't implement debugging we will start things inside Metals, though we still need test discovery to be done on the server side. |
Currently, both sbt BSP and Bloop use the scala-debug-adapter library to run debug and test. However, this means each tool needs to implement integration with that library.
It would make much more sense to implement this in Metals, where any additional bsp server would be able to use it.
The text was updated successfully, but these errors were encountered: