aboutsummaryrefslogtreecommitdiff
path: root/internal/lsp
diff options
context:
space:
mode:
authorRob Findley <rfindley@google.com>2020-04-15 17:14:53 -0400
committerRobert Findley <rfindley@google.com>2020-04-16 21:39:01 +0000
commitf03878568042e284885cd8fbd1824cfa2298086c (patch)
treeddb82c45648a10fd85d1cdea13c6262d1f6e8c56 /internal/lsp
parent92fa1ff4b140942f40a126947c7895a71101ed73 (diff)
downloadgolang-x-tools-f03878568042e284885cd8fbd1824cfa2298086c.tar.gz
internal/lsp/regtest: generalize expectations beyond diagnostic messages
Due to the asynchronous and non-transactional nature of the LSP, writing regtests requires waiting for certain conditions to be met in the client editor. To this point, the only type of condition for which we supported waiting was the presence of diagnostic messages, but we can in principle wait for anything that is triggered by a server notification. This change generalizes the notion of expectations to also encompass log messages. Doing this required expanding the value returned from checking expectations to include a new "Unmeetable" verdict, to account for cases where we know that a condition will never be met (for example if it is a negative assertion). This may be useful for diagnostics as well. A test is added to demonstrate these new expectations, where the initial workspace load fails due to unfetchable dependencies. Additionally, some helper flags are added to help debug regtests without a code change, by allowing changing the test timeout, printing logs, and printing goroutine profiles. Updates golang/go#36879 Change-Id: I4cc194e10a4f181ad36a1a7abbb08ff41954b642 Reviewed-on: https://go-review.googlesource.com/c/tools/+/228399 Run-TryBot: Robert Findley <rfindley@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Diffstat (limited to 'internal/lsp')
-rw-r--r--internal/lsp/regtest/diagnostics_test.go23
-rw-r--r--internal/lsp/regtest/env.go346
-rw-r--r--internal/lsp/regtest/reg_test.go20
3 files changed, 275 insertions, 114 deletions
diff --git a/internal/lsp/regtest/diagnostics_test.go b/internal/lsp/regtest/diagnostics_test.go
index f78c32372..72447f542 100644
--- a/internal/lsp/regtest/diagnostics_test.go
+++ b/internal/lsp/regtest/diagnostics_test.go
@@ -8,6 +8,7 @@ import (
"testing"
"golang.org/x/tools/internal/lsp/fake"
+ "golang.org/x/tools/internal/lsp/protocol"
)
// Use mod.com for all go.mod files due to golang/go#35230.
@@ -31,7 +32,14 @@ func TestDiagnosticErrorInEditedFile(t *testing.T) {
// diagnostic.
env.OpenFile("main.go")
env.RegexpReplace("main.go", "Printl(n)", "")
- env.Await(env.DiagnosticAtRegexp("main.go", "Printl"))
+ env.Await(
+ env.DiagnosticAtRegexp("main.go", "Printl"),
+ // Assert that this test has sent no error logs to the client. This is not
+ // strictly necessary for testing this regression, but is included here
+ // as an example of using the NoErrorLogs() expectation. Feel free to
+ // delete.
+ NoErrorLogs(),
+ )
})
}
@@ -88,7 +96,10 @@ func TestDiagnosticClearingOnEdit(t *testing.T) {
// Fix the error by editing the const name in b.go to `b`.
env.RegexpReplace("b.go", "(a) = 2", "b")
- env.Await(EmptyDiagnostics("a.go"), EmptyDiagnostics("b.go"))
+ env.Await(
+ EmptyDiagnostics("a.go"),
+ EmptyDiagnostics("b.go"),
+ )
})
}
@@ -297,10 +308,16 @@ const Answer = 42
func TestResolveDiagnosticWithDownload(t *testing.T) {
runner.Run(t, testPackageWithRequire, func(t *testing.T, env *Env) {
env.OpenFile("print.go")
- env.W.RunGoCommand(env.Ctx, "mod", "download")
// Check that gopackages correctly loaded this dependency. We should get a
// diagnostic for the wrong formatting type.
// TODO: we should be able to easily also match the diagnostic message.
env.Await(env.DiagnosticAtRegexp("print.go", "fmt.Printf"))
}, WithProxy(testPackageWithRequireProxy))
}
+
+func TestMissingDependency(t *testing.T) {
+ runner.Run(t, testPackageWithRequire, func(t *testing.T, env *Env) {
+ env.OpenFile("print.go")
+ env.Await(LogMatching(protocol.Error, "initial workspace load failed"))
+ })
+}
diff --git a/internal/lsp/regtest/env.go b/internal/lsp/regtest/env.go
index 2faf4256b..7cee9acd5 100644
--- a/internal/lsp/regtest/env.go
+++ b/internal/lsp/regtest/env.go
@@ -14,6 +14,8 @@ import (
"os"
"os/exec"
"path/filepath"
+ "regexp"
+ "runtime/pprof"
"strings"
"sync"
"testing"
@@ -50,9 +52,11 @@ const (
// remote), any tests that execute on the same Runner will share the same
// state.
type Runner struct {
- defaultModes EnvMode
- timeout time.Duration
- goplsPath string
+ DefaultModes EnvMode
+ Timeout time.Duration
+ GoplsPath string
+ AlwaysPrintLogs bool
+ PrintGoroutinesOnFailure bool
mu sync.Mutex
ts *servertest.TCPServer
@@ -62,20 +66,10 @@ type Runner struct {
closers []io.Closer
}
-// NewTestRunner creates a Runner with its shared state initialized, ready to
-// run tests.
-func NewTestRunner(modes EnvMode, testTimeout time.Duration, goplsPath string) *Runner {
- return &Runner{
- defaultModes: modes,
- timeout: testTimeout,
- goplsPath: goplsPath,
- }
-}
-
// Modes returns the bitmask of environment modes this runner is configured to
// test.
func (r *Runner) Modes() EnvMode {
- return r.defaultModes
+ return r.DefaultModes
}
// getTestServer gets the test server instance to connect to, or creates one if
@@ -106,7 +100,7 @@ func (r *Runner) getRemoteSocket(t *testing.T) string {
return filepath.Join(r.socketDir, daemonFile)
}
- if r.goplsPath == "" {
+ if r.GoplsPath == "" {
t.Fatal("cannot run tests with a separate process unless a path to a gopls binary is configured")
}
var err error
@@ -116,7 +110,7 @@ func (r *Runner) getRemoteSocket(t *testing.T) string {
}
socket := filepath.Join(r.socketDir, daemonFile)
args := []string{"serve", "-listen", "unix;" + socket, "-listen.timeout", "10s"}
- cmd := exec.Command(r.goplsPath, args...)
+ cmd := exec.Command(r.GoplsPath, args...)
cmd.Env = append(os.Environ(), runTestAsGoplsEnvvar+"=true")
var stderr bytes.Buffer
cmd.Stderr = &stderr
@@ -171,8 +165,8 @@ type runConfig struct {
func (r *Runner) defaultConfig() *runConfig {
return &runConfig{
- modes: r.defaultModes,
- timeout: r.timeout,
+ modes: r.DefaultModes,
+ timeout: r.Timeout,
}
}
@@ -258,7 +252,10 @@ func (r *Runner) Run(t *testing.T, filedata string, test func(t *testing.T, e *E
}()
env := NewEnv(ctx, t, ws, ts)
defer func() {
- if t.Failed() {
+ if t.Failed() && r.PrintGoroutinesOnFailure {
+ pprof.Lookup("goroutine").WriteTo(os.Stderr, 1)
+ }
+ if t.Failed() || r.AlwaysPrintLogs {
ls.printBuffers(t.Name(), os.Stderr)
}
if err := env.E.Shutdown(ctx); err != nil {
@@ -332,17 +329,47 @@ type Env struct {
// every change to diagnostics.
mu sync.Mutex
// For simplicity, each waiter gets a unique ID.
- nextWaiterID int
- lastDiagnostics map[string]*protocol.PublishDiagnosticsParams
- waiters map[int]*diagnosticCondition
+ nextWaiterID int
+ state State
+ waiters map[int]*condition
+}
+
+// State encapsulates the server state TODO: explain more
+type State struct {
+ // diagnostics are a map of relative path->diagnostics params
+ diagnostics map[string]*protocol.PublishDiagnosticsParams
+ logs []*protocol.LogMessageParams
+}
+
+func (s State) String() string {
+ var b strings.Builder
+ b.WriteString("#### log messages (see RPC logs for full text):\n")
+ for _, msg := range s.logs {
+ summary := fmt.Sprintf("%v: %q", msg.Type, msg.Message)
+ if len(summary) > 60 {
+ summary = summary[:57] + "..."
+ }
+ // Some logs are quite long, and since they should be reproduced in the RPC
+ // logs on any failure we include here just a short summary.
+ fmt.Fprint(&b, "\t"+summary+"\n")
+ }
+ b.WriteString("\n")
+ b.WriteString("#### diagnostics:\n")
+ for name, params := range s.diagnostics {
+ fmt.Fprintf(&b, "\t%s (version %d):\n", name, int(params.Version))
+ for _, d := range params.Diagnostics {
+ fmt.Fprintf(&b, "\t\t(%d, %d): %s\n", int(d.Range.Start.Line), int(d.Range.Start.Character), d.Message)
+ }
+ }
+ return b.String()
}
-// A diagnosticCondition is satisfied when all expectations are simultaneously
+// A condition is satisfied when all expectations are simultaneously
// met. At that point, the 'met' channel is closed. On any failure, err is set
// and the failed channel is closed.
-type diagnosticCondition struct {
- expectations []DiagnosticExpectation
- met chan struct{}
+type condition struct {
+ expectations []Expectation
+ verdict chan Verdict
}
// NewEnv creates a new test environment using the given workspace and gopls
@@ -355,16 +382,19 @@ func NewEnv(ctx context.Context, t *testing.T, ws *fake.Workspace, ts servertest
t.Fatal(err)
}
env := &Env{
- T: t,
- Ctx: ctx,
- W: ws,
- E: editor,
- Server: ts,
- Conn: conn,
- lastDiagnostics: make(map[string]*protocol.PublishDiagnosticsParams),
- waiters: make(map[int]*diagnosticCondition),
+ T: t,
+ Ctx: ctx,
+ W: ws,
+ E: editor,
+ Server: ts,
+ Conn: conn,
+ state: State{
+ diagnostics: make(map[string]*protocol.PublishDiagnosticsParams),
+ },
+ waiters: make(map[int]*condition),
}
env.E.Client().OnDiagnostics(env.onDiagnostics)
+ env.E.Client().OnLogMessage(env.onLogMessage)
return env
}
@@ -373,62 +403,182 @@ func (e *Env) onDiagnostics(_ context.Context, d *protocol.PublishDiagnosticsPar
defer e.mu.Unlock()
pth := e.W.URIToPath(d.URI)
- e.lastDiagnostics[pth] = d
+ e.state.diagnostics[pth] = d
+ e.checkConditionsLocked()
+ return nil
+}
+func (e *Env) onLogMessage(_ context.Context, m *protocol.LogMessageParams) error {
+ e.mu.Lock()
+ defer e.mu.Unlock()
+ e.state.logs = append(e.state.logs, m)
+ e.checkConditionsLocked()
+ return nil
+}
+
+func (e *Env) checkConditionsLocked() {
for id, condition := range e.waiters {
- if meetsExpectations(e.lastDiagnostics, condition.expectations) {
+ if v, _ := checkExpectations(e.state, condition.expectations); v != Unmet {
delete(e.waiters, id)
- close(condition.met)
+ condition.verdict <- v
}
}
- return nil
}
-// ExpectDiagnostics asserts that the current diagnostics in the editor match
-// the given expectations. It is intended to be used together with Env.Await to
-// allow waiting on simpler diagnostic expectations (for example,
-// AnyDiagnosticsACurrenttVersion), followed by more detailed expectations
-// tested by ExpectDiagnostics.
+// ExpectNow asserts that the current state of the editor matches the given
+// expectations.
+//
+// It can be used together with Env.Await to allow waiting on
+// simple expectations, followed by more detailed expectations tested by
+// ExpectNow. For example:
//
-// For example:
// env.RegexpReplace("foo.go", "a", "x")
// env.Await(env.AnyDiagnosticAtCurrentVersion("foo.go"))
-// env.ExpectDiagnostics(env.DiagnosticAtRegexp("foo.go", "x"))
+// env.ExpectNow(env.DiagnosticAtRegexp("foo.go", "x"))
//
// This has the advantage of not timing out if the diagnostic received for
// "foo.go" does not match the expectation: instead it fails early.
-func (e *Env) ExpectDiagnostics(expectations ...DiagnosticExpectation) {
+func (e *Env) ExpectNow(expectations ...Expectation) {
e.T.Helper()
e.mu.Lock()
defer e.mu.Unlock()
- if !meetsExpectations(e.lastDiagnostics, expectations) {
- e.T.Fatalf("diagnostic are unmet:\n%s\nlast diagnostics:\n%s", summarizeExpectations(expectations), formatDiagnostics(e.lastDiagnostics))
+ if verdict, summary := checkExpectations(e.state, expectations); verdict != Met {
+ e.T.Fatalf("expectations unmet:\n%s\ncurrent state:\n%v", summary, e.state)
}
}
-func meetsExpectations(m map[string]*protocol.PublishDiagnosticsParams, expectations []DiagnosticExpectation) bool {
+// checkExpectations reports whether s meets all expectations.
+func checkExpectations(s State, expectations []Expectation) (Verdict, string) {
+ finalVerdict := Met
+ var summary strings.Builder
for _, e := range expectations {
- diags, ok := m[e.Path]
- if !ok {
- return false
+ v := e.Check(s)
+ if v > finalVerdict {
+ finalVerdict = v
+ }
+ summary.WriteString(fmt.Sprintf("\t%v: %s\n", v, e.Description()))
+ }
+ return finalVerdict, summary.String()
+}
+
+// An Expectation asserts that the state of the editor at a point in time
+// matches an expected condition. This is used for signaling in tests when
+// certain conditions in the editor are met.
+type Expectation interface {
+ // Check determines whether the state of the editor satisfies the
+ // expectation.
+ Check(State) Verdict
+ // Description is a human-readable description of the expectation.
+ Description() string
+}
+
+// A Verdict is the result of checking an expectation against the current
+// editor state.
+type Verdict int
+
+// Order matters for the following constants: verdicts are sorted in order of
+// decisiveness.
+const (
+ // Met indicates that an expectation is satisfied by the current state.
+ Met Verdict = iota
+ // Unmet indicates that an expectation is not currently met, but could be met
+ // in the future.
+ Unmet
+ // Unmeetable indicates that an expectation cannot be satisfied in the
+ // future.
+ Unmeetable
+)
+
+func (v Verdict) String() string {
+ switch v {
+ case Met:
+ return "Met"
+ case Unmet:
+ return "Unmet"
+ case Unmeetable:
+ return "Unmeetable"
+ }
+ return fmt.Sprintf("unrecognized verdict %d", v)
+}
+
+// LogExpectation is an expectation on the log messages received by the editor
+// from gopls.
+type LogExpectation struct {
+ check func([]*protocol.LogMessageParams) Verdict
+ description string
+}
+
+// Check implements the Expectation interface.
+func (e LogExpectation) Check(s State) Verdict {
+ return e.check(s.logs)
+}
+
+// Description implements the Expectation interface.
+func (e LogExpectation) Description() string {
+ return e.description
+}
+
+// NoErrorLogs asserts that the client has not received any log messages of
+// error severity.
+func NoErrorLogs() LogExpectation {
+ check := func(msgs []*protocol.LogMessageParams) Verdict {
+ for _, msg := range msgs {
+ if msg.Type == protocol.Error {
+ return Unmeetable
+ }
}
- if !e.IsMet(diags) {
- return false
+ return Met
+ }
+ return LogExpectation{
+ check: check,
+ description: "no errors have been logged",
+ }
+}
+
+// LogMatching asserts that the client has received a log message
+// matching of type typ matching the regexp re.
+func LogMatching(typ protocol.MessageType, re string) LogExpectation {
+ rec, err := regexp.Compile(re)
+ if err != nil {
+ panic(err)
+ }
+ check := func(msgs []*protocol.LogMessageParams) Verdict {
+ for _, msg := range msgs {
+ if msg.Type == typ && rec.Match([]byte(msg.Message)) {
+ return Met
+ }
}
+ return Unmet
+ }
+ return LogExpectation{
+ check: check,
+ description: fmt.Sprintf("log message matching %q", re),
}
- return true
}
// A DiagnosticExpectation is a condition that must be met by the current set
-// of diagnostics.
+// of diagnostics for a file.
type DiagnosticExpectation struct {
// IsMet determines whether the diagnostics for this file version satisfy our
// expectation.
- IsMet func(*protocol.PublishDiagnosticsParams) bool
+ isMet func(*protocol.PublishDiagnosticsParams) bool
// Description is a human-readable description of the diagnostic expectation.
- Description string
+ description string
// Path is the workspace-relative path to the file being asserted on.
- Path string
+ path string
+}
+
+// Check implements the Expectation interface.
+func (e DiagnosticExpectation) Check(s State) Verdict {
+ if diags, ok := s.diagnostics[e.path]; ok && e.isMet(diags) {
+ return Met
+ }
+ return Unmet
+}
+
+// Description implements the Expectation interface.
+func (e DiagnosticExpectation) Description() string {
+ return fmt.Sprintf("%s: %s", e.path, e.description)
}
// EmptyDiagnostics asserts that diagnostics are empty for the
@@ -438,9 +588,9 @@ func EmptyDiagnostics(name string) DiagnosticExpectation {
return len(diags.Diagnostics) == 0
}
return DiagnosticExpectation{
- IsMet: isMet,
- Description: "empty diagnostics",
- Path: name,
+ isMet: isMet,
+ description: "empty diagnostics",
+ path: name,
}
}
@@ -453,9 +603,9 @@ func (e *Env) AnyDiagnosticAtCurrentVersion(name string) DiagnosticExpectation {
return int(diags.Version) == version
}
return DiagnosticExpectation{
- IsMet: isMet,
- Description: fmt.Sprintf("any diagnostics at version %d", version),
- Path: name,
+ isMet: isMet,
+ description: fmt.Sprintf("any diagnostics at version %d", version),
+ path: name,
}
}
@@ -465,7 +615,7 @@ func (e *Env) AnyDiagnosticAtCurrentVersion(name string) DiagnosticExpectation {
func (e *Env) DiagnosticAtRegexp(name, re string) DiagnosticExpectation {
pos := e.RegexpSearch(name, re)
expectation := DiagnosticAt(name, pos.Line, pos.Column)
- expectation.Description += fmt.Sprintf(" (location of %q)", re)
+ expectation.description += fmt.Sprintf(" (location of %q)", re)
return expectation
}
@@ -481,65 +631,51 @@ func DiagnosticAt(name string, line, col int) DiagnosticExpectation {
return false
}
return DiagnosticExpectation{
- IsMet: isMet,
- Description: fmt.Sprintf("diagnostic at {line:%d, column:%d}", line, col),
- Path: name,
+ isMet: isMet,
+ description: fmt.Sprintf("diagnostic at {line:%d, column:%d}", line, col),
+ path: name,
}
}
-// Await waits for all diagnostic expectations to simultaneously be met. It
-// should only be called from the main test goroutine.
-func (e *Env) Await(expectations ...DiagnosticExpectation) {
- // NOTE: in the future this mechanism extend beyond just diagnostics, for
- // example by modifying IsMet to be a func(*Env) boo. However, that would
- // require careful checking of conditions around every state change, so for
- // now we just limit the scope to diagnostic conditions.
-
+// Await waits for all expectations to simultaneously be met. It should only be
+// called from the main test goroutine.
+func (e *Env) Await(expectations ...Expectation) {
e.T.Helper()
e.mu.Lock()
// Before adding the waiter, we check if the condition is currently met or
// failed to avoid a race where the condition was realized before Await was
// called.
- if meetsExpectations(e.lastDiagnostics, expectations) {
- e.mu.Unlock()
+ switch verdict, summary := checkExpectations(e.state, expectations); verdict {
+ case Met:
return
+ case Unmeetable:
+ e.mu.Unlock()
+ e.T.Fatalf("unmeetable expectations:\n%s\nstate:\n%v", summary, e.state)
}
- cond := &diagnosticCondition{
+ cond := &condition{
expectations: expectations,
- met: make(chan struct{}),
+ verdict: make(chan Verdict),
}
e.waiters[e.nextWaiterID] = cond
e.nextWaiterID++
e.mu.Unlock()
+ var err error
select {
case <-e.Ctx.Done():
+ err = e.Ctx.Err()
+ case v := <-cond.verdict:
+ if v != Met {
+ err = fmt.Errorf("condition has final verdict %v", v)
+ }
+ }
+
+ if err != nil {
// Debugging an unmet expectation can be tricky, so we put some effort into
// nicely formatting the failure.
- summary := summarizeExpectations(expectations)
e.mu.Lock()
- diagString := formatDiagnostics(e.lastDiagnostics)
- e.mu.Unlock()
- e.T.Fatalf("waiting on:\n\t%s\nerr: %v\ndiagnostics:\n%s", summary, e.Ctx.Err(), diagString)
- case <-cond.met:
+ defer e.mu.Unlock()
+ _, summary := checkExpectations(e.state, expectations)
+ e.T.Fatalf("waiting on:\n%s\nerr:%v\nstate:\n%v", err, summary, e.state)
}
}
-
-func summarizeExpectations(expectations []DiagnosticExpectation) string {
- var descs []string
- for _, e := range expectations {
- descs = append(descs, fmt.Sprintf("%s: %s", e.Path, e.Description))
- }
- return strings.Join(descs, "\n\t")
-}
-
-func formatDiagnostics(diags map[string]*protocol.PublishDiagnosticsParams) string {
- var b strings.Builder
- for name, params := range diags {
- b.WriteString(fmt.Sprintf("\t%s (version %d):\n", name, int(params.Version)))
- for _, d := range params.Diagnostics {
- b.WriteString(fmt.Sprintf("\t\t(%d, %d): %s\n", int(d.Range.Start.Line), int(d.Range.Start.Character), d.Message))
- }
- }
- return b.String()
-}
diff --git a/internal/lsp/regtest/reg_test.go b/internal/lsp/regtest/reg_test.go
index 9246ccb1b..e4bd9258f 100644
--- a/internal/lsp/regtest/reg_test.go
+++ b/internal/lsp/regtest/reg_test.go
@@ -18,8 +18,11 @@ import (
)
var (
- 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 -gopls_subprocess_testmode flag")
+ 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 -gopls_subprocess_testmode flag")
+ alwaysPrintLogs = flag.Bool("regtest_print_rpc_logs", false, "whether to always print RPC logs")
+ regtestTimeout = flag.Duration("regtest_timeout", 60*time.Second, "default timeout for each regtest")
+ printGoroutinesOnFailure = flag.Bool("regtest_print_goroutines", false, "whether to print goroutine info on failure")
)
var runner *Runner
@@ -33,7 +36,12 @@ func TestMain(m *testing.M) {
resetExitFuncs := lsprpc.OverrideExitFuncsForTest()
defer resetExitFuncs()
- const testTimeout = 60 * time.Second
+ runner = &Runner{
+ DefaultModes: NormalModes,
+ Timeout: *regtestTimeout,
+ AlwaysPrintLogs: *alwaysPrintLogs,
+ PrintGoroutinesOnFailure: *printGoroutinesOnFailure,
+ }
if *runSubprocessTests {
goplsPath := *goplsBinaryPath
if goplsPath == "" {
@@ -43,10 +51,10 @@ func TestMain(m *testing.M) {
panic(fmt.Sprintf("finding test binary path: %v", err))
}
}
- runner = NewTestRunner(NormalModes|SeparateProcess, testTimeout, goplsPath)
- } else {
- runner = NewTestRunner(NormalModes, testTimeout, "")
+ runner.DefaultModes = NormalModes | SeparateProcess
+ runner.GoplsPath = goplsPath
}
+
code := m.Run()
runner.Close()
os.Exit(code)