src/goTestExplorer: explain benchmark output

- Add test_events.md to detail what go test -json looks like
- Add comments
- Also, address gerrit comments

src/goTestExplorer.ts

@@ -262,8 +262,8 @@ async function processSymbol(
 	seen: Set<string>,
 	symbol: DocumentSymbol
 ) {
-	// Skip TestMain(*testing.M)
-	if (symbol.name === 'TestMain' || /\*testing.M\)/.test(symbol.detail)) {
+	// Skip TestMain(*testing.M) - allow TestMain(*testing.T)
+	if (symbol.name === 'TestMain' && /\*testing.M\)/.test(symbol.detail)) {
 		return;
 	}
 
@@ -325,7 +325,7 @@ async function walk(
 	let dirs = [uri];
 
 	// While there are directories to be scanned
-	while (dirs.length) {
+	while (dirs.length > 0) {
 		const d = dirs;
 		dirs = [];
 
@@ -561,6 +561,7 @@ function resolveTestName(ctrl: TestController, tests: Record<string, TestItem>,
 	return test;
 }
 
+// Process benchmark test events (see test_events.md)
 function consumeGoBenchmarkEvent<T>(
 	ctrl: TestController,
 	run: TestRun<T>,
@@ -569,18 +570,19 @@ function consumeGoBenchmarkEvent<T>(
 	e: GoTestOutput
 ) {
 	if (e.Test) {
+		// Find (or create) the (sub)benchmark
 		const test = resolveTestName(ctrl, benchmarks, e.Test);
 		if (!test) {
 			return;
 		}
 
 		switch (e.Action) {
-			case 'fail':
+			case 'fail': // Failed
 				run.setState(test, TestResultState.Failed);
 				complete.add(test);
 				break;
 
-			case 'skip':
+			case 'skip': // Skipped
 				run.setState(test, TestResultState.Skipped);
 				complete.add(test);
 				break;
@@ -589,22 +591,29 @@ function consumeGoBenchmarkEvent<T>(
 		return;
 	}
 
+	// Ignore anything that's not an output event
 	if (!e.Output) {
 		return;
 	}
 
-	// Started: "BenchmarkFooBar"
-	// Completed: "BenchmarkFooBar-4    123456    123.4 ns/op    123 B/op    12 allocs/op"
+	// On start:    "BenchmarkFooBar"
+	// On complete: "BenchmarkFooBar-4    123456    123.4 ns/op    123 B/op    12 allocs/op"
+
+	// Extract the benchmark name and status
 	const m = e.Output.match(/^(?<name>Benchmark[/\w]+)(?:-(?<procs>\d+)\s+(?<result>.*))?(?:$|\n)/);
 	if (!m) {
+		// If the output doesn't start with `BenchmarkFooBar`, ignore it
 		return;
 	}
 
+	// Find (or create) the (sub)benchmark
 	const test = resolveTestName(ctrl, benchmarks, m.groups.name);
 	if (!test) {
 		return;
 	}
 
+	// If output includes benchmark results, the benchmark passed. If output
+	// only includes the benchmark name, the benchmark is running.
 	if (m.groups.result) {
 		run.appendMessage(test, {
 			message: m.groups.result,
@@ -618,6 +627,7 @@ function consumeGoBenchmarkEvent<T>(
 	}
 }
 
+// Pass any incomplete benchmarks (see test_events.md)
 function passBenchmarks<T>(run: TestRun<T>, items: Record<string, TestItem>, complete: Set<TestItem>) {
 	function pass(item: TestItem) {
 		if (!complete.has(item)) {
@@ -746,7 +756,7 @@ async function runTest<T>(ctrl: TestController, request: TestRunRequest<T>) {
 		for (const item of items) {
 			run.setState(item, TestResultState.Queued);
 
-			// Remove any subtests
+			// Clear any dynamic subtests generated by a previous run
 			item.canResolveChildren = false;
 			Array.from(item.children.values()).forEach((x) => x.dispose());
 
@@ -762,7 +772,8 @@ async function runTest<T>(ctrl: TestController, request: TestRunRequest<T>) {
 		const testFns = Object.keys(tests);
 		const benchmarkFns = Object.keys(benchmarks);
 
-		if (testFns.length) {
+		if (testFns.length > 0) {
+			// Run tests
 			await goTest({
 				goConfig,
 				flags,
@@ -774,7 +785,8 @@ async function runTest<T>(ctrl: TestController, request: TestRunRequest<T>) {
 			});
 		}
 
-		if (benchmarkFns.length) {
+		if (benchmarkFns.length > 0) {
+			// Run benchmarks
 			const complete = new Set<TestItem>();
 			await goTest({
 				goConfig,
@@ -787,6 +799,7 @@ async function runTest<T>(ctrl: TestController, request: TestRunRequest<T>) {
 				goTestOutputConsumer: (e) => consumeGoBenchmarkEvent(ctrl, run, benchmarks, complete, e)
 			});
 
+			// Explicitly pass any incomplete benchmarks (see test_events.md)
 			passBenchmarks(run, benchmarks, complete);
 		}
 

src/test_events.md

@@ -0,0 +1,38 @@
+# Go test events
+
+Running tests with the `-json` flag or passing test output through `go tool
+test2json1` will produce a stream of JSON events. Each event specifies an
+action, such as `run`, `pass`, `output`, etc. An event *may* specify what test
+it belongs to. The VSCode Go test controller must capture these events in order
+to notify VSCode of test output and lifecycle events.
+
+## Tests
+
+Processing test events generated by `TestXxx(*testing.T)` functions is easy.
+Events with an empty `Test` field can be ignored, and all other events have a
+meaningful `Action` field. Output is recorded, and run/pass/fail/skip events are
+converted to VSCode test API events.
+
+[go#37555](https://github.com/golang/go/issues/37555) did require special
+handling, but that only appeared in Go 1.14 and was backported to 1.14.1.
+
+## Benchmarks
+
+Test events generated by `BenchmarkXxx(*testing.B)` functions require
+significantly more processing. If a benchmark fails or is skipped, the `Test`
+and `Action` fields are populated appropriately. Otherwise, `Test` is empty and
+`Action` is always `output`. Thus, nominal lifecycle events (run/pass) must be
+deduced purely from test output. When a benchmark begins, an output such as
+`BenchmarkFooBar\n` is produced. When a benchmark completes, an output such as
+`BencmarkFooBar-4  123456  123.4 ns/op  123 B/op  12 allocs/op` is produced. No
+explicit `run` or `pass` events are generated. Thus:
+
+- When `BenchmarkFooBar\n` is seen, the benchmark will be marked as running
+- When an explicit fail/skip is seen, the benchmark will be marked as failed/skipped
+- When benchmark results are seen, the benchmark will be marked as passed
+
+Thus, a benchmark that does not produce results (and does not fail or skip) will
+never produce an event indicating that it has completed. Benchmarks that call
+`(*testing.B).Run` will not produce results. In practice, this means that any
+incomplete benchmarks must be explicitly marked as passed once `go test`
+returns.