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>
This commit is contained in:
Rob Findley 2020-09-24 12:56:09 -04:00 committed by Robert Findley
parent 079ba7bd75
commit 53e0aa8268
5 changed files with 114 additions and 65 deletions

View File

@ -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"),

View File

@ -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"),

View File

@ -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

View File

@ -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)

View File

@ -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 <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")
}
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)
}