src/goTest: group test explorer settings, fix bugs
- Rename settings and update descriptions to improve clarity
- vscode.URI already encodes the fragment - don't double encode
- Dynamically resolved subtests were not labeled as such
Change-Id: Ifb5c4636f9fa06ef8980df270c3afdb20bec0089
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/347669
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
diff --git a/docs/settings.md b/docs/settings.md
index dcb17a8..62dafd1 100644
--- a/docs/settings.md
+++ b/docs/settings.md
@@ -393,25 +393,25 @@
### `go.testEnvVars`
Environment variables that will be passed to the process that runs the Go tests
-### `go.testExplorerConcatenateMessages`
+### `go.testExplorer.alwaysRunBenchmarks`
-If true, test log messages associated with a given location will be shown as a single message.
+Run benchmarks when running all tests in a file or folder.
+
+Default: `false`
+### `go.testExplorer.concatenateMessages`
+
+Concatenate all test log messages for a given location into a single message.
Default: `true`
-### `go.testExplorerPackages`
+### `go.testExplorer.packageDisplayMode`
-Control whether packages in the test explorer are presented flat or nested.<br/>
+Present packages in the test explorer flat or nested.<br/>
Allowed Options: `flat`, `nested`
Default: `"flat"`
-### `go.testExplorerRunBenchmarks`
+### `go.testExplorer.showDynamicSubtestsInEditor`
-Include benchmarks when running all tests in a group.
-
-Default: `false`
-### `go.testExplorerSetDynamicSubtestRange`
-
-If true, the source location of dynamically discovered subtests will be set to the source location of the containing function
+Set the source location of dynamically discovered subtests to the location of the containing function. As a result, dynamically discovered subtests will be added to the gutter test widget of the containing function.
Default: `false`
### `go.testFlags`
diff --git a/package.json b/package.json
index 0404700..d3b6c2f 100644
--- a/package.json
+++ b/package.json
@@ -1302,32 +1302,32 @@
"description": "Flags to pass to `go test`. If null, then buildFlags will be used. This is not propagated to the language server.",
"scope": "resource"
},
- "go.testExplorerPackages": {
+ "go.testExplorer.packageDisplayMode": {
"type": "string",
"enum": [
"flat",
"nested"
],
"default": "flat",
- "description": "Control whether packages in the test explorer are presented flat or nested.",
+ "description": "Present packages in the test explorer flat or nested.",
"scope": "resource"
},
- "go.testExplorerRunBenchmarks": {
+ "go.testExplorer.alwaysRunBenchmarks": {
"type": "boolean",
"default": false,
- "description": "Include benchmarks when running all tests in a group.",
+ "description": "Run benchmarks when running all tests in a file or folder.",
"scope": "resource"
},
- "go.testExplorerConcatenateMessages": {
+ "go.testExplorer.concatenateMessages": {
"type": "boolean",
"default": true,
- "description": "If true, test log messages associated with a given location will be shown as a single message.",
+ "description": "Concatenate all test log messages for a given location into a single message.",
"scope": "resource"
},
- "go.testExplorerSetDynamicSubtestRange": {
+ "go.testExplorer.showDynamicSubtestsInEditor": {
"type": "boolean",
"default": false,
- "description": "If true, the source location of dynamically discovered subtests will be set to the source location of the containing function",
+ "description": "Set the source location of dynamically discovered subtests to the location of the containing function. As a result, dynamically discovered subtests will be added to the gutter test widget of the containing function.",
"scope": "resource"
},
"go.generateTestsFlags": {
diff --git a/src/goTest/resolve.ts b/src/goTest/resolve.ts
index 277a8e5..b5e342a 100644
--- a/src/goTest/resolve.ts
+++ b/src/goTest/resolve.ts
@@ -228,7 +228,7 @@
private shouldSetRange(item: TestItem): boolean {
const config = getGoConfig(item.uri);
- return config.get<boolean>('testExplorerSetDynamicSubtestRange');
+ return config.get<boolean>('testExplorer.showDynamicSubtestsInEditor');
}
// Create an item.
@@ -303,7 +303,7 @@
private async getPackage(uri: Uri): Promise<TestItem> {
let item: TestItem;
- const nested = getGoConfig(uri).get('testExplorerPackages') === 'nested';
+ const nested = getGoConfig(uri).get('testExplorer.packageDisplayMode') === 'nested';
const modDir = await getModFolderPath(uri, true);
const wsfolder = workspace.getWorkspaceFolder(uri);
if (modDir) {
diff --git a/src/goTest/run.ts b/src/goTest/run.ts
index d5da2d8..e9c2178 100644
--- a/src/goTest/run.ts
+++ b/src/goTest/run.ts
@@ -158,7 +158,7 @@
const isMod = isInMod(pkg) || (await isModSupported(pkg.uri, true));
const goConfig = getGoConfig(pkg.uri);
const flags = getTestFlags(goConfig);
- const includeBench = getGoConfig(pkg.uri).get('testExplorerRunBenchmarks');
+ const includeBench = getGoConfig(pkg.uri).get('testExplorer.alwaysRunBenchmarks');
// If any of the tests are test suite methods, add all test functions that call `suite.Run`
const hasTestMethod = items.some(({ item }) => this.resolver.isTestMethod.has(item));
@@ -212,7 +212,7 @@
}
const record = new Map<TestItem, string[]>();
- const concat = goConfig.get<boolean>('testExplorerConcatenateMessages');
+ const concat = goConfig.get<boolean>('testExplorer.concatenateMessages');
// https://github.com/golang/go/issues/39904
if (subItems.length > 0 && Object.keys(tests).length + Object.keys(benchmarks).length > 1) {
@@ -376,12 +376,12 @@
const m = name.substring(pos).match(re);
if (!m) {
if (!parent) return tests[name];
- return this.resolver.getOrCreateSubTest(parent, name.substring(pos), name);
+ return this.resolver.getOrCreateSubTest(parent, name.substring(pos), name, true);
}
const subName = name.substring(0, pos + m.index);
const test = parent
- ? this.resolver.getOrCreateSubTest(parent, name.substring(pos, pos + m.index), subName)
+ ? this.resolver.getOrCreateSubTest(parent, name.substring(pos, pos + m.index), subName, true)
: tests[subName];
return resolve(test, pos + m.index, m[0].length);
};
diff --git a/src/goTest/utils.ts b/src/goTest/utils.ts
index 8610a2b..4d81422 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: encodeURIComponent(name) });
+ if (name) uri = uri.with({ fragment: 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 = decodeURIComponent(u.fragment);
+ const name = u.fragment;
return { name, kind };
}
}
diff --git a/test/integration/goTest.explore.test.ts b/test/integration/goTest.explore.test.ts
index a3e6b39..31a581f 100644
--- a/test/integration/goTest.explore.test.ts
+++ b/test/integration/goTest.explore.test.ts
@@ -4,12 +4,17 @@
*--------------------------------------------------------*/
import assert = require('assert');
import path = require('path');
-import { TextDocument, TestItemCollection, TextDocumentChangeEvent, workspace, Uri } from 'vscode';
+import sinon = require('sinon');
+import { TextDocument, TestItem, TestItemCollection, TextDocumentChangeEvent, workspace, Uri } from 'vscode';
import { GoTestExplorer } from '../../src/goTest/explore';
import { MockTestController, MockTestWorkspace } from '../mocks/MockTest';
import { forceDidOpenTextDocument, getSymbols_Regex, populateModulePathCache } from './goTest.utils';
import { MockExtensionContext } from '../mocks/MockContext';
import { MockMemento } from '../mocks/MockMemento';
+import * as config from '../../src/config';
+import { GoTestResolver } from '../../src/goTest/resolve';
+import * as testUtils from '../../src/testUtils';
+import { GoTest } from '../../src/goTest/utils';
type Files = Record<string, string | { contents: string; language: string }>;
@@ -18,7 +23,7 @@
files: Files;
}
-function setupCtor<T extends GoTestExplorer>(
+function newExplorer<T extends GoTestExplorer>(
folders: string[],
files: Files,
ctor: new (...args: ConstructorParameters<typeof GoTestExplorer>) => T
@@ -42,6 +47,13 @@
assert.deepStrictEqual(actual, expect);
}
+async function forceResolve(resolver: GoTestResolver, item?: TestItem) {
+ await resolver.resolve(item);
+ const items: TestItem[] = [];
+ (item?.children || resolver.items).forEach((x) => items.push(x));
+ await Promise.all(items.map((x) => forceResolve(resolver, x)));
+}
+
suite('Go Test Explorer', () => {
suite('Document opened', () => {
class DUT extends GoTestExplorer {
@@ -89,7 +101,7 @@
for (const name in cases) {
test(name, async () => {
const { workspace, files, open, expect } = cases[name];
- const { ctrl, expl, ws } = setupCtor(workspace, files, DUT);
+ const { ctrl, expl, ws } = newExplorer(workspace, files, DUT);
await expl._didOpen(ws.fs.files.get(open));
@@ -158,7 +170,7 @@
for (const name in cases) {
test(name, async () => {
const { workspace, files, open, changes, expect } = cases[name];
- const { ctrl, expl, ws } = setupCtor(workspace, files, DUT);
+ const { ctrl, expl, ws } = newExplorer(workspace, files, DUT);
await expl._didOpen(ws.fs.files.get(open));
@@ -205,4 +217,206 @@
]);
});
});
+
+ suite('settings', () => {
+ const sandbox = sinon.createSandbox();
+
+ // eslint-disable-next-line @typescript-eslint/no-explicit-any
+ let goConfig: any;
+
+ setup(() => {
+ goConfig = Object.create(config.getGoConfig());
+ sandbox.stub(config, 'getGoConfig').returns(goConfig);
+ });
+
+ teardown(() => {
+ sandbox.restore();
+ });
+
+ suite('packageDisplayMode', () => {
+ let resolver: GoTestResolver;
+
+ setup(() => {
+ const { expl } = newExplorer(
+ ['/src/proj'],
+ {
+ '/src/proj/go.mod': 'module test',
+ '/src/proj/pkg/main_test.go': 'package main\nfunc TestFoo(t *testing.T) {}',
+ '/src/proj/pkg/sub/main_test.go': 'package main\nfunc TestBar(t *testing.T) {}'
+ },
+ GoTestExplorer
+ );
+ resolver = expl.resolver;
+ });
+
+ test('flat', async () => {
+ // Expect:
+ // - module
+ // - package pkg
+ // - package pkg/sub
+
+ sinon.stub(goConfig, 'get').withArgs('testExplorer.packageDisplayMode').returns('flat');
+ await forceResolve(resolver);
+
+ const mod = resolver.items.get('file:///src/proj?module');
+ assert(mod, 'Module is missing or is not at the root');
+
+ const pkg = mod.children.get('file:///src/proj/pkg?package');
+ assert(pkg, 'Package pkg is missing or not a child of the module');
+
+ const sub = mod.children.get('file:///src/proj/pkg/sub?package');
+ assert(sub, 'Package pkg/sub is missing or not a child of the module');
+ });
+
+ test('nested', async () => {
+ // Expect:
+ // - module
+ // - package pkg
+ // - package pkg/sub
+
+ sinon.stub(goConfig, 'get').withArgs('testExplorer.packageDisplayMode').returns('nested');
+ await forceResolve(resolver);
+
+ const mod = resolver.items.get('file:///src/proj?module');
+ assert(mod, 'Module is missing or is not at the root');
+
+ const pkg = mod.children.get('file:///src/proj/pkg?package');
+ assert(pkg, 'Package pkg is missing or not a child of the module');
+
+ const sub = pkg.children.get('file:///src/proj/pkg/sub?package');
+ assert(sub, 'Package pkg/sub is missing or not a child of package pkg');
+ });
+ });
+
+ suite('alwaysRunBenchmarks', () => {
+ let explorer: GoTestExplorer;
+ let runStub: sinon.SinonStub<[testUtils.TestConfig], Promise<boolean>>;
+
+ const expectedTests = [
+ 'file:///src/proj/pkg/main_test.go?file',
+ 'file:///src/proj/pkg/main_test.go?test#TestFoo',
+ 'file:///src/proj/pkg/main_test.go?benchmark#BenchmarkBar'
+ ];
+
+ setup(() => {
+ runStub = sandbox.stub(testUtils, 'goTest');
+ runStub.callsFake(() => Promise.resolve(true));
+
+ explorer = newExplorer(
+ ['/src/proj'],
+ {
+ '/src/proj/go.mod': 'module test',
+ '/src/proj/pkg/main_test.go': `
+ package main
+
+ func TestFoo(t *testing.T) {}
+ func BenchmarkBar(b *testing.B) {}
+ `
+ },
+ GoTestExplorer
+ ).expl;
+ });
+
+ test('false', async () => {
+ // Running the file should only run the test
+
+ sinon.stub(goConfig, 'get').withArgs('testExplorer.alwaysRunBenchmarks').returns(false);
+ await forceResolve(explorer.resolver);
+
+ const tests = explorer.resolver.find(Uri.parse('file:///src/proj/pkg/main_test.go'));
+ assert.deepStrictEqual(
+ tests.map((x) => x.id),
+ expectedTests
+ );
+
+ await explorer.runner.run({ include: [tests[0]] });
+ assert.strictEqual(runStub.callCount, 1, 'Expected goTest to be called once');
+ assert.deepStrictEqual(runStub.lastCall.args[0].functions, ['TestFoo']);
+ });
+
+ test('true', async () => {
+ // Running the file should run the test and the benchmark
+
+ sinon.stub(goConfig, 'get').withArgs('testExplorer.alwaysRunBenchmarks').returns(true);
+ await forceResolve(explorer.resolver);
+
+ const tests = explorer.resolver.find(Uri.parse('file:///src/proj/pkg/main_test.go'));
+ assert.deepStrictEqual(
+ tests.map((x) => x.id),
+ expectedTests
+ );
+
+ await explorer.runner.run({ include: [tests[0]] });
+ assert.strictEqual(runStub.callCount, 2, 'Expected goTest to be called twice');
+ assert.deepStrictEqual(runStub.firstCall.args[0].functions, ['TestFoo']);
+ assert.deepStrictEqual(runStub.secondCall.args[0].functions, ['BenchmarkBar']);
+ });
+ });
+
+ suite('showDynamicSubtestsInEditor', () => {
+ let explorer: GoTestExplorer;
+ let runStub: sinon.SinonStub<[testUtils.TestConfig], Promise<boolean>>;
+
+ setup(() => {
+ runStub = sandbox.stub(testUtils, 'goTest');
+ runStub.callsFake((cfg) => {
+ // Trigger creation of dynamic subtest
+ cfg.goTestOutputConsumer({
+ Test: 'TestFoo/Bar',
+ Action: 'run'
+ });
+ return Promise.resolve(true);
+ });
+
+ explorer = newExplorer(
+ ['/src/proj'],
+ {
+ '/src/proj/go.mod': 'module test',
+ '/src/proj/main_test.go': 'package main\nfunc TestFoo(t *testing.T) {}'
+ },
+ GoTestExplorer
+ ).expl;
+ });
+
+ test('false', async () => {
+ // Dynamic subtests should have no location
+
+ sinon.stub(goConfig, 'get').withArgs('testExplorer.showDynamicSubtestsInEditor').returns(false);
+ await forceResolve(explorer.resolver);
+
+ const test = explorer.resolver
+ .find(Uri.parse('file:///src/proj/main_test.go'))
+ .filter((x) => GoTest.parseId(x.id).name)[0];
+ assert(test, 'Could not find test');
+
+ await explorer.runner.run({ include: [test] });
+ assert.strictEqual(runStub.callCount, 1, 'Expected goTest to be called once');
+
+ const subTest = test.children.get('file:///src/proj/main_test.go?test#TestFoo%2FBar');
+ assert(subTest, 'Could not find subtest');
+
+ assert(!subTest.range, 'Subtest should not have a range');
+ });
+
+ test('true', async () => {
+ // Dynamic subtests should have the same location as their parents
+
+ sinon.stub(goConfig, 'get').withArgs('testExplorer.showDynamicSubtestsInEditor').returns(true);
+ await forceResolve(explorer.resolver);
+
+ const test = explorer.resolver
+ .find(Uri.parse('file:///src/proj/main_test.go'))
+ .filter((x) => GoTest.parseId(x.id).name)[0];
+ assert(test, 'Could not find test');
+
+ await explorer.runner.run({ include: [test] });
+ assert.strictEqual(runStub.callCount, 1, 'Expected goTest to be called once');
+
+ const subTest = test.children.get('file:///src/proj/main_test.go?test#TestFoo%2FBar');
+ assert(subTest, 'Could not find subtest');
+
+ assert.deepStrictEqual(subTest.range, test.range, 'Subtest range should match parent range');
+ });
+ });
+ });
});
diff --git a/test/mocks/MockTest.ts b/test/mocks/MockTest.ts
index 41bc6aa..8372de2 100644
--- a/test/mocks/MockTest.ts
+++ b/test/mocks/MockTest.ts
@@ -15,6 +15,7 @@
TestController,
TestItem,
TestItemCollection,
+ TestMessage,
TestRun,
TestRunProfile,
TestRunProfileKind,
@@ -109,6 +110,24 @@
dispose(): void {}
}
+class MockTestRun implements TestRun {
+ name = 'test run';
+ isPersisted = false;
+
+ get token(): CancellationToken {
+ throw new Error('Method not implemented.');
+ }
+
+ enqueued(test: TestItem): void {}
+ started(test: TestItem): void {}
+ skipped(test: TestItem): void {}
+ failed(test: TestItem, message: TestMessage | readonly TestMessage[], duration?: number): void {}
+ errored(test: TestItem, message: TestMessage | readonly TestMessage[], duration?: number): void {}
+ passed(test: TestItem, duration?: number): void {}
+ appendOutput(output: string): void {}
+ end(): void {}
+}
+
export class MockTestController implements TestController {
id = 'go';
label = 'Go';
@@ -117,7 +136,7 @@
resolveHandler?: (item: TestItem) => void | Thenable<void>;
createTestRun(request: TestRunRequest, name?: string, persist?: boolean): TestRun {
- throw new Error('Method not implemented.');
+ return new MockTestRun();
}
createRunProfile(
