Add diagnostic provider for clients without dynamic registration#77984
Add diagnostic provider for clients without dynamic registration#77984dibarbet merged 2 commits intodotnet:mainfrom SofusA:default-diagnostic-provider
Conversation
@dotnet-policy-service agree |
dibarbet
left a comment
There was a problem hiding this comment.
Apologies for the delay here
|
|
||
| [Theory, CombinatorialData] | ||
| public async Task TestPublicDiagnosticSourcesAreRegisteredWhenSupported(bool mutatingLspWorkspace) | ||
| public async Task TestPublicDiagnosticSourcesAreRegisteredWhenSupported(bool mutatingLspWorkspace, bool dynamicRegistration) |
There was a problem hiding this comment.
This test doesn't seem quite complete - adding the test parameter is fine (or it could be a separate test), but I would expect the test to assert that no dynamic capabilities (and no separate sources) are registered when the client does not support dynamic registration.
Instead I would expect the test to assert that there is only the single static registration.
There was a problem hiding this comment.
Sorry. I pushed this change before i was able to run the test by mistake.
The registrations are empty without dynamic registration, which i think makes sense.
I suppose i should look up the server capabilities somehow. Could you guide me in the right direction?
No worries with the delay. I just appreciate that you take the time.
There was a problem hiding this comment.
The registrations are empty without dynamic registration, which i think makes sense.
Yup I believe so.
I suppose i should look up the server capabilities somehow. Could you guide me in the right direction?
On the testLspServer instance, you should be able to call GetServerCapabilities - https://github.com/dotnet/roslyn/blob/main/src/LanguageServer/Protocol.TestUtilities/LanguageServer/AbstractLanguageServerProtocolTests.cs#L791
There was a problem hiding this comment.
Thanks.
I have added an if / else on dynamicRegistration. Would you prefer an early return in the test instead?
Closes #76624