Multi-target LSP tests to run against .NET #77173
Conversation
ghost
added
Area-IDE
untriaged
dibarbet
force-pushed
the
lsp_net_tests
branch
from
ddfcb0a to
c53e1e0
Compare
dibarbet
changed the title
dibarbet
force-pushed
the
lsp_net_tests
branch
from
5b16913 to
76b364e
Compare
| namespace Microsoft.CodeAnalysis.Test.Utilities; | ||
|
|
||
| public partial class EditorTestWorkspace : TestWorkspace<EditorTestHostDocument, EditorTestHostProject, EditorTestHostSolution>, ILspWorkspace | ||
| public partial class EditorTestWorkspace : TestWorkspace<EditorTestHostDocument, EditorTestHostProject, EditorTestHostSolution> |
There was a problem hiding this comment.
we're not using this anymore for any LSP tests that require mutation, so no need for it to implement the LSP mutation methods. Only used by doc outline tests at the moment (which work with any workspace).
| } | ||
|
|
||
| protected virtual TestComposition Composition => EditorFeaturesLspComposition; | ||
| protected virtual TestComposition Composition => FeaturesLspComposition; |
There was a problem hiding this comment.
we generally don't need editorfeatures in the composition anymore. Previously it was required to test LSP APIs that returned internal VS types - we would have a default service in the protocol layer, and override it in editor features (to return things like classified text runs).
However now we have all our own types and the conversions are in the protocol layer too, so this is generally not necessary.
| /// This ensures that events like workspace registration / workspace changes are processed by the time we exit this method. | ||
| /// </summary> | ||
| protected static async Task WaitForWorkspaceOperationsAsync(EditorTestWorkspace workspace) | ||
| protected static async Task WaitForWorkspaceOperationsAsync<TDocument, TProject, TSolution>(TestWorkspace<TDocument, TProject, TSolution> workspace) |
There was a problem hiding this comment.
generics are required so this can be passed an EditorTestWorkspace (for a few tests in higher layers that use that) or LspTestWorkspace (almost everything else).
| } | ||
| } | ||
|
|
||
| internal abstract class AbstractTestLspServer<TWorkspace, TDocument, TProject, TSolution> : IAsyncDisposable |
There was a problem hiding this comment.
refactored TestLspServer a bit so it can be passed any implementation of TestWorkspace<TDocument, TProject, TSolution>. mostly this is just the LspTestWorkspace, but as above there are a few cases where we still use EditorTestWorkspace.
| /// Opens a document in the workspace only, and waits for workspace operations. | ||
| /// Use <see cref="OpenDocumentAsync(Uri, string, string)"/> if the document should be opened in LSP"/> | ||
| /// </summary> | ||
| public async Task OpenDocumentInWorkspaceAsync(DocumentId documentId, bool openAllLinkedDocuments, SourceText? text = null) |
There was a problem hiding this comment.
previously, some tests opened text buffers to simulate opening a file in the workspace (and setting things like active document context).
We don't actually need the buffers - instead we can just open the documents in the workspace directly.
| var originalText = await testLspServer.GetDocumentTextAsync(textDocumentEdit[0].TextDocument.Uri); | ||
| var edits = textDocumentEdit[0].Edits.Select(e => (LSP.TextEdit)e.Value).ToArray(); | ||
| var updatedText = ApplyTextEdits(edits, originalText); | ||
| Assert.Equal(expectedText, updatedText); |
There was a problem hiding this comment.
The text edits for this test changed slightly when we switched to LspTestWorkspace because the source texts were no longer backed by the text buffer source text implementation. Instead of updating the hardcoded edits, much cleaner to apply the edits and verify the end result.
|
|
||
| Assert.Equal("CS1513", results.Single().Diagnostics.Single().Code); | ||
| Assert.NotNull(results.Single().Diagnostics.Single().CodeDescription!.Href); | ||
| Assert.Equal("CS1513", results.Single().Diagnostics!.Single().Code); |
There was a problem hiding this comment.
everything in this file is removal of the text buffer calls and nullable updates
| return server; | ||
| } | ||
|
|
||
| internal class EditorTestLspServer : AbstractTestLspServer<EditorTestWorkspace, EditorTestHostDocument, EditorTestHostProject, EditorTestHostSolution> |
There was a problem hiding this comment.
these are the tests that still require the EditorTestWorkspace because they deal with text buffers in the tests.
| } | ||
|
|
||
| protected override TestComposition Composition => base.Composition.AddParts(typeof(TypeScriptHandlerFactory)); | ||
| protected override TestComposition Composition => EditorTestCompositions.LanguageServerProtocolEditorFeatures |
There was a problem hiding this comment.
requires Editor features here as the VS typescript EA components live in editor features.
…s.cs Co-authored-by: Joey Robichaud <jorobich@microsoft.com>
3 participants
Previously, all LSP tests ran against net472 only, even though the LSP code running in VSCode ran against .NET9. So we've had a test gap here for a while.
But some refactorings Tomas did a couple weeks ago to TestWorkspace now make it relatively straightforward to multi-target these tests. This gets us both CI coverage on .NET core as well as the ability to debug these tests using VSCode (it only ships with a .net core debugger).
The changes in the PR fit a few categories
LspTestWorkspaceorEditorTestWorkspace(for tests in EditorFeatures that require text buffers, like the document outline tests).CI:
