extension: Test Suite Fails to Parse Subtests Correctly

[caioreix]

Describe the bug

When the code len of a subtest suite is called, it ends up causing an incorrect parse of the function, thus not properly generating the test command. It results in something like:
go test -timeout 30s -coverprofile=/tmp/vscode-xxx/go-code-cover -run ^\(\*testSuite\)\.TestSuite_Example$/^Success$ test

The expected output would be something like:
go test -timeout 30s -coverprofile=/tmp/vscode-xxx/go-code-cover -run ^TestSuite$ -m ^(TestSuite_Example$/^Success)$ test

This is happening due to escaping both the function name and the subtest name in this line:

vscode-go/extension/src/subTestUtils.ts

Lines 17 to 23 in 77a4fdb

	export function escapeSubTestName(testFuncName: string, subTestName: string): string {
	return `${testFuncName}/${subTestName}`
	.replace(/\s/g, '_')
	.split('/')
	.map((part) => escapeRegExp(part), '')
	.join('$/^');
	}

Escaping only the second part would already solve the problem:
https://github.com/caioreix/vscode-go/blob/6061299ff2b1526292a9e1aeff77bf77b3f88f67/extension/src/subTestUtils.ts#L17-L25

Steps to reproduce the behavior:

To reproduce it, you can use the snippet below and click on the codelens 'run test' corresponding to the Success or Fail subtest (I used testify to make visualization easier, but the behavior is the same for any test suite):

package main

import (
	"testing"

	"github.com/stretchr/testify/suite"
)

type testSuite struct {
	suite.Suite
}

func TestSuite(t *testing.T) {
	suite.Run(t, new(testSuite))
}

func (t *testSuite) TestSuite_Example() {
	t.Run("Success", func() {})

	t.Run("Fail", func() {})
}

[h9jiang]

Hi @caioreix

I can confirm the sub test "Success" can not be correctly triggered using the code lenses provided. It end up using

Running tool: go test -timeout 30s -run ^\(\*testSuite2\)\.TestSuite_Example$/^Success$ module1

I was able to use the code lenses to trigger test for TestSuite_Example.

Running tool: go test -timeout 30s -run ^TestSuite$ -testify.m ^(TestSuite_Example)$ module1

But I can not run test using the command you provided (it passes but it actually does not run any test)

go test -v -timeout 30s -run ^TestSuite$ -m ^(TestSuite_Example$/^Success)$ module1
=== RUN   TestSuite
    suite.go:234: warning: no tests to run
--- PASS: TestSuite (0.00s)
PASS

Am I constructing the command wrong?

In addition, testify test framework has best-effort support, the team doesn't have the resources to work on it but we'll accept cls.

[h9jiang]

cc @firelizzard18

[caioreix]

hello @h9jiang!

But I can not run test using the command you provided (it passes but it actually does not run any test)

No, the command itself is correct, but it still doesn’t work because testify doesn’t support subtests. The idea would be to first fix this incorrect interaction from vscode-go. After that, I’m thinking about opening a proposal for implementation in the testify repository, filtering this flag and ignoring the other subtests (I've already made a small POC of the implementation, but I haven't finished it yet.). I used this format ^(METHOD$/^TEST_NAME)$ just to follow the standard from go test, but it’s something we can discuss further to see if there are better options.

[firelizzard18]

it still doesn’t work because testify doesn’t support subtests

@caioreix Are you using an older version of testify? Newer versions do use subtests and don't require the -m/-testify.m flag. vscode-go's legacy test support does have support for that flag but I don't plan on adding it to the test explorer implementation (vscode-go's or Go Companion's). go test -v -run '^TestSuite$/^TestSuite_Example$/^Success$' will run that specific subtest - I tested locally with your example. Go's parsing of the -run flag changed in CL 343883 (released in 1.18?) but I don't think that's relevant since it's about alternations, e.g. (A|B).

That being said, I'm not sure that the legacy test support or either explorer implementation builds the flag correctly. Go Companion may because that was written after CL 343883.

[h9jiang]

Hi @firelizzard18

Thank you so much for your input here. I'm not familiar with testify framework but I'm glad to see that testify no longer require -m / -testify.m in newer versions. You are right, the -run '^TestSuite$/^TestSuite_Example$/^Success$' successfully triggers the subtest without any issues.

If this works, I don't think we really need to introduce -testify.m flag in vscode-go's testing framework or Go Companion test explorer to avoid over-complicate this. Upon checking the source of vscode-go, I find this place have special handling for testify. Also some other place where vscode-go can be aware of this is a testify test.

the legacy test support or either explorer implementation builds the flag correctly.

Current test support build testify test correctly but does not build testify subtest correctly. The testify test is build using the special handling testify.m mentioned above.

We can make some changes in current vscode-go's test to cleanup those testify related special handling and make this testify subtest work but I wonder if this is something worth doing. Consider the Go Companion is under-development and it works for testify subtest, it might not worth changing the existing vscode-go since it's going to be replaced by Go Companion.

Temporarily move it from v0.48.0 milestone to backlog.

[caioreix]

Since the testing part will be replaced by Go Companion, I don't see a problem leaving this implementation for when the migration happens. However, while testing with Go Companion, I noticed that tests written with testify are not recognized by the Test Explorer at the moment. Only functions in the format TestX(t *testing.T) are picked up. Methods like (t *testSuite) TestSuite_Example() as well as their subtests do not appear in the Lens.

[firelizzard18]

However, while testing with Go Companion, I noticed that tests written with testify are not recognized by the Test Explorer at the moment.

Correct; Go Companion eliminates JavaScript based test discovery in favor of using gopls, and that's a new feature (for gopls) which doesn't have support for frameworks like testify. That is something I'd like to add but my main focus for now is supporting table-driven tests.

That being said, they should appear once you execute the test. Granted, those test items are "dynamic" subtests and thus don't have a location (because go test doesn't tell me where the t.Run call happens) among other limitations.