runtime/trace: dump test traces on validation failure

We currently dump traces for internal/trace tests on validation failure,
but not for the runtime/trace package.

This change moves some of the machinery to do this into the testtrace
package and then uses it from the runtime/trace package.

For #75665.

Change-Id: Ibe2d4f3945c1fd21dcbccf56820865f8d2ea41f9
Reviewed-on: https://go-review.googlesource.com/c/go/+/710755
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

2 files changed, 12 insertions, 16 deletions

diff --git a/src/runtime/trace/flightrecorder_test.go b/src/runtime/trace/flightrecorder_test.go
index 61cb03dcf6..221f9c6b67 100644
--- a/src/runtime/trace/flightrecorder_test.go
+++ b/src/runtime/trace/flightrecorder_test.go
@@ -119,7 +119,7 @@ func TestFlightRecorderConcurrentWriteTo(t *testing.T) {
 		if buf.Len() == 0 {
 			continue
 		}
-		testReader(t, buf, testtrace.ExpectSuccess())
+		testReader(t, buf.Bytes(), testtrace.ExpectSuccess())
 	}
 }
 
@@ -260,12 +260,12 @@ func testFlightRecorder(t *testing.T, fr *trace.FlightRecorder, f flightRecorder
 	traceBytes := buf.Bytes()
 
 	// Parse the trace to make sure it's not broken.
-	testReader(t, bytes.NewReader(traceBytes), testtrace.ExpectSuccess())
+	testReader(t, traceBytes, testtrace.ExpectSuccess())
 	return traceBytes
 }
 
-func testReader(t *testing.T, tr io.Reader, exp *testtrace.Expectation) {
-	r, err := inttrace.NewReader(tr)
+func testReader(t *testing.T, tb []byte, exp *testtrace.Expectation) {
+	r, err := inttrace.NewReader(bytes.NewReader(tb))
 	if err != nil {
 		if err := exp.Check(err); err != nil {
 			t.Error(err)
@@ -292,6 +292,9 @@ func testReader(t *testing.T, tr io.Reader, exp *testtrace.Expectation) {
 	if err := exp.Check(nil); err != nil {
 		t.Error(err)
 	}
+	if t.Failed() || *dumpTraces {
+		testtrace.Dump(t, "trace", tb, *dumpTraces)
+	}
 }
 
 func TestTraceAndFlightRecorder(t *testing.T) {
diff --git a/src/runtime/trace/trace_test.go b/src/runtime/trace/trace_test.go
index b891c8c8f9..2174be061d 100644
--- a/src/runtime/trace/trace_test.go
+++ b/src/runtime/trace/trace_test.go
@@ -7,13 +7,16 @@ package trace_test
 import (
 	"bytes"
 	"flag"
-	"os"
 	. "runtime/trace"
 	"testing"
 	"time"
 )
 
-var saveTraces = flag.Bool("savetraces", false, "save traces collected by tests")
+var dumpTraces = flag.Bool("dump-traces", false, "dump traces to a file, even on success")
+
+// This file just contains smoke tests and tests of runtime/trace logic only.
+// It doesn't validate the resulting traces. See the internal/trace package for
+// more comprehensive end-to-end tests.
 
 func TestTraceStartStop(t *testing.T) {
 	if IsEnabled() {
@@ -32,7 +35,6 @@ func TestTraceStartStop(t *testing.T) {
 	if size != buf.Len() {
 		t.Fatalf("trace writes after stop: %v -> %v", size, buf.Len())
 	}
-	saveTrace(t, buf, "TestTraceStartStop")
 }
 
 func TestTraceDoubleStart(t *testing.T) {
@@ -50,12 +52,3 @@ func TestTraceDoubleStart(t *testing.T) {
 	Stop()
 	Stop()
 }
-
-func saveTrace(t *testing.T, buf *bytes.Buffer, name string) {
-	if !*saveTraces {
-		return
-	}
-	if err := os.WriteFile(name+".trace", buf.Bytes(), 0600); err != nil {
-		t.Errorf("failed to write trace file: %s", err)
-	}
-}