internal/lsp: use subtests for all lsp categories

This makes it possible to run just one type of test if needed
Also add some verification that the right number of tests is being run
And finally collect all the expectations up front, including the completions.

Change-Id: Iee6045a8ad89fa399fefd03bc0712770701ec6f8
Reviewed-on: https://go-review.googlesource.com/c/149737
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>

internal/lsp/lsp_test.go

@@ -28,6 +28,9 @@ func TestLSP(t *testing.T) {
 
 func testLSP(t *testing.T, exporter packagestest.Exporter) {
 	const dir = "testdata"
+	const expectedCompletionsCount = 4
+	const expectedDiagnosticsCount = 7
+	const expectedFormatCount = 3
 
 	files := packagestest.MustCopyFileTree(dir)
 	for fragment, operation := range files {
@@ -48,9 +51,10 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) {
 	dirs := make(map[string]bool)
 
 	// collect results for certain tests
-	expectedDiagnostics := make(map[string][]protocol.Diagnostic)
+	expectedDiagnostics := make(diagnostics)
-	expectedCompletions := make(map[token.Position]*protocol.CompletionItem)
+	completionItems := make(completionItems)
-	expectedFormat := make(map[string]string)
+	expectedCompletions := make(completions)
+	expectedFormat := make(formats)
 
 	s := &server{
 		view: source.NewView(),
@@ -81,70 +85,76 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) {
 	}
 	// Collect any data that needs to be used by subsequent tests.
 	if err := exported.Expect(map[string]interface{}{
-		"diag": func(pos token.Position, msg string) {
+		"diag":     expectedDiagnostics.collect,
-			collectDiagnostics(t, expectedDiagnostics, pos, msg)
+		"item":     completionItems.collect,
-		},
+		"complete": expectedCompletions.collect,
-		"item": func(pos token.Position, label, detail, kind string) {
+		"format":   expectedFormat.collect,
-			collectCompletionItems(expectedCompletions, pos, label, detail, kind)
-		},
-		"format": func(pos token.Position) {
-			collectFormat(expectedFormat, pos)
-		},
 	}); err != nil {
 		t.Fatal(err)
 	}
 
-	// test completion
+	t.Run("Completion", func(t *testing.T) {
-	testCompletion(t, exported, s, expectedCompletions)
+		t.Helper()
+		if len(expectedCompletions) != expectedCompletionsCount {
+			t.Errorf("got %v completions expected %v", len(expectedCompletions), expectedCompletionsCount)
+		}
+		expectedCompletions.test(t, exported, s, completionItems)
+	})
 
-	// test diagnostics
+	t.Run("Diagnostics", func(t *testing.T) {
-	var dirList []string
+		t.Helper()
-	for dir := range dirs {
+		diagnosticsCount := expectedDiagnostics.test(t, exported, s.view, dirs)
-		dirList = append(dirList, dir)
+		if diagnosticsCount != expectedDiagnosticsCount {
-	}
+			t.Errorf("got %v diagnostics expected %v", diagnosticsCount, expectedDiagnosticsCount)
-	exported.Config.Mode = packages.LoadFiles
+		}
-	pkgs, err := packages.Load(exported.Config, dirList...)
+	})
-	if err != nil {
-		t.Fatal(err)
-	}
-	testDiagnostics(t, s.view, pkgs, expectedDiagnostics)
 
-	// test format
+	t.Run("Format", func(t *testing.T) {
-	testFormat(t, s, expectedFormat)
+		t.Helper()
+		if len(expectedFormat) != expectedFormatCount {
+			t.Errorf("got %v formats expected %v", len(expectedFormat), expectedFormatCount)
+		}
+		expectedFormat.test(t, s)
+	})
 }
 
-func testCompletion(t *testing.T, exported *packagestest.Exported, s *server, wants map[token.Position]*protocol.CompletionItem) {
+type diagnostics map[string][]protocol.Diagnostic
-	if err := exported.Expect(map[string]interface{}{
+type completionItems map[token.Pos]*protocol.CompletionItem
-		"complete": func(src token.Position, expected []token.Position) {
+type completions map[token.Position][]token.Pos
-			var want []protocol.CompletionItem
+type formats map[string]string
-			for _, pos := range expected {
+
-				want = append(want, *wants[pos])
+func (c completions) test(t *testing.T, exported *packagestest.Exported, s *server, items completionItems) {
-			}
+	for src, itemList := range c {
-			list, err := s.Completion(context.Background(), &protocol.CompletionParams{
+		var want []protocol.CompletionItem
-				TextDocumentPositionParams: protocol.TextDocumentPositionParams{
+		for _, pos := range itemList {
-					TextDocument: protocol.TextDocumentIdentifier{
+			want = append(want, *items[pos])
-						URI: protocol.DocumentURI(source.ToURI(src.Filename)),
+		}
-					},
+		list, err := s.Completion(context.Background(), &protocol.CompletionParams{
-					Position: protocol.Position{
+			TextDocumentPositionParams: protocol.TextDocumentPositionParams{
-						Line:      float64(src.Line - 1),
+				TextDocument: protocol.TextDocumentIdentifier{
-						Character: float64(src.Column - 1),
+					URI: protocol.DocumentURI(source.ToURI(src.Filename)),
-					},
 				},
-			})
+				Position: protocol.Position{
-			if err != nil {
+					Line:      float64(src.Line - 1),
-				t.Fatal(err)
+					Character: float64(src.Column - 1),
-			}
+				},
-			got := list.Items
+			},
-			if equal := reflect.DeepEqual(want, got); !equal {
+		})
-				t.Errorf("completion failed for %s:%v:%v: (expected: %v), (got: %v)", filepath.Base(src.Filename), src.Line, src.Column, want, got)
+		if err != nil {
-			}
+			t.Fatal(err)
-		},
+		}
-	}); err != nil {
+		got := list.Items
-		t.Fatal(err)
+		if equal := reflect.DeepEqual(want, got); !equal {
+			t.Errorf("completion failed for %s:%v:%v: (expected: %v), (got: %v)", filepath.Base(src.Filename), src.Line, src.Column, want, got)
+		}
 	}
 }
 
-func collectCompletionItems(expectedCompletions map[token.Position]*protocol.CompletionItem, pos token.Position, label, detail, kind string) {
+func (c completions) collect(src token.Position, expected []token.Pos) {
+	c[src] = expected
+}
+
+func (i completionItems) collect(pos token.Pos, label, detail, kind string) {
 	var k protocol.CompletionItemKind
 	switch kind {
 	case "struct":
@@ -164,14 +174,26 @@ func collectCompletionItems(expectedCompletions map[token.Position]*protocol.Com
 	case "method":
 		k = protocol.MethodCompletion
 	}
-	expectedCompletions[pos] = &protocol.CompletionItem{
+	i[pos] = &protocol.CompletionItem{
 		Label:  label,
 		Detail: detail,
 		Kind:   float64(k),
 	}
 }
 
-func testDiagnostics(t *testing.T, v *source.View, pkgs []*packages.Package, wants map[string][]protocol.Diagnostic) {
+func (d diagnostics) test(t *testing.T, exported *packagestest.Exported, v *source.View, dirs map[string]bool) int {
+	// first trigger a load to get the diagnostics
+	var dirList []string
+	for dir := range dirs {
+		dirList = append(dirList, dir)
+	}
+	exported.Config.Mode = packages.LoadFiles
+	pkgs, err := packages.Load(exported.Config, dirList...)
+	if err != nil {
+		t.Fatal(err)
+	}
+	// and now see if they match the expected ones
+	count := 0
 	for _, pkg := range pkgs {
 		for _, filename := range pkg.GoFiles {
 			f := v.GetFile(source.ToURI(filename))
@@ -183,7 +205,7 @@ func testDiagnostics(t *testing.T, v *source.View, pkgs []*packages.Package, wan
 			sort.Slice(got, func(i int, j int) bool {
 				return got[i].Range.Start.Line < got[j].Range.Start.Line
 			})
-			want := wants[filename]
+			want := d[filename]
 			if equal := reflect.DeepEqual(want, got); !equal {
 				msg := &bytes.Buffer{}
 				fmt.Fprintf(msg, "diagnostics failed for %s: expected:\n", filepath.Base(filename))
@@ -196,11 +218,13 @@ func testDiagnostics(t *testing.T, v *source.View, pkgs []*packages.Package, wan
 				}
 				t.Error(msg.String())
 			}
+			count += len(want)
 		}
 	}
+	return count
 }
 
-func collectDiagnostics(t *testing.T, expectedDiagnostics map[string][]protocol.Diagnostic, pos token.Position, msg string) {
+func (d diagnostics) collect(pos token.Position, msg string) {
 	line := float64(pos.Line - 1)
 	col := float64(pos.Column - 1)
 	want := protocol.Diagnostic{
@@ -218,15 +242,11 @@ func collectDiagnostics(t *testing.T, expectedDiagnostics map[string][]protocol.
 		Source:   "LSP",
 		Message:  msg,
 	}
-	if _, ok := expectedDiagnostics[pos.Filename]; ok {
+	d[pos.Filename] = append(d[pos.Filename], want)
-		expectedDiagnostics[pos.Filename] = append(expectedDiagnostics[pos.Filename], want)
-	} else {
-		t.Errorf("unexpected filename: %v", pos.Filename)
-	}
 }
 
-func testFormat(t *testing.T, s *server, expectedFormat map[string]string) {
+func (f formats) test(t *testing.T, s *server) {
-	for filename, gofmted := range expectedFormat {
+	for filename, gofmted := range f {
 		edits, err := s.Formatting(context.Background(), &protocol.DocumentFormattingParams{
 			TextDocument: protocol.TextDocumentIdentifier{
 				URI: protocol.DocumentURI(source.ToURI(filename)),
@@ -245,10 +265,10 @@ func testFormat(t *testing.T, s *server, expectedFormat map[string]string) {
 	}
 }
 
-func collectFormat(expectedFormat map[string]string, pos token.Position) {
+func (f formats) collect(pos token.Position) {
 	cmd := exec.Command("gofmt", pos.Filename)
 	stdout := bytes.NewBuffer(nil)
 	cmd.Stdout = stdout
 	cmd.Run() // ignore error, sometimes we have intentionally ungofmt-able files
-	expectedFormat[pos.Filename] = stdout.String()
+	f[pos.Filename] = stdout.String()
 }