[vscode-go] Support check.v1 Test Suites
113 views
Skip to first unread message
Gerrit Bot (Gerrit)
May 10, 2021, 10:06:27 PM5/10/21
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Gerrit Bot has uploaded this change for review.
Support check.v1 Test Suites
Fixes https://github.com/golang/vscode-go/issues/111
This PR roughly follows the code snippet outlined here: https://github.com/microsoft/vscode-go/issues/1921#issuecomment-514892108 to ensure that codelens is available for not just stretchr/testify, but also gopkg.in/check.v1.
The associated issue (https://github.com/golang/vscode-go/issues/111) and linked issues discuss support for other testing suites as well (https://github.com/golang/vscode-go/issues/228), however I did not add them as I am not familiar with them.
Taking that in to consideration, I'd love comments on:
- Is this a reasonable approach in general? It does change a few functions to by async, but the logical flow remains the same as all callers simply await as well.
- Should we spend the time to integrate a more flexible system for adding/updating testing packages? That would probably be outside the scope of this PR, but if that approach is preferred, that's pretty reasonable.
Also, this is my first contribution, and I believe I've followed the contributing guide, but please let me know if there is anything I can do to make this smoother! Thanks!
Change-Id: Idc8a9c74bf062d63c06237ebcc0b572ef99c50c2
GitHub-Last-Rev: e54fc9b85853ad6df201ae8e9b6e45e9c597fce7
GitHub-Pull-Request: golang/vscode-go#1494
---
M src/goTest.ts
M src/testUtils.ts
M test/integration/test.test.ts
3 files changed, 91 insertions(+), 42 deletions(-)
diff --git a/src/goTest.ts b/src/goTest.ts
index e25c9d7..52d6958 100644
--- a/src/goTest.ts
+++ b/src/goTest.ts
@@ -180,7 +180,7 @@
testFunctions: vscode.DocumentSymbol[],
goConfig: vscode.WorkspaceConfiguration
) {
- const args = getTestFunctionDebugArgs(editor.document, testFunctionName, testFunctions);
+ const args = await getTestFunctionDebugArgs(editor.document, testFunctionName, testFunctions);
const tags = getTestTags(goConfig);
const buildFlags = tags ? ['-tags', tags] : [];
const flagsFromConfig = getTestFlags(goConfig);
diff --git a/src/testUtils.ts b/src/testUtils.ts
index 77e812d..de9cfda 100644
--- a/src/testUtils.ts
+++ b/src/testUtils.ts
@@ -41,6 +41,10 @@
const testMethodRegex = /^\(([^)]+)\)\.(Test\P{Ll}.*)$/u;
const benchmarkRegex = /^Benchmark\P{Ll}.*/u;
+const checkPkg = '"gopkg.in/check.v1"';
+const testifyPkg = '"github.com/stretchr/testify/suite"';
+const allExternalPackages = [checkPkg, testifyPkg];
+
/**
* Input to goTest.
*/
@@ -144,13 +148,19 @@
return;
}
const children = symbol.children;
- const testify = children.some(
- (sym) => sym.kind === vscode.SymbolKind.Namespace && sym.name === '"github.com/stretchr/testify/suite"'
- );
+
+ // include test functions and methods from 3rd party testing packages
+ let containsExternal = false;
+ allExternalPackages.forEach((pkg) => {
+ const ext = children.some((sym) => sym.kind === vscode.SymbolKind.Namespace && sym.name === pkg);
+ if (ext) {
+ containsExternal = ext;
+ }
+ });
return children.filter(
(sym) =>
sym.kind === vscode.SymbolKind.Function &&
- (testFuncRegex.test(sym.name) || (testify && testMethodRegex.test(sym.name)))
+ (testFuncRegex.test(sym.name) || (containsExternal && testMethodRegex.test(sym.name)))
);
}
@@ -174,16 +184,20 @@
* @param testFunctionName The test function to get the debug args
* @param testFunctions The test functions found in the document
*/
-export function getTestFunctionDebugArgs(
+export async function getTestFunctionDebugArgs(
document: vscode.TextDocument,
testFunctionName: string,
testFunctions: vscode.DocumentSymbol[]
-): string[] {
+): Promise<string[]> {
if (benchmarkRegex.test(testFunctionName)) {
return ['-test.bench', '^' + testFunctionName + '$', '-test.run', 'a^'];
}
const instanceMethod = extractInstanceTestName(testFunctionName);
if (instanceMethod) {
+ if (await containsThirdPartyTestPackages(document, null, [checkPkg])) {
+ return ['-check.f', `^${instanceMethod}$`];
+ }
+
const testFns = findAllTestSuiteRuns(document, testFunctions);
const testSuiteRuns = ['-test.run', `^${testFns.map((t) => t.name).join('|')}$`];
const testSuiteTests = ['-testify.m', `^${instanceMethod}$`];
@@ -281,7 +295,7 @@
const { targets, pkgMap, currentGoWorkspace } = await getTestTargetPackages(testconfig, outputChannel);
// generate full test args.
- const { args, outArgs, tmpCoverPath, addJSONFlag } = computeTestCommand(testconfig, targets);
+ const { args, outArgs, tmpCoverPath, addJSONFlag } = await computeTestCommand(testconfig, targets);
outputChannel.appendLine(['Running tool:', goRuntimePath, ...outArgs].join(' '));
outputChannel.appendLine('');
@@ -388,15 +402,15 @@
// computeTestCommand returns the test command argument list and extra info necessary
// to post process the test results.
// Exported for testing.
-export function computeTestCommand(
+export async function computeTestCommand(
testconfig: TestConfig,
targets: string[]
-): {
+): Promise<{
args: Array<string>; // test command args.
outArgs: Array<string>; // compact test command args to show to user.
tmpCoverPath?: string; // coverage file path if coverage info is necessary.
addJSONFlag: boolean; // true if we add extra -json flag for stream processing.
-} {
+}> {
const args: Array<string> = ['test'];
// user-specified flags
const argsFlagIdx = testconfig.flags?.indexOf('-args') ?? -1;
@@ -438,7 +452,7 @@
}
// all other test run/benchmark flags
- args.push(...targetArgs(testconfig));
+ args.push(...(await targetArgs(testconfig)));
const outArgs = args.slice(0); // command to show
@@ -561,24 +575,34 @@
*
* @param testconfig Configuration for the Go extension.
*/
-function targetArgs(testconfig: TestConfig): Array<string> {
+async function targetArgs(testconfig: TestConfig): Promise<Array<string>> {
let params: string[] = [];
if (testconfig.functions) {
if (testconfig.isBenchmark) {
params = ['-bench', util.format('^(%s)$', testconfig.functions.join('|'))];
} else {
- let testFunctions = testconfig.functions;
- let testifyMethods = testFunctions.filter((fn) => testMethodRegex.test(fn));
- if (testifyMethods.length > 0) {
- // filter out testify methods
- testFunctions = testFunctions.filter((fn) => !testMethodRegex.test(fn));
- testifyMethods = testifyMethods.map(extractInstanceTestName);
+ const editor = vscode.window.activeTextEditor;
+ if (!editor) {
+ vscode.window.showInformationMessage('No editor is active.');
+ return;
+ }
+ if (!editor.document.fileName.endsWith('_test.go')) {
+ vscode.window.showInformationMessage('No tests found. Current file is not a test file.');
+ return;
}
- // we might skip the '-run' param when running only testify methods, which will result
- // in running all the test methods, but one of them should call testify's `suite.Run(...)`
- // which will result in the correct thing to happen
+ let testFunctions = testconfig.functions;
+ let testMethods = testFunctions.filter((fn) => testMethodRegex.test(fn));
+ if (testMethods.length > 0) {
+ // filter out methods
+ testFunctions = testFunctions.filter((fn) => !testMethodRegex.test(fn));
+ testMethods = testMethods.map(extractInstanceTestName);
+ }
+
+ // we might skip the '-run' param when running only external test package methods, which will
+ // result in running all the test methods, but in the case of testify, one of them should call
+ // testify's `suite.Run(...)`, which will cause the correct thing to happen
if (testFunctions.length > 0) {
if (testFunctions.length === 1) {
params = params.concat(['-run', util.format('^%s$', testFunctions[0])]);
@@ -586,8 +610,12 @@
params = params.concat(['-run', util.format('^(%s)$', testFunctions.join('|'))]);
}
}
- if (testifyMethods.length > 0) {
- params = params.concat(['-testify.m', util.format('^(%s)$', testifyMethods.join('|'))]);
+ if (testMethods.length > 0) {
+ if (await containsThirdPartyTestPackages(editor.document, null, [checkPkg])) {
+ params = params.concat(['-check.f', util.format('^(%s)$', testMethods.join('|'))]);
+ } else if (await containsThirdPartyTestPackages(editor.document, null, [testifyPkg])) {
+ params = params.concat(['-testify.m', util.format('^(%s)$', testMethods.join('|'))]);
+ }
}
}
return params;
@@ -605,3 +633,24 @@
flags.splice(index, 2);
}
}
+
+export async function containsThirdPartyTestPackages(
+ doc: vscode.TextDocument,
+ token: vscode.CancellationToken,
+ pkgs: string[]
+): Promise<boolean> {
+ const documentSymbolProvider = new GoDocumentSymbolProvider(true);
+ const allPackages = await documentSymbolProvider
+ .provideDocumentSymbols(doc, token)
+ .then((symbols) => symbols[0].children)
+ .then((symbols) => {
+ return symbols.filter(
+ (sym) =>
+ sym.kind === vscode.SymbolKind.Namespace &&
+ pkgs.some((pkg) => {
+ return sym.name === pkg;
+ })
+ );
+ });
+ return allPackages.length > 0;
+}
diff --git a/test/integration/test.test.ts b/test/integration/test.test.ts
index 0896ea0..9f42873 100644
--- a/test/integration/test.test.ts
+++ b/test/integration/test.test.ts
@@ -17,14 +17,14 @@
import vscode = require('vscode');
suite('Test Go Test Args', () => {
- function runTest(param: {
+ async function runTest(param: {
expectedArgs: string;
expectedOutArgs: string;
flags?: string[];
functions?: string[];
isBenchmark?: boolean;
}) {
- const { args, outArgs } = computeTestCommand(
+ const { args, outArgs } = await computeTestCommand(
{
dir: '',
goConfig: getGoConfig(),
@@ -40,57 +40,57 @@
assert.strictEqual(outArgs.join(' '), param.expectedOutArgs, 'displayed command');
}
- test('default config', () => {
- runTest({
+ test('default config', async () => {
+ await runTest({
expectedArgs: 'test -timeout 30s ./...',
expectedOutArgs: 'test -timeout 30s ./...'
});
});
- test('user flag [-v] enables -json flag', () => {
- runTest({
+ test('user flag [-v] enables -json flag', async () => {
+ await runTest({
expectedArgs: 'test -timeout 30s -json ./... -v',
expectedOutArgs: 'test -timeout 30s ./... -v',
flags: ['-v']
});
});
- test('user flag [-json -v] prevents -json flag addition', () => {
- runTest({
+ test('user flag [-json -v] prevents -json flag addition', async () => {
+ await runTest({
expectedArgs: 'test -timeout 30s ./... -json -v',
expectedOutArgs: 'test -timeout 30s ./... -json -v',
flags: ['-json', '-v']
});
});
- test('user flag [-args] does not crash', () => {
- runTest({
+ test('user flag [-args] does not crash', async () => {
+ await runTest({
expectedArgs: 'test -timeout 30s ./... -args',
expectedOutArgs: 'test -timeout 30s ./... -args',
flags: ['-args']
});
});
- test('user flag [-args -v] does not enable -json flag', () => {
- runTest({
+ test('user flag [-args -v] does not enable -json flag', async () => {
+ await runTest({
expectedArgs: 'test -timeout 30s ./... -args -v',
expectedOutArgs: 'test -timeout 30s ./... -args -v',
flags: ['-args', '-v']
});
});
- test('specifying functions adds -run flags', () => {
- runTest({
+ test('specifying functions adds -run flags', async () => {
+ await runTest({
expectedArgs: 'test -timeout 30s -run ^(TestA|TestB)$ ./...',
expectedOutArgs: 'test -timeout 30s -run ^(TestA|TestB)$ ./...',
functions: ['TestA', 'TestB']
});
});
- test('functions & benchmark adds -bench flags and skips timeout', () => {
- runTest({
+ test('functions & benchmark adds -bench flags and skips timeout', async () => {
+ await runTest({
expectedArgs: 'test -benchmem -run=^$ -bench ^(TestA|TestB)$ ./...',
expectedOutArgs: 'test -benchmem -run=^$ -bench ^(TestA|TestB)$ ./...',
functions: ['TestA', 'TestB'],
isBenchmark: true
});
});
- test('user -run flag is ignored when functions are provided', () => {
- runTest({
+ test('user -run flag is ignored when functions are provided', async () => {
+ await runTest({
expectedArgs: 'test -timeout 30s -run ^(TestA|TestB)$ ./...',
expectedOutArgs: 'test -timeout 30s -run ^(TestA|TestB)$ ./...',
functions: ['TestA', 'TestB'],
To view, visit change 318689. To unsubscribe, or for help writing mail filters, visit settings.
Hyang-Ah Hana Kim (Gerrit)
May 11, 2021, 4:33:35 PM5/11/21
to Gerrit Bot, goph...@pubsubhelper.golang.org, Rebecca Stambler, Hyang-Ah Hana Kim, Suzy Mueller, Go Bot, golang-co...@googlegroups.com
Attention is currently required from: Rebecca Stambler.
Patch set 1:Run-TryBot +1
7 comments:
Commit Message:
Patch Set #1, Line 7:
Support check.v1 Test Suitessrc/testUtils: support check.v1 test suites
(it's not officially established, but somehow we ended up prefixing the title with the most affected module path. Since it's submitted using GitHub PR, update the first PR message and the PR title - gopherbot will pick it up and make it as the commit message for the squashed commit)
Patch Set #1, Line 9:
Fixes https://github.com/golang/vscode-go/issues/111Place this at the end of the PR message (https://golang.org/doc/contribute#ref_issues)
Fixes golang/vscode-go#111
Patch Set #1, Line 18:
- Should we spend the time to integrate a more flexible system for adding/updating testing packages? That would probably be outside the scope of this PR, but if that approach is preferred, that's pretty reasonable.Before investing more this way, I think we need to think about how to extend/utilize gopls-based symbol info providers. Unlike go-outline, gopls can also provide the package, import info across files, so we can avoid issues like https://github.com/golang/vscode-go/issues/899 too.
Patchset:
Thanks and congrats for the first contribution!
File src/testUtils.ts:
Patch Set #1, Line 137:
export async function getTestFunctions(I understand that the testify PR didn't add any tests.
It would be nice if you can help adding tests - so when GoDocumentSymbolProvider is removed, we can avoid regression.test/integration/test.test.ts or codelens.test.ts may give some hints.
Running tests will add extra dependency, which isn't nice. So, it's sufficient to check whether these or the targetArg detects the right methods and returns the expected args for now.Patch Set #1, Line 455:
(await targetArgs(testconfig)if targetArgs fails to return Array, won't it cause an issue?
const editor = vscode.window.activeTextEditor;
if (!editor) {
vscode.window.showInformationMessage('No editor is active.');
return;
}
if (!editor.document.fileName.endsWith('_test.go')) {
vscode.window.showInformationMessage('No tests found. Current file is not a test file.');
return;
}Is it possible to avoid this editor check here? This seems too late to show the popup and I am not sure about the consequence of returning here without any result.
Instead of calling containsThirdPartyTestPackages here with the current editor, what do you think about having getTestFunctions return more info and letting TestConfig carry extra info (whether it's testify or check)? So, we can avoid running containsThirdPartyTestPackages here.
To view, visit change 318689. To unsubscribe, or for help writing mail filters, visit settings.
kokoro (Gerrit)
May 11, 2021, 4:44:35 PM5/11/21
to Gerrit Bot, goph...@pubsubhelper.golang.org, Rebecca Stambler, Hyang-Ah Hana Kim, Suzy Mueller, Go Bot, golang-co...@googlegroups.com
Attention is currently required from: Rebecca Stambler.
Kokoro presubmit build finished with status: FAILURE
Logs at: https://source.cloud.google.com/results/invocations/b94e7efa-d6e6-4a3d-a752-19d65a51011c
Patch set 1:TryBot-Result -1
Gopher Robot (Gerrit)
Mar 18, 2022, 10:25:27 AM3/18/22
to Gerrit Bot, goph...@pubsubhelper.golang.org, kokoro, Hyang-Ah Hana Kim, Suzy Mueller, golang-co...@googlegroups.com
Gopher Robot abandoned this change.
Abandoned
GitHub PR golang/vscode-go#1494 has been closed.
To view, visit change 318689. To unsubscribe, or for help writing mail filters, visit settings.
Reply all
Reply to author
Forward
0 new messages