From 006b16f6cf7fbcd3e40539140eef5f1e69aaf847 Mon Sep 17 00:00:00 2001 From: Ian Cottrell Date: Mon, 27 Apr 2020 12:12:05 -0400 Subject: [PATCH] internal/lsp: share common command line test functionality This moves the common code from the cmd and gopls tests to the shared cmdtest package, they were starting to drift apart. This change was extracted from another larger cl where I was trying to work out why it broke in one but not the other. Change-Id: I554ce364f4152e6b61f989da8162d968426d4ae5 Reviewed-on: https://go-review.googlesource.com/c/tools/+/230301 Run-TryBot: Ian Cottrell TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler --- gopls/test/gopls_test.go | 32 ++++++------------------------ internal/lsp/cmd/cmd_test.go | 34 +++++++------------------------- internal/lsp/cmd/test/cmdtest.go | 29 +++++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 53 deletions(-) diff --git a/gopls/test/gopls_test.go b/gopls/test/gopls_test.go index e074b0c556..feceea3d9a 100644 --- a/gopls/test/gopls_test.go +++ b/gopls/test/gopls_test.go @@ -10,13 +10,8 @@ import ( "golang.org/x/tools/go/packages/packagestest" "golang.org/x/tools/gopls/internal/hooks" - "golang.org/x/tools/internal/jsonrpc2/servertest" - "golang.org/x/tools/internal/lsp/cache" cmdtest "golang.org/x/tools/internal/lsp/cmd/test" - "golang.org/x/tools/internal/lsp/debug" - "golang.org/x/tools/internal/lsp/lsprpc" "golang.org/x/tools/internal/lsp/source" - "golang.org/x/tools/internal/lsp/tests" "golang.org/x/tools/internal/testenv" ) @@ -26,7 +21,12 @@ func TestMain(m *testing.M) { } func TestCommandLine(t *testing.T) { - packagestest.TestAll(t, testCommandLine) + packagestest.TestAll(t, + cmdtest.TestCommandLine( + "../../internal/lsp/testdata", + commandLineOptions, + ), + ) } func commandLineOptions(options *source.Options) { @@ -34,23 +34,3 @@ func commandLineOptions(options *source.Options) { options.GoDiff = false hooks.Options(options) } - -func testCommandLine(t *testing.T, exporter packagestest.Exporter) { - const testdata = "../../internal/lsp/testdata" - if stat, err := os.Stat(testdata); err != nil || !stat.IsDir() { - t.Skip("testdata directory not present") - } - data := tests.Load(t, exporter, testdata) - ctx := tests.Context(t) - ctx = debug.WithInstance(ctx, "", "") - cache := cache.New(ctx, commandLineOptions) - ss := lsprpc.NewStreamServer(cache) - ts := servertest.NewTCPServer(ctx, ss) - for _, data := range data { - defer data.Exported.Cleanup() - t.Run(data.Folder, func(t *testing.T) { - t.Helper() - tests.Run(t, cmdtest.NewRunner(exporter, data, tests.Context(t), ts.Addr, commandLineOptions), data) - }) - } -} diff --git a/internal/lsp/cmd/cmd_test.go b/internal/lsp/cmd/cmd_test.go index 0f0ca86e10..be054954a2 100644 --- a/internal/lsp/cmd/cmd_test.go +++ b/internal/lsp/cmd/cmd_test.go @@ -5,7 +5,6 @@ package cmd_test import ( - "context" "fmt" "os" "path/filepath" @@ -14,12 +13,8 @@ import ( "testing" "golang.org/x/tools/go/packages/packagestest" - "golang.org/x/tools/internal/jsonrpc2/servertest" - "golang.org/x/tools/internal/lsp/cache" "golang.org/x/tools/internal/lsp/cmd" cmdtest "golang.org/x/tools/internal/lsp/cmd/test" - "golang.org/x/tools/internal/lsp/debug" - "golang.org/x/tools/internal/lsp/lsprpc" "golang.org/x/tools/internal/lsp/tests" "golang.org/x/tools/internal/testenv" ) @@ -30,27 +25,12 @@ func TestMain(m *testing.M) { } func TestCommandLine(t *testing.T) { - packagestest.TestAll(t, testCommandLine) -} - -func testCommandLine(t *testing.T, exporter packagestest.Exporter) { - ctx := tests.Context(t) - ts := testServer(ctx) - data := tests.Load(t, exporter, "../testdata") - for _, datum := range data { - defer datum.Exported.Cleanup() - t.Run(tests.FormatFolderName(datum.Folder), func(t *testing.T) { - t.Helper() - tests.Run(t, cmdtest.NewRunner(exporter, datum, ctx, ts.Addr, nil), datum) - }) - } -} - -func testServer(ctx context.Context) *servertest.TCPServer { - ctx = debug.WithInstance(ctx, "", "") - cache := cache.New(ctx, nil) - ss := lsprpc.NewStreamServer(cache) - return servertest.NewTCPServer(ctx, ss) + packagestest.TestAll(t, + cmdtest.TestCommandLine( + "../testdata", + nil, + ), + ) } func TestDefinitionHelpExample(t *testing.T) { @@ -65,7 +45,7 @@ func TestDefinitionHelpExample(t *testing.T) { return } ctx := tests.Context(t) - ts := testServer(ctx) + ts := cmdtest.NewTestServer(ctx, nil) thisFile := filepath.Join(dir, "definition.go") baseArgs := []string{"query", "definition"} expect := regexp.MustCompile(`(?s)^[\w/\\:_-]+flag[/\\]flag.go:\d+:\d+-\d+: defined here as FlagSet struct {.*}$`) diff --git a/internal/lsp/cmd/test/cmdtest.go b/internal/lsp/cmd/test/cmdtest.go index a1bff9bfd7..863e7edb02 100644 --- a/internal/lsp/cmd/test/cmdtest.go +++ b/internal/lsp/cmd/test/cmdtest.go @@ -18,7 +18,11 @@ import ( "testing" "golang.org/x/tools/go/packages/packagestest" + "golang.org/x/tools/internal/jsonrpc2/servertest" + "golang.org/x/tools/internal/lsp/cache" "golang.org/x/tools/internal/lsp/cmd" + "golang.org/x/tools/internal/lsp/debug" + "golang.org/x/tools/internal/lsp/lsprpc" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/lsp/tests" @@ -42,6 +46,31 @@ type normalizer struct { fragment string } +func TestCommandLine(testdata string, options func(*source.Options)) func(*testing.T, packagestest.Exporter) { + return func(t *testing.T, exporter packagestest.Exporter) { + if stat, err := os.Stat(testdata); err != nil || !stat.IsDir() { + t.Skip("testdata directory not present") + } + ctx := tests.Context(t) + ts := NewTestServer(ctx, options) + data := tests.Load(t, exporter, testdata) + for _, datum := range data { + defer datum.Exported.Cleanup() + t.Run(tests.FormatFolderName(datum.Folder), func(t *testing.T) { + t.Helper() + tests.Run(t, NewRunner(exporter, datum, ctx, ts.Addr, options), datum) + }) + } + } +} + +func NewTestServer(ctx context.Context, options func(*source.Options)) *servertest.TCPServer { + ctx = debug.WithInstance(ctx, "", "") + cache := cache.New(ctx, options) + ss := lsprpc.NewStreamServer(cache) + return servertest.NewTCPServer(ctx, ss) +} + func NewRunner(exporter packagestest.Exporter, data *tests.Data, ctx context.Context, remote string, options func(*source.Options)) *runner { r := &runner{ exporter: exporter,