Skip to content

Commit

Permalink
src/goTest: clean up test ID parsing
Browse files Browse the repository at this point in the history 
Adds a string union type to insure test kinds are used correctly, adds
a utility function for parsing test IDs, and updates usage to prefer
using TestItem.uri over extracting the URI from the test ID.

Change-Id: I5528b1ce0fac4329428271b383a0685914d0aa4d
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/345178
Trust: Alexander Rakoczy <[email protected]>
Trust: Hyang-Ah Hana Kim <[email protected]>
Run-TryBot: Alexander Rakoczy <[email protected]>
TryBot-Result: kokoro <[email protected]>
Reviewed-by: Hyang-Ah Hana Kim <[email protected]>
  • Loading branch information
@firelizzard18 @hyangah
firelizzard18 authored and hyangah committed Aug 26, 2021
1 parent 3e12e73 commit e53d60e
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 49 deletions.
9 changes: 4 additions & 5 deletions src/goTest/explore.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,12 +176,12 @@ export class GoTestExplorer {
}

this.ctrl.items.forEach((item) => {
const uri = Uri.parse(item.id);
if (uri.query === 'package') {
const { kind } = GoTest.parseId(item.id);
if (kind === 'package') {
return;
}

const ws = this.workspace.getWorkspaceFolder(uri);
const ws = this.workspace.getWorkspaceFolder(item.uri);
if (!ws) {
dispose(item);
}
Expand All @@ -200,8 +200,7 @@ export class GoTestExplorer {
return item;
}

const uri = Uri.parse(item.id);
if (!file.path.startsWith(uri.path)) {
if (!file.path.startsWith(item.uri.path)) {
return;
}

Expand Down
41 changes: 21 additions & 20 deletions src/goTest/resolve.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ import path = require('path');
import { getModFolderPath } from '../goModules';
import { getCurrentGoPath } from '../util';
import { getGoConfig } from '../config';
import { dispose, disposeIfEmpty, FileSystem, GoTest, Workspace } from './utils';
import { dispose, disposeIfEmpty, FileSystem, GoTest, GoTestKind, Workspace } from './utils';
import { walk, WalkStop } from './walk';
import { goTest } from '../testUtils';

export type ProvideSymbols = (doc: TextDocument, token: CancellationToken) => Thenable<DocumentSymbol[]>;

Expand Down Expand Up @@ -55,12 +56,12 @@ export class GoTestResolver {
if (!item) {
// Dispose of package entries at the root if they are now part of a workspace folder
this.ctrl.items.forEach((item) => {
const uri = Uri.parse(item.id);
if (uri.query !== 'package') {
const { kind } = GoTest.parseId(item.id);
if (kind !== 'package') {
return;
}

if (this.workspace.getWorkspaceFolder(uri)) {
if (this.workspace.getWorkspaceFolder(item.uri)) {
dispose(item);
}
});
Expand All @@ -86,29 +87,29 @@ export class GoTestResolver {
return;
}

const uri = Uri.parse(item.id);
const { kind } = GoTest.parseId(item.id);

// The user expanded a module or workspace - find all packages
if (uri.query === 'module' || uri.query === 'workspace') {
await walkPackages(this.workspace.fs, uri, async (uri) => {
if (kind === 'module' || kind === 'workspace') {
await walkPackages(this.workspace.fs, item.uri, async (uri) => {
await this.getPackage(uri);
});
}

// The user expanded a module or package - find all files
if (uri.query === 'module' || uri.query === 'package') {
for (const [file, type] of await this.workspace.fs.readDirectory(uri)) {
if (kind === 'module' || kind === 'package') {
for (const [file, type] of await this.workspace.fs.readDirectory(item.uri)) {
if (type !== FileType.File || !file.endsWith('_test.go')) {
continue;
}

await this.getFile(Uri.joinPath(uri, file));
await this.getFile(Uri.joinPath(item.uri, file));
}
}

// The user expanded a file - find all functions
if (uri.query === 'file') {
const doc = await this.workspace.openTextDocument(uri.with({ query: '', fragment: '' }));
if (kind === 'file') {
const doc = await this.workspace.openTextDocument(item.uri);
await this.processDocument(doc);
}

Expand Down Expand Up @@ -140,7 +141,7 @@ 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 { fragment: parentName, query: kind } = Uri.parse(item.id);
const { kind, name: parentName } = GoTest.parseId(item.id);

let existing: TestItem | undefined;
item.children.forEach((child) => {
Expand Down Expand Up @@ -176,8 +177,8 @@ export class GoTestResolver {
}

item.children.forEach((child) => {
const uri = Uri.parse(child.id);
if (!seen.has(uri.fragment)) {
const { name } = GoTest.parseId(child.id);
if (!seen.has(name)) {
dispose(child);
return;
}
Expand All @@ -193,12 +194,12 @@ export class GoTestResolver {
/* ***** Private ***** */

// Create an item.
private createItem(label: string, uri: Uri, kind: string, name?: string): TestItem {
private createItem(label: string, uri: Uri, kind: GoTestKind, name?: string): TestItem {
return this.ctrl.createTestItem(GoTest.id(uri, kind, name), label, uri.with({ query: '', fragment: '' }));
}

// Retrieve an item.
private getItem(parent: TestItem | undefined, uri: Uri, kind: string, name?: string): TestItem {
private getItem(parent: TestItem | undefined, uri: Uri, kind: GoTestKind, name?: string): TestItem {
return (parent?.children || this.ctrl.items).get(GoTest.id(uri, kind, name));
}

Expand All @@ -207,7 +208,7 @@ export class GoTestResolver {
parent: TestItem | undefined,
label: string,
uri: Uri,
kind: string,
kind: GoTestKind,
name?: string
): TestItem {
const existing = this.getItem(parent, uri, kind, name);
Expand Down Expand Up @@ -367,7 +368,7 @@ export class GoTestResolver {

seen.add(symbol.name);

const kind = match.groups.kind.toLowerCase();
const kind = match.groups.kind.toLowerCase() as GoTestKind;
const suite = match.groups.type ? this.getTestSuite(match.groups.type) : undefined;
const existing =
this.getItem(file, doc.uri, kind, symbol.name) ||
Expand Down Expand Up @@ -411,7 +412,7 @@ export class GoTestResolver {
suite.func = item;

for (const method of suite.methods) {
if (Uri.parse(method.parent.id).query !== 'file') {
if (GoTest.parseId(method.parent.id).kind !== 'file') {
continue;
}

Expand Down
36 changes: 18 additions & 18 deletions src/goTest/run.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { isModSupported } from '../goModules';
import { getGoConfig } from '../config';
import { getTestFlags, goTest, GoTestOutput } from '../testUtils';
import { GoTestResolver } from './resolve';
import { dispose, forEachAsync, Workspace } from './utils';
import { dispose, forEachAsync, GoTest, Workspace } from './utils';

type CollectedTest = { item: TestItem; explicitlyIncluded?: boolean };

Expand Down Expand Up @@ -83,15 +83,15 @@ export class GoTestRunner {
hasNonBench = false;
for (const items of collected.values()) {
for (const { item } of items) {
const uri = Uri.parse(item.id);
if (uri.query === 'benchmark') hasBench = true;
const { kind } = GoTest.parseId(item.id);
if (kind === 'benchmark') hasBench = true;
else hasNonBench = true;
}
}

function isInMod(item: TestItem): boolean {
const uri = Uri.parse(item.id);
if (uri.query === 'module') return true;
const { kind } = GoTest.parseId(item.id);
if (kind === 'module') return true;
if (!item.parent) return false;
return isInMod(item.parent);
}
Expand Down Expand Up @@ -123,10 +123,10 @@ export class GoTestRunner {
const tests: Record<string, TestItem> = {};
const benchmarks: Record<string, TestItem> = {};
for (const { item, explicitlyIncluded } of items) {
const uri = Uri.parse(item.id);
if (/[/#]/.test(uri.fragment)) {
const { kind, name } = GoTest.parseId(item.id);
if (/[/#]/.test(name)) {
// running sub-tests is not currently supported
vscode.window.showErrorMessage(`Cannot run ${uri.fragment} - running sub-tests is not supported`);
vscode.window.showErrorMessage(`Cannot run ${name} - running sub-tests is not supported`);
continue;
}

Expand All @@ -138,7 +138,7 @@ export class GoTestRunner {
// However, if the user clicks the run button on a file or package
// that contains benchmarks and nothing else, they likely expect
// those benchmarks to run.
if (uri.query === 'benchmark' && !explicitlyIncluded && !includeBench && !(hasBench && !hasNonBench)) {
if (kind === 'benchmark' && !explicitlyIncluded && !includeBench && !(hasBench && !hasNonBench)) {
continue;
}

Expand All @@ -152,10 +152,10 @@ export class GoTestRunner {
}
});

if (uri.query === 'benchmark') {
benchmarks[uri.fragment] = item;
if (kind === 'benchmark') {
benchmarks[name] = item;
} else {
tests[uri.fragment] = item;
tests[name] = item;
}
}

Expand Down Expand Up @@ -238,8 +238,8 @@ export class GoTestRunner {
}
}

const uri = Uri.parse(item.id);
if (!uri.fragment) {
const { name } = GoTest.parseId(item.id);
if (!name) {
if (item.children.size === 0) {
await this.resolver.resolve(item);
}
Expand All @@ -251,8 +251,8 @@ export class GoTestRunner {
}

function getFile(item: TestItem): TestItem {
const uri = Uri.parse(item.id);
if (uri.query === 'file') return item;
const { kind } = GoTest.parseId(item.id);
if (kind === 'file') return item;
return getFile(item.parent);
}

Expand Down Expand Up @@ -436,10 +436,10 @@ export class GoTestRunner {
parseOutput(test: TestItem, output: string[]): TestMessage[] {
const messages: TestMessage[] = [];

const uri = Uri.parse(test.id);
const { kind } = GoTest.parseId(test.id);
const gotI = output.indexOf('got:\n');
const wantI = output.indexOf('want:\n');
if (uri.query === 'example' && gotI >= 0 && wantI >= 0) {
if (kind === 'example' && gotI >= 0 && wantI >= 0) {
const got = output.slice(gotI + 1, wantI).join('');
const want = output.slice(wantI + 1).join('');
const message = TestMessage.diff('Output does not match', want, got);
Expand Down
41 changes: 37 additions & 4 deletions src/goTest/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,52 @@
*--------------------------------------------------------*/
import * as vscode from 'vscode';

// GoTestKind indicates the Go construct represented by a test item.
//
// - A 'module' is a folder that contains a go.mod file
// - A 'workspace' is a VSCode workspace folder that contains .go files outside
// of a module
// - A 'package' is a folder that contains .go files (and is not a module)
// - A 'file' is a file ending with _test.go
// - A 'test' is a Go test, e.g. func TestXxx(t *testing.T)
// - A 'benchmark' is a Go benchmark, e.g. func BenchmarkXxx(t *testing.T)
// - An 'example' is a Go example, e.g. func ExampleXxx()
//
// The top-level test item for a workspace folder is always either a module or a
// workspace. If the user opens a file (containing tests) that is not contained
// within any workspace folder, a top-level package will be created as a parent
// of that file.
export type GoTestKind = 'module' | 'workspace' | 'package' | 'file' | 'test' | 'benchmark' | 'example';

export class GoTest {
// Construct an ID for an item. Exported for tests.
// Constructs an ID for an item. The ID of a test item consists of the URI
// for the relevant file or folder with the URI query set to the test item
// kind (see GoTestKind) and the URI fragment set to the function name, if
// the item represents a test, benchmark, or example function.
//
// - Module: file:///path/to/mod?module
// - Workspace: file:///path/to/src?workspace
// - Package: file:///path/to/mod/pkg?package
// - File: file:///path/to/mod/file.go?file
// - Test: file:///path/to/mod/file.go?test#TestXxx
// - Benchmark: file:///path/to/mod/file.go?benchmark#BenchmarkXxx
// - Example: file:///path/to/mod/file.go?example#ExampleXxx
static id(uri: vscode.Uri, kind: string, name?: string): string {
static id(uri: vscode.Uri, kind: GoTestKind, name?: string): string {
uri = uri.with({ query: kind });
if (name) uri = uri.with({ fragment: name });
return uri.toString();
}

// Parses the ID as a URI and extracts the kind and name.
//
// The URI of the relevant file or folder should be retrieved wil
// TestItem.uri.
static parseId(id: string): { kind: GoTestKind; name?: string } {
const u = vscode.Uri.parse(id);
const kind = u.query as GoTestKind;
const name = u.fragment;
return { name, kind };
}
}

// The subset of vscode.FileSystem that is used by the test explorer.
Expand Down Expand Up @@ -61,8 +94,8 @@ export function dispose(item: vscode.TestItem) {
// cleaning up package/file trees that contain no tests.
export function disposeIfEmpty(item: vscode.TestItem) {
// Don't dispose of empty top-level items
const uri = vscode.Uri.parse(item.id);
if (uri.query === 'module' || uri.query === 'workspace' || (uri.query === 'package' && !item.parent)) {
const { kind } = GoTest.parseId(item.id);
if (kind === 'module' || kind === 'workspace' || (kind === 'package' && !item.parent)) {
return;
}

Expand Down
4 changes: 2 additions & 2 deletions test/integration/goTest.resolve.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import assert = require('assert');
import { TestItem, Uri } from 'vscode';
import { GoTestResolver } from '../../src/goTest/resolve';
import { GoTest } from '../../src/goTest/utils';
import { GoTest, GoTestKind } from '../../src/goTest/utils';
import { MockTestController, MockTestWorkspace } from '../mocks/MockTest';
import { getSymbols_Regex, populateModulePathCache } from './goTest.utils';

Expand All @@ -26,7 +26,7 @@ function setup(folders: string[], files: Files) {

suite('Go Test Resolver', () => {
interface TC extends TestCase {
item?: ([string, string, string] | [string, string, string, string])[];
item?: ([string, string, GoTestKind] | [string, string, GoTestKind, string])[];
expect: string[];
}

Expand Down

0 comments on commit e53d60e

Please sign in to comment.