gopls/internal/regtest: add an option to nest the workdir
Add a regtest run option NestWorkdir, which causes the working directory
to be nested in the editor workspace, and use it in some modfile tests
to exercise the new workspace logic.
For now we only run the nested tests while using experimental workspace
modules. In a later CL, the 'legacy' mode will be updated to find a
solitary nested module, at which point we should be able to run nested
in all modes.
Fixes golang/go#42111
Change-Id: I0bd3b31969684bc1ba1935633cbb9a3f26de1587
Reviewed-on: https://go-review.googlesource.com/c/tools/+/257138
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Trust: Robert Findley <rfindley@google.com>
diff --git a/gopls/internal/regtest/diagnostics_test.go b/gopls/internal/regtest/diagnostics_test.go
index d3685dd..e8190cf 100644
--- a/gopls/internal/regtest/diagnostics_test.go
+++ b/gopls/internal/regtest/diagnostics_test.go
@@ -1246,7 +1246,7 @@
env.DiagnosticAtRegexp("main.go", "x"),
)
})
- withOptions(WithRootPath("a"), WithLimitWorkspaceScope()).run(t, mod, func(t *testing.T, env *Env) {
+ withOptions(WithRootPath("a"), LimitWorkspaceScope()).run(t, mod, func(t *testing.T, env *Env) {
env.OpenFile("a/main.go")
env.Await(
NoDiagnostics("main.go"),
diff --git a/gopls/internal/regtest/modfile_test.go b/gopls/internal/regtest/modfile_test.go
index 2f897a2..22d780a 100644
--- a/gopls/internal/regtest/modfile_test.go
+++ b/gopls/internal/regtest/modfile_test.go
@@ -33,6 +33,15 @@
const Name = "Hello"
`
+func runModfileTest(t *testing.T, files, proxy string, f TestFunc) {
+ t.Run("normal", func(t *testing.T) {
+ withOptions(WithProxyFiles(proxy)).run(t, files, f)
+ })
+ t.Run("nested", func(t *testing.T) {
+ withOptions(WithProxyFiles(proxy), NestWorkdir(), WithModes(Experimental)).run(t, files, f)
+ })
+}
+
func TestModFileModification(t *testing.T) {
testenv.NeedsGo1Point(t, 14)
@@ -50,7 +59,7 @@
}
`
t.Run("basic", func(t *testing.T) {
- withOptions(WithProxyFiles(proxy)).run(t, untidyModule, func(t *testing.T, env *Env) {
+ runModfileTest(t, untidyModule, proxy, func(t *testing.T, env *Env) {
// Open the file and make sure that the initial workspace load does not
// modify the go.mod file.
goModContent := env.ReadWorkspaceFile("go.mod")
@@ -77,7 +86,7 @@
t.Run("delete main.go", func(t *testing.T) {
t.Skip("This test will be flaky until golang/go#40269 is resolved.")
- withOptions(WithProxyFiles(proxy)).run(t, untidyModule, func(t *testing.T, env *Env) {
+ runModfileTest(t, untidyModule, proxy, func(t *testing.T, env *Env) {
goModContent := env.ReadWorkspaceFile("go.mod")
mainContent := env.ReadWorkspaceFile("main.go")
env.OpenFile("main.go")
@@ -126,7 +135,7 @@
require random.org v1.2.3
`
- withOptions(WithProxyFiles(proxy)).run(t, mod, func(t *testing.T, env *Env) {
+ runModfileTest(t, mod, proxy, func(t *testing.T, env *Env) {
env.OpenFile("main.go")
var d protocol.PublishDiagnosticsParams
env.Await(
@@ -172,7 +181,7 @@
require example.com v1.2.3
`
- runner.Run(t, mod, func(t *testing.T, env *Env) {
+ runModfileTest(t, mod, proxy, func(t *testing.T, env *Env) {
env.OpenFile("go.mod")
var d protocol.PublishDiagnosticsParams
env.Await(
@@ -185,7 +194,7 @@
if got := env.Editor.BufferText("go.mod"); got != want {
t.Fatalf("unexpected go.mod content:\n%s", tests.Diff(want, got))
}
- }, WithProxyFiles(proxy))
+ })
}
func TestUnusedDiag(t *testing.T) {
@@ -212,7 +221,7 @@
go 1.14
`
- withOptions(WithProxyFiles(proxy)).run(t, files, func(t *testing.T, env *Env) {
+ runModfileTest(t, files, proxy, func(t *testing.T, env *Env) {
env.OpenFile("go.mod")
var d protocol.PublishDiagnosticsParams
env.Await(
@@ -266,7 +275,7 @@
func _() {
caire.RemoveTempImage()
}`
- runner.Run(t, repro, func(t *testing.T, env *Env) {
+ runModfileTest(t, repro, proxy, func(t *testing.T, env *Env) {
env.OpenFile("main.go")
var d protocol.PublishDiagnosticsParams
env.Await(
@@ -288,7 +297,7 @@
if got := env.ReadWorkspaceFile("go.mod"); got != want {
t.Fatalf("TestNewDepWithUnusedDep failed:\n%s", tests.Diff(want, got))
}
- }, WithProxyFiles(proxy))
+ })
}
// TODO: For this test to be effective, the sandbox's file watcher must respect
@@ -310,13 +319,13 @@
func main() {
fmt.Println(blah.Name)
`
- runner.Run(t, mod, func(t *testing.T, env *Env) {
+ runModfileTest(t, mod, proxy, func(t *testing.T, env *Env) {
env.Await(env.DiagnosticAtRegexp("go.mod", "require"))
env.RunGoCommand("mod", "tidy")
env.Await(
EmptyDiagnostics("go.mod"),
)
- }, WithProxyFiles(proxy))
+ })
}
// Tests golang/go#39784: a missing indirect dependency, necessary
@@ -409,7 +418,7 @@
// Start from a bad state/bad IWL, and confirm that we recover.
t.Run("bad", func(t *testing.T) {
- runner.Run(t, unknown, func(t *testing.T, env *Env) {
+ runModfileTest(t, unknown, proxy, func(t *testing.T, env *Env) {
env.OpenFile("go.mod")
env.Await(
env.DiagnosticAtRegexp("go.mod", "example.com v1.2.2"),
@@ -419,7 +428,7 @@
env.Await(
env.DiagnosticAtRegexp("main.go", "x = "),
)
- }, WithProxyFiles(proxy))
+ })
})
const known = `
@@ -441,7 +450,7 @@
// Start from a good state, transform to a bad state, and confirm that we
// still recover.
t.Run("good", func(t *testing.T) {
- runner.Run(t, known, func(t *testing.T, env *Env) {
+ runModfileTest(t, known, proxy, func(t *testing.T, env *Env) {
env.OpenFile("go.mod")
env.Await(
env.DiagnosticAtRegexp("main.go", "x = "),
@@ -456,7 +465,7 @@
env.Await(
env.DiagnosticAtRegexp("main.go", "x = "),
)
- }, WithProxyFiles(proxy))
+ })
})
}
@@ -501,7 +510,7 @@
println(blah.Name)
}
`
- withOptions(WithProxyFiles(badProxy)).run(t, module, func(t *testing.T, env *Env) {
+ runModfileTest(t, module, badProxy, func(t *testing.T, env *Env) {
env.OpenFile("go.mod")
env.Await(
env.DiagnosticAtRegexp("go.mod", "require example.com v1.2.3"),
diff --git a/gopls/internal/regtest/runner.go b/gopls/internal/regtest/runner.go
index 8c5efe0..b9d646e 100644
--- a/gopls/internal/regtest/runner.go
+++ b/gopls/internal/regtest/runner.go
@@ -39,7 +39,6 @@
// and communicates over pipes to mimic the gopls sidecar execution mode,
// which communicates over stdin/stderr.
Singleton Mode = 1 << iota
-
// Forwarded forwards connections to a shared in-process gopls instance.
Forwarded
// SeparateProcess forwards connection to a shared separate gopls process.
@@ -77,13 +76,14 @@
}
type runConfig struct {
- editor fake.EditorConfig
- sandbox fake.SandboxConfig
- modes Mode
- timeout time.Duration
- debugAddr string
- skipLogs bool
- skipHooks bool
+ editor fake.EditorConfig
+ sandbox fake.SandboxConfig
+ modes Mode
+ timeout time.Duration
+ debugAddr string
+ skipLogs bool
+ skipHooks bool
+ nestWorkdir bool
}
func (r *Runner) defaultConfig() *runConfig {
@@ -153,7 +153,7 @@
// tests need to check other cases.
func WithRootPath(path string) RunOption {
return optionSetter(func(opts *runConfig) {
- opts.editor.EditorRootPath = path
+ opts.editor.WorkspaceRoot = path
})
}
@@ -209,13 +209,21 @@
})
}
-// WithLimitWorkspaceScope sets the LimitWorkspaceScope configuration.
-func WithLimitWorkspaceScope() RunOption {
+// LimitWorkspaceScope sets the LimitWorkspaceScope configuration.
+func LimitWorkspaceScope() RunOption {
return optionSetter(func(opts *runConfig) {
opts.editor.LimitWorkspaceScope = true
})
}
+// NestWorkdir inserts the sandbox working directory in a subdirectory of the
+// editor workspace.
+func NestWorkdir() RunOption {
+ return optionSetter(func(opts *runConfig) {
+ opts.nestWorkdir = true
+ })
+}
+
type TestFunc func(t *testing.T, env *Env)
// Run executes the test function in the default configured gopls execution
@@ -262,16 +270,25 @@
di.MonitorMemory(ctx)
}
- tempDir := filepath.Join(r.TempDir, filepath.FromSlash(t.Name()))
- if err := os.MkdirAll(tempDir, 0755); err != nil {
+ rootDir := filepath.Join(r.TempDir, filepath.FromSlash(t.Name()))
+ if err := os.MkdirAll(rootDir, 0755); err != nil {
t.Fatal(err)
}
+ if config.nestWorkdir {
+ config.sandbox.Workdir = "work/nested"
+ }
config.sandbox.Files = files
- config.sandbox.RootDir = tempDir
+ config.sandbox.RootDir = rootDir
sandbox, err := fake.NewSandbox(&config.sandbox)
if err != nil {
t.Fatal(err)
}
+ workdir := sandbox.Workdir.RootURI().SpanURI().Filename()
+ if config.nestWorkdir {
+ // Now that we know the actual workdir, set our workspace to be the
+ // parent directory.
+ config.editor.WorkspaceRoot = filepath.Clean(filepath.Join(workdir, ".."))
+ }
// Deferring the closure of ws until the end of the entire test suite
// has, in testing, given the LSP server time to properly shutdown and
// release any file locks held in workspace, which is a problem on
diff --git a/internal/lsp/fake/editor.go b/internal/lsp/fake/editor.go
index 3cc1c06..fed01ec 100644
--- a/internal/lsp/fake/editor.go
+++ b/internal/lsp/fake/editor.go
@@ -81,9 +81,9 @@
// workspace folders nor a root URI.
WithoutWorkspaceFolders bool
- // EditorRootPath specifies the root path of the workspace folder used when
+ // WorkspaceRoot specifies the root path of the workspace folder used when
// initializing gopls in the sandbox. If empty, the Workdir is used.
- EditorRootPath string
+ WorkspaceRoot string
// EnableStaticcheck enables staticcheck analyzers.
EnableStaticcheck bool
@@ -121,7 +121,7 @@
protocol.Handlers(
protocol.ClientHandler(e.client,
jsonrpc2.MethodNotFound)))
- if err := e.initialize(ctx, e.Config.WithoutWorkspaceFolders, e.Config.EditorRootPath); err != nil {
+ if err := e.initialize(ctx, e.Config.WithoutWorkspaceFolders, e.Config.WorkspaceRoot); err != nil {
return nil, err
}
e.sandbox.Workdir.AddWatcher(e.onFileChanges)
diff --git a/internal/lsp/fake/sandbox.go b/internal/lsp/fake/sandbox.go
index f240f9b..3b92dc0 100644
--- a/internal/lsp/fake/sandbox.go
+++ b/internal/lsp/fake/sandbox.go
@@ -22,7 +22,7 @@
// code in tests.
type Sandbox struct {
gopath string
- basedir string
+ rootdir string
goproxy string
Workdir *Workdir
}
@@ -42,9 +42,12 @@
// InGoPath specifies that the working directory should be within the
// temporary GOPATH.
InGoPath bool
- // Workdir configures the working directory of the Sandbox, for running in a
- // pre-existing directory. If unset, a new working directory will be created
- // under RootDir.
+ // Workdir configures the working directory of the Sandbox. It behaves as
+ // follows:
+ // - if set to an absolute path, use that path as the working directory.
+ // - if set to a relative path, create and use that path relative to the
+ // sandbox.
+ // - if unset, default to a the 'work' subdirectory of the sandbox.
//
// This option is incompatible with InGoPath or Files.
Workdir string
@@ -70,13 +73,8 @@
if config == nil {
config = new(SandboxConfig)
}
-
- if config.Workdir != "" && (config.Files != "" || config.InGoPath) {
- return nil, fmt.Errorf("invalid SandboxConfig: Workdir cannot be used in conjunction with Files or InGoPath. Got %+v", config)
- }
-
- if config.GOPROXY != "" && config.ProxyFiles != "" {
- return nil, fmt.Errorf("invalid SandboxConfig: GOPROXY cannot be set in conjunction with ProxyFiles. Got %+v", config)
+ if err := validateConfig(*config); err != nil {
+ return nil, fmt.Errorf("invalid SandboxConfig: %v", err)
}
sb := &Sandbox{}
@@ -87,19 +85,22 @@
}
}()
- baseDir, err := ioutil.TempDir(config.RootDir, "gopls-sandbox-")
- if err != nil {
- return nil, fmt.Errorf("creating temporary workdir: %v", err)
+ rootDir := config.RootDir
+ if rootDir == "" {
+ rootDir, err = ioutil.TempDir(config.RootDir, "gopls-sandbox-")
+ if err != nil {
+ return nil, fmt.Errorf("creating temporary workdir: %v", err)
+ }
}
- sb.basedir = baseDir
- sb.gopath = filepath.Join(sb.basedir, "gopath")
+ sb.rootdir = rootDir
+ sb.gopath = filepath.Join(sb.rootdir, "gopath")
if err := os.Mkdir(sb.gopath, 0755); err != nil {
return nil, err
}
if config.GOPROXY != "" {
sb.goproxy = config.GOPROXY
} else {
- proxydir := filepath.Join(sb.basedir, "proxy")
+ proxydir := filepath.Join(sb.rootdir, "proxy")
if err := os.Mkdir(proxydir, 0755); err != nil {
return nil, err
}
@@ -108,27 +109,32 @@
return nil, err
}
}
- if config.Workdir != "" {
+ // Short-circuit writing the workdir if we're given an absolute path, since
+ // this is used for running in an existing directory.
+ // TODO(findleyr): refactor this to be less of a workaround.
+ if filepath.IsAbs(config.Workdir) {
sb.Workdir = NewWorkdir(config.Workdir)
- } else {
- workdir := config.Workdir
- // If we don't have a pre-existing work dir, we want to create either
- // $GOPATH/src or <RootDir/work>.
+ return sb, nil
+ }
+ var workdir string
+ if config.Workdir == "" {
if config.InGoPath {
// Set the working directory as $GOPATH/src.
workdir = filepath.Join(sb.gopath, "src")
} else if workdir == "" {
- workdir = filepath.Join(sb.basedir, "work")
+ workdir = filepath.Join(sb.rootdir, "work")
}
- if err := os.Mkdir(workdir, 0755); err != nil {
- return nil, err
- }
- sb.Workdir = NewWorkdir(workdir)
- if err := sb.Workdir.writeInitialFiles(config.Files); err != nil {
- return nil, err
- }
+ } else {
+ // relative path
+ workdir = filepath.Join(sb.rootdir, config.Workdir)
}
-
+ if err := os.MkdirAll(workdir, 0755); err != nil {
+ return nil, err
+ }
+ sb.Workdir = NewWorkdir(workdir)
+ if err := sb.Workdir.writeInitialFiles(config.Files); err != nil {
+ return nil, err
+ }
return sb, nil
}
@@ -155,6 +161,19 @@
return dataMap
}
+func validateConfig(config SandboxConfig) error {
+ if filepath.IsAbs(config.Workdir) && (config.Files != "" || config.InGoPath) {
+ return errors.New("absolute Workdir cannot be set in conjunction with Files or InGoPath")
+ }
+ if config.Workdir != "" && config.InGoPath {
+ return errors.New("Workdir cannot be set in conjunction with InGoPath")
+ }
+ if config.GOPROXY != "" && config.ProxyFiles != "" {
+ return errors.New("GOPROXY cannot be set in conjunction with ProxyFiles")
+ }
+ return nil
+}
+
// splitModuleVersionPath extracts module information from files stored in the
// directory structure modulePath@version/suffix.
// For example:
@@ -174,6 +193,10 @@
return path, "", ""
}
+func (sb *Sandbox) RootDir() string {
+ return sb.rootdir
+}
+
// GOPATH returns the value of the Sandbox GOPATH.
func (sb *Sandbox) GOPATH() string {
return sb.gopath
@@ -236,7 +259,7 @@
if sb.gopath != "" {
goCleanErr = sb.RunGoCommand(context.Background(), "", "clean", []string{"-modcache"})
}
- err := os.RemoveAll(sb.basedir)
+ err := os.RemoveAll(sb.rootdir)
if err != nil || goCleanErr != nil {
return fmt.Errorf("error(s) cleaning sandbox: cleaning modcache: %v; removing files: %v", goCleanErr, err)
}
