internal/lsp/regtest: use a common directory for regtest sandboxes
For easier debugging (and less cruft if regtests are ctrl-c'ed), root
all regtest sandboxes in a common directory.
This also tries one last time to clean up the directory, and fails on an
error. This might be flaky on windows, but hasn't been so far...
Also give regtest sandboxes names derived from their test name.
Updates golang/go#39384
Updates golang/go#38490
Change-Id: Iae53c29e75f5eb2b8d938d205fbeb463ffc82eb2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240059
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/internal/lsp/fake/editor_test.go b/internal/lsp/fake/editor_test.go
index bb5e19e..df36222 100644
--- a/internal/lsp/fake/editor_test.go
+++ b/internal/lsp/fake/editor_test.go
@@ -48,7 +48,7 @@
`
func TestClientEditing(t *testing.T) {
- ws, err := NewSandbox("TestClientEditing", exampleProgram, "", false, false)
+ ws, err := NewSandbox("", exampleProgram, "", false, false)
if err != nil {
t.Fatal(err)
}
diff --git a/internal/lsp/fake/sandbox.go b/internal/lsp/fake/sandbox.go
index 2c56abd..f41c454 100644
--- a/internal/lsp/fake/sandbox.go
+++ b/internal/lsp/fake/sandbox.go
@@ -20,7 +20,6 @@
// Sandbox holds a collection of temporary resources to use for working with Go
// code in tests.
type Sandbox struct {
- name string
gopath string
basedir string
Proxy *Proxy
@@ -36,21 +35,24 @@
// working directory populated by the txtar-encoded content in srctxt, and a
// file-based module proxy populated with the txtar-encoded content in
// proxytxt.
-func NewSandbox(name, srctxt, proxytxt string, inGopath bool, withoutWorkspaceFolders bool) (_ *Sandbox, err error) {
- sb := &Sandbox{
- name: name,
- }
+//
+// If rootDir is non-empty, it will be used as the root of temporary
+// directories created for the sandbox. Otherwise, a new temporary directory
+// will be used as root.
+func NewSandbox(rootDir, srctxt, proxytxt string, inGopath bool, withoutWorkspaceFolders bool) (_ *Sandbox, err error) {
+ sb := &Sandbox{}
defer func() {
// Clean up if we fail at any point in this constructor.
if err != nil {
sb.Close()
}
}()
- basedir, err := ioutil.TempDir("", fmt.Sprintf("goplstest-sandbox-%s-", name))
+
+ baseDir, err := ioutil.TempDir(rootDir, "gopls-sandbox-")
if err != nil {
return nil, fmt.Errorf("creating temporary workdir: %v", err)
}
- sb.basedir = basedir
+ sb.basedir = baseDir
proxydir := filepath.Join(sb.basedir, "proxy")
sb.gopath = filepath.Join(sb.basedir, "gopath")
// Set the working directory as $GOPATH/src if inGopath is true.
diff --git a/internal/lsp/lsprpc/lsprpc_test.go b/internal/lsp/lsprpc/lsprpc_test.go
index d7c2ee2..1f0c626 100644
--- a/internal/lsp/lsprpc/lsprpc_test.go
+++ b/internal/lsp/lsprpc/lsprpc_test.go
@@ -196,7 +196,7 @@
}`
func TestDebugInfoLifecycle(t *testing.T) {
- sb, err := fake.NewSandbox("gopls-lsprpc-test", exampleProgram, "", false, false)
+ sb, err := fake.NewSandbox("", exampleProgram, "", false, false)
if err != nil {
t.Fatal(err)
}
diff --git a/internal/lsp/regtest/reg_test.go b/internal/lsp/regtest/reg_test.go
index 9f0e283..0837fae 100644
--- a/internal/lsp/regtest/reg_test.go
+++ b/internal/lsp/regtest/reg_test.go
@@ -8,6 +8,7 @@
"context"
"flag"
"fmt"
+ "io/ioutil"
"os"
"testing"
"time"
@@ -20,6 +21,7 @@
runSubprocessTests = flag.Bool("enable_gopls_subprocess_tests", false, "run regtests against a gopls subprocess")
goplsBinaryPath = flag.String("gopls_test_binary", "", "path to the gopls binary for use as a remote, for use with the -enable_gopls_subprocess_tests flag")
regtestTimeout = flag.Duration("regtest_timeout", 60*time.Second, "default timeout for each regtest")
+ skipCleanup = flag.Bool("regtest_skip_cleanup", false, "whether to skip cleaning up temp directories")
printGoroutinesOnFailure = flag.Bool("regtest_print_goroutines", false, "whether to print goroutines info on failure")
)
@@ -36,6 +38,7 @@
DefaultModes: NormalModes,
Timeout: *regtestTimeout,
PrintGoroutinesOnFailure: *printGoroutinesOnFailure,
+ SkipCleanup: *skipCleanup,
}
if *runSubprocessTests {
goplsPath := *goplsBinaryPath
@@ -49,8 +52,16 @@
runner.DefaultModes = NormalModes | SeparateProcess
runner.GoplsPath = goplsPath
}
+ dir, err := ioutil.TempDir("", "gopls-regtest-")
+ if err != nil {
+ panic(fmt.Errorf("creating regtest temp directory: %v", err))
+ }
+ runner.TempDir = dir
code := m.Run()
- runner.Close()
+ if err := runner.Close(); err != nil {
+ fmt.Fprintf(os.Stderr, "closing test runner: %v\n", err)
+ os.Exit(1)
+ }
os.Exit(code)
}
diff --git a/internal/lsp/regtest/runner.go b/internal/lsp/regtest/runner.go
index aa5f665..ba42598 100644
--- a/internal/lsp/regtest/runner.go
+++ b/internal/lsp/regtest/runner.go
@@ -56,6 +56,8 @@
Timeout time.Duration
GoplsPath string
PrintGoroutinesOnFailure bool
+ TempDir string
+ SkipCleanup bool
mu sync.Mutex
ts *servertest.TCPServer
@@ -70,7 +72,6 @@
modes Mode
proxyTxt string
timeout time.Duration
- skipCleanup bool
gopath bool
withoutWorkspaceFolders bool
}
@@ -139,14 +140,6 @@
})
}
-// SkipCleanup is used only for debugging: is skips cleaning up the tests state
-// after completion.
-func SkipCleanup() RunOption {
- return optionSetter(func(opts *runConfig) {
- opts.skipCleanup = true
- })
-}
-
// Run executes the test function in the default configured gopls execution
// modes. For each a test run, a new workspace is created containing the
// un-txtared files specified by filedata.
@@ -173,12 +166,16 @@
continue
}
t.Run(tc.name, func(t *testing.T) {
- t.Helper()
ctx, cancel := context.WithTimeout(context.Background(), config.timeout)
defer cancel()
ctx = debug.WithInstance(ctx, "", "")
- sandbox, err := fake.NewSandbox("regtest", filedata, config.proxyTxt, config.gopath, config.withoutWorkspaceFolders)
+ tempDir := filepath.Join(r.TempDir, filepath.FromSlash(t.Name()))
+ if err := os.MkdirAll(tempDir, 0755); err != nil {
+ t.Fatal(err)
+ }
+
+ sandbox, err := fake.NewSandbox(tempDir, filedata, config.proxyTxt, config.gopath, config.withoutWorkspaceFolders)
if err != nil {
t.Fatal(err)
}
@@ -188,13 +185,7 @@
// Windows. This may still be flaky however, and in the future we need a
// better solution to ensure that all Go processes started by gopls have
// exited before we clean up.
- if config.skipCleanup {
- defer func() {
- t.Logf("Skipping workspace cleanup: running in %s", sandbox.Workdir.RootURI())
- }()
- } else {
- r.AddCloser(sandbox)
- }
+ r.AddCloser(sandbox)
ss := tc.getServer(ctx, t)
ls := &loggingFramer{}
framer := ls.framer(jsonrpc2.NewRawStream)
@@ -291,7 +282,7 @@
t.Fatal("cannot run tests with a separate process unless a path to a gopls binary is configured")
}
var err error
- r.socketDir, err = ioutil.TempDir("", "gopls-regtests")
+ r.socketDir, err = ioutil.TempDir(r.TempDir, "gopls-regtest-socket")
if err != nil {
t.Fatalf("creating tempdir: %v", err)
}
@@ -333,8 +324,13 @@
errmsgs = append(errmsgs, err.Error())
}
}
- for _, closer := range r.closers {
- if err := closer.Close(); err != nil {
+ if !r.SkipCleanup {
+ for _, closer := range r.closers {
+ if err := closer.Close(); err != nil {
+ errmsgs = append(errmsgs, err.Error())
+ }
+ }
+ if err := os.RemoveAll(r.TempDir); err != nil {
errmsgs = append(errmsgs, err.Error())
}
}
