This change adds a test for this case (empty test file in GOPATH mode)
to go/packages. It is skipped for now, as the go/packages work-around is
too brittle to justify adding. Instead, we suppress gopls-generated
error messages in pop-ups--users should only see error messages if they
come from go/packages.
Fixesgolang/go#40825
Change-Id: I0e2af53578d65739ebae582075498d997fc019d7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/250949
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
packages.Load returns bad data for an empty xtest in GOPATH mode.
Updates golang/go#40825.
Change-Id: Ibab8dcbeafc22a71a2a4c3be0008b728c13c4c80
Reviewed-on: https://go-review.googlesource.com/c/tools/+/249703
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 244117 introduced a bug when modFile == os.DevNull: v.root is left
uninitialized, resulting in a view that appears to own all files. Fixing
that exposes a problem where opening a folder with no Go files and
GO111MODULE=on shows a popup. Skip the popup when no Go files are found.
Change-Id: I7f8b2d6fd2f954af64c3a65156ff44c649f3a5b2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248620
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Some users may intentionally be opening subdirectories to avoid having
gopls load the whole module. Allow this via a configuration.
Fixesgolang/go#40567
Change-Id: I6167f62a74a1c0b7cf07c1cb247adda839ee41f2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247617
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
When a package consists of files that only fail to parse, we fail to
associate parse errors with the package, and therefore return no
diagnostics. To address this, we need to associate the errors with the
package. This involves adding a *token.File to the parseGoData so that
error messages and positions can be properly extracted, even when there
is no associated AST.
Fixesgolang/go#39763
Change-Id: I5620821b9bcbeb499c498f9061dcf487d159bed5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243579
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
A bunch of options are added to enable long-running performance-oriented
tests in existing directories. They will be used in a later CL to
implement a simple stress test, as an example of what is possible.
Change-Id: I531b201b415362ea135978238b3d64b903226359
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244440
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This removes some unncessary mangling of strings, and allows the editor
to fully own its configuration.
Change-Id: I0fde206824964124182c9138ee06fb670461d486
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244360
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
CL 242579 changed the mechanism of reporting diagnostics so that we now
combine reports from multiple sources. Previously, we overwrote existing
diagnostics if new ones were reported. This was fine because
source.Diagnostics generated all of the diagnostics for Go files, and
mod.Diagnostics generated all of the diagnostics for go.mod files.
Now, we combine diagnostics from both sources -- mod.Diagnostics can
generate reports for Go files. We may get duplicate reports for packages
with test variants, so now, we check for those and dedupe.
Change-Id: I42e98079b4eead380058dd029a3a0c72a1796ebb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243778
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
We had previously been treating file changes with no content in the same
way as the deletion of content. Now, we distinguish between the two.
Fixesgolang/go#38424.
Change-Id: I44b338006af0c13a8a3f842844521789ea378470
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243577
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
getFile still returns a FileHandle, even if the file doesn't exist on
disk. Work-around this by checking if the file exists before adding
the file handle to the map.
Fixesgolang/go#38498
Change-Id: Ie02679068b37bf4f3d19966c6cfcf2361086b1de
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242924
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This test exposes a lot of fundamental issues with `go list` overlays.
Now that we have the regression test framework, we can copy it over and
make any flakes completely reproducible by waiting for each change to
complete.
The issue here ended up being that initial workspace load returns
workspace packages with no associated files due to golang/go#39986.
Working around this allows invalidation to proceed as usual.
In the process of debugging this, I also noticed that packages can get
loaded as command-line-arguments even when we can get the correct
package path for them. It's pretty complicated to fix this, so
restructured the code a tiny bit and left a TODO. I'd like to come back
to this at some point, but it's not pressing since I was able to fix
this bug.
Fixesgolang/go#39646.
Updates golang/go#39986.
Change-Id: Id6b08a5e92d28eddc731feb0ef3fd3b3fc69e64b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240098
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
This change permits starting gopls without a root URI or any workspace
folders. If no view is found for an opened file, we try to create a new
view based on the module root of that file. In GOPATH mode, we just
use the directory containing the file.
I wrote a regtest for this by adding a new configuration that gets
propagated to the sandbox. I'm not sure if this is the best way to do
that, so I'll let Rob advise.
Fixesgolang/go#34160
Change-Id: I3deca3ac1b86b69eba416891a1c28fd35658a2ed
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240099
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This CL got away from me a little.
For a number of reasons, the existing goimports API of passing in values
for various GO* values was not working. For one, the number of necessary
variables kept growing. For another, we tried to avoid calling `go env`
in GOPATH mode by using `build.Default`, but that turns out to be buggy;
see golang/go#39838. And finally, it created massive confusion about
whether the values were intended to be read from the OS environment, or
fully evaluated by the `go` command.
There are only two users of the internal imports API, so there really
shouldn't need to be more than two modes. For the command line tool, we
have to call `go env` to deal with the `go/build` bug. So we just do it.
Tests use that same path, but can augment the enviroment to set
themselves up. In contrast, `gopls` needs to fully specify the
environment. It can simply pass in the fully evaluated GO* values.
Finally, make the change I was actually here to make: propagate
GOMODCACHE and use it where appropriate.
Fixesgolang/go#39761.
Change-Id: I720c69839d91d66d98e94dfc5f065ba0279c5542
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239754
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
When a workspace package is deleted, we need to stop trying to load it.
Check that every workspace package still has at least one .go file when
we copy them between snapshots.
I think this is correct, but even if it's not, orphaned file loading
should patch it up.
Fixesgolang/go#38977.
Change-Id: I0b11010a40aac09f619f54b5ba02e2467b15a36c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238028
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Bad go/packages results were being cached, causing unexpected behavior
when new x tests are created. This fix also allows us to enable a
regression test that had been previously disabled.
Change-Id: I93c05df2f4d32c6c8a43e9f5aaeeb30bc4a32f3a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238058
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
The go command ignores go files in specially-named directories like
testdata. We probably still want to try to offer services like
autocomplete and jump-to-definition there, but diagnostics are less
likely to be helpful. As a compromise, just squash diagnostics.
Note that the rules we use are slightly wrong; see the comment on
ignoreFiles for details.
Fixesgolang/go#39563.
Change-Id: I0bc00639e68bd71ea55d867af36e07ef4ec780a1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237638
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This test partially reproduces some strange behavior with creating
new tests files. In particular, it creates a new x test in a package
that already has a test variant and adds content with a missing import.
In the test, the import is never added. However, in my own experience
debugging this in VS Code, I see the import get added but the diagnostic
never get removed. One thing at a time though...
Updates golang/go#39315
Change-Id: I724a145688b915d04abd1f21efc6f9a7506be043
Reviewed-on: https://go-review.googlesource.com/c/tools/+/235581
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
When a file with errors is removed outside the editor, sometimes its
errors are cleared by the editor and sometimes they are not. If the file
is still open in the editor gopls does not clear the errors, taking the
editor's version as the truth. Otherwise the errors are cleared.
(This behavior depends on the editor sending gopls a notification that
the workspace changed.)
There seems to be no good way yet to test that gopls takes no action after
receiving the didChangeWatchedFiles notification.
Updates golang/go#38878
Change-Id: Ie418dd786d4c5f827cf0665a31f0f9913f7cfdc0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/235377
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Now that we're not using build tags any more we can consolidate files.
Do so.
I tried a little to find good places for the moved code, but only a
little.
Change-Id: I6b66afb7cad870471d7d4a3d86c13fadb94a40e1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/235457
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
It's in the build tagged tests because the fix is only in 1.14.
Fixesgolang/go#36960
Change-Id: I606dfb4f73735c6db8c48f67f19720d340be8361
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232991
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Some code lenses may be undesirable for certain users or editors -- for
example a code lens that runs tests, when VSCode already supports this
functionality outside of the LSP. To handle such situations, support
configuring code lenses via a new 'codelens' gopls option.
Add support for code lens in regtests, and use this to test the new
configuration. To achieve this, thread through a new 'EditorConfig' type
that configures the fake editor's LSP session. It made sense to move the
test Env overlay onto this config object as well.
While looking at them, document some types in source.Options.
Change-Id: I961077422a273829c5cbd83c3b87fae29f77eeda
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232680
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We were previously sending diagnostics for nonexistent files, and then
adding them to the snapshot in the process. Remove this behavior, and
add a regression test. Case insensitive filesystems were too confusing
to write a test for, but fortunately, Filippo reported another instance
of this bug, so I used that for the regression test.
Fixesgolang/go#38602
Change-Id: I4ef6b51944f3338e838875a5aafffd066e8392f4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230315
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>
The Workspace type has accumulated too much additional functionality of
late: managing the Env, GOPATH, and GOPROXY in addition to the working
directory. Additionally, the name 'Workspace' can easily be confused
with 'workspaceFolder' in the LSP spec, and they're not quite
equivalent.
Split off a Proxy type to be responsible for the fake module proxy, and
a Workdir type to be responsible for working with the temporary
directory. Rename what remains of 'Workspace' to a more appropriate name
for such a collection of resources: Sandbox.
This is mostly just moving things around, with one significant change in
functionality: previously our three temporary directories (workdir,
gopath, and goproxy) were in separate toplevel directories below
$TMPDIR. Now they are all below a new sandbox temp directory, so that
they are correlated in the filesystem and can be cleaned up with one
call to os.RemoveAll.
Change-Id: I1e160a31ae22f0132355117df941fe65822900eb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230758
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
If the user's workspace is neither in GOPATH nor a module, and there
are errors in their code, send a message (with ShowMessage) only on
the first load, or when the configuration changes. The previous
behavior sent the message more frequently.
There is a regtest, and two new Expectations for when the fake
editor sees (or does not see) a ShowMessage notification.
Fixesgolang/go#37279
Change-Id: I076bd95105359b9310dcf97019e3559159271356
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230897
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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>
Add issue comments to tests for issues, move a couple constants closer
to their use.
Change-Id: I34fa2643195ae81e463a070952a1a1a4af2c6132
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230997
Reviewed-by: Robert Findley <rfindley@google.com>
A common problem when writing regtests is that if you have an error in
your expectations, you must wait until the regtest times out to see what
went wrong.
With the integration of additional progress reporting in the LSP server,
we know when diagnostic work should have been completed, and we should
be able to fail tests early once we know that our diagnostic
expectations will never be met.
This CL adds a new OnceMet Expectation, which combines two expecations:
the first is a precondition that must be met before checking the second.
The second is an arbitrary expectation, but is translated as follows:
once the precondition is met, the second condition is checked and any
Unmet verdicts are translated into Unmeetable.
Change-Id: Ie8c677229a347c624e2659a3ef9104304b175243
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229977
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
In order for regtests to wait until file diagnostics are complete,
instrument diagnostics with verbose WorkDone reporting. In order for
this to be granular enough for use, the modification source needed to be
threaded through to the didModifyFiles function (which is where the
diagnostic goroutine is spun off).
A new expectation is added: CompletedWork, to allow specifying that a
specific work item has been completed. The problem with using
NoOutstandingWork was that it required a continuous chain of work to
prevent the regtest from succeeding when the bug was present, meaning
that by the time we have sent the didChange notification successfully
the server must have started work on its behalf. This was inherently
racy, and too tricky to get right.
Additionally, a couple bugs are fixed:
- EmptyDiagnostics is corrected to account for the case where we have
received zero diagnostics for a given file.
- A deadlock is fixed in Await when expectations are immediately met.
Updates golang/go#36879Fixesgolang/go#32149
Change-Id: I49ee011860351eed96a3b4f6795804b57a10dc60
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229777
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We were using strings.Split on env vars, which did bad stuff when the
var contained an =, e.g. GOFLAGS=-tags=foo. Only split on the first =.
Irritatingly, this breaks only `go mod` commands, so almost nothing in
gopls failed, just organize imports and the `go.mod` code lens stuff.
Fixesgolang/go#38669
Change-Id: I8d28c806b77a8df92100af1fa4fbcca5edf97cff
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230560
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change also required the addition of a new run configuration -
WithEnv, which adds extra environment variables to the configuration.
Please let me know if this is the wrong approach.
Fixesgolang/go#37984
Change-Id: Ied2a53a443dc74c7ed723b91765eeddc1a7c1c00
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229128
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Not sure when this got fixed, but confirmed manually and via a
regression test.
Fixesgolang/go#36951
Change-Id: I7833a5d1119c34fd3fb45ea658e954eadbee750c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229129
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
I believe that other changes in gopls/v0.4.0 also fixed the issue that
this regression test was written for. Re-enable it, and change it to
assert that diagnostics are cleared.
Fixesgolang/go#37195
Change-Id: I4538186ad288d9c6f70cc450f948b62f3868941f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228723
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
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>
Certain regtests require referencing external data. To support this, add
the ability to use a file-based proxy populated with testdata.
To expose this configuration, augment the regtest runner with variadic
options. Also use this to replace the Runner.RunInMode function.
Add a simple regtest that uses this functionality.
Updates golang/go#36879
Change-Id: I7e6314430abcd127dbb7bca12574ef9935bf1f83
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228235
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
In https://golang.org/cl/225157, I removed the redundant T and Context
parameters from the regtest func signature in an effort to reduce
confusion. In hindsight, removing the testing.T did not reduce
confusion, because there is a T in the test signature anyway (and now
it's the wrong T!).
Change-Id: I2e84009e9cb33cd51055012cfef9fe4987e1b9bd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228229
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Paul Jolly <paul@myitcv.org.uk>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Gopls gets confused about the package if the package name is edited.
(See golang.org/issues/32149) This skipped test will succeed as long
as the bug is present.
Change-Id: Ic99ceda133f92f306baf94e2f8ad0381ed565814
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227760
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The code for `gofmt -s` directly modifies the AST, since the ASTs are
not long-lived. Some of this code made it into our analysis
implementations, causing very strange bugs to manifest. Added a
regression test for this specific case.
Fixesgolang/go#38267
Change-Id: I235620adcbf2bbc7027c6d83ff2c7fe74729062e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227299
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
A couple small improvements for regtests:
- module names fixed to include a '.'. This was fixed for diagnostic
tests already.
- For regtests that were written to exercise specifig github issues,
include the issue number in the test name.
Change-Id: I67da6d78dc166e854d2543cf63dac382202f9dbc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226844
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
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>
This change adds a RunGoCommand function to the workspace, which will
allow us to test how gopls responds when a certain go command is
executed. Add a test that checks behavior after "go mod init" runs.
Change-Id: I679249c1654b136d44187397b4196b8a10b5615e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226478
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
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>
If a package starts out as command-line-arguments, and then becomes
"valid" (i.e., gets a package declaration), we shouldn't continue to try
to diagnose "command-line-arguments". We should remove
"command-line-arguments" from workspace packages any time its metadata
is invalidated (assuming it may get added back if a file= query produces
it again).
Include the relevant regression test.
Fixesgolang/go#37978
Change-Id: I7fc51edeb58007b4b4a163336cbeb752a53da322
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225317
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>
In an effort to be idiomatic, I made the regtest func signature
func(context.Context, testing.T, *Env), despite the fact that Env
already has a Context and a T.
This just ended up causing more confusion, as it's not clear which
Context or T to use. Remove this and just use the fields on Env.
Change-Id: I54da1fdfe6ce17a67601b2af8640d4d2ea676e8c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225157
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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>
This change adds a test for diagnostics not disappearing when you create the first .go file for a project and add an import, then save.
Updates golang/go#37195
Change-Id: I871e77d9ab129f13be58931fe009e4297c7f2b38
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221222
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Originally I decided to use t.Parallel() in hopes of uncovering new
bugs. That may have worked... but manifested as rare flakes that are
difficult to diagnose (golang.org/issues/37318).
Since this level of parallelism is extremely unlikely in normal gopls
workloads, I'm going to remove the t.Parallel() calls in hopes of
eliminating this flakiness. I'd rather be able to continue running these
tests.
Also, don't run in the 'Shared' execution mode by default: normal gopls
execution is either as a sidecar (the Singleton execution mode), or as a
daemon (the Forwarded execution mode).
Un-skip the TestGoToStdlibDefinition test, as hopefully it will no
longer flake.
Updates golang/go#37318.
Change-Id: Id73ee3c8702ab4ab1d039baa038fbce879e38df8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221379
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
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>
Diagnostics should be cleared for files which are (1) deleted on disk
and not open in the editor, and (2) closed and only open in the editor.
Enable the corresponding regression test, and fix a few issues raised by
staticcheck.
Fixesgolang/go#37049
Change-Id: Iff736a7f6c3eaacda4237c2e4cf7926e9949dece
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220079
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
