[release] 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 #2570

Change-Id: I595f34daee257c85bafd3e5706176f73f891fdf1
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/458999
Reviewed-by: Hyang-Ah Hana Kim <[email protected]>
Run-TryBot: Suzy Mueller <[email protected]>
TryBot-Result: kokoro <[email protected]>
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/462376
Run-TryBot: Hyang-Ah Hana Kim <[email protected]>
Reviewed-by: Robert Findley <[email protected]>

Author: suzmue

src/goDocumentSymbols.ts

@@ -25,6 +25,10 @@ export class GoplsDocumentSymbolProvider implements vscode.DocumentSymbolProvide
 	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;

src/goTest/explore.ts

@@ -258,7 +258,12 @@ export class GoTestExplorer {
 	}
 
 	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) {

src/goTest/resolve.ts

@@ -233,6 +233,21 @@ export class GoTestResolver {
 		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 @@ export class GoTestResolver {
 		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;

test/integration/goTest.explore.test.ts

@@ -60,11 +60,16 @@ suite('Go Test Explorer', () => {
 			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 @@ suite('Go Test Explorer', () => {
 					'/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 @@ suite('Go Test Explorer', () => {
 					'/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 @@ suite('Go Test Explorer', () => {
 
 		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);
 			});
 		}
 	});