Skip to content

Commit 6df855e

Browse files
jbagopherbot
authored andcommitted
testing: fix panic in t.Log
If a testing.TB is no longer on the stack, t.Log would panic because its outputWriter is nil. Check for nil and drop the write, which is the previous behavior. Change-Id: Ifde97997a3aa26ae604ac9c218588c1980110cbf Reviewed-on: https://go-review.googlesource.com/c/go/+/673215 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Alan Donovan <[email protected]> Auto-Submit: Jonathan Amsterdam <[email protected]>
1 parent fac2ccb commit 6df855e

File tree

2 files changed

+34
-9
lines changed

2 files changed

+34
-9
lines changed

src/testing/sub_test.go

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,6 @@ func TestTRun(t *T) {
280280
t.Run("c", func(t *T) {
281281
t.Parallel()
282282
})
283-
284283
})
285284
})
286285
},
@@ -305,7 +304,6 @@ func TestTRun(t *T) {
305304
time.Sleep(time.Nanosecond)
306305
})
307306
}
308-
309307
})
310308
}
311309
})
@@ -841,7 +839,7 @@ func TestBenchmarkOutput(t *T) {
841839
}
842840

843841
func TestBenchmarkStartsFrom1(t *T) {
844-
var first = true
842+
first := true
845843
Benchmark(func(b *B) {
846844
if first && b.N != 1 {
847845
panic(fmt.Sprintf("Benchmark() first N=%v; want 1", b.N))
@@ -851,7 +849,7 @@ func TestBenchmarkStartsFrom1(t *T) {
851849
}
852850

853851
func TestBenchmarkReadMemStatsBeforeFirstRun(t *T) {
854-
var first = true
852+
first := true
855853
Benchmark(func(b *B) {
856854
if first && (b.startAllocs == 0 || b.startBytes == 0) {
857855
panic("ReadMemStats not called before first run")
@@ -1250,3 +1248,21 @@ func TestOutputWriteAfterComplete(t *T) {
12501248
t.Error(s)
12511249
}
12521250
}
1251+
1252+
// Verify that logging to an inactive top-level testing.T does not panic.
1253+
// These tests can run in either order.
1254+
1255+
func TestOutputEscape1(t *T) { testOutputEscape(t) }
1256+
func TestOutputEscape2(t *T) { testOutputEscape(t) }
1257+
1258+
var global *T
1259+
1260+
func testOutputEscape(t *T) {
1261+
if global == nil {
1262+
// Store t in a global, to set up for the second execution.
1263+
global = t
1264+
} else {
1265+
// global is inactive here.
1266+
global.Log("hello")
1267+
}
1268+
}

src/testing/testing.go

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -904,8 +904,10 @@ type TB interface {
904904
private()
905905
}
906906

907-
var _ TB = (*T)(nil)
908-
var _ TB = (*B)(nil)
907+
var (
908+
_ TB = (*T)(nil)
909+
_ TB = (*B)(nil)
910+
)
909911

910912
// T is a type passed to Test functions to manage test state and support formatted test logs.
911913
//
@@ -1119,6 +1121,11 @@ type outputWriter struct {
11191121
// Write writes a log message to the test's output stream, properly formatted and
11201122
// indented. It may not be called after a test function and all its parents return.
11211123
func (o *outputWriter) Write(p []byte) (int, error) {
1124+
// o can be nil if this is called from a top-level *TB that is no longer active.
1125+
// Just ignore the message in that case.
1126+
if o == nil || o.c == nil {
1127+
return 0, nil
1128+
}
11221129
if o.c.destination() == nil {
11231130
panic("Write called after " + o.c.name + " has completed")
11241131
}
@@ -1369,7 +1376,7 @@ func (c *common) TempDir() string {
13691376
}
13701377

13711378
dir := fmt.Sprintf("%s%c%03d", c.tempDir, os.PathSeparator, seq)
1372-
if err := os.Mkdir(dir, 0777); err != nil {
1379+
if err := os.Mkdir(dir, 0o777); err != nil {
13731380
c.Fatalf("TempDir: %v", err)
13741381
}
13751382
return dir
@@ -2132,8 +2139,10 @@ func MainStart(deps testDeps, tests []InternalTest, benchmarks []InternalBenchma
21322139
}
21332140
}
21342141

2135-
var testingTesting bool
2136-
var realStderr *os.File
2142+
var (
2143+
testingTesting bool
2144+
realStderr *os.File
2145+
)
21372146

21382147
// Run runs the tests. It returns an exit code to pass to os.Exit.
21392148
// The exit code is zero when all tests pass, and non-zero for any kind

0 commit comments

Comments
 (0)