cmd/coordinator: parse "header" instead of "banner"

In preparation of supporting a longer test header, change
parseOutputAndBanner to parseOutputAndHeader which returns the full
human-readable header.

The only behavior change is that output with only a banner and no
trailing newline will still return the banner as part of the header.

For golang/go#50146

Change-Id: Ie34fe3891e9bc29a3da79fddacd4829a89d2dbfb
Reviewed-on: https://go-review.googlesource.com/c/build/+/372537
Reviewed-by: Alex Rakoczy <[email protected]>
Trust: Michael Pratt <[email protected]>
Run-TryBot: Michael Pratt <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>

Author: prattmic

cmd/coordinator/buildstatus.go

@@ -1400,7 +1400,7 @@ func (st *buildStatus) runTests(helpers <-chan buildlet.Client) (remoteErr, err
 		close(buildletsGone)
 	}()
 
-	var lastBanner string
+	var lastHeader string
 	var serialDuration time.Duration
 	for _, ti := range set.items {
 	AwaitDone:
@@ -1420,10 +1420,10 @@ func (st *buildStatus) runTests(helpers <-chan buildlet.Client) (remoteErr, err
 
 		serialDuration += ti.execDuration
 		if len(ti.output) > 0 {
-			banner, out := parseOutputAndBanner(ti.output)
-			if banner != lastBanner {
-				lastBanner = banner
-				fmt.Fprintf(st, "\n##### %s\n", banner)
+			header, out := parseOutputAndHeader(ti.output)
+			if header != lastHeader {
+				lastHeader = header
+				fmt.Fprintf(st, "\n%s\n", header)
 			}
 			if pool.NewGCEConfiguration().InStaging() {
 				out = bytes.TrimSuffix(out, nl)
@@ -1454,20 +1454,32 @@ func (st *buildStatus) runTests(helpers <-chan buildlet.Client) (remoteErr, err
 const (
 	banner       = "XXXBANNERXXX:" // flag passed to dist
 	bannerPrefix = "\n" + banner   // with the newline added by dist
+
+	outputBanner = "##### " // banner to display in output.
 )
 
 var bannerPrefixBytes = []byte(bannerPrefix)
 
-func parseOutputAndBanner(b []byte) (banner string, out []byte) {
-	if bytes.HasPrefix(b, bannerPrefixBytes) {
-		b = b[len(bannerPrefixBytes):]
-		nl := bytes.IndexByte(b, '\n')
-		if nl != -1 {
-			banner = string(b[:nl])
-			b = b[nl+1:]
-		}
+// parseOutputAndHeader parses b and returns the test display header (e.g.,
+// "##### Testing packages.") and the following output.
+func parseOutputAndHeader(b []byte) (header string, out []byte) {
+	if !bytes.HasPrefix(b, bannerPrefixBytes) {
+		return "", b
+	}
+
+	b = b[1:] // skip newline
+	nl := bytes.IndexByte(b, '\n')
+	if nl == -1 {
+		header = string(b)
+		b = nil
+	} else {
+		header = string(b[:nl])
+		b = b[nl+1:]
 	}
-	return banner, b
+	// Replace internal marker banner with the human-friendly
+	// version.
+	header = strings.ReplaceAll(header, banner, outputBanner)
+	return header, b
 }
 
 // maxTestExecError is the number of test execution failures at which

cmd/coordinator/buildstatus_test.go

@@ -12,12 +12,12 @@ import (
 	"testing"
 )
 
-// TestParseOutputAndBanner tests banner parsing by parseOutputAndBanner.
-func TestParseOutputAndBanner(t *testing.T) {
+// TestParseOutputAndHeader tests header parsing by parseOutputAndHeader.
+func TestParseOutputAndHeader(t *testing.T) {
 	for _, tc := range []struct {
 		name       string
 		input      []byte
-		wantBanner string
+		wantHeader string
 		wantOut    []byte
 	}{
 		{
@@ -28,35 +28,34 @@ ok	archive/tar	0.015s
 ok	archive/zip	0.406s
 ok	bufio	0.075s
 `),
-			wantBanner: "Testing packages.",
+			wantHeader: "##### Testing packages.",
 			wantOut: []byte(`ok	archive/tar	0.015s
 ok	archive/zip	0.406s
 ok	bufio	0.075s
 `),
 		},
 		{
-			name: "banner only",
+			name: "header only",
 			input: []byte(`
 XXXBANNERXXX:Testing packages.
 `),
-			wantBanner: "Testing packages.",
+			wantHeader: "##### Testing packages.",
 			wantOut:    []byte(``),
 		},
 		{
-			// TODO(prattmic): This is likely not desirable behavior.
-			name: "banner only missing trailing newline",
+			name: "header only missing trailing newline",
 			input: []byte(`
 XXXBANNERXXX:Testing packages.`),
-			wantBanner: "",
-			wantOut:    []byte(`Testing packages.`),
+			wantHeader: "##### Testing packages.",
+			wantOut:    []byte(``),
 		},
 		{
 			name: "no banner",
 			input: []byte(`ok	archive/tar	0.015s
 ok	archive/zip	0.406s
 ok	bufio	0.075s
 `),
-			wantBanner: "",
+			wantHeader: "",
 			wantOut: []byte(`ok	archive/tar	0.015s
 ok	archive/zip	0.406s
 ok	bufio	0.075s
@@ -69,7 +68,7 @@ ok	archive/tar	0.015s
 ok	archive/zip	0.406s
 ok	bufio	0.075s
 `),
-			wantBanner: "",
+			wantHeader: "",
 			wantOut: []byte(`XXXBANNERXXX:Testing packages.
 ok	archive/tar	0.015s
 ok	archive/zip	0.406s
@@ -84,7 +83,7 @@ ok	archive/tar	0.015s
 ok	archive/zip	0.406s
 ok	bufio	0.075s
 `),
-			wantBanner: "",
+			wantHeader: "",
 			wantOut: []byte(`
 ##### Testing packages.
 ok	archive/tar	0.015s
@@ -94,9 +93,9 @@ ok	bufio	0.075s
 		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
-			gotBanner, gotOut := parseOutputAndBanner(tc.input)
-			if gotBanner != tc.wantBanner {
-				t.Errorf("parseOutputAndBanner(%q) got banner %q want banner %q", string(tc.input), gotBanner, tc.wantBanner)
+			gotHeader, gotOut := parseOutputAndHeader(tc.input)
+			if gotHeader != tc.wantHeader {
+				t.Errorf("parseOutputAndBanner(%q) got banner %q want banner %q", string(tc.input), gotHeader, tc.wantHeader)
 			}
 			if string(gotOut) != string(tc.wantOut) {
 				t.Errorf("parseOutputAndBanner(%q) got out %q want out %q", string(tc.input), string(gotOut), string(tc.wantOut))