-
Notifications
You must be signed in to change notification settings - Fork 345
Add LSP test for completions #4256
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,109 @@ | ||
| // Copyright 2020-2025 Buf Technologies, Inc. | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| package buflsp_test | ||
|
|
||
| import ( | ||
| "path/filepath" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| "go.lsp.dev/protocol" | ||
| ) | ||
|
|
||
| func TestCompletion(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| ctx := t.Context() | ||
|
|
||
| testProtoPath, err := filepath.Abs("testdata/completion/test.proto") | ||
| require.NoError(t, err) | ||
|
|
||
| clientJSONConn, testURI := setupLSPServer(t, testProtoPath) | ||
|
|
||
| tests := []struct { | ||
| name string | ||
| line uint32 | ||
| character uint32 | ||
| expectedContains []string | ||
| expectedNotContains []string | ||
| expectNoCompletions bool | ||
| }{ | ||
| { | ||
| name: "complete_builtin_toplevel", | ||
| line: 5, | ||
| character: 1, // After the "m" | ||
| expectedContains: []string{"message"}, | ||
| }, | ||
| { | ||
| name: "complete_message_field_types", | ||
| line: 9, // Empty line in User message where field would go | ||
| character: 2, // Indented position where field type would be | ||
| expectedContains: []string{"string", "int32", "int64", "bool", "bytes", "User", "GetUserRequest", "GetUserResponse"}, | ||
| }, | ||
| { | ||
| name: "complete_builtin_service", | ||
| line: 14, | ||
| character: 2, // Indented position where "rpc" would be | ||
| expectedContains: []string{"rpc", "option"}, | ||
| }, | ||
| { | ||
| name: "complete_rpc_request_type", | ||
| line: 13, | ||
| character: uint32(len(" rpc GetUser(Get") - 1), | ||
| expectedContains: []string{"GetUserRequest", "GetUserResponse"}, | ||
| }, | ||
| { | ||
| name: "complete_rpc_response_type", | ||
| line: 13, | ||
| character: uint32(len(" rpc GetUser(Get) returns (Get") - 1), | ||
| expectedContains: []string{"GetUserRequest", "GetUserResponse"}, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| t.Parallel() | ||
| var completionList *protocol.CompletionList | ||
| _, completionErr := clientJSONConn.Call(ctx, protocol.MethodTextDocumentCompletion, protocol.CompletionParams{ | ||
| TextDocumentPositionParams: protocol.TextDocumentPositionParams{ | ||
| TextDocument: protocol.TextDocumentIdentifier{ | ||
| URI: testURI, | ||
| }, | ||
| Position: protocol.Position{ | ||
| Line: tt.line, | ||
| Character: tt.character, | ||
| }, | ||
| }, | ||
| }, &completionList) | ||
| require.NoError(t, completionErr) | ||
| if tt.expectNoCompletions { | ||
| assert.Nil(t, completionList, "expected no completions") | ||
| return | ||
| } | ||
| require.NotNil(t, completionList, "expected completion list to be non-nil") | ||
| labels := make([]string, 0, len(completionList.Items)) | ||
| for _, item := range completionList.Items { | ||
| labels = append(labels, item.Label) | ||
| } | ||
| for _, expected := range tt.expectedContains { | ||
| assert.Contains(t, labels, expected, "expected completion list to contain %q", expected) | ||
| } | ||
|
Comment on lines
+101
to
+103
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I realize I may have done a similar thing in my tests, but would it make sense to do an
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think theres too many. |
||
| for _, notExpected := range tt.expectedNotContains { | ||
| assert.NotContains(t, labels, notExpected, "expected completion list to not contain %q", notExpected) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| version: v2 | ||
| modules: | ||
| - path: . | ||
| lint: | ||
| use: | ||
| - STANDARD | ||
| breaking: | ||
| use: | ||
| - FILE |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| syntax = "proto3"; | ||
|
|
||
| package example.v1; | ||
|
|
||
| // complete_builtin_toplevel | ||
| m {} | ||
|
|
||
| message User { | ||
| string id = 1; | ||
| // complete_message_field_types | ||
| } | ||
|
|
||
| service UserService { | ||
| rpc GetUser(Get) returns (Get); // complete_rpc_request_type, complete_rpc_response_type | ||
| // complete_builtin_service | ||
| } | ||
|
|
||
| message GetUserRequest { | ||
| string user_id = 1; | ||
| } | ||
|
|
||
| message GetUserResponse { | ||
| User user = 1; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: we don't use either of these — not sure if expectedNotContains is useful if we switch to using
assert.ElementsMatchbelow, but can probably remove it, and it might be nice to have a couple of negative tests for places where we don't have completion suggestions (usingexpectNoCompletions).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are missing a bunch here, was going to follow up with some fixes needed. Theres an issue with state which isn't captured in this test. I think we need to have an update, followed by a completion to show this issue.