make bsp build on TestModule success instead of fail #4103
Conversation
| @@ -385,6 +385,7 @@ private class MillBuildServer( | |||
| case (m: SemanticDbJavaModule, ev) if clientWantsSemanticDb => | |||
| (m.compiledClassesAndSemanticDbFiles, ev) | |||
| case (m: JavaModule, ev) => (m.compile, ev) | |||
| case (m: TestModule, ev) => (Task.Anon{()}, ev) | |||
There was a problem hiding this comment.
Alternatively, we could trigger testClasspath, which should trigger the compilation, if any. It might trigger a bit more than just compilation. I also can't imagine, why we not already caught the compilation by the matches above, so doing nothing might be valid too, although I'd like to leave a log message in such case, so whoever has issues and inspects the logs, she can see that we had to make a decision along the way.
There was a problem hiding this comment.
Technically, calling compile on such a module is wrong. We either report the wrong module capabilities (canCompile) or IJ is doing it wrong and we should open an issue there.
There was a problem hiding this comment.
@chikei Could you provide a reproducer for you initial issue?
There was a problem hiding this comment.
So, I just read the older issue and PRs to that topic and I think this is an issue in IJ. Can you please open an issue there? We should also link that issue as a code comment here, since otherwise a future maintainer will reasonably decide to remove this case.
There was a problem hiding this comment.
I guess a way to handle it instead of failing the whole request would be to just ignore the specific build target identifier which don't represent compilable modules and omit it in the result list. This is a defensive consequence of BSP working on bulk requests, and we already handle partial error the same way in other BSP requests.
There was a problem hiding this comment.
I guess a way to handle it instead of failing the whole request would be to just ignore the specific build target identifier which don't represent compilable modules and omit it in the result list. This is a defensive consequence of BSP working on bulk requests, and we already handle partial error the same way in other BSP requests.
Forget that. That won't work here, since the result value does not have a list with an item per build target identifier.
|
reproducer mill project: https://github.com/chikei/mill-bsp-test-only-module |
| @@ -385,6 +385,7 @@ private class MillBuildServer( | |||
| case (m: SemanticDbJavaModule, ev) if clientWantsSemanticDb => | |||
| (m.compiledClassesAndSemanticDbFiles, ev) | |||
| case (m: JavaModule, ev) => (m.compile, ev) | |||
| case (m: TestModule, ev) => (Task.Anon { () }, ev) | |||
There was a problem hiding this comment.
| case (m: TestModule, ev) => (Task.Anon { () }, ev) | |
| // See also https://youtrack.jetbrains.com/issue/SCL-23352 | |
| case (m: TestModule, ev) => | |
| val task = Task.Anon { | |
| T.log.debug("Ignoring invalid compile request for test module ${m.bspBuildTarget.displayName}") | |
| } | |
| (task, ev) |
IJ with BSP will make bsp call
BuildTargetCompilewith allBuildTarget, additional test module (introduced in #3064) will cause the bsp call fail as it falls into third case in the pattern match. This PR makes it does nothing and always success.