internal/lsp/regtest: simplify, consolidate, and document settings

Configuration of LSP settings within the regression test runner had become a bit of a grab-bag: some were configured via explicit fields on EditorConfig, some via the catch-all EditorConfig.Settings field, and others via custom RunOption implementations. Consolidate these fields as follows: - Add an EnvVars and Settings field, for configuring environment and LSP settings. - Eliminate the EditorConfig RunOption wrapper. RunOptions help build the config. - Remove RunOptions that just wrap a key-value settings pair. By definition settings are user-facing and cannot change without breaking compatibility. Therefore, our tests can and should set the exact string keys they are using. - Eliminate the unused SendPID option. Also clean up some logic to change configuration. For golang/go#39384 Change-Id: Id5d1614f139550cbc62db2bab1d1e1f545ad9393 Reviewed-on: https://go-review.googlesource.com/c/tools/+/416876 TryBot-Result: Gopher Robot <gobot@golang.org> gopls-CI: kokoro <noreply+kokoro@google.com> Run-TryBot: Robert Findley <rfindley@google.com> Reviewed-by: Alan Donovan <adonovan@google.com>
golang · Jul 12, 2022 · 6e6f313 · 6e6f313
1 parent 3db2cdc
commit 6e6f313
Show file tree

Hide file tree

Showing 25 changed files with 201 additions and 301 deletions.
diff --git a/gopls/internal/regtest/bench/bench_test.go b/gopls/internal/regtest/bench/bench_test.go
@@ -93,14 +93,14 @@ func TestBenchmarkSymbols(t *testing.T) {
 	}
 
 	opts := benchmarkOptions(symbolOptions.workdir)
-	conf := EditorConfig{}
+	settings := make(Settings)
 	if symbolOptions.matcher != "" {
-		conf.SymbolMatcher = &symbolOptions.matcher
+		settings["symbolMatcher"] = symbolOptions.matcher
 	}
 	if symbolOptions.style != "" {
-		conf.SymbolStyle = &symbolOptions.style
+		settings["symbolStyle"] = symbolOptions.style
 	}
-	opts = append(opts, conf)
+	opts = append(opts, settings)
 
 	WithOptions(opts...).Run(t, "", func(t *testing.T, env *Env) {
 		// We can't Await in this test, since we have disabled hooks. Instead, run
@@ -200,9 +200,10 @@ func TestBenchmarkDidChange(t *testing.T) {
 // Always run it in isolation since it measures global heap usage.
 //
 // Kubernetes example:
-//   $ go test -run=TestPrintMemStats -didchange_dir=$HOME/w/kubernetes
-//   TotalAlloc:      5766 MB
-//   HeapAlloc:       1984 MB
+//
+//	$ go test -run=TestPrintMemStats -didchange_dir=$HOME/w/kubernetes
+//	TotalAlloc:      5766 MB
+//	HeapAlloc:       1984 MB
 //
 // Both figures exhibit variance of less than 1%.
 func TestPrintMemStats(t *testing.T) {

diff --git a/gopls/internal/regtest/codelens/codelens_test.go b/gopls/internal/regtest/codelens/codelens_test.go
@@ -63,9 +63,7 @@ const (
 	for _, test := range tests {
 		t.Run(test.label, func(t *testing.T) {
 			WithOptions(
-				EditorConfig{
-					CodeLenses: test.enabled,
-				},
+				Settings{"codelenses": test.enabled},
 			).Run(t, workspace, func(t *testing.T, env *Env) {
 				env.OpenFile("lib.go")
 				lens := env.CodeLens("lib.go")
@@ -308,10 +306,11 @@ func main() {
 }
 `
 	WithOptions(
-		EditorConfig{
-			CodeLenses: map[string]bool{
+		Settings{
+			"codelenses": map[string]bool{
 				"gc_details": true,
-			}},
+			},
+		},
 	).Run(t, mod, func(t *testing.T, env *Env) {
 		env.OpenFile("main.go")
 		env.ExecuteCodeLensCommand("main.go", command.GCDetails)

diff --git a/gopls/internal/regtest/completion/completion_test.go b/gopls/internal/regtest/completion/completion_test.go
@@ -529,7 +529,7 @@ func main() {
 }
 `
 	WithOptions(
-		EditorConfig{WindowsLineEndings: true},
+		WindowsLineEndings(),
 	).Run(t, src, func(t *testing.T, env *Env) {
 		// Trigger unimported completions for the example.com/blah package.
 		env.OpenFile("main.go")

diff --git a/gopls/internal/regtest/debug/debug_test.go b/gopls/internal/regtest/debug/debug_test.go
@@ -21,11 +21,7 @@ func TestBugNotification(t *testing.T) {
 	// server.
 	WithOptions(
 		Modes(Singleton), // must be in-process to receive the bug report below
-		EditorConfig{
-			Settings: map[string]interface{}{
-				"showBugReports": true,
-			},
-		},
+		Settings{"showBugReports": true},
 	).Run(t, "", func(t *testing.T, env *Env) {
 		const desc = "got a bug"
 		bug.Report(desc, nil)

diff --git a/gopls/internal/regtest/diagnostics/diagnostics_test.go b/gopls/internal/regtest/diagnostics/diagnostics_test.go
@@ -471,12 +471,11 @@ func _() {
 }
 `
 	WithOptions(
-		EditorConfig{
-			Env: map[string]string{
-				"GOPATH":      "",
-				"GO111MODULE": "off",
-			},
-		}).Run(t, files, func(t *testing.T, env *Env) {
+		EnvVars{
+			"GOPATH":      "",
+			"GO111MODULE": "off",
+		},
+	).Run(t, files, func(t *testing.T, env *Env) {
 		env.OpenFile("main.go")
 		env.Await(env.DiagnosticAtRegexp("main.go", "fmt"))
 		env.SaveBuffer("main.go")
@@ -500,8 +499,9 @@ package x
 
 var X = 0
 `
-	editorConfig := EditorConfig{Env: map[string]string{"GOFLAGS": "-tags=foo"}}
-	WithOptions(editorConfig).Run(t, files, func(t *testing.T, env *Env) {
+	WithOptions(
+		EnvVars{"GOFLAGS": "-tags=foo"},
+	).Run(t, files, func(t *testing.T, env *Env) {
 		env.OpenFile("main.go")
 		env.OrganizeImports("main.go")
 		env.Await(EmptyDiagnostics("main.go"))
@@ -573,9 +573,9 @@ hi mom
 `
 	for _, go111module := range []string{"on", "off", ""} {
 		t.Run(fmt.Sprintf("GO111MODULE_%v", go111module), func(t *testing.T) {
-			WithOptions(EditorConfig{
-				Env: map[string]string{"GO111MODULE": go111module},
-			}).Run(t, files, func(t *testing.T, env *Env) {
+			WithOptions(
+				EnvVars{"GO111MODULE": go111module},
+			).Run(t, files, func(t *testing.T, env *Env) {
 				env.Await(
 					NoOutstandingWork(),
 				)
@@ -605,11 +605,7 @@ func main() {
 `
 	WithOptions(
 		InGOPATH(),
-		EditorConfig{
-			Env: map[string]string{
-				"GO111MODULE": "off",
-			},
-		},
+		EnvVars{"GO111MODULE": "off"},
 	).Run(t, collision, func(t *testing.T, env *Env) {
 		env.OpenFile("x/x.go")
 		env.Await(
@@ -1236,7 +1232,7 @@ func main() {
 	})
 	WithOptions(
 		WorkspaceFolders("a"),
-		LimitWorkspaceScope(),
+		Settings{"expandWorkspaceToModule": false},
 	).Run(t, mod, func(t *testing.T, env *Env) {
 		env.OpenFile("a/main.go")
 		env.Await(
@@ -1267,11 +1263,7 @@ func main() {
 `
 
 	WithOptions(
-		EditorConfig{
-			Settings: map[string]interface{}{
-				"staticcheck": true,
-			},
-		},
+		Settings{"staticcheck": true},
 	).Run(t, files, func(t *testing.T, env *Env) {
 		env.OpenFile("main.go")
 		var d protocol.PublishDiagnosticsParams
@@ -1381,9 +1373,7 @@ func b(c bytes.Buffer) {
 }
 `
 	WithOptions(
-		EditorConfig{
-			AllExperiments: true,
-		},
+		Settings{"allExperiments": true},
 	).Run(t, mod, func(t *testing.T, env *Env) {
 		// Confirm that the setting doesn't cause any warnings.
 		env.Await(NoShowMessage())
@@ -1495,11 +1485,7 @@ package foo_
 	WithOptions(
 		ProxyFiles(proxy),
 		InGOPATH(),
-		EditorConfig{
-			Env: map[string]string{
-				"GO111MODULE": "off",
-			},
-		},
+		EnvVars{"GO111MODULE": "off"},
 	).Run(t, contents, func(t *testing.T, env *Env) {
 		// Simulate typing character by character.
 		env.OpenFile("foo/foo_test.go")
@@ -1698,9 +1684,7 @@ import (
 	t.Run("GOPATH", func(t *testing.T) {
 		WithOptions(
 			InGOPATH(),
-			EditorConfig{
-				Env: map[string]string{"GO111MODULE": "off"},
-			},
+			EnvVars{"GO111MODULE": "off"},
 			Modes(Singleton),
 		).Run(t, mod, func(t *testing.T, env *Env) {
 			env.Await(
@@ -1729,11 +1713,7 @@ package b
 		t.Run("GO111MODULE="+go111module, func(t *testing.T) {
 			WithOptions(
 				Modes(Singleton),
-				EditorConfig{
-					Env: map[string]string{
-						"GO111MODULE": go111module,
-					},
-				},
+				EnvVars{"GO111MODULE": go111module},
 			).Run(t, modules, func(t *testing.T, env *Env) {
 				env.OpenFile("a/a.go")
 				env.OpenFile("b/go.mod")
@@ -1750,11 +1730,7 @@ package b
 	t.Run("GOPATH_GO111MODULE_auto", func(t *testing.T) {
 		WithOptions(
 			Modes(Singleton),
-			EditorConfig{
-				Env: map[string]string{
-					"GO111MODULE": "auto",
-				},
-			},
+			EnvVars{"GO111MODULE": "auto"},
 			InGOPATH(),
 		).Run(t, modules, func(t *testing.T, env *Env) {
 			env.OpenFile("a/a.go")
@@ -2026,9 +2002,7 @@ package a
 func Hello() {}
 `
 	WithOptions(
-		EditorConfig{
-			ExperimentalUseInvalidMetadata: true,
-		},
+		Settings{"experimentalUseInvalidMetadata": true},
 		Modes(Singleton),
 	).Run(t, mod, func(t *testing.T, env *Env) {
 		env.OpenFile("go.mod")
@@ -2082,9 +2056,7 @@ package main
 func _() {}
 `
 	WithOptions(
-		EditorConfig{
-			ExperimentalUseInvalidMetadata: true,
-		},
+		Settings{"experimentalUseInvalidMetadata": true},
 		// ExperimentalWorkspaceModule has a different failure mode for this
 		// case.
 		Modes(Singleton),

diff --git a/gopls/internal/regtest/inlayhints/inlayhints_test.go b/gopls/internal/regtest/inlayhints/inlayhints_test.go
@@ -56,10 +56,8 @@ const (
 	for _, test := range tests {
 		t.Run(test.label, func(t *testing.T) {
 			WithOptions(
-				EditorConfig{
-					Settings: map[string]interface{}{
-						"hints": test.enabled,
-					},
+				Settings{
+					"hints": test.enabled,
 				},
 			).Run(t, workspace, func(t *testing.T, env *Env) {
 				env.OpenFile("lib.go")

diff --git a/gopls/internal/regtest/misc/configuration_test.go b/gopls/internal/regtest/misc/configuration_test.go
@@ -9,7 +9,6 @@ import (
 
 	. "golang.org/x/tools/internal/lsp/regtest"
 
-	"golang.org/x/tools/internal/lsp/fake"
 	"golang.org/x/tools/internal/testenv"
 )
 
@@ -40,12 +39,11 @@ var FooErr = errors.New("foo")
 			env.DoneWithOpen(),
 			NoDiagnostics("a/a.go"),
 		)
-		cfg := &fake.EditorConfig{}
-		*cfg = env.Editor.Config
+		cfg := env.Editor.Config()
 		cfg.Settings = map[string]interface{}{
 			"staticcheck": true,
 		}
-		env.ChangeConfiguration(t, cfg)
+		env.ChangeConfiguration(cfg)
 		env.Await(
 			DiagnosticAt("a/a.go", 5, 4),
 		)
@@ -70,11 +68,9 @@ import "errors"
 var FooErr = errors.New("foo")
 `
 
-	WithOptions(EditorConfig{
-		Settings: map[string]interface{}{
-			"staticcheck": true,
-		},
-	}).Run(t, files, func(t *testing.T, env *Env) {
+	WithOptions(
+		Settings{"staticcheck": true},
+	).Run(t, files, func(t *testing.T, env *Env) {
 		env.Await(ShownMessage("staticcheck is not supported"))
 	})
 }
diff --git a/gopls/internal/regtest/misc/definition_test.go b/gopls/internal/regtest/misc/definition_test.go
@@ -162,9 +162,7 @@ func main() {}
 	} {
 		t.Run(tt.importShortcut, func(t *testing.T) {
 			WithOptions(
-				EditorConfig{
-					ImportShortcut: tt.importShortcut,
-				},
+				Settings{"importShortcut": tt.importShortcut},
 			).Run(t, mod, func(t *testing.T, env *Env) {
 				env.OpenFile("main.go")
 				file, pos := env.GoToDefinition("main.go", env.RegexpSearch("main.go", `"fmt"`))

diff --git a/gopls/internal/regtest/misc/formatting_test.go b/gopls/internal/regtest/misc/formatting_test.go
@@ -352,10 +352,8 @@ const Bar = 42
 `
 
 	WithOptions(
-		EditorConfig{
-			Settings: map[string]interface{}{
-				"gofumpt": true,
-			},
+		Settings{
+			"gofumpt": true,
 		},
 	).Run(t, input, func(t *testing.T, env *Env) {
 		env.OpenFile("foo.go")

diff --git a/gopls/internal/regtest/misc/imports_test.go b/gopls/internal/regtest/misc/imports_test.go
@@ -153,9 +153,8 @@ var _, _ = x.X, y.Y
 		t.Fatal(err)
 	}
 	defer os.RemoveAll(modcache)
-	editorConfig := EditorConfig{Env: map[string]string{"GOMODCACHE": modcache}}
 	WithOptions(
-		editorConfig,
+		EnvVars{"GOMODCACHE": modcache},
 		ProxyFiles(proxy),
 	).Run(t, files, func(t *testing.T, env *Env) {
 		env.OpenFile("main.go")

diff --git a/gopls/internal/regtest/misc/link_test.go b/gopls/internal/regtest/misc/link_test.go
@@ -75,7 +75,9 @@ const Hello = "Hello"
 		}
 
 		// Then change the environment to make these links private.
-		env.ChangeEnv(map[string]string{"GOPRIVATE": "import.test"})
+		cfg := env.Editor.Config()
+		cfg.Env = map[string]string{"GOPRIVATE": "import.test"}
+		env.ChangeConfiguration(cfg)
 
 		// Finally, verify that the links are gone.
 		content, _ = env.Hover("main.go", env.RegexpSearch("main.go", "pkg.Hello"))

diff --git a/gopls/internal/regtest/misc/semantictokens_test.go b/gopls/internal/regtest/misc/semantictokens_test.go
@@ -26,9 +26,7 @@ func main() {}
 `
 	WithOptions(
 		Modes(Singleton),
-		EditorConfig{
-			AllExperiments: true,
-		},
+		Settings{"allExperiments": true},
 	).Run(t, src, func(t *testing.T, env *Env) {
 		params := &protocol.SemanticTokensParams{}
 		const badURI = "http://foo"

diff --git a/gopls/internal/regtest/misc/settings_test.go b/gopls/internal/regtest/misc/settings_test.go
@@ -24,11 +24,7 @@ func main() {
 `
 
 	WithOptions(
-		EditorConfig{
-			Settings: map[string]interface{}{
-				"directoryFilters": []string{""},
-			},
-		},
+		Settings{"directoryFilters": []string{""}},
 	).Run(t, src, func(t *testing.T, env *Env) {
 		// No need to do anything. Issue golang/go#51843 is triggered by the empty
 		// directory filter above.

diff --git a/gopls/internal/regtest/misc/shared_test.go b/gopls/internal/regtest/misc/shared_test.go
@@ -30,7 +30,7 @@ func runShared(t *testing.T, testFunc func(env1 *Env, env2 *Env)) {
 	WithOptions(Modes(modes)).Run(t, sharedProgram, func(t *testing.T, env1 *Env) {
 		// Create a second test session connected to the same workspace and server
 		// as the first.
-		env2, cleanup := NewEnv(env1.Ctx, t, env1.Sandbox, env1.Server, env1.Editor.Config, true)
+		env2, cleanup := NewEnv(env1.Ctx, t, env1.Sandbox, env1.Server, env1.Editor.Config(), true)
 		defer cleanup()
 		env2.Await(InitialWorkspaceLoad)
 		testFunc(env1, env2)

diff --git a/gopls/internal/regtest/misc/staticcheck_test.go b/gopls/internal/regtest/misc/staticcheck_test.go
@@ -60,11 +60,9 @@ func testGenerics[P *T, T any](p P) {
 var FooErr error = errors.New("foo")
 `
 
-	WithOptions(EditorConfig{
-		Settings: map[string]interface{}{
-			"staticcheck": true,
-		},
-	}).Run(t, files, func(t *testing.T, env *Env) {
+	WithOptions(
+		Settings{"staticcheck": true},
+	).Run(t, files, func(t *testing.T, env *Env) {
 		env.OpenFile("a/a.go")
 		env.Await(
 			env.DiagnosticAtRegexpFromSource("a/a.go", "sort.Slice", "sortslice"),