metals icon indicating copy to clipboard operation
metals copied to clipboard

improvement: add debug adapter for running main class to metals

Open kasiaMarek opened this issue 9 months ago • 1 comments

connected to: https://github.com/scalameta/metals/issues/5928 requires https://github.com/scalacenter/scala-debug-adapter/pull/704 first

kasiaMarek avatar May 07 '24 09:05 kasiaMarek

@kasiaMarek It's great that Metals will be able to debug without the BSP server having to support it.

In MainClassDebugAdapter you setup a process to run the mainclass and pass java options of -agentlib:jdwp=transport=dt_socket,server=y,suspend=y,quiet=n which makes sense.

Why not use the BSP buildTarget/run call instead? You can specify the above jvmOptions in ScalaMainClass.

It wouldn't simplify running main classes much but then you can debug tests in the same way using buildTarget/test. Metals wouldn't need to know anything about which test framework it's using.

Arthurm1 avatar May 10 '24 11:05 Arthurm1

Sorry for taking so long to write back, it slipped my mind.

Why not use the BSP buildTarget/run call instead?

We can do that but as you've noticed there isn't much of a benefit to it.

you can debug tests in the same way using buildTarget/test

We need events, if we just run the test like this we'd only get the output. So it's not really an easier option. (But I feel like I only have a partial understanding of this so correct if I'm wrong.)

kasiaMarek avatar May 21 '24 08:05 kasiaMarek

We need events, if we just run the test like this we'd only get the output. So it's not really an easier option. (But I feel like I only have a partial understanding of this so correct if I'm wrong.)

Do you mean test events? You'd get those through BSP Test Report messages.

Does Metals currently discover/run tests itself rather than delegating to BSP?

I'd argue that it's the Build Server's responsibility to do this as it should already understand the test frameworks it supports and then Metals doesn't have to understand any test frameworks, however BSP doesn't allow test discovery at the method level (only class level) and I can't currently see how it's possible to obtain location in the Gradle BSP which is fairly key in tracing failed tests. So I'm wondering if tests in BSP can't actually deliver as good a user experience as tests in Metals can.

EDIT: I don't think there's a way to send back stacktraces of failed tests from BSP either.

Arthurm1 avatar May 21 '24 09:05 Arthurm1

You'd get those through BSP Test Report messages.

Oh, yes, that should in theory work. But besides the inability to run only test selection there seem to be other issues there. The biggest is that the run output gets send to onBuildLogMessage, so it's hard distinguish it from any other logs. In theory there exists OnRunPrintStdout but I don't think any of the build servers actually use that. Bloop and sbt do not send Test Reports and looking at the code both sbt and mill seem to ignore java options send for running tests (but trying to test it on mill I run into other issues). I don't know how the situation looks for gradle but it seems that having the build servers obey specification (and more if we want test selection) might be more effort than running things from metals.

Does Metals currently discover/run tests itself rather than delegating to BSP?

I think we simply always start debug server to run tests.

kasiaMarek avatar May 24 '24 09:05 kasiaMarek

Ah - I hadn't realised that the build tools don't even send back TestReports. Probably best to just handle it all within Metals I guess.

Arthurm1 avatar May 24 '24 15:05 Arthurm1