Our editor interaction for running `go generate` was untested. Add
support for triggering generate from the fake editor, and a simple test.
To enable this, some helpers were added to list Workspace files and
check for file state changes, to avoid having to synthetically create
file events. This workaround is not ideal as it results in a leaky
abstraction: in other cases the regtest may assume that FileEvents are
triggered by workspace interactions (e.g. ws.WriteFile), but in this
case it cannot. Unfortunately the only real solution for this would be
to make file watching more realistic, by polling file state on an
interval or using an actual file watching library. Neither of those
options seemed worthwhile just to keep the fake.Editor API pristine.
A new debugging option is added, SkipCleanup, to allow inspecting
regtest working directories after a test with minimal code change.
Change-Id: I64dceeb21a4eb9eff2b6936e44f80f4bd24b82da
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230313
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This fixes a bunch of fmt.Errorf calls to use %w rather than %v when wrapping
an error with additional context.
Change-Id: I03088376fbf89aa537555e825e5d02544d813ed2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231477
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
As suggested on Slack, a better fix for golang/go#38467 would be to hide
suggested fixes on generated files. This way, the diagnostics are still
visible but files are not unintentionally modified.
Also, deleted the SuggestedFixes field on source.Diagnostic, since it's
entirely unused.
Change-Id: I10756471e0f913465b1cccd7f222eea0f4de77fe
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230999
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
In order to experiment with adding more progress reporting to gopls, add
a new experimental configuration for verbose work done reporting.
Also, pass configuration in InitializationOptions when initializing the
editor.
Change-Id: Id2336790719d9c0b6d663997e1aef672aec43d28
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229458
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
A few more updates to the regtest framework - add support for all
codeActions, not just organize imports. Also, return diagnostics from
env.Await to pass into code actions. Not sure if that's the correct way
to do it, so please let me know if there's a better way.
The test is for 1.14 only, since -modfile is only supported after 1.14.
Fixesgolang/go#38211
Change-Id: I5cdd35e2ec06e2b2487d1f3491197030008be9c9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226958
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
While trying to be pragmatic, I simply locked the fake.Editor while
calling textDocument/formatting. This did indeed manifest later on as a
deadlock, so instead we release the lock but fail if the buffer version
changes while awaiting the formatting call.
Change-Id: I0ca24a502f3118e76de306a016449bbe665e70e4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228230
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Configuration was not being set correctly, because the configuration
response was not honoring the configuration section order.
Change-Id: I8418535b45e6a24fd8f0605d58cb370e22664f17
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228257
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This pushes the handler construction out to the user, allowing flexability of
use, and is the final stage of the switch to the new handler API.
Change-Id: Id2e61813a817df0d6e4d20dd47ce8c92b0ae87db
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227024
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Handler is now a function type that mapps to what used to be the Deliver method.
The only handler that used other methods was Canceller, for now that still
exists as LegacyHooks. Once the handlers are fully cleaned up we should be able
to re-implement canceller as handler middleware.
Each connection is now only allowed one handler, and it is passed to the Run
method, but handlers are composable.
Change-Id: I370e0459df851bb9c9c2a679b99cff073b94489e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226479
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
One of the tricky things about asserting on conditions in regtests is
the asynchronous nature of LSP. For example, as the LSP client we cannot
be sure when we've received all diagnostics for a given file.
Currently, regtests are implemented by awaiting specific diagnostic
expectations. This means that if gopls generates diagnostics that do
not match those expectations, we can only time out the test.
Ideally, we would want to know that gopls is done generating all diagnostics
for the current file state. This is not possible without knowing the
status of diagnostics for. Barring this, we would want to know that
diagnostics are done for the current file version. Unfortunately, that
also is not possible, because a new version of file B can affect
diagnostics in file A.
So in lieu of this information, this CL exposes a few tools that can be
used to improve the experience of writing new regtests.
- A new expectation is added: AnyDiagnosticAtCurrentVersion, that is
satisfied if any diagnostics have been received for the current
buffer version.
- ExpectDiagnostics is added to Env, to help check whether the current
diagnostics matches expectations.
Updates golang/go#38113
Change-Id: I48d2c3db87c13ac3ab424d01d9444cbc285af9e1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226842
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
https://github.com/microsoft/vscode-go/issues/3076#issuecomment-605062933
inspired me to write a regression test for this case. Turns out we
weren't handling it correctly after all...
This change makes sure that we only rebuild the view once a new go.mod
file is saved, not just created. It also preserves the snapshot ID
number when the view is recreated so that diagnostic caching continues
to work as expected.
Change-Id: I63bd559c3bd33b91828171cd7ddb3d099c31cddb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226017
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Expressing regtests in terms of textual coordinates is hard to read: the
reader ends up counting lines and characters to understand the text edit
or assertion.
To address, this, add two new functions for fake.Editor: RegexpSearch
and RegexpReplace, as well as a symmetric RegexpSearch function for
workspace files and wrappers for regtext.Env.
This allows expressing edits as well as buffer locations in terms of
easily scannable regexps.
An alternative solution to this problem is to integrate markers ala
packagestest. I tried this, but it ended up being cumbersome to
implement and less usable than regexps, due to the static nature of
markers: after the buffer has been edited all markers must be
updated.
Updates golang/go#36879
Change-Id: Iad087cf0d529737034197beef7b729816a159c69
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224757
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Add two new fake editor commands: Formatting and OrganizeImports, which
delegate to textDocument/formatting and textDocument/codeAction
respectively. Use this in simple regtests, as well as on save.
Implementing this required fixing a broken assumption about text edits
in the editor: previously these edits were incrementally mutating the
buffer, but the correct implementation should simultaneously mutate the
buffer (i.e., all positions in an edit set refer to the starting buffer
state). This never mattered before because we were only operating on one
edit at a time.
Updates golang/go#36879
Change-Id: I6dec343c4e202288fa20c26df2fbafe9340a1bce
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221539
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
Helper functions on the regtest.Env wrapping workspace or editor
functionality are moved to a new wrappers.go file.
Also, rename 'WriteBuffer' to 'SaveBuffer', for less confusion with
'WriteFile'.
Change-Id: Ide9a5cf919dcee6e4a4fbfdb167eddf751a26eeb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221538
Reviewed-by: Rohan Challa <rohan@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Fix various things related to regtest execution:
+ Check the error from OpenFile in fake.Editor.GoToDefinition.
+ Add an error-checked wrapper to env for CloseBuffer.
+ Use env wrappers in TestDiagnosticClearingOnClose.
+ Use os.Executable to get the test binary path.
+ Add a -listen.timeout to the remote gopls process, so that it is
cleaned up.
Updates golang/go#36879
Change-Id: I056ff50bdb611a78ad78e4f5e2a94a4f155cc9de
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220902
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
I'm still not sure if we need to handle any other non-standard package
path apart from "command-line-arguments".
Also, a couple of staticcheck fixes.
Change-Id: I0bb3e60f6ffe104ff9027dbebb628020caaa1af4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220138
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Not all regtests resulted in LSP shutdown, which caused temp modfiles to
be leaked. After this fix I have confirmed that /tmp is clean after a
successful run of the regtests.
Also proactively clean up the unix socket file when serving jsonrpc2
over UDS.
Change-Id: I745fbd3d2adeeb165cadf7c54fd815d8df81d4e4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220061
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
A lot of bug reports originating from LSP clients are related to either
the timing or sequence of editor interactions with gopls (or at least
they're originally reported this way). For example: "when I open a
package and then create a new file, I lose diagnostics for existing
files". These conditions are often hard to reproduce, and to isolate as
either a gopls bug or a bug in the editor.
Right now we're relying on govim integration tests to catch these
regressions, but it's important to also have a testing framework that
can exercise this functionality in-process. As a starting point this CL
adds test fakes that implement a high level API for scripting editor
interactions. A fake workspace can be used to sandbox file operations; a
fake editor provides an interface for text editing operations; a fake
LSP client can be used to connect the fake editor to a gopls instance.
Some tests are added to the lsprpc package to demonstrate the API.
The primary goal of these fakes should be to simulate an client that
complies to the LSP spec. Put another way: if we have a bug report that
we can't reproduce with our regression tests, it should either be a bug
in our test fakes or a bug in the LSP client originating the report.
I did my best to comply with the spec in this implementation, but it
will certainly develop as we write more tests. We will also need to add
to the editor API in the future for testing more language features.
Updates golang/go#36879
Updates golang/go#34111
Change-Id: Ib81188683a7066184b8a254275ed5525191a2d68
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217598
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
