internal/lsp/regtest: only run /default tests with -short
Due to shared caching, the "default" tests can run faster than other
execution modes and still retain most of the test coverage. Update the
test runner to only run the singleton tests if testing.Short() is true,
independent of GOOS.
On the other hand, we lost noticeable coverage when disabling the
Forwarded testing mode. Now that the regtests are lighter weight in
general, reenable it on the longtests builders.
While at it, clean up tests that used the server-side Options hook to
instead use Settings, since clients would configure their server via
Settings and Options can't work with a shared daemon.
Updates golang/go#39384
Change-Id: I33e8b746188d795e88841727e6b7116cd4851dc2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/418774
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
diff --git a/gopls/internal/regtest/completion/postfix_snippet_test.go b/gopls/internal/regtest/completion/postfix_snippet_test.go
index 5a7ffb8..5486047 100644
--- a/gopls/internal/regtest/completion/postfix_snippet_test.go
+++ b/gopls/internal/regtest/completion/postfix_snippet_test.go
@@ -9,7 +9,6 @@
"testing"
. "golang.org/x/tools/internal/lsp/regtest"
- "golang.org/x/tools/internal/lsp/source"
)
func TestPostfixSnippetCompletion(t *testing.T) {
@@ -433,9 +432,11 @@
},
}
- r := WithOptions(Options(func(o *source.Options) {
- o.ExperimentalPostfixCompletions = true
- }))
+ r := WithOptions(
+ Settings{
+ "experimentalPostfixCompletions": true,
+ },
+ )
r.Run(t, mod, func(t *testing.T, env *Env) {
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
diff --git a/gopls/internal/regtest/misc/shared_test.go b/gopls/internal/regtest/misc/shared_test.go
index 6b5acd0..170e164 100644
--- a/gopls/internal/regtest/misc/shared_test.go
+++ b/gopls/internal/regtest/misc/shared_test.go
@@ -24,7 +24,10 @@
fmt.Println("Hello World.")
}`
-func runShared(t *testing.T, testFunc func(env1 *Env, env2 *Env)) {
+// runShared is a helper to run a test in the same directory using both the
+// original env, and an additional other environment connected to the same
+// server.
+func runShared(t *testing.T, testFunc func(origEnv *Env, otherEnv *Env)) {
// Only run these tests in forwarded modes.
modes := DefaultModes() & (Forwarded | SeparateProcess)
WithOptions(Modes(modes)).Run(t, sharedProgram, func(t *testing.T, env1 *Env) {
@@ -38,28 +41,32 @@
}
func TestSimultaneousEdits(t *testing.T) {
- runShared(t, func(env1 *Env, env2 *Env) {
+ runShared(t, func(origEnv *Env, otherEnv *Env) {
// In editor #1, break fmt.Println as before.
- env1.OpenFile("main.go")
- env1.RegexpReplace("main.go", "Printl(n)", "")
+ origEnv.OpenFile("main.go")
+ origEnv.RegexpReplace("main.go", "Printl(n)", "")
// In editor #2 remove the closing brace.
- env2.OpenFile("main.go")
- env2.RegexpReplace("main.go", "\\)\n(})", "")
+ otherEnv.OpenFile("main.go")
+ otherEnv.RegexpReplace("main.go", "\\)\n(})", "")
// Now check that we got different diagnostics in each environment.
- env1.Await(env1.DiagnosticAtRegexp("main.go", "Printl"))
- env2.Await(env2.DiagnosticAtRegexp("main.go", "$"))
+ origEnv.Await(origEnv.DiagnosticAtRegexp("main.go", "Printl"))
+ otherEnv.Await(otherEnv.DiagnosticAtRegexp("main.go", "$"))
})
}
func TestShutdown(t *testing.T) {
- runShared(t, func(env1 *Env, env2 *Env) {
- if err := env1.Editor.Close(env1.Ctx); err != nil {
+ runShared(t, func(origEnv *Env, otherEnv *Env) {
+ // Close otherEnv, and verify that operation in the original environment is
+ // unaffected. Note: 'otherEnv' must be the environment being closed here.
+ // If we were to instead close 'env' here, we'd run into a duplicate
+ // shutdown when the test runner closes the original env.
+ if err := otherEnv.Editor.Close(otherEnv.Ctx); err != nil {
t.Errorf("closing first editor: %v", err)
}
// Now make an edit in editor #2 to trigger diagnostics.
- env2.OpenFile("main.go")
- env2.RegexpReplace("main.go", "\\)\n(})", "")
- env2.Await(env2.DiagnosticAtRegexp("main.go", "$"))
+ origEnv.OpenFile("main.go")
+ origEnv.RegexpReplace("main.go", "\\)\n(})", "")
+ origEnv.Await(origEnv.DiagnosticAtRegexp("main.go", "$"))
})
}
diff --git a/gopls/internal/regtest/workspace/workspace_test.go b/gopls/internal/regtest/workspace/workspace_test.go
index e4a1c4b..deb8a83 100644
--- a/gopls/internal/regtest/workspace/workspace_test.go
+++ b/gopls/internal/regtest/workspace/workspace_test.go
@@ -15,7 +15,6 @@
"golang.org/x/tools/internal/lsp/bug"
"golang.org/x/tools/internal/lsp/fake"
"golang.org/x/tools/internal/lsp/protocol"
- "golang.org/x/tools/internal/lsp/source"
"golang.org/x/tools/internal/testenv"
. "golang.org/x/tools/internal/lsp/regtest"
@@ -138,36 +137,22 @@
}
}
-// make sure that directory filters work
-func TestFilters(t *testing.T) {
- for _, tt := range []struct {
- name, rootPath string
- }{
- {
- name: "module root",
- rootPath: "pkg",
+func TestDirectoryFilters(t *testing.T) {
+ WithOptions(
+ ProxyFiles(workspaceProxy),
+ WorkspaceFolders("pkg"),
+ Settings{
+ "directoryFilters": []string{"-inner"},
},
- } {
- t.Run(tt.name, func(t *testing.T) {
- opts := []RunOption{ProxyFiles(workspaceProxy)}
- if tt.rootPath != "" {
- opts = append(opts, WorkspaceFolders(tt.rootPath))
+ ).Run(t, workspaceModule, func(t *testing.T, env *Env) {
+ syms := env.WorkspaceSymbol("Hi")
+ sort.Slice(syms, func(i, j int) bool { return syms[i].ContainerName < syms[j].ContainerName })
+ for _, s := range syms {
+ if strings.Contains(s.ContainerName, "inner") {
+ t.Errorf("WorkspaceSymbol: found symbol %q with container %q, want \"inner\" excluded", s.Name, s.ContainerName)
}
- f := func(o *source.Options) {
- o.DirectoryFilters = append(o.DirectoryFilters, "-inner")
- }
- opts = append(opts, Options(f))
- WithOptions(opts...).Run(t, workspaceModule, func(t *testing.T, env *Env) {
- syms := env.WorkspaceSymbol("Hi")
- sort.Slice(syms, func(i, j int) bool { return syms[i].ContainerName < syms[j].ContainerName })
- for i, s := range syms {
- if strings.Contains(s.ContainerName, "/inner") {
- t.Errorf("%s %v %s %s %d\n", s.Name, s.Kind, s.ContainerName, tt.name, i)
- }
- }
- })
- })
- }
+ }
+ })
}
// Make sure that analysis diagnostics are cleared for the whole package when
diff --git a/internal/lsp/regtest/regtest.go b/internal/lsp/regtest/regtest.go
index b3a543d..cfd999d 100644
--- a/internal/lsp/regtest/regtest.go
+++ b/internal/lsp/regtest/regtest.go
@@ -79,26 +79,17 @@
}
}
-// The regtests run significantly slower on these operating systems, due to (we
-// believe) kernel locking behavior. Only run in singleton mode on these
-// operating system when using -short.
-var slowGOOS = map[string]bool{
- "darwin": true,
- "openbsd": true,
- "plan9": true,
-}
-
+// DefaultModes returns the default modes to run for each regression test (they
+// may be reconfigured by the tests themselves).
func DefaultModes() Mode {
- // TODO(rfindley): these modes should *not* depend on GOOS. Depending on
- // testing.Short() should be sufficient.
- normal := Default | Experimental
- if slowGOOS[runtime.GOOS] && testing.Short() {
- normal = Default
+ modes := Default
+ if !testing.Short() {
+ modes |= Experimental | Forwarded
}
if *runSubprocessTests {
- return normal | SeparateProcess
+ modes |= SeparateProcess
}
- return normal
+ return modes
}
// Main sets up and tears down the shared regtest state.
diff --git a/internal/lsp/regtest/runner.go b/internal/lsp/regtest/runner.go
index 6c96e61..1d22aaa 100644
--- a/internal/lsp/regtest/runner.go
+++ b/internal/lsp/regtest/runner.go
@@ -85,6 +85,21 @@
Experimental
)
+func (m Mode) String() string {
+ switch m {
+ case Default:
+ return "default"
+ case Forwarded:
+ return "forwarded"
+ case SeparateProcess:
+ return "separate process"
+ case Experimental:
+ return "experimental"
+ default:
+ return "unknown mode"
+ }
+}
+
// A Runner runs tests in gopls execution environments, as specified by its
// modes. For modes that share state (for example, a shared cache or common
// remote), any tests that execute on the same Runner will share the same
@@ -117,14 +132,6 @@
debugAddr string
skipLogs bool
skipHooks bool
- optionsHook func(*source.Options)
-}
-
-func (r *Runner) defaultConfig() *runConfig {
- return &runConfig{
- modes: r.DefaultModes,
- optionsHook: r.OptionsHook,
- }
}
// A RunOption augments the behavior of the test runner.
@@ -155,22 +162,16 @@
}
// Modes configures the execution modes that the test should run in.
+//
+// By default, modes are configured by the test runner. If this option is set,
+// it overrides the set of default modes and the test runs in exactly these
+// modes.
func Modes(modes Mode) RunOption {
return optionSetter(func(opts *runConfig) {
- opts.modes = modes
- })
-}
-
-// Options configures the various server and user options.
-func Options(hook func(*source.Options)) RunOption {
- return optionSetter(func(opts *runConfig) {
- old := opts.optionsHook
- opts.optionsHook = func(o *source.Options) {
- if old != nil {
- old(o)
- }
- hook(o)
+ if opts.modes != 0 {
+ panic("modes set more than once")
}
+ opts.modes = modes
})
}
@@ -301,13 +302,18 @@
for _, tc := range tests {
tc := tc
- config := r.defaultConfig()
+ var config runConfig
for _, opt := range opts {
- opt.set(config)
+ opt.set(&config)
}
- if config.modes&tc.mode == 0 {
+ modes := r.DefaultModes
+ if config.modes != 0 {
+ modes = config.modes
+ }
+ if modes&tc.mode == 0 {
continue
}
+
if config.debugAddr != "" && tc.mode != Default {
// Debugging is useful for running stress tests, but since the daemon has
// likely already been started, it would be too late to debug.
@@ -364,7 +370,9 @@
}
}
}()
- ss := tc.getServer(t, config.optionsHook)
+
+ ss := tc.getServer(t, r.OptionsHook)
+
framer := jsonrpc2.NewRawStream
ls := &loggingFramer{}
if !config.skipLogs {
