diff --git a/gopls/internal/regtest/diagnostics_test.go b/gopls/internal/regtest/diagnostics_test.go index d3685ddac1..e8190cf7dc 100644 --- a/gopls/internal/regtest/diagnostics_test.go +++ b/gopls/internal/regtest/diagnostics_test.go @@ -1246,7 +1246,7 @@ func main() { 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 2f897a2fa9..22d780a36a 100644 --- a/gopls/internal/regtest/modfile_test.go +++ b/gopls/internal/regtest/modfile_test.go @@ -33,6 +33,15 @@ package hello 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 @@ func main() { } ` 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 @@ func main() { 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 @@ go 1.12 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 @@ go 1.12 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 @@ require example.com v1.2.3 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 @@ func main() {} 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 @@ import ( 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 @@ require ( 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 @@ package main 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 @@ func main() { // 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 @@ func main() { env.Await( env.DiagnosticAtRegexp("main.go", "x = "), ) - }, WithProxyFiles(proxy)) + }) }) const known = ` @@ -441,7 +450,7 @@ func main() { // 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 @@ func main() { env.Await( env.DiagnosticAtRegexp("main.go", "x = "), ) - }, WithProxyFiles(proxy)) + }) }) } @@ -501,7 +510,7 @@ func main() { 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 8c5efe0f17..b9d646e689 100644 --- a/gopls/internal/regtest/runner.go +++ b/gopls/internal/regtest/runner.go @@ -39,7 +39,6 @@ const ( // 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 Runner struct { } 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 @@ func WithoutWorkspaceFolders() RunOption { // 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 @@ func WithGOPROXY(goproxy string) RunOption { }) } -// 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 @@ func (r *Runner) Run(t *testing.T, files string, test TestFunc, opts ...RunOptio 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 3cc1c061c8..fed01eca95 100644 --- a/internal/lsp/fake/editor.go +++ b/internal/lsp/fake/editor.go @@ -81,9 +81,9 @@ type EditorConfig struct { // 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 @@ func (e *Editor) Connect(ctx context.Context, conn jsonrpc2.Conn, hooks ClientHo 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 f240f9b54a..3b92dc0c98 100644 --- a/internal/lsp/fake/sandbox.go +++ b/internal/lsp/fake/sandbox.go @@ -22,7 +22,7 @@ import ( // code in tests. type Sandbox struct { gopath string - basedir string + rootdir string goproxy string Workdir *Workdir } @@ -42,9 +42,12 @@ type SandboxConfig struct { // 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 @@ func NewSandbox(config *SandboxConfig) (_ *Sandbox, err error) { 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 @@ func NewSandbox(config *SandboxConfig) (_ *Sandbox, err error) { } }() - 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 @@ func NewSandbox(config *SandboxConfig) (_ *Sandbox, err error) { 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 . + 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") - } - 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 + workdir = filepath.Join(sb.rootdir, "work") } + } 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 @@ func unpackTxt(txt string) map[string][]byte { 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 @@ func splitModuleVersionPath(path string) (modulePath, version, suffix string) { 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 @@ func (sb *Sandbox) Close() error { 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) }