extension/test: restore process.env modified during testing

Some tests mutate process.env, or call updateGoVarsFromConfig that
mutates process.env (setting GOROOT, GOPATH, GOMODCACHE, GO111MODULE...)
That makes subsequent tests operate in strange state.

Fix tests to recover the process env state cached from suite/test start.

The ideal solution would be to avoid mutating process.env whenever
possible. There were brief attempts to clean up this and replace
updateGoVarsFromConfig with a Configuration storage object and
dependency injection, but they were not complete yet.

Change-Id: Iecd3bb639b50875a3df652f2060c2c836499913e
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/619776
kokoro-CI: kokoro <[email protected]>
Commit-Queue: Hyang-Ah Hana Kim <[email protected]>
Reviewed-by: Hongxiang Jiang <[email protected]>

Author: hyangah

extension/src/goEnv.ts

@@ -26,20 +26,6 @@ export function toolInstallationEnvironment(): NodeJS.Dict<string> {
 	} else {
 		toolsGopath = getCurrentGoPath();
 	}
-	if (!toolsGopath) {
-		const msg = 'Cannot install Go tools. Set either go.gopath or go.toolsGopath in settings.';
-		vscode.window.showInformationMessage(msg, 'Open User Settings', 'Open Workspace Settings').then((selected) => {
-			switch (selected) {
-				case 'Open User Settings':
-					vscode.commands.executeCommand('workbench.action.openGlobalSettings');
-					break;
-				case 'Open Workspace Settings':
-					vscode.commands.executeCommand('workbench.action.openWorkspaceSettings');
-					break;
-			}
-		});
-		return {};
-	}
 	env['GOPATH'] = toolsGopath;
 
 	// Explicitly set 'auto' so tools that require

extension/src/goInstallTools.ts

@@ -480,6 +480,9 @@ export async function promptForUpdatingTool(
 }
 
 export function updateGoVarsFromConfig(goCtx: GoExtensionContext): Promise<void> {
+	// TODO(hyangah): can we avoid modifying process.env? The `go env` output
+	// can be cached in memory and queried when functions in goEnv.ts are called
+	// instead of mutating process.env.
 	const { binPath, why } = getBinPathWithExplanation('go', false);
 	const goRuntimePath = binPath;
 
@@ -492,11 +495,14 @@ export function updateGoVarsFromConfig(goCtx: GoExtensionContext): Promise<void>
 	}
 
 	return new Promise<void>((resolve, reject) => {
+		const env = toolExecutionEnvironment();
+		const cwd = getWorkspaceFolderPath();
+
 		cp.execFile(
 			goRuntimePath,
 			// -json is supported since go1.9
 			['env', '-json', 'GOPATH', 'GOROOT', 'GOPROXY', 'GOBIN', 'GOMODCACHE'],
-			{ env: toolExecutionEnvironment(), cwd: getWorkspaceFolderPath() },
+			{ env: env, cwd: cwd },
 			(err, stdout, stderr) => {
 				if (err) {
 					outputChannel.append(

extension/test/gopls/codelens.test.ts

@@ -37,6 +37,10 @@ suite('Code lenses for testing and benchmarking', function () {
 		sinon.restore();
 	});
 
+	// updaetGoVarsFromConfig mutates env vars. Cache the value
+	// so we can restore it in suiteTeardown.
+	const prevEnv = Object.assign({}, process.env);
+
 	suiteSetup(async () => {
 		await updateGoVarsFromConfig({});
 
@@ -47,6 +51,7 @@ suite('Code lenses for testing and benchmarking', function () {
 
 	suiteTeardown(async () => {
 		await env.teardown();
+		process.env = prevEnv;
 	});
 
 	test('Subtests - runs a test with cursor on t.Run line', async () => {

extension/test/gopls/goTest.run.test.ts

@@ -11,13 +11,20 @@ import { Env } from './goplsTestEnv.utils';
 import { updateGoVarsFromConfig } from '../../src/goInstallTools';
 
 suite('Go Test Runner', () => {
+	// updateGoVarsFromConfig mutates process.env. Restore the cached
+	// prevEnv when teardown.
+	// TODO: avoid updateGoVarsFromConfig call.
+	const prevEnv = Object.assign({}, process.env);
 	const fixtureDir = path.join(__dirname, '..', '..', '..', 'test', 'testdata');
 
 	let testExplorer: GoTestExplorer;
 
 	suiteSetup(async () => {
 		await updateGoVarsFromConfig({});
 	});
+	suiteTeardown(() => {
+		process.env = prevEnv;
+	});
 
 	suite('parseOutput', () => {
 		const ctx = MockExtensionContext.new();
@@ -170,10 +177,13 @@ suite('Go Test Runner', () => {
 	});
 
 	suite('Subtest', function () {
+		// This test is slow, especially on Windows.
 		// WARNING: each call to testExplorer.runner.run triggers one or more
 		// `go test` command runs (testUtils.goTest is spied, not mocked or replaced).
 		// Each `go test` command invocation can take seconds on slow machines.
 		// As we add more cases, the timeout should be increased accordingly.
+		this.timeout(20000); // I don't know why but timeout chained after `suite` didn't work.
+
 		const sandbox = sinon.createSandbox();
 		const subTestDir = path.join(fixtureDir, 'subTest');
 		const ctx = MockExtensionContext.new();
@@ -204,7 +214,6 @@ suite('Go Test Runner', () => {
 		});
 
 		test('discover and run', async () => {
-			console.log('discover and run');
 			// 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));
@@ -284,6 +293,6 @@ suite('Go Test Runner', () => {
 				'Failed to execute `go test`'
 			);
 			assert.strictEqual(spy.callCount, 0, 'expected no calls to goTest');
-		}).timeout(10000);
+		});
 	});
 });

extension/test/integration/coverage.test.ts

@@ -22,6 +22,9 @@ suite('Coverage for tests', function () {
 	let fixtureSourcePath: string;
 	let coverFilePath: string;
 
+	// updateGoVarsFromConfig mutates process.env. Restore to prevEnv in suiteTeardown.
+	// TODO: avoid updateGoVarsFromConfig.
+	const prevEnv = Object.assign({}, process.env);
 	suiteSetup(async () => {
 		await updateGoVarsFromConfig({});
 
@@ -30,6 +33,9 @@ suite('Coverage for tests', function () {
 		coverFilePath = path.join(fixtureSourcePath, 'cover.out');
 		return;
 	});
+	suiteTeardown(() => {
+		process.env = prevEnv;
+	});
 	test('resolve import paths', async () => {
 		initForTest();
 		const x = vscode.workspace.openTextDocument(coverFilePath);

extension/test/integration/goDebugConfiguration.test.ts

@@ -23,13 +23,18 @@ suite('Debug Environment Variable Merge Test', () => {
 	const fixtureSourcePath = path.join(__dirname, '..', '..', '..', 'test', 'testdata');
 	const filePath = path.join(fixtureSourcePath, 'baseTest', 'test.go');
 
+	// updateGoVarsFromConfig mutates process.env.
+	// Stash the original value and restore it in suiteTeardown.
+	// TODO: avoid updateGoVarsFromConfig.
+	const prevEnv = Object.assign({}, process.env);
 	suiteSetup(async () => {
 		await goInstallTools.updateGoVarsFromConfig({});
 		await vscode.workspace.openTextDocument(vscode.Uri.file(filePath));
 	});
 
 	suiteTeardown(() => {
 		vscode.commands.executeCommand('workbench.action.closeActiveEditor');
+		process.env = prevEnv;
 	});
 
 	let sandbox: sinon.SinonSandbox;

extension/test/integration/statusbar.test.ts

@@ -29,12 +29,14 @@ import ourutil = require('../../src/util');
 import { setGOROOTEnvVar } from '../../src/goEnv';
 
 describe('#initGoStatusBar()', function () {
+	const prevEnv = Object.assign({}, process.env);
 	this.beforeAll(async () => {
 		await updateGoVarsFromConfig({}); // should initialize the status bar.
 	});
 
 	this.afterAll(() => {
 		disposeGoStatusBar();
+		process.env = prevEnv;
 	});
 
 	it('should create a status bar item', () => {