[vscode-go] src/goTestExplorer: support running sub-tests
71 views
Skip to first unread message
Hyang-Ah Hana Kim (Gerrit)
Sep 3, 2021, 2:38:07 PM9/3/21
to Ethan Reesor, Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Rebecca Stambler, kokoro, Suzy Mueller, golang-co...@googlegroups.com
Hyang-Ah Hana Kim submitted this change.
Approvals:
Hyang-Ah Hana Kim: Looks good to me, approved; Trusted; Run TryBots
Rebecca Stambler: Trusted
kokoro: TryBots succeeded
src/goTest: support running sub-tests
Changes the logic for creating sub-tests on the fly. Prevously, test
IDs were created in such a way that it was impractical to support
running subtests. This changes the way IDs are created for subtests
such that running subtests is simple.
Additionally, this CL updates 'goTest' to run `go test -run=^$ -bench
^BenchmarkFoo/Bar$` (without the parentheses) when a single benchmark
is selected, as a hack to get around golang/go#47800.
Updates golang/vscode-go#1641
Change-Id: I26eac8a5a396df3923073274ed93d9c59107d9c3
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/343433
Reviewed-by: Hyang-Ah Hana Kim <hya...@gmail.com>
Trust: Hyang-Ah Hana Kim <hya...@gmail.com>
Trust: Rebecca Stambler <rsta...@golang.org>
Run-TryBot: Hyang-Ah Hana Kim <hya...@gmail.com>
TryBot-Result: kokoro <noreply...@google.com>
---
M .vscode/launch.json
M src/goTest/explore.ts
M src/goTest/resolve.ts
M src/goTest/run.ts
M src/goTest/utils.ts
M src/testUtils.ts
M test/integration/goTest.explore.test.ts
A test/integration/goTest.run.test.ts
M test/integration/goTest.utils.ts
M test/integration/index.ts
A test/mocks/MockContext.ts
A test/testdata/subTest/go.mod
A test/testdata/subTest/sub_test.go
13 files changed, 199 insertions(+), 67 deletions(-)
diff --git a/.vscode/launch.json b/.vscode/launch.json
index 24c9e32..ab3634d 100644
--- a/.vscode/launch.json
+++ b/.vscode/launch.json
@@ -61,7 +61,8 @@
"999999"
],
"env": {
- "VSCODE_GO_IN_TEST": "1" // Disable code that shouldn't be used in test
+ "VSCODE_GO_IN_TEST": "1", // Disable code that shouldn't be used in test
+ "MOCHA_TIMEOUT": "999999",
},
"stopOnEntry": false,
"sourceMaps": true,
diff --git a/src/goTest/explore.ts b/src/goTest/explore.ts
index abcc9cc..691140b 100644
--- a/src/goTest/explore.ts
+++ b/src/goTest/explore.ts
@@ -114,6 +114,7 @@
}
public readonly resolver: GoTestResolver;
+ public readonly runner: GoTestRunner;
constructor(
private readonly workspace: Workspace,
@@ -124,6 +125,7 @@
const runner = new GoTestRunner(workspace, ctrl, resolver);
this.resolver = resolver;
+ this.runner = runner;
ctrl.resolveHandler = async (item) => {
try {
diff --git a/src/goTest/resolve.ts b/src/goTest/resolve.ts
index 9cc393a..812d7fc 100644
--- a/src/goTest/resolve.ts
+++ b/src/goTest/resolve.ts
@@ -138,9 +138,9 @@
}
// Create or Retrieve a sub test or benchmark. The ID will be of the form:
- // file:///path/to/mod/file.go?test#TestXxx/A/B/C
- getOrCreateSubTest(item: TestItem, name: string, dynamic?: boolean): TestItem {
- const { kind, name: parentName } = GoTest.parseId(item.id);
+ // file:///path/to/mod/file.go?test#TestXxx%2fA%2fB%2fC
+ getOrCreateSubTest(item: TestItem, label: string, name: string, dynamic?: boolean): TestItem {
+ const { kind } = GoTest.parseId(item.id);
let existing: TestItem | undefined;
item.children.forEach((child) => {
@@ -149,7 +149,7 @@
if (existing) return existing;
item.canResolveChildren = true;
- const sub = this.createItem(name, item.uri, kind, `${parentName}/${name}`);
+ const sub = this.createItem(label, item.uri, kind, name);
item.children.add(sub);
if (dynamic) {
diff --git a/src/goTest/run.ts b/src/goTest/run.ts
index 10bfb2c..1bd6382 100644
--- a/src/goTest/run.ts
+++ b/src/goTest/run.ts
@@ -20,6 +20,7 @@
import { getTestFlags, goTest, GoTestOutput } from '../testUtils';
import { GoTestResolver } from './resolve';
import { dispose, forEachAsync, GoTest, Workspace } from './utils';
+import { outputChannel } from '../goStatus';
type CollectedTest = { item: TestItem; explicitlyIncluded?: boolean };
@@ -57,7 +58,7 @@
) {}
// Execute tests - TestController.runTest callback
- async run(request: TestRunRequest, token: CancellationToken) {
+ async run(request: TestRunRequest, token?: CancellationToken) {
const collected = new Map<TestItem, CollectedTest[]>();
const files = new Set<TestItem>();
if (request.include) {
@@ -96,7 +97,8 @@
}
const run = this.ctrl.createTestRun(request);
- const outputChannel = new TestRunOutput(run);
+ const testRunOutput = new TestRunOutput(run);
+ const subItems: string[] = [];
for (const [pkg, items] of collected.entries()) {
const isMod = isInMod(pkg) || (await isModSupported(pkg.uri, true));
const goConfig = getGoConfig(pkg.uri);
@@ -123,11 +125,7 @@
const benchmarks: Record<string, TestItem> = {};
for (const { item, explicitlyIncluded } of items) {
const { kind, name } = GoTest.parseId(item.id);
- if (/[/#]/.test(name)) {
- // running sub-tests is not currently supported
- vscode.window.showErrorMessage(`Cannot run ${name} - running sub-tests is not supported`);
- continue;
- }
+ if (/[/#]/.test(name)) subItems.push(name);
// When the user clicks the run button on a package, they expect all
// of the tests within that package to run - they probably don't
@@ -163,6 +161,19 @@
const benchmarkFns = Object.keys(benchmarks);
const concat = goConfig.get<boolean>('testExplorerConcatenateMessages');
+ // https://github.com/golang/go/issues/39904
+ if (subItems.length > 0 && testFns.length + benchmarkFns.length > 1) {
+ outputChannel.appendLine(
+ `The following tests in ${pkg.uri} failed to run, as go test will only run a sub-test or sub-benchmark if it is by itself:`
+ );
+ testFns.concat(benchmarkFns).forEach((x) => outputChannel.appendLine(x));
+ outputChannel.show();
+ vscode.window.showErrorMessage(
+ `Cannot run the selected tests in package ${pkg.label} - see the Go output panel for details`
+ );
+ continue;
+ }
+
// Run tests
if (testFns.length > 0) {
const complete = new Set<TestItem>();
@@ -170,14 +181,14 @@
goConfig,
flags,
isMod,
- outputChannel,
+ outputChannel: testRunOutput,
dir: pkg.uri.fsPath,
functions: testFns,
cancel: token,
goTestOutputConsumer: (e) => this.consumeGoTestEvent(run, tests, record, complete, concat, e)
});
if (!success) {
- if (this.isBuildFailure(outputChannel.lines)) {
+ if (this.isBuildFailure(testRunOutput.lines)) {
this.markComplete(tests, new Set(), (item) => {
run.errored(item, { message: 'Compilation failed' });
item.error = 'Compilation failed';
@@ -195,7 +206,7 @@
goConfig,
flags,
isMod,
- outputChannel,
+ outputChannel: testRunOutput,
dir: pkg.uri.fsPath,
functions: benchmarkFns,
isBenchmark: true,
@@ -206,7 +217,7 @@
// Explicitly complete any incomplete benchmarks (see test_events.md)
if (success) {
this.markComplete(benchmarks, complete, (x) => run.passed(x));
- } else if (this.isBuildFailure(outputChannel.lines)) {
+ } else if (this.isBuildFailure(testRunOutput.lines)) {
this.markComplete(benchmarks, new Set(), (item) => {
// TODO change to errored when that is added back
run.failed(item, { message: 'Compilation failed' });
@@ -275,16 +286,24 @@
return;
}
- const parts = name.split(/[#/]+/);
- let test = tests[parts[0]];
- if (!test) {
- return;
- }
+ const re = /[#/]+/;
- for (const part of parts.slice(1)) {
- test = this.resolver.getOrCreateSubTest(test, part, true);
- }
- return test;
+ const resolve = (parent?: TestItem, start = 0, length = 0): TestItem | undefined => {
+ const pos = start + length;
+ const m = name.substring(pos).match(re);
+ if (!m) {
+ if (!parent) return tests[name];
+ return this.resolver.getOrCreateSubTest(parent, name.substring(pos), name);
+ }
+
+ const subName = name.substring(0, pos + m.index);
+ const test = parent
+ ? this.resolver.getOrCreateSubTest(parent, name.substring(pos, pos + m.index), subName)
+ : tests[subName];
+ return resolve(test, pos + m.index, m[0].length);
+ };
+
+ return resolve();
}
// Process benchmark events (see test_events.md)
diff --git a/src/goTest/utils.ts b/src/goTest/utils.ts
index f6850a6..0f7a02c 100644
--- a/src/goTest/utils.ts
+++ b/src/goTest/utils.ts
@@ -36,7 +36,7 @@
// - Example: file:///path/to/mod/file.go?example#ExampleXxx
static id(uri: vscode.Uri, kind: GoTestKind, name?: string): string {
uri = uri.with({ query: kind });
- if (name) uri = uri.with({ fragment: name });
+ if (name) uri = uri.with({ fragment: encodeURIComponent(name) });
return uri.toString();
}
@@ -47,7 +47,7 @@
static parseId(id: string): { kind: GoTestKind; name?: string } {
const u = vscode.Uri.parse(id);
const kind = u.query as GoTestKind;
- const name = u.fragment;
+ const name = decodeURIComponent(u.fragment);
return { name, kind };
}
}
diff --git a/src/testUtils.ts b/src/testUtils.ts
index ef91f6d..3d4c1e7 100644
--- a/src/testUtils.ts
+++ b/src/testUtils.ts
@@ -587,7 +587,11 @@
if (testconfig.functions) {
if (testconfig.isBenchmark) {
- params = ['-bench', util.format('^(%s)$', testconfig.functions.join('|'))];
+ if (testconfig.functions.length === 1) {
+ params = ['-bench', util.format('^%s$', testconfig.functions[0])];
+ } else {
+ params = ['-bench', util.format('^(%s)$', testconfig.functions.join('|'))];
+ }
} else {
let testFunctions = testconfig.functions;
let testifyMethods = testFunctions.filter((fn) => testMethodRegex.test(fn));
diff --git a/test/integration/goTest.explore.test.ts b/test/integration/goTest.explore.test.ts
index 2222678..83fa612 100644
--- a/test/integration/goTest.explore.test.ts
+++ b/test/integration/goTest.explore.test.ts
@@ -5,11 +5,12 @@
import assert = require('assert');
import path = require('path');
import fs = require('fs-extra');
-import { TextDocument, TestItemCollection, TextDocumentChangeEvent, ExtensionContext, workspace, Uri } from 'vscode';
+import { TextDocument, TestItemCollection, TextDocumentChangeEvent, workspace, Uri } from 'vscode';
import { GoTestExplorer } from '../../src/goTest/explore';
import { getCurrentGoPath } from '../../src/util';
import { MockTestController, MockTestWorkspace } from '../mocks/MockTest';
-import { getSymbols_Regex, populateModulePathCache } from './goTest.utils';
+import { forceDidOpenTextDocument, getSymbols_Regex, populateModulePathCache } from './goTest.utils';
+import { MockExtensionContext } from '../mocks/MockContext';
type Files = Record<string, string | { contents: string; language: string }>;
@@ -179,49 +180,21 @@
});
suite('stretchr', () => {
- let gopath: string;
- let repoPath: string;
- let fixturePath: string;
- let fixtureSourcePath: string;
+ const fixtureDir = path.join(__dirname, '..', '..', '..', 'test', 'testdata', 'stretchrTestSuite');
+ const ctx = MockExtensionContext.new();
+
let document: TextDocument;
let testExplorer: GoTestExplorer;
- const ctx: Partial<ExtensionContext> = {
- subscriptions: []
- };
-
suiteSetup(async () => {
- gopath = getCurrentGoPath();
- if (!gopath) {
- assert.fail('Cannot run tests without a configured GOPATH');
- }
- console.log(`Using GOPATH: ${gopath}`);
+ testExplorer = GoTestExplorer.setup(ctx);
- // Set up the test fixtures.
- repoPath = path.join(gopath, 'src', 'test');
- fixturePath = path.join(repoPath, 'testfixture');
- fixtureSourcePath = path.join(__dirname, '..', '..', '..', 'test', 'testdata', 'stretchrTestSuite');
-
- await fs.remove(repoPath);
- await fs.copy(fixtureSourcePath, fixturePath, {
- recursive: true
- });
-
- testExplorer = GoTestExplorer.setup(ctx as ExtensionContext);
-
- const uri = Uri.file(path.join(fixturePath, 'suite_test.go'));
- document = await workspace.openTextDocument(uri);
-
- // Force didOpenTextDocument to fire. Without this, the test may run
- // before the event is handled.
- //
- // eslint-disable-next-line @typescript-eslint/no-explicit-any
- await (testExplorer as any).didOpenTextDocument(document);
+ const uri = Uri.file(path.join(fixtureDir, 'suite_test.go'));
+ document = await forceDidOpenTextDocument(workspace, testExplorer, uri);
});
suiteTeardown(() => {
- fs.removeSync(repoPath);
- ctx.subscriptions.forEach((x) => x.dispose());
+ ctx.teardown();
});
test('discovery', () => {
diff --git a/test/integration/goTest.run.test.ts b/test/integration/goTest.run.test.ts
new file mode 100644
index 0000000..1512f14
--- /dev/null
+++ b/test/integration/goTest.run.test.ts
@@ -0,0 +1,83 @@
+import assert = require('assert');
+import path = require('path');
+import sinon = require('sinon');
+import { Uri, workspace } from 'vscode';
+import * as testUtils from '../../src/testUtils';
+import { forceDidOpenTextDocument } from './goTest.utils';
+import { GoTestExplorer } from '../../src/goTest/explore';
+import { MockExtensionContext } from '../mocks/MockContext';
+import { GoTest } from '../../src/goTest/utils';
+
+suite('Go Test Runner', () => {
+ const sandbox = sinon.createSandbox();
+ const fixtureDir = path.join(__dirname, '..', '..', '..', 'test', 'testdata', 'subTest');
+
+ let testExplorer: GoTestExplorer;
+ let runSpy: sinon.SinonSpy<[testUtils.TestConfig], Promise<boolean>>;
+
+ setup(() => {
+ runSpy = sinon.spy(testUtils, 'goTest');
+ });
+
+ teardown(() => {
+ sandbox.restore();
+ });
+
+ suite('Subtest', () => {
+ const ctx = MockExtensionContext.new();
+ let uri: Uri;
+
+ suiteSetup(async () => {
+ testExplorer = GoTestExplorer.setup(ctx);
+
+ uri = Uri.file(path.join(fixtureDir, 'sub_test.go'));
+ await forceDidOpenTextDocument(workspace, testExplorer, uri);
+ });
+
+ suiteTeardown(() => {
+ ctx.teardown();
+ });
+
+ test('discover and run', async () => {
+ // Locate TestMain and TestOther
+ const tests = testExplorer.resolver.find(uri).filter((x) => GoTest.parseId(x.id).kind === 'test');
+ tests.sort((a, b) => a.label.localeCompare(b.label));
+ assert.deepStrictEqual(
+ tests.map((x) => x.label),
+ ['TestMain', 'TestOther']
+ );
+ const [tMain, tOther] = tests;
+
+ // Run TestMain
+ await testExplorer.runner.run({ include: [tMain] });
+ assert(runSpy.calledOnce, 'goTest was not called');
+
+ // Verify TestMain was run
+ let call = runSpy.lastCall.args[0];
+ assert.strictEqual(call.dir, fixtureDir);
+ assert.deepStrictEqual(call.functions, ['TestMain']);
+ runSpy.resetHistory();
+
+ // Locate subtest
+ const tSub = tMain.children.get(GoTest.id(uri, 'test', 'TestMain/Sub'));
+ assert(tSub, 'Subtest was not created');
+
+ // Run subtest by itself
+ await testExplorer.runner.run({ include: [tSub] });
+ assert(runSpy.calledOnce, 'goTest was not called');
+
+ // Verify TestMain/Sub was run
+ call = runSpy.lastCall.args[0];
+ assert.strictEqual(call.dir, fixtureDir);
+ assert.deepStrictEqual(call.functions, ['TestMain/Sub']);
+ runSpy.resetHistory();
+
+ // Ensure the subtest hasn't been disposed
+ assert(tSub.parent, 'Subtest was disposed');
+
+ // Attempt to run subtest and other test - should not work
+ await testExplorer.runner.run({ include: [tSub, tOther] });
+ assert(!runSpy.called, 'goTest was called');
+ });
+ });
+});
diff --git a/test/integration/goTest.utils.ts b/test/integration/goTest.utils.ts
index e95a3b2..9250f98 100644
--- a/test/integration/goTest.utils.ts
+++ b/test/integration/goTest.utils.ts
@@ -5,6 +5,8 @@
import path = require('path');
import { DocumentSymbol, FileType, Uri, TextDocument, SymbolKind, Range, Position } from 'vscode';
import { packagePathToGoModPathMap } from '../../src/goModules';
+import { GoTestExplorer } from '../../src/goTest/explore';
+import { Workspace } from '../../src/goTest/utils';
import { MockTestWorkspace } from '../mocks/MockTest';
// eslint-disable-next-line @typescript-eslint/no-unused-vars
@@ -41,3 +43,19 @@
}
walk(Uri.file('/'));
}
+
+export async function forceDidOpenTextDocument(
+ workspace: Workspace,
+ testExplorer: GoTestExplorer,
+ uri: Uri
+): Promise<TextDocument> {
+ const doc = await workspace.openTextDocument(uri);
+
+ // Force didOpenTextDocument to fire. Without this, the test may run
+ // before the event is handled.
+ //
+ // eslint-disable-next-line @typescript-eslint/no-explicit-any
+ await (testExplorer as any).didOpenTextDocument(doc);
+
+ return doc;
+}
diff --git a/test/integration/index.ts b/test/integration/index.ts
index efc7404..9f4fb2a 100644
--- a/test/integration/index.ts
+++ b/test/integration/index.ts
@@ -8,10 +8,14 @@
import * as Mocha from 'mocha';
import * as path from 'path';
export function run(): Promise<void> {
- const mocha = new Mocha({
+ const options: Mocha.MochaOptions = {
grep: process.env.MOCHA_GREP,
ui: 'tdd'
- });
+ };
+ if (process.env.MOCHA_TIMEOUT) {
+ options.timeout = Number(process.env.MOCHA_TIMEOUT);
+ }
+ const mocha = new Mocha(options);
// @types/mocha is outdated
(mocha as any).color(true);
diff --git a/test/mocks/MockContext.ts b/test/mocks/MockContext.ts
new file mode 100644
index 0000000..5b66359
--- /dev/null
+++ b/test/mocks/MockContext.ts
@@ -0,0 +1,15 @@
+import { Disposable, ExtensionContext } from 'vscode';
+
+type ExtensionContextPlus = ExtensionContext & Pick<MockExtensionContext, 'teardown'>;
+
+export class MockExtensionContext implements Partial<ExtensionContext> {
+ subscriptions: Disposable[] = [];
+
+ static new(): ExtensionContextPlus {
+ return (new this() as unknown) as ExtensionContextPlus;
+ }
+
+ teardown() {
+ this.subscriptions.forEach((x) => x.dispose());
+ }
+}
diff --git a/test/testdata/subTest/go.mod b/test/testdata/subTest/go.mod
new file mode 100644
index 0000000..2dce7c7
--- /dev/null
+++ b/test/testdata/subTest/go.mod
@@ -0,0 +1 @@
+module subTest
\ No newline at end of file
diff --git a/test/testdata/subTest/sub_test.go b/test/testdata/subTest/sub_test.go
new file mode 100644
index 0000000..d78d47a
--- /dev/null
+++ b/test/testdata/subTest/sub_test.go
@@ -0,0 +1,12 @@
+package subTest
+
+import "testing"
+
+func TestMain(t *testing.T) {
+ t.Log("Main")
+ t.Run("Sub", func(t *testing.T) { t.Log("Sub") })
+}
+
+func TestOther(t *testing.T) {
+ t.Log("Other")
+}
To view, visit change 343433. To unsubscribe, or for help writing mail filters, visit settings.
Reply all
Reply to author
Forward
0 new messages