Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/tools/internal/lsp/regtest: change behavior when awaiting diagnostics #38113

Closed
stamblerre opened this issue Mar 27, 2020 · 4 comments
Closed
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Testing An issue that has been verified to require only test changes, not just a test failure. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@stamblerre
Copy link
Contributor

Struggled to come up with a good issue description, so sorry if it's not clear.

As I've been writing more regtests, I've noticed that waiting on diagnostics can be tedious if you're not certain what the diagnostic should be - you end up having to wait 30 seconds for the tests to run just to confirm that you made a mistake in the column number or something like that. Is it reasonable for the regtests to see that a diagnostic came in for the file in question, but it was just not the expected one, and then exit early?

/cc @findleyr

@stamblerre stamblerre added this to the gopls/v0.4.0 milestone Mar 27, 2020
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Mar 27, 2020
@stamblerre stamblerre added Testing An issue that has been verified to require only test changes, not just a test failure. and removed Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Mar 27, 2020
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Mar 27, 2020
@findleyr findleyr self-assigned this Mar 27, 2020
@findleyr
Copy link
Contributor

I started working on something related to this in https://golang.org/cl/225158 earlier this week, but ran into strange problems with the non-standard request that I haven't figured out.

You can also change the timeout in reg_test.go -- that's what I do when writing a new test.

@gopherbot
Copy link

Change https://golang.org/cl/226842 mentions this issue: internal/lsp/regtest: add functions to make diagnostic assertions easier

@stamblerre stamblerre modified the milestones: gopls/v0.4.0, gopls/v0.5.0 Apr 2, 2020
gopherbot pushed a commit to golang/tools that referenced this issue Apr 2, 2020
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>
@stamblerre stamblerre modified the milestones: gopls/v0.4.0, gopls/v0.5.0 Apr 2, 2020
@findleyr
Copy link
Contributor

@stamblerre I think this was fixed (at least as well as it can be) by https://golang.org/cl/229977. Do you agree?

@stamblerre
Copy link
Contributor Author

Yep! Let's close.

@stamblerre stamblerre modified the milestones: gopls/v0.5.0, gopls/v0.4.2 Jun 18, 2020
@golang golang locked and limited conversation to collaborators Jun 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Testing An issue that has been verified to require only test changes, not just a test failure. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

3 participants