From a5e28d8dabebcf3d9ced7db6c96577fa607a0b52 Mon Sep 17 00:00:00 2001 From: Rob Findley Date: Wed, 22 Jul 2020 20:06:06 -0400 Subject: [PATCH] internal/lsp/fake: move to a struct for configuring the sandbox The Sandbox constructor was getting out of control, and this allows binding regtest options directly to the sandbox configuration struct. Change-Id: I18da4ddf43c86b3b652516c3ddca7726cd35e3b2 Reviewed-on: https://go-review.googlesource.com/c/tools/+/244439 Run-TryBot: Robert Findley TryBot-Result: Gobot Gobot Reviewed-by: Heschi Kreinick --- internal/lsp/fake/editor.go | 11 ++++++++- internal/lsp/fake/editor_test.go | 2 +- internal/lsp/fake/sandbox.go | 39 ++++++++++++++++++------------ internal/lsp/lsprpc/lsprpc_test.go | 2 +- internal/lsp/regtest/runner.go | 38 ++++++++++++++--------------- 5 files changed, 55 insertions(+), 37 deletions(-) diff --git a/internal/lsp/fake/editor.go b/internal/lsp/fake/editor.go index a2fbd3c24d..5e16b027d2 100644 --- a/internal/lsp/fake/editor.go +++ b/internal/lsp/fake/editor.go @@ -68,6 +68,15 @@ type EditorConfig struct { // SymbolMatcher is the config associated with the "symbolMatcher" gopls // config option. SymbolMatcher *string + + // WithoutWorkspaceFolders is used to simulate opening a single file in the + // editor, without a workspace root. In that case, the client sends neither + // workspace folders nor a root URI. + WithoutWorkspaceFolders bool + + // EditorRootPath specifies the root path of the workspace folder used when + // initializing gopls in the sandbox. If empty, the Workdir is used. + EditorRootPath string } // NewEditor Creates a new Editor. @@ -94,7 +103,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.sandbox.withoutWorkspaceFolders, e.sandbox.editorRootPath); err != nil { + if err := e.initialize(ctx, e.Config.WithoutWorkspaceFolders, e.Config.EditorRootPath); err != nil { return nil, err } e.sandbox.Workdir.AddWatcher(e.onFileChanges) diff --git a/internal/lsp/fake/editor_test.go b/internal/lsp/fake/editor_test.go index e01ac81206..f1ce7537ae 100644 --- a/internal/lsp/fake/editor_test.go +++ b/internal/lsp/fake/editor_test.go @@ -48,7 +48,7 @@ func main() { ` func TestClientEditing(t *testing.T) { - ws, err := NewSandbox("", exampleProgram, "", false, false, "") + ws, err := NewSandbox(&SandboxConfig{Files: exampleProgram}) if err != nil { t.Fatal(err) } diff --git a/internal/lsp/fake/sandbox.go b/internal/lsp/fake/sandbox.go index 3b3b005e92..ca9b34f98f 100644 --- a/internal/lsp/fake/sandbox.go +++ b/internal/lsp/fake/sandbox.go @@ -24,15 +24,23 @@ type Sandbox struct { basedir string Proxy *Proxy Workdir *Workdir +} - // withoutWorkspaceFolders is used to simulate opening a single file in the - // editor, without a workspace root. In that case, the client sends neither - // workspace folders nor a root URI. - withoutWorkspaceFolders bool - - // editorRootPath specifies the root path of the workspace folder used when - // initializing gopls in the sandbox. If empty, the Workdir is used. - editorRootPath string +// SandboxConfig controls the behavior of a test sandbox. The zero value +// defines a reasonable default. +type SandboxConfig struct { + // RootDir sets the base directory to use when creating temporary + // directories. If not specified, defaults to a new temporary directory. + RootDir string + // Files holds a txtar-encoded archive of files to populate the initial state + // of the working directory. + Files string + // ProxyFiles holds a txtar-encoded archive of files to populate a file-based + // Go proxy. + ProxyFiles string + // InGoPath specifies that the working directory should be within the + // temporary GOPATH. + InGoPath bool } // NewSandbox creates a collection of named temporary resources, with a @@ -43,7 +51,10 @@ type Sandbox struct { // 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, editorRootPath string) (_ *Sandbox, err error) { +func NewSandbox(config *SandboxConfig) (_ *Sandbox, err error) { + if config == nil { + config = new(SandboxConfig) + } sb := &Sandbox{} defer func() { // Clean up if we fail at any point in this constructor. @@ -52,7 +63,7 @@ func NewSandbox(rootDir, srctxt, proxytxt string, inGopath bool, withoutWorkspac } }() - baseDir, err := ioutil.TempDir(rootDir, "gopls-sandbox-") + baseDir, err := ioutil.TempDir(config.RootDir, "gopls-sandbox-") if err != nil { return nil, fmt.Errorf("creating temporary workdir: %v", err) } @@ -62,7 +73,7 @@ func NewSandbox(rootDir, srctxt, proxytxt string, inGopath bool, withoutWorkspac // Set the working directory as $GOPATH/src if inGopath is true. workdir := filepath.Join(sb.gopath, "src") dirs := []string{sb.gopath, proxydir} - if !inGopath { + if !config.InGoPath { workdir = filepath.Join(sb.basedir, "work") dirs = append(dirs, workdir) } @@ -71,10 +82,8 @@ func NewSandbox(rootDir, srctxt, proxytxt string, inGopath bool, withoutWorkspac return nil, err } } - sb.Proxy, err = NewProxy(proxydir, proxytxt) - sb.Workdir, err = NewWorkdir(workdir, srctxt) - sb.withoutWorkspaceFolders = withoutWorkspaceFolders - sb.editorRootPath = editorRootPath + sb.Proxy, err = NewProxy(proxydir, config.ProxyFiles) + sb.Workdir, err = NewWorkdir(workdir, config.Files) return sb, nil } diff --git a/internal/lsp/lsprpc/lsprpc_test.go b/internal/lsp/lsprpc/lsprpc_test.go index 8cdc011744..339d1d14df 100644 --- a/internal/lsp/lsprpc/lsprpc_test.go +++ b/internal/lsp/lsprpc/lsprpc_test.go @@ -196,7 +196,7 @@ func main() { }` func TestDebugInfoLifecycle(t *testing.T) { - sb, err := fake.NewSandbox("", exampleProgram, "", false, false, "") + sb, err := fake.NewSandbox(&fake.SandboxConfig{Files: exampleProgram}) if err != nil { t.Fatal(err) } diff --git a/internal/lsp/regtest/runner.go b/internal/lsp/regtest/runner.go index 33aa90c4f7..1cf38d8b88 100644 --- a/internal/lsp/regtest/runner.go +++ b/internal/lsp/regtest/runner.go @@ -68,13 +68,10 @@ type Runner struct { } type runConfig struct { - editorConfig fake.EditorConfig - modes Mode - proxyTxt string - timeout time.Duration - gopath bool - withoutWorkspaceFolders bool - rootPath string + editor fake.EditorConfig + sandbox fake.SandboxConfig + modes Mode + timeout time.Duration } func (r *Runner) defaultConfig() *runConfig { @@ -105,7 +102,7 @@ func WithTimeout(d time.Duration) RunOption { // WithProxy configures a file proxy using the given txtar-encoded string. func WithProxy(txt string) RunOption { return optionSetter(func(opts *runConfig) { - opts.proxyTxt = txt + opts.sandbox.ProxyFiles = txt }) } @@ -119,7 +116,7 @@ func WithModes(modes Mode) RunOption { // WithEditorConfig configures the editor's LSP session. func WithEditorConfig(config fake.EditorConfig) RunOption { return optionSetter(func(opts *runConfig) { - opts.editorConfig = config + opts.editor = config }) } @@ -129,7 +126,8 @@ func WithEditorConfig(config fake.EditorConfig) RunOption { // neither workspace folders nor a root URI. func WithoutWorkspaceFolders() RunOption { return optionSetter(func(opts *runConfig) { - opts.withoutWorkspaceFolders = false + // TODO: this cannot be right. + opts.editor.WithoutWorkspaceFolders = false }) } @@ -138,7 +136,7 @@ func WithoutWorkspaceFolders() RunOption { // tests need to check other cases. func WithRootPath(path string) RunOption { return optionSetter(func(opts *runConfig) { - opts.rootPath = path + opts.editor.EditorRootPath = path }) } @@ -146,7 +144,7 @@ func WithRootPath(path string) RunOption { // than a separate working directory for use with modules. func InGOPATH() RunOption { return optionSetter(func(opts *runConfig) { - opts.gopath = true + opts.sandbox.InGoPath = true }) } @@ -155,12 +153,8 @@ type TestFunc func(t *testing.T, env *Env) // 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. -func (r *Runner) Run(t *testing.T, filedata string, test TestFunc, opts ...RunOption) { +func (r *Runner) Run(t *testing.T, files string, test TestFunc, opts ...RunOption) { t.Helper() - config := r.defaultConfig() - for _, opt := range opts { - opt.set(config) - } tests := []struct { name string @@ -174,6 +168,10 @@ func (r *Runner) Run(t *testing.T, filedata string, test TestFunc, opts ...RunOp for _, tc := range tests { tc := tc + config := r.defaultConfig() + for _, opt := range opts { + opt.set(config) + } if config.modes&tc.mode == 0 { continue } @@ -186,7 +184,9 @@ func (r *Runner) Run(t *testing.T, filedata string, test TestFunc, opts ...RunOp if err := os.MkdirAll(tempDir, 0755); err != nil { t.Fatal(err) } - sandbox, err := fake.NewSandbox(tempDir, filedata, config.proxyTxt, config.gopath, config.withoutWorkspaceFolders, config.rootPath) + config.sandbox.Files = files + config.sandbox.RootDir = tempDir + sandbox, err := fake.NewSandbox(&config.sandbox) if err != nil { t.Fatal(err) } @@ -201,7 +201,7 @@ func (r *Runner) Run(t *testing.T, filedata string, test TestFunc, opts ...RunOp ls := &loggingFramer{} framer := ls.framer(jsonrpc2.NewRawStream) ts := servertest.NewPipeServer(ctx, ss, framer) - env := NewEnv(ctx, t, sandbox, ts, config.editorConfig) + env := NewEnv(ctx, t, sandbox, ts, config.editor) defer func() { if t.Failed() && r.PrintGoroutinesOnFailure { pprof.Lookup("goroutine").WriteTo(os.Stderr, 1)