Skip to content

Commit 2cab203

Browse files
author
Bryan C. Mills
committed
[release-branch.go1.14] testing: treat PAUSE lines as changing the active test name
We could instead fix cmd/test2json to treat PAUSE lines as *not* changing the active test name, but that seems like it would be more confusing to humans, and also wouldn't fix tools that parse output using existing builds of cmd/test2json. Fixes #40848 Updates #40657 Change-Id: I937611778f5b1e7dd1d6e9f44424d7e725a589ed Reviewed-on: https://go-review.googlesource.com/c/go/+/248727 Run-TryBot: Bryan C. Mills <[email protected]> Reviewed-by: Jean de Klerk <[email protected]> (cherry picked from commit cdc77d3) Reviewed-on: https://go-review.googlesource.com/c/go/+/249098 TryBot-Result: Gobot Gobot <[email protected]>
1 parent 053469c commit 2cab203

File tree

2 files changed

+47
-5
lines changed

2 files changed

+47
-5
lines changed
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Regression test for https://golang.org/issue/40657: output from the main test
2+
# function should be attributed correctly even if interleaved with the PAUSE
3+
# line for a new parallel subtest.
4+
5+
[short] skip
6+
7+
go test -json
8+
stdout '"Test":"TestWeirdTiming","Output":"[^"]* logging to outer again\\n"'
9+
10+
-- go.mod --
11+
module example.com
12+
go 1.15
13+
-- main_test.go --
14+
package main
15+
16+
import (
17+
"testing"
18+
)
19+
20+
func TestWeirdTiming(outer *testing.T) {
21+
outer.Run("pauser", func(pauser *testing.T) {
22+
outer.Logf("logging to outer")
23+
pauser.Parallel()
24+
})
25+
26+
outer.Logf("logging to outer again")
27+
}

src/testing/testing.go

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -352,10 +352,19 @@ func (p *testPrinter) Fprint(w io.Writer, testName, out string) {
352352
defer p.lastNameMu.Unlock()
353353

354354
if !p.chatty ||
355-
strings.HasPrefix(out, "--- PASS") ||
356-
strings.HasPrefix(out, "--- FAIL") ||
357-
strings.HasPrefix(out, "=== CONT") ||
358-
strings.HasPrefix(out, "=== RUN") {
355+
strings.HasPrefix(out, "--- PASS: ") ||
356+
strings.HasPrefix(out, "--- FAIL: ") ||
357+
strings.HasPrefix(out, "--- SKIP: ") ||
358+
strings.HasPrefix(out, "=== RUN ") ||
359+
strings.HasPrefix(out, "=== CONT ") ||
360+
strings.HasPrefix(out, "=== PAUSE ") {
361+
// If we're buffering test output (!p.chatty), we don't really care which
362+
// test is emitting which line so long as they are serialized.
363+
//
364+
// If the message already implies an association with a specific new test,
365+
// we don't need to check what the old test name was or log an extra CONT
366+
// line for it. (We're updating it anyway, and the current message already
367+
// includes the test name.)
359368
p.lastName = testName
360369
fmt.Fprint(w, out)
361370
return
@@ -907,7 +916,13 @@ func (t *T) Parallel() {
907916
for ; root.parent != nil; root = root.parent {
908917
}
909918
root.mu.Lock()
910-
fmt.Fprintf(root.w, "=== PAUSE %s\n", t.name)
919+
// Unfortunately, even though PAUSE indicates that the named test is *no
920+
// longer* running, cmd/test2json interprets it as changing the active test
921+
// for the purpose of log parsing. We could fix cmd/test2json, but that
922+
// won't fix existing deployments of third-party tools that already shell
923+
// out to older builds of cmd/test2json — so merely fixing cmd/test2json
924+
// isn't enough for now.
925+
printer.Fprint(root.w, t.name, fmt.Sprintf("=== PAUSE %s\n", t.name))
911926
root.mu.Unlock()
912927
}
913928

0 commit comments

Comments
 (0)