Skip to content

Commit 2ea69a3

Browse files
prattmicprattmic
committed
cmd/coordinator: collapse duplicate execution headers
Following CL 371474, cmd/dist may include a "Test execution environment" header which describes the test execution environment. When sharding tests, we want to surface the proper execution environment for each test, which may vary from shard to shard. We could simply print the metadata for each shard in its output, as shard results are printed in an atomic block. However, shards are pretty small and that adds quite a bit of noise to logs. Instead, treat the metadata block like the test header banners: as long as the metadata doesn't change, we don't need to print it again. On the other hand, if the metadata changes, we do print the test header banner again. This isn't strictly necessary, it just serves to improve readability by ensuring that tests are always immediately preceeded by their banner rather than metadata (in the case that metadata changes in the middle of a header block). This CL should be submitted and deployed before CL 371474. For golang/go#50146. Change-Id: Ifca30f7f31237fd8cd0fcd801d198d9c341f695e Reviewed-on: https://go-review.googlesource.com/c/build/+/372538 Trust: Michael Pratt <[email protected]> Run-TryBot: Michael Pratt <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Alex Rakoczy <[email protected]>
1 parent 8ddb22a commit 2ea69a3

File tree

2 files changed

+168
-25
lines changed

2 files changed

+168
-25
lines changed

‎cmd/coordinator/buildstatus.go

Lines changed: 59 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1496,6 +1496,7 @@ func (st *buildStatus) runTests(helpers <-chan buildlet.Client) (remoteErr, err
14961496
close(buildletsGone)
14971497
}()
14981498

1499+
var lastMetadata string
14991500
var lastHeader string
15001501
var serialDuration time.Duration
15011502
for _, ti := range set.items {
@@ -1516,9 +1517,24 @@ func (st *buildStatus) runTests(helpers <-chan buildlet.Client) (remoteErr, err
15161517

15171518
serialDuration += ti.execDuration
15181519
if len(ti.output) > 0 {
1519-
header, out := parseOutputAndHeader(ti.output)
1520+
metadata, header, out := parseOutputAndHeader(ti.output)
1521+
printHeader := false
1522+
if metadata != lastMetadata {
1523+
lastMetadata = metadata
1524+
fmt.Fprintf(st, "\n%s\n", metadata)
1525+
// Always include the test header after
1526+
// metadata changes. This is a readability
1527+
// optimization that ensures that tests are
1528+
// always immediately preceeded by their test
1529+
// banner, even if it is duplicate banner
1530+
// because the test metadata changed.
1531+
printHeader = true
1532+
}
15201533
if header != lastHeader {
15211534
lastHeader = header
1535+
printHeader = true
1536+
}
1537+
if printHeader {
15221538
fmt.Fprintf(st, "\n%s\n", header)
15231539
}
15241540
if pool.NewGCEConfiguration().InStaging() {
@@ -1551,31 +1567,63 @@ const (
15511567
banner = "XXXBANNERXXX:" // flag passed to dist
15521568
bannerPrefix = "\n" + banner // with the newline added by dist
15531569

1570+
metadataBannerPrefix = bannerPrefix + "Test execution environment."
1571+
15541572
outputBanner = "##### " // banner to display in output.
15551573
)
15561574

1557-
var bannerPrefixBytes = []byte(bannerPrefix)
1575+
var (
1576+
bannerPrefixBytes = []byte(bannerPrefix)
1577+
metadataBannerPrefixBytes = []byte(metadataBannerPrefix)
1578+
)
15581579

1559-
// parseOutputAndHeader parses b and returns the test display header (e.g.,
1560-
// "##### Testing packages.") and the following output.
1561-
func parseOutputAndHeader(b []byte) (header string, out []byte) {
1580+
// parseOutputAndHeader parses b and returns the test (optional) environment
1581+
// metaadata, display header (e.g., "##### Testing packages.") and the
1582+
// following output.
1583+
//
1584+
// metadata is the optional execution environment metadata block. e.g.,
1585+
//
1586+
// ##### Test execution environment.
1587+
// # GOARCH: amd64
1588+
// # CPU: Intel(R) Xeon(R) W-2135 CPU @ 3.70GHz
1589+
func parseOutputAndHeader(b []byte) (metadata, header string, out []byte) {
15621590
if !bytes.HasPrefix(b, bannerPrefixBytes) {
1563-
return "", b
1591+
return "", "", b
1592+
}
1593+
1594+
if bytes.HasPrefix(b, metadataBannerPrefixBytes) {
1595+
// Header includes everything up to and including the next
1596+
// banner.
1597+
rem := b[len(metadataBannerPrefixBytes):]
1598+
i := bytes.Index(rem, bannerPrefixBytes)
1599+
if i == -1 {
1600+
// Metadata block without a following block doesn't
1601+
// make sense. Bail.
1602+
return "", "", b
1603+
}
1604+
bi := i + len(metadataBannerPrefixBytes)
1605+
// Metadata portion of header, skipping initial and trailing newlines.
1606+
metadata = strings.Trim(string(b[:bi]), "\n")
1607+
metadata = strings.Replace(metadata, banner, outputBanner, 1)
1608+
b = b[bi+1:] // skip newline at start of next banner.
1609+
} else {
1610+
b = b[1:] // skip newline
15641611
}
15651612

1566-
b = b[1:] // skip newline
1613+
// Find end of primary test banner.
15671614
nl := bytes.IndexByte(b, '\n')
15681615
if nl == -1 {
1616+
// No newline, everything is header.
15691617
header = string(b)
15701618
b = nil
15711619
} else {
15721620
header = string(b[:nl])
15731621
b = b[nl+1:]
15741622
}
1575-
// Replace internal marker banner with the human-friendly
1576-
// version.
1577-
header = strings.ReplaceAll(header, banner, outputBanner)
1578-
return header, b
1623+
1624+
// Replace internal marker banner with the human-friendly version.
1625+
header = strings.Replace(header, banner, outputBanner, 1)
1626+
return metadata, header, b
15791627
}
15801628

15811629
// maxTestExecError is the number of test execution failures at which

‎cmd/coordinator/buildstatus_test.go

Lines changed: 109 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,11 @@ import (
1515
// TestParseOutputAndHeader tests header parsing by parseOutputAndHeader.
1616
func TestParseOutputAndHeader(t *testing.T) {
1717
for _, tc := range []struct {
18-
name string
19-
input []byte
20-
wantHeader string
21-
wantOut []byte
18+
name string
19+
input []byte
20+
wantMetadata string
21+
wantHeader string
22+
wantOut []byte
2223
}{
2324
{
2425
name: "standard",
@@ -28,7 +29,8 @@ ok archive/tar 0.015s
2829
ok archive/zip 0.406s
2930
ok bufio 0.075s
3031
`),
31-
wantHeader: "##### Testing packages.",
32+
wantMetadata: "",
33+
wantHeader: "##### Testing packages.",
3234
wantOut: []byte(`ok archive/tar 0.015s
3335
ok archive/zip 0.406s
3436
ok bufio 0.075s
@@ -39,23 +41,26 @@ ok bufio 0.075s
3941
input: []byte(`
4042
XXXBANNERXXX:Testing packages.
4143
`),
42-
wantHeader: "##### Testing packages.",
43-
wantOut: []byte(``),
44+
wantMetadata: "",
45+
wantHeader: "##### Testing packages.",
46+
wantOut: []byte(``),
4447
},
4548
{
4649
name: "header only missing trailing newline",
4750
input: []byte(`
4851
XXXBANNERXXX:Testing packages.`),
49-
wantHeader: "##### Testing packages.",
50-
wantOut: []byte(``),
52+
wantMetadata: "",
53+
wantHeader: "##### Testing packages.",
54+
wantOut: []byte(``),
5155
},
5256
{
5357
name: "no banner",
5458
input: []byte(`ok archive/tar 0.015s
5559
ok archive/zip 0.406s
5660
ok bufio 0.075s
5761
`),
58-
wantHeader: "",
62+
wantMetadata: "",
63+
wantHeader: "",
5964
wantOut: []byte(`ok archive/tar 0.015s
6065
ok archive/zip 0.406s
6166
ok bufio 0.075s
@@ -68,7 +73,8 @@ ok archive/tar 0.015s
6873
ok archive/zip 0.406s
6974
ok bufio 0.075s
7075
`),
71-
wantHeader: "",
76+
wantMetadata: "",
77+
wantHeader: "",
7278
wantOut: []byte(`XXXBANNERXXX:Testing packages.
7379
ok archive/tar 0.015s
7480
ok archive/zip 0.406s
@@ -83,19 +89,108 @@ ok archive/tar 0.015s
8389
ok archive/zip 0.406s
8490
ok bufio 0.075s
8591
`),
86-
wantHeader: "",
92+
wantMetadata: "",
93+
wantHeader: "",
8794
wantOut: []byte(`
8895
##### Testing packages.
8996
ok archive/tar 0.015s
9097
ok archive/zip 0.406s
9198
ok bufio 0.075s
9299
`),
93100
},
101+
{
102+
name: "metadata",
103+
input: []byte(`
104+
XXXBANNERXXX:Test execution environment.
105+
# GOARCH: amd64
106+
# CPU: Intel(R) Xeon(R) W-2135 CPU @ 3.70GHz
107+
108+
XXXBANNERXXX:Testing packages.
109+
ok archive/tar 0.015s
110+
ok archive/zip 0.406s
111+
ok bufio 0.075s
112+
`),
113+
wantMetadata: `##### Test execution environment.
114+
# GOARCH: amd64
115+
# CPU: Intel(R) Xeon(R) W-2135 CPU @ 3.70GHz`,
116+
wantHeader: "##### Testing packages.",
117+
wantOut: []byte(`ok archive/tar 0.015s
118+
ok archive/zip 0.406s
119+
ok bufio 0.075s
120+
`),
121+
},
122+
{
123+
name: "metadata missing separator newline",
124+
input: []byte(`
125+
XXXBANNERXXX:Test execution environment.
126+
# GOARCH: amd64
127+
# CPU: Intel(R) Xeon(R) W-2135 CPU @ 3.70GHz
128+
XXXBANNERXXX:Testing packages.
129+
ok archive/tar 0.015s
130+
ok archive/zip 0.406s
131+
ok bufio 0.075s
132+
`),
133+
wantMetadata: `##### Test execution environment.
134+
# GOARCH: amd64
135+
# CPU: Intel(R) Xeon(R) W-2135 CPU @ 3.70GHz`,
136+
wantHeader: "##### Testing packages.",
137+
wantOut: []byte(`ok archive/tar 0.015s
138+
ok archive/zip 0.406s
139+
ok bufio 0.075s
140+
`),
141+
},
142+
{
143+
name: "metadata missing second banner",
144+
input: []byte(`
145+
XXXBANNERXXX:Test execution environment.
146+
# GOARCH: amd64
147+
# CPU: Intel(R) Xeon(R) W-2135 CPU @ 3.70GHz
148+
`),
149+
wantMetadata: "",
150+
wantHeader: "",
151+
wantOut: []byte(`
152+
XXXBANNERXXX:Test execution environment.
153+
# GOARCH: amd64
154+
# CPU: Intel(R) Xeon(R) W-2135 CPU @ 3.70GHz
155+
`),
156+
},
157+
{
158+
name: "metadata missing body",
159+
input: []byte(`
160+
XXXBANNERXXX:Test execution environment.
161+
# GOARCH: amd64
162+
# CPU: Intel(R) Xeon(R) W-2135 CPU @ 3.70GHz
163+
164+
XXXBANNERXXX:Testing packages.
165+
`),
166+
wantMetadata: `##### Test execution environment.
167+
# GOARCH: amd64
168+
# CPU: Intel(R) Xeon(R) W-2135 CPU @ 3.70GHz`,
169+
wantHeader: "##### Testing packages.",
170+
wantOut: []byte(``),
171+
},
172+
{
173+
name: "metadata missing body and newline",
174+
input: []byte(`
175+
XXXBANNERXXX:Test execution environment.
176+
# GOARCH: amd64
177+
# CPU: Intel(R) Xeon(R) W-2135 CPU @ 3.70GHz
178+
179+
XXXBANNERXXX:Testing packages.`),
180+
wantMetadata: `##### Test execution environment.
181+
# GOARCH: amd64
182+
# CPU: Intel(R) Xeon(R) W-2135 CPU @ 3.70GHz`,
183+
wantHeader: "##### Testing packages.",
184+
wantOut: []byte(``),
185+
},
94186
} {
95187
t.Run(tc.name, func(t *testing.T) {
96-
gotHeader, gotOut := parseOutputAndHeader(tc.input)
188+
gotMetadata, gotHeader, gotOut := parseOutputAndHeader(tc.input)
189+
if gotMetadata != tc.wantMetadata {
190+
t.Errorf("parseOutputAndBanner(%q) got metadata %q want metadata %q", string(tc.input), gotMetadata, tc.wantMetadata)
191+
}
97192
if gotHeader != tc.wantHeader {
98-
t.Errorf("parseOutputAndBanner(%q) got banner %q want banner %q", string(tc.input), gotHeader, tc.wantHeader)
193+
t.Errorf("parseOutputAndBanner(%q) got header %q want header %q", string(tc.input), gotHeader, tc.wantHeader)
99194
}
100195
if string(gotOut) != string(tc.wantOut) {
101196
t.Errorf("parseOutputAndBanner(%q) got out %q want out %q", string(tc.input), string(gotOut), string(tc.wantOut))

0 commit comments

Comments
 (0)