This CL changes some unit test functions, making sure that these tests (and goroutines spawned during test) won't block.
Since they are just test functions, I use one CL to fix them all. I hope this won't cause trouble to reviewers and can save time for us.
There are three main categories of incorrect logic fixed by this CL:
1. Use testing.Fatal()/Fatalf() in spawned goroutines, which is forbidden by Go's document.
2. Channels are used in such a way that, when errors or timeout happen, the test will be blocked and never return.
3. Channels are used in such a way that, when errors or timeout happen, the test can return but some spawned goroutines will be leaked, occupying resource until all other tests return and the process is killed.
Change-Id: I3df931ec380794a0cf1404e632c1dd57c65d63e8
Reviewed-on: https://go-review.googlesource.com/c/go/+/219380
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
for _, tt := range tests {
p.err = make(chan syntax.Error)
- gotch := make(chan [][]string)
+ gotch := make(chan [][]string, 1)
go func() {
p.pragcgobuf = nil
p.pragcgo(nopos, tt.in)
func main1(t *testing.T) {
const N = 100000
- done := make(chan struct{})
+ done := make(chan bool, 2)
b := make([]byte, 2)
go func() {
for i := 0; i < N; i++ {
b[0] = byte(i)
b[1] = byte(i)
}
- done <- struct{}{}
+ done <- true
}()
go func() {
for i := 0; i < N; i++ {
x, y := read1(b)
if byte(x) != byte(y) {
- t.Fatalf("x=%x y=%x\n", x, y)
+ t.Errorf("x=%x y=%x\n", x, y)
+ done <- false
+ return
}
}
- done <- struct{}{}
+ done <- true
}()
<-done
<-done
func main2(t *testing.T) {
const N = 100000
- done := make(chan struct{})
+ done := make(chan bool, 2)
b := make([]byte, 2)
go func() {
for i := 0; i < N; i++ {
b[0] = byte(i)
b[1] = byte(i)
}
- done <- struct{}{}
+ done <- true
}()
go func() {
for i := 0; i < N; i++ {
x, y := read2(b)
if x&0xff00 != y&0xff00 {
- t.Fatalf("x=%x y=%x\n", x, y)
+ t.Errorf("x=%x y=%x\n", x, y)
+ done <- false
+ return
}
}
- done <- struct{}{}
+ done <- true
}()
<-done
<-done
startWrite := make(chan struct{})
startRead := make(chan struct{})
- readDone := make(chan struct{})
+ readDone := make(chan struct{}, 1)
client := Client(c, testConfig)
go func() {
cipherSuites: []uint16{TLS_RSA_WITH_RC4_128_SHA},
}
- bufChan := make(chan []byte)
+ bufChan := make(chan []byte, 1)
c, s := localPipe(t)
go func() {
return nil, nil, err
}
- connChan := make(chan interface{})
+ connChan := make(chan interface{}, 1)
go func() {
tcpConn, err := l.Accept()
if err != nil {
connChan <- err
+ return
}
connChan <- tcpConn
}()
defer ln.Close()
serverTLSUniques := make(chan []byte)
+ parentDone := make(chan struct{})
+ childDone := make(chan struct{})
+ defer close(parentDone)
go func() {
+ defer close(childDone)
for i := 0; i < 2; i++ {
sconn, err := ln.Accept()
if err != nil {
t.Error(err)
return
}
- serverTLSUniques <- srv.ConnectionState().TLSUnique
+ select {
+ case <-parentDone:
+ return
+ case serverTLSUniques <- srv.ConnectionState().TLSUnique:
+ }
}
}()
if err != nil {
t.Fatal(err)
}
- if !bytes.Equal(conn.ConnectionState().TLSUnique, <-serverTLSUniques) {
+
+ var serverTLSUniquesValue []byte
+ select {
+ case <-childDone:
+ return
+ case serverTLSUniquesValue = <-serverTLSUniques:
+ }
+
+ if !bytes.Equal(conn.ConnectionState().TLSUnique, serverTLSUniquesValue) {
t.Error("client and server channel bindings differ")
}
conn.Close()
if !conn.ConnectionState().DidResume {
t.Error("second session did not use resumption")
}
- if !bytes.Equal(conn.ConnectionState().TLSUnique, <-serverTLSUniques) {
+
+ select {
+ case <-childDone:
+ return
+ case serverTLSUniquesValue = <-serverTLSUniques:
+ }
+
+ if !bytes.Equal(conn.ConnectionState().TLSUnique, serverTLSUniquesValue) {
t.Error("client and server channel bindings differ when session resumption is used")
}
}
source: "VHdhcyBicmlsbGlnLCBhbmQgdGhlIHNsaXRoeSB0b3Zlcw==", // twas brillig...
nextc: next,
})
- errc := make(chan error)
+ errc := make(chan error, 1)
go func() {
_, err := ioutil.ReadAll(d)
errc <- err
b.Fatalf("Listen failed: %v", err)
}
defer ln.Close()
- done := make(chan bool)
+ done := make(chan bool, 1)
go func() {
for p := 0; p < P; p++ {
s, err := ln.Accept()
if err != nil {
b.Errorf("Accept failed: %v", err)
+ done <- false
return
}
servers[p] = s
for p := 0; p < P; p++ {
c, err := net.Dial("tcp", ln.Addr().String())
if err != nil {
+ <-done
b.Fatalf("Dial failed: %v", err)
}
clients[p] = c
}
- <-done
+ if !<-done {
+ b.FailNow()
+ }
b.StartTimer()
func check(t *testing.T, source, golden string, mode checkMode) {
// run the test
- cc := make(chan int)
+ cc := make(chan int, 1)
go func() {
runcheck(t, source, golden, mode)
cc <- 0
}
func TestMutexCloseUnblock(t *testing.T) {
- c := make(chan bool)
+ c := make(chan bool, 4)
var mu FDMutex
mu.RWLock(true)
for i := 0; i < 4; i++ {
N = 1e4
}
defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(P))
- done := make(chan bool)
+ done := make(chan bool, P)
var mu FDMutex
var readState [2]uint64
var writeState [2]uint64
for p := 0; p < P; p++ {
go func() {
+ defer func() {
+ done <- !t.Failed()
+ }()
r := rand.New(rand.NewSource(rand.Int63()))
for i := 0; i < N; i++ {
switch r.Intn(3) {
}
}
}
- done <- true
}()
}
for p := 0; p < P; p++ {
- <-done
+ if !<-done {
+ t.FailNow()
+ }
}
if !mu.IncrefAndClose() {
t.Fatal("broken")
}
// count all the messages arriving
- count := make(chan int)
+ count := make(chan int, 1)
go func() {
ct := 0
for range done {
c := ts.Client()
- errc := make(chan error)
+ errc := make(chan error, 1)
go func() {
res, err := c.Get(ts.URL)
if err != nil {
}
done := make(chan error)
+ stop := make(chan struct{})
+ defer close(stop)
go func() {
- done <- cmd.Wait()
+ select {
+ case <-stop:
+ return
+ case done <- cmd.Wait():
+ }
}()
// Do b.N requests to the server.
func TestTransportMaxPerHostIdleConns(t *testing.T) {
defer afterTest(t)
+ stop := make(chan struct{}) // stop marks the exit of main Test goroutine
+ defer close(stop)
+
resch := make(chan string)
gotReq := make(chan bool)
ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) {
gotReq <- true
- msg := <-resch
+ var msg string
+ select {
+ case <-stop:
+ return
+ case msg = <-resch:
+ }
_, err := w.Write([]byte(msg))
if err != nil {
- t.Fatalf("Write: %v", err)
+ t.Errorf("Write: %v", err)
+ return
}
}))
defer ts.Close()
// Their responses will hang until we write to resch, though.
donech := make(chan bool)
doReq := func() {
+ defer func() {
+ select {
+ case <-stop:
+ return
+ case donech <- t.Failed():
+ }
+ }()
resp, err := c.Get(ts.URL)
if err != nil {
t.Error(err)
t.Errorf("ReadAll: %v", err)
return
}
- donech <- true
}
go doReq()
<-gotReq
// where we won the race.
res.Body.Close()
}
- activityc <- true
+ if !<-activityc { // Receives false when close(activityc) is executed
+ return
+ }
}
}()
}
// Make sure all the request come back, one way or another.
for i := 0; i < numClients*reqsPerClient; i++ {
select {
- case <-activityc:
+ case activityc <- true:
case <-time.After(5 * time.Second):
+ close(activityc)
t.Fatalf("presumed deadlock; no HTTP client activity seen in awhile")
}
}
tr := &Transport{
Dial: func(network, addr string) (net.Conn, error) {
eventLog.Println("dial: blocking")
- inDial <- true
+ if !<-inDial {
+ return nil, errors.New("main Test goroutine exited")
+ }
<-unblockDial
return nil, errors.New("nope")
},
}()
select {
- case <-inDial:
+ case inDial <- true:
case <-time.After(5 * time.Second):
+ close(inDial)
t.Fatal("timeout; never saw blocking dial")
}
// Test that Chdir+Getwd is program-wide.
func TestProgWideChdir(t *testing.T) {
const N = 10
+ const ErrPwd = "Error!"
c := make(chan bool)
- cpwd := make(chan string)
+ cpwd := make(chan string, N)
for i := 0; i < N; i++ {
go func(i int) {
// Lock half the goroutines in their own operating system
// See issue 9428.
runtime.LockOSThread()
}
- <-c
+ hasErr, closed := <-c
+ if !closed && hasErr {
+ cpwd <- ErrPwd
+ return
+ }
pwd, err := Getwd()
if err != nil {
t.Errorf("Getwd on goroutine %d: %v", i, err)
+ cpwd <- ErrPwd
return
}
cpwd <- pwd
}
oldwd, err := Getwd()
if err != nil {
+ c <- true
t.Fatalf("Getwd: %v", err)
}
d, err := ioutil.TempDir("", "test")
if err != nil {
+ c <- true
t.Fatalf("TempDir: %v", err)
}
defer func() {
RemoveAll(d)
}()
if err := Chdir(d); err != nil {
+ c <- true
t.Fatalf("Chdir: %v", err)
}
// OS X sets TMPDIR to a symbolic link.
// So we resolve our working directory again before the test.
d, err = Getwd()
if err != nil {
+ c <- true
t.Fatalf("Getwd: %v", err)
}
close(c)
for i := 0; i < N; i++ {
pwd := <-cpwd
+ if pwd == ErrPwd {
+ t.FailNow()
+ }
if pwd != d {
t.Errorf("Getwd returned %q; want %q", pwd, d)
}
}
for _, test := range tests {
var v Value
- done := make(chan bool)
+ done := make(chan bool, p)
for i := 0; i < p; i++ {
go func() {
r := rand.New(rand.NewSource(rand.Int63()))
+ expected := true
loop:
for j := 0; j < N; j++ {
x := test[r.Intn(len(test))]
}
}
t.Logf("loaded unexpected value %+v, want %+v", x, test)
- done <- false
+ expected = false
+ break
}
- done <- true
+ done <- expected
}()
}
for i := 0; i < p; i++ {