src/goCover: display coverage correctly for multiple packages

The output file from test coverage contains import paths, but when
vscode-go displays the coverage it only knows file system paths. The
code replaced by this CL used a heuristic that only got the mapping
right for a single package.

The tests do not start an editing session, but only check that the
import paths in the coverage file are properly converted to the
file system paths that vscode needs.

Change-Id: I60c8622a90134a18d0e64a239a865f0ba13ffb09
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/238697
Reviewed-by: Hyang-Ah Hana Kim <[email protected]>

src/goCover.ts

@@ -5,13 +5,15 @@
 
 'use strict';
 
+import cp = require('child_process');
 import fs = require('fs');
 import path = require('path');
-import rl = require('readline');
+import util = require('util');
 import vscode = require('vscode');
 import { isModSupported } from './goModules';
+import { envPath } from './goPath';
 import { getTestFlags, goTest, showTestOutput, TestConfig } from './testUtils';
-import { getGoConfig } from './util';
+import { getBinPath, getCurrentGoPath, getGoConfig, getWorkspaceFolderPath } from './util';
 
 let gutterSvgs: { [key: string]: string };
 let decorators: {
@@ -93,6 +95,8 @@ export function initCoverageDecorators(ctx: vscode.ExtensionContext) {
 export function updateCodeCoverageDecorators(coverageDecoratorConfig: any) {
 	// These defaults are chosen to be distinguishable in nearly any color scheme (even Red)
 	// as well as by people who have difficulties with color perception.
+	// (how do these relate the defaults in package.json?)
+	// and where do the defaults actually come from? (raised as issue #256)
 	decoratorConfig = {
 		type: 'highlight',
 		coveredHighlightColor: 'rgba(64,128,128,0.5)',
@@ -152,13 +156,17 @@ interface CoverageData {
 }
 
 let coverageFiles: { [key: string]: CoverageData } = {};
+let coveragePath = new Map<string, CoverageData>();
+let pathsToDirs = new Map<string, string>();
 let isCoverageApplied: boolean = false;
 
 /**
  * Clear the coverage on all files
  */
 function clearCoverage() {
 	coverageFiles = {};
+	coveragePath = new Map<string, CoverageData>();
+	pathsToDirs = new Map<string, string>();
 	disposeDecorators();
 	isCoverageApplied = false;
 }
@@ -167,48 +175,49 @@ function clearCoverage() {
  * Extract the coverage data from the given cover profile & apply them on the files in the open editors.
  * @param coverProfilePath Path to the file that has the cover profile data
  * @param packageDirPath Absolute path of the package for which the coverage was calculated
+ * @param testDir Directory to execute go list in, when there is no workspace, for some tests
  */
-export function applyCodeCoverageToAllEditors(coverProfilePath: string, packageDirPath: string): Promise<void> {
-	return new Promise((resolve, reject) => {
+export function applyCodeCoverageToAllEditors(coverProfilePath: string, testDir?: string): Promise<void> {
+	const v = new Promise<void>((resolve, reject) => {
 		try {
 			// Clear existing coverage files
 			clearCoverage();
 
-			const lines = rl.createInterface({
-				input: fs.createReadStream(coverProfilePath),
-				output: undefined
-			});
-
-			lines.on('line', (data: string) => {
-				// go test coverageprofile generates output:
-				//    filename:StartLine.StartColumn,EndLine.EndColumn Hits CoverCount
-				// The first line will be "mode: set" which will be ignored
-				const fileRange = data.match(/([^:]+)\:([\d]+)\.([\d]+)\,([\d]+)\.([\d]+)\s([\d]+)\s([\d]+)/);
-				if (!fileRange) {
-					return;
+			// collect the packages named in the coverage file
+			const seenPaths = new Set<string>();
+			// for now read synchronously and hope for no errors
+			const contents = fs.readFileSync(coverProfilePath).toString();
+			contents.split('\n').forEach((line) => {
+				const parse = line.match(/([^:]+)\:([\d]+)\.([\d]+)\,([\d]+)\.([\d]+)\s([\d]+)\s([\d]+)/);
+				if (!parse) { return; }
+				const lastSlash = parse[1].lastIndexOf('/'); // ok for windows?
+				if (lastSlash !== -1) {
+					seenPaths.add(parse[1].slice(0, lastSlash));
 				}
 
-				const filePath = path.join(packageDirPath, path.basename(fileRange[1]));
-				const coverage = getCoverageData(filePath);
+				// and fill in coveragePath
+				const coverage = getPathData(parse[1]);
 				const range = new vscode.Range(
 					// Start Line converted to zero based
-					parseInt(fileRange[2], 10) - 1,
+					parseInt(parse[2], 10) - 1,
 					// Start Column converted to zero based
-					parseInt(fileRange[3], 10) - 1,
+					parseInt(parse[3], 10) - 1,
 					// End Line converted to zero based
-					parseInt(fileRange[4], 10) - 1,
+					parseInt(parse[4], 10) - 1,
 					// End Column converted to zero based
-					parseInt(fileRange[5], 10) - 1
+					parseInt(parse[5], 10) - 1
 				);
 				// If is Covered (CoverCount > 0)
-				if (parseInt(fileRange[7], 10) > 0) {
+				if (parseInt(parse[7], 10) > 0) {
 					coverage.coveredRange.push(range);
 				} else {
 					coverage.uncoveredRange.push(range);
 				}
-				setCoverageData(filePath, coverage);
+				setPathData(parse[1], coverage);
 			});
-			lines.on('close', () => {
+			const pathPromise = getPathsToDirs(seenPaths, pathsToDirs, testDir);
+			pathPromise.then(() => {
+				createCoverageData();
 				setDecorators();
 				vscode.window.visibleTextEditors.forEach(applyCodeCoverage);
 				resolve();
@@ -218,6 +227,7 @@ export function applyCodeCoverageToAllEditors(coverProfilePath: string, packageD
 			reject(e);
 		}
 	});
+	return v;
 }
 
 /**
@@ -237,6 +247,33 @@ function getCoverageData(filePath: string): CoverageData {
 	return coverageFiles[filePath] || { coveredRange: [], uncoveredRange: [] };
 }
 
+/**
+ * Get the CoverageData for an import path.
+ * @param importPath
+ */
+function getPathData(importPath: string): CoverageData {
+	return coveragePath.get(importPath) || { coveredRange: [], uncoveredRange: [] };
+}
+
+/**
+ * Set the CoverageData for an import path.
+ * @param importPath
+ * @param data
+ */
+function setPathData(importPath: string, data: CoverageData) {
+	coveragePath.set(importPath, data);
+}
+
+function createCoverageData() {
+	coveragePath.forEach((cd, ip) => {
+		const lastSlash = ip.lastIndexOf('/');
+		const importPath = ip.slice(0, lastSlash);
+		const fileDir = pathsToDirs.get(importPath);
+		const file = fileDir + ip.slice(lastSlash); // what about Windows?
+		setCoverageData(file, cd);
+	});
+}
+
 /**
  * Set the object that holds the coverage data for given file path.
  * @param filePath
@@ -333,6 +370,41 @@ export function trackCodeCoverageRemovalOnFileChange(e: vscode.TextDocumentChang
 	modifiedFiles[e.document.fileName] = true;
 }
 
+/**
+ * Fill the map of directory paths corresponding to input package paths
+ * @param set Set<string> of import package paths
+ */
+async function getPathsToDirs(set: Set<string>, res: Map<string, string>, testDir?: string) {
+	const goRuntimePath = getBinPath('go');
+	if (!goRuntimePath) {
+		vscode.window.showErrorMessage(
+			`Failed to run, as the "go" binary cannot be found in either GOROOT(${process.env['GOROOT']}) or PATH(${envPath})`
+		);
+	}
+	const args: string[] = ['list', '-f', '{{.ImportPath}}:{{.Dir}}'];
+	set.forEach((s) => args.push(s));
+
+	const options: { [key: string]: any } = {
+		env: Object.assign({}, process.env, { GOPATH: getCurrentGoPath() })
+	};
+	const workDir = getWorkspaceFolderPath();
+	// If there is a workDir then probably it is what we want
+	// Otherwise maybe a test suggested a directory.
+	if (workDir) {
+		options['cwd'] = workDir;
+	} else if (testDir) {
+		options['cwd'] = testDir;
+	}
+
+	const execFile = util.promisify(cp.execFile);
+	const { stdout } = await execFile(goRuntimePath, args, options);
+	stdout.split('\n').forEach((line) => {
+		const flds = line.split(':');
+		if (flds.length !== 2) { return; }
+		res.set(flds[0], flds[1]);
+	});
+}
+
 /**
  * If current editor has Code coverage applied, then remove it.
  * Else run tests to get the coverage and apply.
@@ -383,3 +455,27 @@ export function isPartOfComment(e: vscode.TextDocumentChangeEvent): boolean {
 		return idx > -1 && idx <= change.range.start.character;
 	});
 }
+
+// These routines enable testing without starting an editing session.
+
+export function coverageFilesForTest():  { [key: string]: CoverageData; } {
+	return coverageFiles;
+}
+
+export function initForTest() {
+	if (!decoratorConfig) {
+		// this code is unnecessary except for testing, where there may be no workspace
+		// nor the normal flow of initializations
+		const x = 'rgba(0,0,0,0)';
+		if (!gutterSvgs) {
+			gutterSvgs = { x };
+		}
+		decoratorConfig = {
+			type: 'highlight',
+			coveredHighlightColor: x,
+			uncoveredHighlightColor: x,
+			coveredGutterStyle: x,
+			uncoveredGutterStyle: x
+		};
+	}
+}

src/goMain.ts

@@ -57,7 +57,7 @@ import {
 	getToolsGopath,
 	getWorkspaceFolderPath,
 	handleDiagnosticErrors,
-	isGoPathSet,
+	isGoPathSet
 } from './util';
 
 export let buildDiagnosticCollection: vscode.DiagnosticCollection;
@@ -554,8 +554,7 @@ export function activate(ctx: vscode.ExtensionContext): void {
 						updateWorkspaceState(lastCoverProfilePathKey, coverProfilePath);
 					}
 					applyCodeCoverageToAllEditors(
-						coverProfilePath,
-						path.dirname(vscode.window.activeTextEditor.document.fileName)
+						coverProfilePath
 					);
 				});
 		})

src/testUtils.ts

@@ -20,7 +20,7 @@ import {
 	getTempFilePath,
 	killTree,
 	LineBuffer,
-	resolvePath,
+	resolvePath
 } from './util';
 
 const outputChannel = vscode.window.createOutputChannel('Go Tests');
@@ -402,7 +402,7 @@ export async function goTest(testconfig: TestConfig): Promise<boolean> {
 		);
 	});
 	if (testconfig.applyCodeCoverage) {
-		await applyCodeCoverageToAllEditors(tmpCoverPath, testconfig.dir);
+		await applyCodeCoverageToAllEditors(tmpCoverPath);
 	}
 	return testResult;
 }

test/fixtures/coverage/a/a.go

@@ -0,0 +1,7 @@
+package a
+
+func main() {
+	x := 12
+	y := x + 17
+	panic(y)
+}

test/fixtures/coverage/b/b.go

@@ -0,0 +1,11 @@
+package b
+
+import (
+	"fmt"
+	"os"
+)
+
+func main() {
+	v := os.Env()
+	fmt.Print(v)
+}

test/fixtures/coverage/cover.out

@@ -0,0 +1,3 @@
+mode: set
+github.com/microsoft/vscode-go/gofixtures/coveragetest/a/a.go:19.71,22.25 3 1
+github.com/microsoft/vscode-go/gofixtures/coveragetest/b/b.go:35.2,35.14 1 1

test/fixtures/coverage/go.mod

@@ -0,0 +1,3 @@
+module github.com/microsoft/vscode-go/gofixtures/coveragetest
+
+go 1.14

test/integration/coverage.test.ts

@@ -0,0 +1,49 @@
+/*---------------------------------------------------------
+ * Copyright 2020 The Go Authors. All rights reserved.
+ * Licensed under the MIT License. See LICENSE in the project root for license information.
+ *--------------------------------------------------------*/
+
+'use strict';
+
+import * as assert from 'assert';
+import fs = require('fs-extra');
+import path = require('path');
+import sinon = require('sinon');
+import vscode = require('vscode');
+import { applyCodeCoverageToAllEditors, coverageFilesForTest, initForTest } from '../../src/goCover';
+import { updateGoVarsFromConfig } from '../../src/goInstallTools';
+import { getCurrentGoPath, getWorkspaceFolderPath } from '../../src/util';
+
+// The ideal test would check that each open editor containing a file with coverage
+// information is displayed correctly. We cannot see the applied decorations, so the
+// test checks that the cover.out file has been read correctly, and the import paths
+// have been correctly converted to file system paths, which are what vscode uses.
+suite('Coverage for tests', function () {
+	this.timeout(10000);
+
+	let fixtureSourcePath: string;
+	let coverFilePath: string;
+
+	suiteSetup(async () => {
+		await updateGoVarsFromConfig();
+
+		// Set up the test fixtures.
+		fixtureSourcePath = path.join(__dirname, '..', '..', '..', 'test', 'fixtures', 'coverage');
+		coverFilePath = path.join(fixtureSourcePath, 'cover.out');
+		return;
+	});
+	test('resolve import paths', async () => {
+		initForTest();
+		const x = vscode.workspace.openTextDocument(coverFilePath);
+		await applyCodeCoverageToAllEditors(coverFilePath, fixtureSourcePath);
+		let aDotGo: boolean;
+		let bDotGo: boolean;
+		for (const fn in coverageFilesForTest()) {
+			if (true) { // TSLint insists the body for for..in.. be an if-statement
+				if (fn === `${fixtureSourcePath}/a/a.go`) { aDotGo = true; }
+				if (fn === `${fixtureSourcePath}/b/b.go`) { bDotGo = true; }
+		}
+	}
+		assert.equal(aDotGo && bDotGo, true);
+	});
+});