search

microsoft / build-server-for-gradle

An implementation of the Build Server Protocol for Gradle
MIT License
45 stars 7 forks source link

Support debug test. #161

Open jdneo opened 2 days ago

jdneo commented 2 days ago

@jdneo I think Metals used debugSessionStart but for DebugSessionParamsDataKind used the same data kinds we use for tests e.g. scala-test-suites-selection and scala-test.

What's kind of interesting is that different clients might want different debug adapters....

Metals is primarily for Scala development so it uses https://github.com/scalacenter/scala-debug-adapter For Kotlin there is https://github.com/fwcd/kotlin-debug-adapter Java has https://github.com/microsoft/java-debug

I imagine the Java one is good enough for all but it would be handy to be able to specify the type in the BSP command and have the build server kick start the relevant one. Another thing to propose to add to BSP spec.

Originally posted by @Arthurm1 in https://github.com/microsoft/build-server-for-gradle/issues/144#issuecomment-2199608819

jdneo commented 2 days ago

Ah this is different from what I was thinking about.

My original thought is that, just having a flag to tell whether this is a debug request. And then use TestLauncher.debugTestsOn(int) at gradle tooling api side.

Arthurm1 commented 2 days ago

And then on the client side you attach to process?

jdneo commented 2 days ago

Yes, and we don't need to care about the client debug adapter implementation.

Arthurm1 commented 1 day ago

You could test this without any BSP changes.

Both ScalaTestSuites and ScalaTestParams allow the client to pass jvmoptions.

debugTestsOn adds -agentlib:jdwp=transport=dt_socket,server=n,suspend=n,address=localhost:<port> to jvmoptions which you could pass in.

Although debugTestsOn also disables parallel test execution.

The issue with adding a flag to BSP is that's not language agnostic. Do all languages debug by attaching to a port number? Maybe it doesn't matter if we're using ScalaTestSuites and ScalaTestParams as these are JVM languages only.

jdneo commented 1 day ago

Although debugTestsOn also disables parallel test execution.

Yes this is the reason I was thinking about not to append jdwp args at first.

I'll think twice to see what is the best way to do that.