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

Add a command to run test with custom value for -run flag #1734

Open
pckhoi opened this issue Aug 31, 2021 · 12 comments · May be fixed by #1745
Open

Add a command to run test with custom value for -run flag #1734

pckhoi opened this issue Aug 31, 2021 · 12 comments · May be fixed by #1745
Labels
FeatureRequest go-test issues related to go test support (test output, test explorer, ...) HelpWanted Issues that are not prioritized by the maintainers. Help is requested from community contributors.

Comments

@pckhoi
Copy link

pckhoi commented Aug 31, 2021

Is your feature request related to a problem? Please describe.
Until this extension can run 'subtest at cursor' reliably (#1602), an easier and more flexible way to allow people to run arbitrary subtests is to add a "Test -run" command that allows people to run tests with a custom value for -run flag. I think this command will still remain useful even when 'subtest at cursor' command can run reliably because you can write regexp to match and run multiple subtests at any level, not just one specific subtest under your cursor.

Describe the solution you'd like
Add a command that works like "Test Package" except it should show an input box that allows specifying a custom value for -run flag before running tests. Subsequent runs of this command should remember the previous value to make typing minimal.

@gopherbot gopherbot added this to the Untriaged milestone Aug 31, 2021
@findleyr findleyr modified the milestones: Untriaged, Backlog Aug 31, 2021
@findleyr findleyr added the HelpWanted Issues that are not prioritized by the maintainers. Help is requested from community contributors. label Aug 31, 2021
@findleyr
Copy link
Contributor

Thanks for the feature request. This makes sense to me, but is probably not going to be a high priority for the team right now (will discuss with others in our weekly triage meeting).

Added 'help wanted' as I think this would be relatively straightforward to contribute.

@n1lesh
Copy link
Contributor

n1lesh commented Aug 31, 2021

@findleyr Can I have a go at it? Though I would need some guidance to get started.

@findleyr
Copy link
Contributor

Sure, though I think @hyangah or @firelizzard18 might be better able to advise on how to get started.

@firelizzard18
Copy link
Contributor

firelizzard18 commented Aug 31, 2021

As I see it, today the main advantage of adding this command over running the equivalent on the command line is integration with go.testEnvVars, go.testFlags, etc, as @pckhoi pointed out #1602 (comment). Down the road, it may be possible to integrate such a command with the testing API, so that test items in the test explorer are updated.

@n1lesh The main problem will be deciding the current directory and/or packages to test. In a single workspace environment, you could set the CWD to the workspace root and test all packages (./...), but that could be problematic if the same function name is used in multiple packages. And it wouldn't work for multi-root workspaces. You could derive the CWD from the active editor, but if that's how you do it, the command should be "Go: Test current package with custom -run" or something like that. You could expose the command only as a context menu option on folders in the explorer (right click, "Go: Test -run"), but that limits its utility. You could open a quick pick or something, but I'm not sure how to make that work well for large projects.

For the actual implementation, I would change the type of the functions property of TestConfig to string[] | string and then update the function that builds the -run flag arguments: if testconfig.functions is a string, return ['-run', testconfig.functions]. Then, the command implementation (in src/goTest.ts probably) can be:

const goConfig = getGoConfig(packageDirUri);
const isMod = await isModSupported(packageDirUri);

const testConfig: TestConfig = {
	goConfig,
	dir: packageDirUri.fsPath,
	flags: getTestFlags(goConfig, args),
	functions: userSuppliedRunFlagArgument,
	isMod,
};
// Remember this config as the last executed test.
lastTestConfig = testConfig;
return goTest(testConfig);

EDIT: I fixed the link to TestConfig.functions.

@n1lesh
Copy link
Contributor

n1lesh commented Sep 1, 2021

@firelizzard18 I guess it's better to implement it at the current active package level for the reasons you mentioned.

Just one question, to run the custom -run flag at the package level, wouldn't path.dirname(editor.document.fileName) give the current active package path, as the same is used by testCurrentPackage as well? Is there any significance of packageDirUri which I am not understanding?

@firelizzard18
Copy link
Contributor

@n1lesh I meant packageDirUri as a placeholder for whatever logic you use to decide on a directory. path.dirname(vscode.window.activeTextEditor.document.fileName) will be the path of the directory of the currently active editor, which is what the run package tests codelens and Go: Test Package command do.

@n1lesh
Copy link
Contributor

n1lesh commented Sep 1, 2021

@firelizzard18 Got it and thanks for all your suggestions and feedback.

@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/347209 mentions this issue: src/goTest: Add command Test Current Package With Custom Run

@bep
Copy link

bep commented Nov 29, 2021

I have tested the patch in 347209 and while it may do what it says, I find it very limited.

A frequent thing for me is to rerun a test (or test case) until I have fixed it. The above command allows me to run a specific test case, but I constantly need to paste in the -run pattern on every iteration, which is not something that I'm prepared to do.

@n1lesh
Copy link
Contributor

n1lesh commented Nov 29, 2021

@bep I wouldn't know if we can keep the initial command cached for the user to re-use it, if that's what you're looking for. @firelizzard18 Would you be able to suggest something on this?

@bep
Copy link

bep commented Nov 29, 2021

@n1lesh yes, something like that ... I tested that and that works kind of naturally ... until you switch to another package. But I guess that's an OK compromise.

@hyangah
Copy link
Contributor

hyangah commented Nov 29, 2021

"Go: Test Previous" (and also Debug Previous) is the command for the purpose and it seems the current CL does it already.
A question that hold me back on this change is though what prevents users from configuring their own tasks.json as they want.

@hyangah hyangah added the go-test issues related to go test support (test output, test explorer, ...) label Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FeatureRequest go-test issues related to go test support (test output, test explorer, ...) HelpWanted Issues that are not prioritized by the maintainers. Help is requested from community contributors.
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

7 participants