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 running specific test case of a test table #2445

Open
ofabricio opened this issue Sep 8, 2022 · 22 comments
Open

Allow running specific test case of a test table #2445

ofabricio opened this issue Sep 8, 2022 · 22 comments
Assignees
Labels
FeatureRequest Go Companion Issues relating to the Go Companion extension go-test issues related to go test support (test output, test explorer, ...) gopls gopls related issues

Comments

@ofabricio
Copy link

ofabricio commented Sep 8, 2022

Related issues

Issue Work to be done
This issue Statically detect simple table-driven tests
#1602 Support "run at cursor" for entries in a table
#1734 Let the user manually enter a test or subtest name to execute

I'd like to suggest a feature that allows running a specific test case in a test table.

When the cursor is in a test case line, a run button gutter could appear allowing you to run that specific case. Or a new option on top of the test function (see image below).

I have tests with a lot of cases and when I have to focus in one to debug it I have either to comment all the other cases or separate that one in a new test function.

image

@gopherbot gopherbot added this to the Untriaged milestone Sep 8, 2022
@adonovan
Copy link
Member

adonovan commented Sep 8, 2022

How would such a thing work? It seems like an impossible static analysis problem in all but the most trivial cases.

@ofabricio
Copy link
Author

Maybe name convention? Identify tt or testTable or testCases1 with a []struct{ ... }. Then scan through its items until a { ... } that contains the cursor position and create a new test just with that item or comment out the other items behind scenes?

Footnotes

  1. this one is used by the tdt (table driven test) snippet

@hyangah
Copy link
Contributor

hyangah commented Sep 29, 2022

Behind the codelens, the extension simply invokes go test or dlv test commands. Even with name convention around tt, testTable, testCases, I don't know how to wire go test and dlv test commands. There is a concept of subtest (testing.T.Run) that go test and dlv test recognizes, but it seems like you aren't using it in the code example.

If users use the subtest, the extension currently has subtest support through the test explorer view.
It does by running all tests and discover the list of dynamically found subtests.

cc @firelizzard18

@firelizzard18
Copy link
Contributor

firelizzard18 commented Sep 30, 2022

@ofabricio In your example, it is not possible to run a specific test case without changing tt. In my opinion it is not reasonable for the extension to modify your code (e.g. commenting out the other test cases) or to create a copy of your test with only one case. Not to mention that would have to interpret your code and decide what part of it constituted a test case. If you instead define subtests by making calls to (*testing.T).Run(name string, fn func(*testing.T)), determining what block of code is a subtest is straight forward: the code inside the callback is the subtest. For example:

func TestExample(t *testing.T) {
    tt := []struct{
        give int
        then int
    }{
        {give: 1, then: 3},
        {give: 2, then: 4},
        {give: 3, then: 5},
    }

    for i, tc := range tt {
        t.Run(fmt.Sprintf("Case %d", i+1), func(t *testing.T) {
            v := tc.give + 2
            assertEqual(t, v, tc.then, tc)
        })
    }
}

If you know the name of the sub test you want to run, passing the right arguments to go test is relatively simple. As @hyangah said the test explorer uses the output of go test to dynamically discover subtests. Discovered subtests are listed in the test explorer and can be individually run from there. However, statically discovering what values should be considered a test case is non trivial in any but the simplest situations.

Statically discovering test cases is a complex problem. Finding a static subtest such as t.Run("name", subtest) within TestXxx(t *testing.T) is somewhat feasible, but even then the call might be within a conditional or loop. Any more complex static analysis is not feasible to do on the extension side unless the team decides they want the extension to include a Go test static analyzer written in JavaScript (and someone volunteers to write it).

If we move test discovery out of the extension into gopls, it is more feasible to statically detect subtests. Though that will still be limited in scope as the halting problem makes it (literally) impossible to build a static analyzer that is guaranteed to run in finite time and find every subtest for an arbitrary input. @hyangah is moving test discovery to gopls an option?

@adonovan
Copy link
Member

A simpler implementation would be to comment out all of the table except the line you want to execute. :)

@ofabricio
Copy link
Author

I didn't know the subtests appear in that view, as I don't use subtests often. I made a few tests with it and, although not so convenient, it kinda meet my needs tbh.

So I don't think there is a need to implement my suggestion unless you think it worth it -- it still looks like a cool, handy feature :p

What if instead of running a case, we add a "Extract test cases" option similar to "Extract function"? It would create a new test with just the selected cases. That would serve two purposes: I could debug one case separately by isolating it (as I currently manually do); or split a big test table (currently I have a test table with 600+ cases)

@firelizzard18
Copy link
Contributor

Test manipulation utilities like "Extract test cases" sounds like a candidate for a separate extension. You might be able to use an approach similar to how I implemented a Go test explorer for the Test Explorer UI: https://gitlab.com/firelizzard/vscode-go-test-adapter/-/blob/5fe67440e8dfa2be14a840e084df165d59eb3c4e/src/model.ts#L111.

@tkgalk
Copy link

tkgalk commented Jan 5, 2023

How would such a thing work? It seems like an impossible static analysis problem in all but the most trivial cases.

I wouldn't say it's impossible, given that GoLand team has done it fairly reliably. Sure, we might not have the manpower, resources and the backend tooling - but it is doable.

@piavgh
Copy link

piavgh commented Sep 25, 2023

Is there any update on this issue?
This is definitely a great to have feature

@firelizzard18
Copy link
Contributor

Realistically this requires static analysis, and the only reasonable way to do that is in Go (an analyzer for Go written in JavaScript is not reasonable), so it needs to be part of a separate service. gopls is the obvious choice for that and golang/go#59445 is a prerequisite.

@tkgalk I'd believe GoLand can statically locate subtests (as in t.Run(name, callback)) and that is something I'd be interested in contributing once golang/go#59445 is done.

Or are you saying GoLand knows how to execute test cases when they're defined without t.Run as in the example at the top? I find that harder to believe as that would require modifying the code before executing it, which does not seem feasible to me from a tooling perspective. That would get messy fast.

@tkgalk
Copy link

tkgalk commented Sep 28, 2023

image

GoLand can deal with table tests without individual t.Runs, yes. I'm not sure how it goes about it (they are run in a t.Run later, in a loop), but it can run a specific case from a table.

https://www.jetbrains.com/go/guide/tips/run-test-in-table-test/ - here's more information about this. I think they are still relying on go test -v run <name> and are using some regex/treesitter-like thing to grab the test names from "name" field, because they have a bunch of rules on those test names for the IDE to catch them.

@firelizzard18
Copy link
Contributor

they are run in a t.Run later, in a loop

This is the key distinction. If you look at the screenshot in the issue description, there is no call to t.Run. In that scenario it is not possible to select a single subtest without modifying the source.

vscode-go already supports running subtests, and as you guessed it uses the -run flag. The issue is that it is based on dynamic discovery. When you run a test, the test adapter scans the output for test names and tries to identify subtests. The heuristics are a big messy but it works when the subtest aren't nested. There are bugs with nested subtests but I haven't had the time and energy to hunt them down. A complicating factor is that the subtests are deleted as soon as the file is modified; originally that wasn't the case but it quickly got confusing when old subtests didn't get removed.

I would like to work on this myself, but golang/go#59445 is a prerequisite at least for me since I have absolutely no interest in attempting that analysis in JavaScript. And as per golang/go#59445 (comment) it is probably not feasible for me as a contributor to handle the migration of test discovery to gopls, so I'm waiting for that task to be completed.

@hyangah hyangah added the go-test issues related to go test support (test output, test explorer, ...) label Dec 5, 2023
@jonnylangefeld
Copy link

In case this helps anyone, this is my current acceptable workflow while this is not a first-class feature:
Run either all tests or test at cursor once via

  • ⌘ Command; + A or type > Tests: Ran All Tests in the command palette
  • ⌘ Command; + C or type > Tests: Ran Test at Cursor in the command palette
    This doesn't debug just yet, but it opens the TEST RESULTS tab with the history on the right. In the history you now have buttons to run or debug for each individual table test entry.

Screenshot 2024-04-20 at 2 00 16 PM

And voilà, now you can debug sub-table-tests via button click.

@firelizzard18
Copy link
Contributor

this is my current acceptable workflow while this is not a first-class feature

This is the same workaround that I use. But I want to set expectations - subtests will never be fully first-class. I want to improve the experience of dynamically discovered subtests and I want to add static subtest discover for common patterns but the halting problem means it's impossible to write a general algorithm that is 100% accurate for every possible case.

@safareli
Copy link

something that's accurate for 80% case would be a win

@firelizzard18
Copy link
Contributor

We can't possibly guarantee 80% or any percent. The best we can do is support a list of common, simple patterns. Without running a survey of the ecosystem (which I personally don't have time for) there's no way to know which patterns are used most. And complex patterns are somewhere between difficult and impossible to support. This should be easy to detect:

func TestFoo(t *testing.T) {
	cases := []struct{
		Name string
		Data any
	}{
		{Name: "foo", Data: 1},
		{Name: "bar", Data: 2},
	}
	for _, c := range cases {
		t.Run(c.Name, func(t *testing.T) { /* ... */ })
	}
}

On the other hand, this is impossible to statically analyze:

func TestBar(t *testing.T) {
	r, err := http.Get("https://example.com/api/user/1/posts")
	if err != nil {
		panic(err)
	}

	b, err := io.ReadAll(r.Body)
	if err != nil {
		panic(err)
	}

	var posts []struct {
		Title string
		Data  any
	}
	err = json.Unmarshal(b, &posts)
	if err != nil {
		panic(err)
	}

	for _, c := range posts {
		t.Run(c.Title, func(t *testing.T) { /* ... */ })
	}
}

@gopherbot
Copy link
Collaborator

Change https://go.dev/cl/601015 mentions this issue: gopls/internal/cache/tests: detect simple subtests

@gopherbot
Copy link
Collaborator

Change https://go.dev/cl/548675 mentions this issue: gopls/internal: test discovery

gopherbot pushed a commit to golang/tools that referenced this issue Sep 10, 2024
Implements test discovery. Tests are discovered as part of the type
checking process, at the same time as method sets and xrefs, and cached.
Does not implement the Modules command.

Adds static detection of simple subtests. This provides a framework for
static analysis of subtests but intentionally does not support more than
the most trivial case in order to minimize the complexity of this CL.

Fixes golang/go#59445. Updates golang/go#59445, golang/vscode-go#1602,
golang/vscode-go#2445.

Change-Id: Ief497977da09a1e07831e6c5f3b7d28d6874fd9f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/548675
Reviewed-by: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@firelizzard18 firelizzard18 added gopls gopls related issues Go Companion Issues relating to the Go Companion extension labels Oct 3, 2024
@firelizzard18
Copy link
Contributor

Moving forward, test discovery will be handled by gopls. gopls does not yet have the ability to detect table-driven subtests, but it can identify subtests with hard-coded names, e.g. t.Run("subtests", ...). If you wish to test this out, you can do so with the latest (unreleased) version of gopls or wait for gopls v0.17 along with the Go Companion extension.

@firelizzard18
Copy link
Contributor

This is somewhat relevant: microsoft/vscode#126932 (comment)

@gopherbot
Copy link
Collaborator

Change https://go.dev/cl/618216 mentions this issue: gopls/internal/cache/testfuncs: detect table-driven subtests

@firelizzard18 firelizzard18 moved this to In Progress in Testing UI Renewal Oct 7, 2024
@firelizzard18 firelizzard18 self-assigned this Oct 7, 2024
@hyangah
Copy link
Contributor

hyangah commented Nov 15, 2024

@findleyr @adonovan Can CL 618216 be included in gopls v0.17.0 milestone?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FeatureRequest Go Companion Issues relating to the Go Companion extension go-test issues related to go test support (test output, test explorer, ...) gopls gopls related issues
Projects
Status: In Progress
Development

No branches or pull requests

9 participants