Closed
Description
This is a follow-up to #36879. We've been using the gopls regtests for a few months now, and several usability bugs / missing features / overall themes have emerged. None of these seemed worthy of an issue, but I want to track them in aggregate. I've been trying to let the regtests soak in their current form, but plan to take another pass soon.
Here's a list of what I've noted along the way, either from my own usage or feedback from others'. Feel free to comment with more, and I'll edit this comment to keep it up to date.
- Once @ianthehat's jsonrpc2 shutdown fixes are merged, we should t.Parallel the regtests once more.
- Add more documentation about how to develop new regtests (tricks like using
-regtest_timeout
to avoid waiting for the default regtest timeout). - Having the run options after the test body in
runner.Run
is hard to spot. Use a different options pattern to allow passing them before the test body. - Fix the 'NoOutstandingWork' expectation, by making sure that 'progress finished' messages never fail to be sent due to a cancelled context.
- Somehow fix regtest cleanup on Windows (which runs into problems with file locking from the
go
command). Perhaps awaiting 'NoOutstandingWork' could help here. - Move regtests into the
gopls
module, so that they can run staticcheck. - Once regtests are in the
gopls
module, replace the synthetic file watching with actual file watching, using fsnotify. The fact that file-watching is faked has been a source of bugs. The file watching should also respect the GlobPattern provided in the file watching capability registration. - Make the
DiagnosticAtRegexp
expectation lazy, so that it doesn't eagerly evaluate the regexp position (and therefore no longer needs theEnv
receiver). - Make
DiagnosticAtRegexp
match the full range of the regexp match, not just the starting position (this was a cause of some confusion in a CL). -
Return a copy of(the underlying requirement was instead achieved with less effort by adding a 'ReadDiagnostics' expectation that atomically reads from State).State
from Await: whatever caused expectations to be be met - Explicitly set
GOPACKAGESDRIVER=off
for all regtests. - Use a top-level
TMPDIR
for the entire regtest suite, rather than letting each regtest create it's own sandbox in/tmp
. This reduces cruft in/tmp
when tests are ctrl-c'ed before they can clean themselves up. - Add continuous integration to run the regtests against a true daemon, not an in-process daemon. This is not done currently because it will be problematic on Windows, and would increase the test duration.
- Fix handling of multi-byte runes in source text.
- Remove the need to count didChange/didOpen requests when writing expectations.
CC @pjweinbgo @stamblerre @heschik