Skip to content

Commit

Permalink
src/goTest: support running sub-tests
Browse files Browse the repository at this point in the history
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 #1641

Change-Id: I26eac8a5a396df3923073274ed93d9c59107d9c3
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/343433
Reviewed-by: Hyang-Ah Hana Kim <[email protected]>
Trust: Hyang-Ah Hana Kim <[email protected]>
Trust: Rebecca Stambler <[email protected]>
Run-TryBot: Hyang-Ah Hana Kim <[email protected]>
TryBot-Result: kokoro <[email protected]>
  • Loading branch information
firelizzard18 authored and hyangah committed Sep 3, 2021
1 parent 9021bff commit 6019957
Show file tree
Hide file tree
Showing 13 changed files with 199 additions and 67 deletions.
3 changes: 2 additions & 1 deletion .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions src/goTest/explore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ export class GoTestExplorer {
}

public readonly resolver: GoTestResolver;
public readonly runner: GoTestRunner;

constructor(
private readonly workspace: Workspace,
Expand All @@ -124,6 +125,7 @@ export class GoTestExplorer {
const runner = new GoTestRunner(workspace, ctrl, resolver);

this.resolver = resolver;
this.runner = runner;

ctrl.resolveHandler = async (item) => {
try {
Expand Down
8 changes: 4 additions & 4 deletions src/goTest/resolve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,9 @@ export class GoTestResolver {
}

// 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) => {
Expand All @@ -149,7 +149,7 @@ export class GoTestResolver {
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) {
Expand Down
59 changes: 39 additions & 20 deletions src/goTest/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { getGoConfig } from '../config';
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 };

Expand Down Expand Up @@ -57,7 +58,7 @@ export class GoTestRunner {
) {}

// 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) {
Expand Down Expand Up @@ -96,7 +97,8 @@ export class GoTestRunner {
}

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);
Expand All @@ -123,11 +125,7 @@ export class GoTestRunner {
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
Expand Down Expand Up @@ -163,21 +161,34 @@ export class GoTestRunner {
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>();
const success = await goTest({
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';
Expand All @@ -195,7 +206,7 @@ export class GoTestRunner {
goConfig,
flags,
isMod,
outputChannel,
outputChannel: testRunOutput,
dir: pkg.uri.fsPath,
functions: benchmarkFns,
isBenchmark: true,
Expand All @@ -206,7 +217,7 @@ export class GoTestRunner {
// 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' });
Expand Down Expand Up @@ -275,16 +286,24 @@ export class GoTestRunner {
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)
Expand Down
4 changes: 2 additions & 2 deletions src/goTest/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export class GoTest {
// - 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();
}

Expand All @@ -47,7 +47,7 @@ export class GoTest {
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 };
}
}
Expand Down
6 changes: 5 additions & 1 deletion src/testUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,11 @@ function targetArgs(testconfig: TestConfig): Array<string> {

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));
Expand Down
47 changes: 10 additions & 37 deletions test/integration/goTest.explore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }>;

Expand Down Expand Up @@ -179,49 +180,21 @@ suite('Go Test Explorer', () => {
});

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}`);

// 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);
testExplorer = GoTestExplorer.setup(ctx);

// 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', () => {
Expand Down
83 changes: 83 additions & 0 deletions test/integration/goTest.run.test.ts
Original file line number Diff line number Diff line change
@@ -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');
});
});
});
18 changes: 18 additions & 0 deletions test/integration/goTest.utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -41,3 +43,19 @@ export function populateModulePathCache(workspace: MockTestWorkspace) {
}
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;
}
Loading

0 comments on commit 6019957

Please sign in to comment.