Improve test BSP service
#1769
Improve test BSP service
#1769
Conversation
There was a problem hiding this comment.
Solid job @oyvindberg. I think we should add a testcase for fixed scenario (something near e.g. BspMetalsClientSpec?)
| runAll(testTasks).map { testRuns => | ||
| // When the test execution is over report no matter what the result is | ||
| testEventHandler.report() | ||
| logger.debug(s"Test suites failed: $failure") | ||
| val isOk = !failure && exitCodes.forall(_ == 0) | ||
| if (isOk) state.mergeStatus(ExitStatus.Ok) | ||
| else state.copy(status = ExitStatus.TestExecutionError) | ||
| TestRuns(testRuns) |
There was a problem hiding this comment.
I wonder if testEventHandler is suitable for handling parallel execution of task, I see mutable collections there and plain Ints :/ but ofc this isn't concern of this PR
There was a problem hiding this comment.
the test code seems a bit half baked, so I wouldn't be surprised if some fixes are necessary. A related issue I came across is that clients are not (AFAIU) able to request parallel execution at all for now. Perhaps that could be a bloop specific field inside the request data strucure again.
|
I agree there should be tests. I'm thinking to duplicate the structure for the compile tests and adapt the structure for test end point. or something like that. It'll be a day or two before I get to it |
|
added a test now |
|
Just an update. I've had an hour here and there over the last week, and numerous times I've jumped in to fix this test. I'll probably get there, but it's incredibly slow and difficult work. |
Do you need help with that? What is slowing you down most? |
|
It would help a lot. I'm working towards an MVP of bleep and really need this feature, but I have much less time than I would have wanted. It's a deep call stack, and some state handling I haven't had time to understand.
|
|
Re slow, it's a combination of many things. I'm unable to run sbt in arm64 mode (is there even an issue for that?), so everything with the build is just incredibly slow. roundtrip for compiling |
|
I will try and take a look, but that might be next week unfortunately. |
|
That would be perfect. Gives me some time to try to figure it out myself first :) |
There was a problem hiding this comment.
@oyvindberg Thanks for contribution!
Overall looks good. The only issue is that BspTestSpec.bsp test fails`` test case doesn't pass. No idea why. Could you look at it?
| @@ -1094,7 +1102,7 @@ final class BloopBspServices( | |||
| def matchesGlobs(document: Path, project: Project): Boolean = | |||
| project.sourcesGlobs.exists(glob => glob.matches(document)) | |||
| val document = AbsolutePath(request.textDocument.uri.toPath).underlying | |||
| ifInitialized(None) { (state: State, _: BspServerLogger) => | |||
| ifInitialized(None) { (state: State, _: BspServerLogger) => | |||
- add `TestRun` and `TestRuns` structures to aggregate all produced `TestSuiteEvent.Results`. - propagate all test results back up to service - use test results to return `bsp.StatusCode.Error` on failed tests (Fixing scalacenter#960) - ensure that the compilation before test finished with `StatusCode.Ok`
- not much for now, assert that we get correct exit codes
tgodzik
force-pushed
the
improve-test-bsp
branch
from
5ad8164
to
7fb5711
Compare
|
thanks @tgodzik ! |
TestRunandTestRunsstructures to aggregate all producedTestSuiteEvent.Results.bsp.StatusCode.Erroron failed tests (Fixing Respond with adequate test reports to BSP test #960)StatusCode.Ok