src/goTest: do not open text document on didCreateFile
The Test Explorer watches all _test.go files in the workspace in
order to keep the list of tests updated in the test explorer ui.
On branch switches, there may be many test files that are created,
causing the test explorer to process these. Previously, part of the
processing resulted in opening the file. This led to issues due to
the interaction with the language server. This change updates the
logic on file creation, to add the parent test item only, which will
not require opening the document.
Fixes golang/vscode-go#2570
Change-Id: I595f34daee257c85bafd3e5706176f73f891fdf1
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/458999
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Run-TryBot: Suzy Mueller <suzmue@golang.org>
TryBot-Result: kokoro <noreply+kokoro@google.com>
diff --git a/src/goDocumentSymbols.ts b/src/goDocumentSymbols.ts
index 5809237..1aa814e 100644
--- a/src/goDocumentSymbols.ts
+++ b/src/goDocumentSymbols.ts
@@ -25,6 +25,10 @@
constructor(private readonly goCtx: GoExtensionContext, private includeImports?: boolean) {}
public async provideDocumentSymbols(document: vscode.TextDocument): Promise<vscode.DocumentSymbol[]> {
+ // TODO(suzmue): consider providing an interface for providing document symbols that only requires
+ // the URI. Getting a TextDocument from a filename requires opening the file, which can lead to
+ // files being opened that were not requested by the user in order to get information that we just
+ // need the URI to access.
if (typeof this.includeImports !== 'boolean') {
const gotoSymbolConfig = getGoConfig(document.uri)['gotoSymbol'];
this.includeImports = gotoSymbolConfig ? gotoSymbolConfig['includeImports'] : false;
diff --git a/src/goTest/explore.ts b/src/goTest/explore.ts
index 5493b0e..a9a75fe 100644
--- a/src/goTest/explore.ts
+++ b/src/goTest/explore.ts
@@ -258,7 +258,12 @@
}
protected async didCreateFile(file: Uri) {
- await this.documentUpdate(await this.workspace.openTextDocument(file));
+ // Do not use openTextDocument to get the TextDocument for file,
+ // since this sends a didOpen text document notification to gopls,
+ // leading to spurious diagnostics from gopls:
+ // https://github.com/golang/vscode-go/issues/2570
+ // Instead, get the test item for this file only.
+ await this.resolver.getFile(file);
}
protected async didDeleteFile(file: Uri) {
diff --git a/src/goTest/resolve.ts b/src/goTest/resolve.ts
index 2b4d93c..48c38bf 100644
--- a/src/goTest/resolve.ts
+++ b/src/goTest/resolve.ts
@@ -233,6 +233,21 @@
vscode.commands.executeCommand('setContext', 'go.tests', items);
}
+ // Retrieve or create an item for a Go file.
+ public async getFile(uri: Uri): Promise<TestItem> {
+ const dir = path.dirname(uri.path);
+ const pkg = await this.getPackage(uri.with({ path: dir, query: '', fragment: '' }));
+ const existing = this.getItem(pkg, uri, 'file');
+ if (existing) {
+ return existing;
+ }
+
+ const label = path.basename(uri.path);
+ const item = this.getOrCreateItem(pkg, label, uri, 'file');
+ item.canResolveChildren = true;
+ return item;
+ }
+
/* ***** Private ***** */
private shouldSetRange(item: TestItem): boolean {
@@ -375,21 +390,6 @@
return item;
}
- // Retrieve or create an item for a Go file.
- private async getFile(uri: Uri): Promise<TestItem> {
- const dir = path.dirname(uri.path);
- const pkg = await this.getPackage(uri.with({ path: dir, query: '', fragment: '' }));
- const existing = this.getItem(pkg, uri, 'file');
- if (existing) {
- return existing;
- }
-
- const label = path.basename(uri.path);
- const item = this.getOrCreateItem(pkg, label, uri, 'file');
- item.canResolveChildren = true;
- return item;
- }
-
private getTestSuite(type: string): TestSuite {
if (this.testSuites.has(type)) {
return this.testSuites.get(type) as TestSuite;
diff --git a/test/integration/goTest.explore.test.ts b/test/integration/goTest.explore.test.ts
index 0beda07..ebef7b0 100644
--- a/test/integration/goTest.explore.test.ts
+++ b/test/integration/goTest.explore.test.ts
@@ -60,11 +60,16 @@
async _didOpen(doc: TextDocument) {
await this.didOpenTextDocument(doc);
}
+
+ async _didCreate(doc: TextDocument) {
+ await this.didCreateFile(doc.uri);
+ }
}
interface TC extends TestCase {
- open: string;
- expect: string[];
+ uri: string;
+ expectCreate: string[];
+ expectOpen: string[];
}
const cases: Record<string, TC> = {
@@ -76,8 +81,9 @@
'/src/proj/bar_test.go': 'package main\nfunc TestBar(*testing.T) {}',
'/src/proj/baz/main_test.go': 'package main\nfunc TestBaz(*testing.T) {}'
},
- open: 'file:///src/proj/foo_test.go',
- expect: [
+ uri: 'file:///src/proj/foo_test.go',
+ expectCreate: ['file:///src/proj?module', 'file:///src/proj/foo_test.go?file'],
+ expectOpen: [
'file:///src/proj?module',
'file:///src/proj/foo_test.go?file',
'file:///src/proj/foo_test.go?test#TestFoo'
@@ -89,8 +95,9 @@
'/src/proj/go.mod': 'module test',
'/src/proj/foo_test.go': 'package main\nfunc TestFoo(*testing.T) {}'
},
- open: 'file:///src/proj/foo_test.go',
- expect: [
+ uri: 'file:///src/proj/foo_test.go',
+ expectCreate: ['file:///src/proj?module', 'file:///src/proj/foo_test.go?file'],
+ expectOpen: [
'file:///src/proj?module',
'file:///src/proj/foo_test.go?file',
'file:///src/proj/foo_test.go?test#TestFoo'
@@ -100,14 +107,17 @@
for (const name in cases) {
test(name, async () => {
- const { workspace, files, open, expect } = cases[name];
+ const { workspace, files, uri: uri, expectCreate: expectCreate, expectOpen: expectOpen } = cases[name];
const { ctrl, expl, ws } = newExplorer(workspace, files, DUT);
- const doc = ws.fs.files.get(open);
+ const doc = ws.fs.files.get(uri);
assert(doc);
- await expl._didOpen(doc);
- assertTestItems(ctrl.items, expect);
+ await expl._didCreate(doc);
+ assertTestItems(ctrl.items, expectCreate);
+
+ await expl._didOpen(doc);
+ assertTestItems(ctrl.items, expectOpen);
});
}
});
