From 83f62c7fb1380379f7161cbc04ec8f575aa95522 Mon Sep 17 00:00:00 2001
From: Ethan Reesor <ethan.reesor@gmail.com>
Date: Fri, 15 Nov 2024 16:20:39 -0700
Subject: [PATCH] Improve test events for benchmarks

---
 src/test/manager.ts             |  2 +-
 src/test/runner.ts              |  2 +-
 src/test/{run.ts => testRun.ts} | 65 +++++++++++++++++++--------------
 src/test/utils.ts               |  4 ++
 4 files changed, 44 insertions(+), 29 deletions(-)
 rename src/test/{run.ts => testRun.ts} (91%)

diff --git a/src/test/manager.ts b/src/test/manager.ts
index 8271564..81a127d 100644
--- a/src/test/manager.ts
+++ b/src/test/manager.ts
@@ -7,7 +7,7 @@ import { Context, doSafe, Tail, TestController } from './testing';
 import { TestResolver } from './resolver';
 import { GoTestItem } from './item';
 import { TestRunner } from './runner';
-import { TestRunRequest } from './run';
+import { TestRunRequest } from './testRun';
 import { CodeLensProvider } from './codeLens';
 import { EventEmitter } from '../utils/eventEmitter';
 import { RunConfig } from './config';
diff --git a/src/test/runner.ts b/src/test/runner.ts
index 74d9cd8..a345ec5 100644
--- a/src/test/runner.ts
+++ b/src/test/runner.ts
@@ -4,7 +4,7 @@ import { CancellationToken, FileCoverage, TestRun, TestRunProfileKind, Uri } fro
 import type vscode from 'vscode';
 import { Package, StaticTestCase, TestCase, TestFile } from './item';
 import { Context, Workspace } from './testing';
-import { PackageTestRun, TestRunRequest } from './run';
+import { PackageTestRun, TestRunRequest } from './testRun';
 import { flags2args, Spawner } from './utils';
 import { ProfileType } from './profile';
 import { TestResolver } from './resolver';
diff --git a/src/test/run.ts b/src/test/testRun.ts
similarity index 91%
rename from src/test/run.ts
rename to src/test/testRun.ts
index da1f4b8..32f0670 100644
--- a/src/test/run.ts
+++ b/src/test/testRun.ts
@@ -255,40 +255,51 @@ export class PackageTestRun {
 		const item = test && (await this.#request.manager.resolveTestItem(test, true));
 
 		const elapsed = typeof msg.Elapsed === 'number' ? msg.Elapsed * 1000 : undefined;
-		switch (msg.Action) {
-			case 'output': {
-				if (!msg.Output) {
-					break;
-				}
-
-				// Track output
-				const { id } = item || this.testItem;
-				if (!this.output.has(id)) {
-					this.output.set(id, []);
-				}
-				this.output.get(id)!.push(msg.Output);
-
-				if (!item || /^(=== RUN|\s*--- (FAIL|PASS): )/.test(msg.Output)) {
-					this.append(msg.Output, undefined, this.testItem);
-					break;
-				}
+		if (msg.Action === 'output') {
+			if (!msg.Output) {
+				return;
+			}
 
-				const { message, location } = parseOutputLocation(msg.Output, path.join(item.uri!.fsPath, '..'));
+			// Track output
+			const { id } = item || this.testItem;
+			if (!this.output.has(id)) {
+				this.output.set(id, []);
+			}
+			this.output.get(id)!.push(msg.Output);
+
+			let location: Location | undefined;
+			let message = msg.Output;
+			if (item && !/^(=== RUN|\s*--- (FAIL|PASS): )/.test(msg.Output)) {
+				const parsed = parseOutputLocation(msg.Output, path.join(item.uri!.fsPath, '..'));
+				message = parsed.message;
+				location = parsed.location;
 				if (location) {
 					this.currentLocation.set(id, location);
+				} else {
+					location = this.currentLocation.get(id);
 				}
-				this.append(message, location || this.currentLocation.get(id), item);
-
-				// Detect benchmark completion, e.g.
-				//   "BenchmarkFooBar-4    123456    123.4 ns/op    123 B/op    12 allocs/op"
-				const m = msg.Output.match(/^(?<name>Benchmark[#/\w+]+)(?:-(?<procs>\d+)\s+(?<result>.*))?(?:$|\n)/);
-				if (m && msg.Test && m.groups?.name === msg.Test) {
-					this.#run.passed(item);
-				}
+			}
+			this.append(message, location, item || this.testItem);
 
-				break;
+			// go test is not good about reporting the start and end of benchmarks
+			// so we'll synthesize those events to make life easier.
+			if (!msg.Test?.startsWith('Benchmark')) {
+				return;
 			}
+			if (msg.Output === `=== RUN   ${msg.Test}\n`) {
+				// === RUN   BenchmarkFooBar
+				msg.Action = 'run';
+			} else if (
+				msg.Output?.match(/^(?<name>Benchmark[/\w]+)-(?<procs>\d+)\s+(?<result>.*)(?:$|\n)/)?.[1] === msg.Test
+			) {
+				// BenchmarkFooBar-4    123456    123.4 ns/op    123 B/op    12 allocs/op
+				msg.Action = 'pass';
+			} else {
+				return;
+			}
+		}
 
+		switch (msg.Action) {
 			case 'run':
 			case 'start':
 				if (!msg.Test) {
diff --git a/src/test/utils.ts b/src/test/utils.ts
index 196ac56..c5c81e3 100644
--- a/src/test/utils.ts
+++ b/src/test/utils.ts
@@ -53,7 +53,11 @@ export function spawnProcess(context: Context, scope: Uri, flags: Flags, options
 		errbuf.onLine(stderr);
 		errbuf.onDone((x) => x && stderr(x));
 
+		// Always use -json, but don't combine it with -v because weird things
+		// happen (https://github.com/golang/go/issues/70384)
 		flags.json = true;
+		delete flags.v;
+
 		const tp = cp.spawn(binPath, ['test', ...flags2args(flags)], {
 			...rest,
 			stdio: 'pipe',