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

Allow the execution and debug of sub-tests via VSCode UI #2536

Closed
devuo opened this issue Nov 17, 2022 · 8 comments
Closed

Allow the execution and debug of sub-tests via VSCode UI #2536

devuo opened this issue Nov 17, 2022 · 8 comments
Labels
FeatureRequest FrozenDueToAge go-test issues related to go test support (test output, test explorer, ...)
Milestone

Comments

@devuo
Copy link
Contributor

devuo commented Nov 17, 2022

Is your feature request related to a problem? Please describe.

See below for concrete details of the problem and request.

Describe the solution you'd like
The solution I'd like is for vscode-go to offer the same kind of UX that it currently provides at root test level, but also for sub-tests like JetBrains GoLand, that other languages and test runners in VSCode have.

Here's what it looks like in GoLand:

Screenshot 2022-11-17 at 16 36 26

JetBrains GoLand

And here's how the same file looks like in VSCode:

Screenshot 2022-11-17 at 16 38 18

There's no way via the UI to run a sub-test other than with "Go: Subtest At Cursor" command and no way whatsoever to run it in debug mode.

The request therefore is to add support in vscode-go for the same UI buttons that appear in root tests (specifically, "run test" and "debug test") to also appear for sub tests, like in GoLand.

Here's an example of the same exact thing in VSCode but provided by Jest Runner for Jest tests:

VSCode Jest Runner

@gopherbot gopherbot added this to the Untriaged milestone Nov 17, 2022
@suzmue suzmue modified the milestones: Untriaged, vscode-go/unplanned Nov 17, 2022
@suzmue suzmue added the go-test issues related to go test support (test output, test explorer, ...) label Nov 17, 2022
@devuo
Copy link
Contributor Author

devuo commented Nov 17, 2022

Created a PR with a working solution for this issue: #2537

Screenshot 2022-11-17 at 19 30 03

@hyangah
Copy link
Contributor

hyangah commented Nov 17, 2022

Test explorer (the beaker icon) provides access to the subtests and that works not only with the subtests with simple string names, but subtests with generated test names.

Wonder if there is a way to generate code lenses based on what the test explorer knows about.

@devuo
Copy link
Contributor Author

devuo commented Nov 18, 2022

The limitation of the test explorer is that it's only aware of the tests only after you run them:

Before the main test is executed:
Screenshot 2022-11-18 at 10 33 04

After the test is executed:
Screenshot 2022-11-18 at 10 33 26

My guess without even looking at code from the behavior I can see, is that it parses the tests results output here to generate the test tree we see there:
Screenshot 2022-11-18 at 10 35 03

Because as soon as I make any change to the test file, the sub tests disappear from the test explorer and need to re-run the entire root test for them to appear, which defeats the whole purpose, which is the ability to quickly modify tests and re-run/debug them without having to re-run the root test.

@devuo
Copy link
Contributor Author

devuo commented Nov 18, 2022

The solution I presented in #2537, while admittedly not bulletproof in anyway, follows exactly the same logic that the extension is already using for the subtest at cursor command:

vscode-go/src/goTest.ts

Lines 182 to 195 in 1da1ea2

const testFunction = currentTestFunctions[0];
const simpleRunRegex = /t.Run\("([^"]+)",/;
const runRegex = /t.Run\(/;
let lineText: string;
let runMatch: RegExpMatchArray | null | undefined;
let simpleMatch: RegExpMatchArray | null | undefined;
for (let i = editor.selection.start.line; i >= testFunction.range.start.line; i--) {
lineText = editor.document.lineAt(i).text;
simpleMatch = lineText.match(simpleRunRegex);
runMatch = lineText.match(runRegex);
if (simpleMatch || (runMatch && !simpleMatch)) {
break;
}

@devuo
Copy link
Contributor Author

devuo commented Nov 20, 2022

Meanwhile to solve the brittleness issues with regular expressions to find the tests and sub tests I implemented a small CLI tool that parses the AST of a go test source file and returns a JSON outline of all tests and subtests within: https://github.com/devuo/gotestoutline

@hyangah let me know if you would be happy with this approach instead of the regular expressions and I'll implement a PR with something based off the gotestoutline instead.

@hyangah
Copy link
Contributor

hyangah commented Nov 21, 2022

Thanks @devuo Eventually, the codelens logic belongs to gopls (see the existing test codelens code in gopls. The current version of extension doesn't use this logic yet, but we will eventually let gopls produces test code lenses. (You can test it by setting "gopls.ui.codelenses" "test" codelens).

Meanwhile, to get the PR 2537 going

  • Can you follow the instruction the gopherbot added in the PR description?
  • Can you make the codelens simply call the command corresponding to "Go: Subtest At Cursor"?

Still I see incorrect subtest arg generation - in case of nested subtests. But that is a bug in "Go: Subtest At Cursor", which can be addressed in a separate issue or as we migrate to gopls based codelens.

@gopherbot
Copy link
Collaborator

Change https://go.dev/cl/451755 mentions this issue: add codelens for sub tests

@devuo
Copy link
Contributor Author

devuo commented Nov 22, 2022

@hyangah made the requested changes (and a few more as I found some issues while manually testing). I'll wait for your input and OK in the implemented approach before investing time in writing/fixing new/existing tests.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FeatureRequest FrozenDueToAge go-test issues related to go test support (test output, test explorer, ...)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants