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/gopls: better "run this test" UX #67400

Open
adonovan opened this issue May 15, 2024 · 5 comments
Open

x/tools/gopls: better "run this test" UX #67400

adonovan opened this issue May 15, 2024 · 5 comments
Labels
FeatureRequest gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@adonovan
Copy link
Member

adonovan commented May 15, 2024

Currently, gopls offers many ways to run the test under the cursor or selected tests:

VS Code is clearly going the path of custom client side logic, but for other editors, I think we can improve the experience.

The first challenge is to enable either the code lens or the code action, which is not trivial. Documentation may help. One possibility is to sniff the client type and enable the lens or action by default for all clients other than VS Code.

The second is the UX around the test output. Each client displays the streamed output and final showMessage in a different UI element: VS Code feeds it into a sticky dialog box; Vim accumulates the stream into a buffer which is displayed at the end. Emacs+eglot displays the output in the little echo area (bottom line), which is ephemeral and not suitable for large amounts of text.

Personally I use a custom Emacs Lisp function to fork+exec 'gopls codelens' through Emacs' compile package, so that its output is immediately streamed into a first-class editor buffer. I like this because it is immediate, real-time, editable, searchable, and durable, and because it displays the test logs even when the test passes, which is sometimes helpful. Also, Emacs treats the buffer just like a compiler output, so that locations become links, etc. The UX here is pretty close to ideal for me, but the mechanism is very clunky, and of course no-one but me benefits from it.

Yet another possibility would be to stream the output into a file and immediately open the file (using showDocument) in the client. I don't know how well different clients handle a file that is growing. (For example, Emacs has non-file buffers for things like shell sessions, which grow in real time, but for actual files, it only reloads the content periodically after polling.)

Another possibility is to stream the test output using gopls's new web server. I mention it for completeness, but it's a very loose integration and probably not a good idea.

I feel like the LSP ought to provide some way for a command to indicate to the client that its output is best handled like a terminal stream: large, continuously growing, durable, searchable. Perhaps we need to come up with a coherent plan for that and lobby our LSP overlords.

@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 May 15, 2024
@gopherbot gopherbot added this to the Unreleased milestone May 15, 2024
@hyangah hyangah modified the milestones: Unreleased, gopls/backlog May 20, 2024
@firelizzard18
Copy link
Contributor

@adonovan Do you think it would be appropriate for VSCode to utilize command.Test? That may require updates to gopls to enable various flags, and it would definitely need streaming support. Currently I'm using -json, -fullpath, -run, -bench, and -skip though once my work is merged into vscode-go, I plan on adding/migrating support for the go.testFlags and go.testTags configuration options. I split it into two functions - determining what to pass to -run, -bench, and -skip, and a second function to make the call to go. The latter essentially just adds -json and -fullpath and handles spawning the process and stdout/stderr piping, cancellation, etc.

I also have support for executing a test via the debugger, but that's a lot more complex and probably not suitable to move to gopls. Because of the way vscode works, to get the output from the debug session I have to use a "debug adapter tracker" and because dlv test doesn't support -json I have to manually spawn test2json and pipe data around.

@adonovan
Copy link
Member Author

adonovan commented Sep 9, 2024

Do you think it would be appropriate for VSCode to utilize command.Test?

In principle it's fine for VS Code to use LSP commands (which since https://go.dev/cl/597276 are officially undocumented) since both sides are maintained by us. But I thought you were working on a richer VS Code-only testing UX: does it still need the server to run the go test command?

It would be good to discuss any changes you need to make to the protocol before spending too much time implementing them.

@hyangah

@firelizzard18
Copy link
Contributor

But I thought you were working on a richer VS Code-only testing UX: does it still need the server to run the go test command?

It does not need the server to run go test, but presumably neither to other editors. Today it doesn't make much difference, though in general I tend towards preferring to handle go invocations via gopls. But if there are features that could be handled by command.Test, then having vscode-go use that would allow those features to be shared by other editors. As a concrete example, I have proposed adding a flag to test binaries to emit JSON events directly on a separate file descriptor; if that proposal was accepted then gopls could use that to sidestep the limitations of go tool test2json. The response to that proposal wasn't notably positive so that probably won't happen, but that's the kind of thing I'm imagining. My dream is for the testing internals to be exposed sufficiently that gopls could build and execute tests via some API (without invoking go test and without reimplementing the entire thing) but I doubt that will happen.

@hyangah
Copy link
Contributor

hyangah commented Sep 10, 2024

VSCode Go extension experimented with the gopls's test command some years ago - part of the legacy Test implementation in gopls was made during the experiment - but dropped the idea.

Lack of good output streaming is the main blocker. As @adonovan described, the current implementation combines and sends both stderr and stdout as progress report messages to simulate streaming. Progress message is hard to work with from vscode side. Later we encountered the exact same issue when adding govulncheck integration (RunGovulncheck) and worked around it by implementing a two stage protocol on top of the LSP executeCommand and the progress report. (stage 1: RunGovulncheck returns the progress report token, stage 2: client intercepts all the progress report messages with the token). This wasn't great, and I don't think other editors would benefit much from the custom protocol implemented in this way.

The current implementation also sends back the failure message using showMessage at the end, which is translated as a big prompt in vscode. That is not great either.

Some paths that I hope to explore eventually

    1. look into the BSP (BuildServerProtocol) - it defines stdout/stdin streaming - and implement relevant methods, or
    1. design a custom protocol, clearly document the protocol, and hope other editors to implement it,
    1. just use gopls to compute the test run arguments/flags/envs, then use the info when running/debugging tests from the extension.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FeatureRequest gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

4 participants