From 4a37449d33b0a2b725cbf44e054760a310cf4ba8 Mon Sep 17 00:00:00 2001 From: Andy Wang Date: Sat, 19 Dec 2020 20:14:43 +0000 Subject: [PATCH] Replace all time-sensitive tests with assert.Eventually --- go.mod | 2 +- go.sum | 5 +++ internal/multiplex/mux_test.go | 24 +++++++------- internal/multiplex/session_test.go | 9 +++--- internal/multiplex/stream_test.go | 11 ++++--- internal/multiplex/switchboard_test.go | 19 ++++++----- internal/test/integration_test.go | 45 +++++++------------------- 7 files changed, 51 insertions(+), 64 deletions(-) diff --git a/go.mod b/go.mod index b3ebe02..e950ab7 100644 --- a/go.mod +++ b/go.mod @@ -15,7 +15,7 @@ require ( github.com/refraction-networking/utls v0.0.0-20190909200633-43c36d3c1f57 github.com/sirupsen/logrus v1.5.0 github.com/stephens2424/writerset v1.0.2 // indirect - github.com/stretchr/testify v1.3.0 + github.com/stretchr/testify v1.6.1 go.etcd.io/bbolt v1.3.4 golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9 golang.org/x/sys v0.0.0-20200413165638-669c56c373c4 // indirect diff --git a/go.sum b/go.sum index d24d520..150adcb 100644 --- a/go.sum +++ b/go.sum @@ -43,6 +43,8 @@ github.com/stretchr/testify v1.2.2 h1:bSDNvY7ZPG5RlJ8otE/7V6gMiyenm9RtJ7IUVIAoJ1 github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= +github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0= +github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= go.etcd.io/bbolt v1.3.4 h1:hi1bXHMVrlQh6WwxAy+qZCV/SYIlqo+Ushwdpa4tAKg= go.etcd.io/bbolt v1.3.4/go.mod h1:G5EMThwa9y8QZGBClrRx5EY+Yw9kAhnjy3bSjsnlVTQ= @@ -76,5 +78,8 @@ golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8T golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE= golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33q108Sa+fhmuc+sWQYwY= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/internal/multiplex/mux_test.go b/internal/multiplex/mux_test.go index d3deaf9..76344ca 100644 --- a/internal/multiplex/mux_test.go +++ b/internal/multiplex/mux_test.go @@ -4,6 +4,7 @@ import ( "bytes" "github.com/cbeuw/Cloak/internal/common" "github.com/cbeuw/connutil" + "github.com/stretchr/testify/assert" "io" "math/rand" "net" @@ -12,8 +13,6 @@ import ( "time" ) -const eventualConsistencyTolerance = 500 * time.Millisecond - func serveEcho(l net.Listener) { for { conn, err := l.Accept() @@ -114,13 +113,13 @@ func TestMultiplex(t *testing.T) { //test echo runEchoTest(t, streams, maxMsgLen) - time.Sleep(eventualConsistencyTolerance) - if clientSession.streamCount() != numStreams { - t.Errorf("client stream count is wrong: %v", clientSession.streamCount()) - } - if serverSession.streamCount() != numStreams { - t.Errorf("server stream count is wrong: %v", serverSession.streamCount()) - } + assert.Eventuallyf(t, func() bool { + return clientSession.streamCount() == numStreams + }, time.Second, 10*time.Millisecond, "client stream count is wrong: %v", clientSession.streamCount()) + + assert.Eventuallyf(t, func() bool { + return serverSession.streamCount() == numStreams + }, time.Second, 10*time.Millisecond, "server stream count is wrong: %v", serverSession.streamCount()) // close one stream closing, streams := streams[0], streams[1:] @@ -152,9 +151,12 @@ func TestMux_StreamClosing(t *testing.T) { t.Errorf("can't write to stream: %v", err) } - time.Sleep(eventualConsistencyTolerance) + _, err = io.ReadFull(toBeClosed, recvBuf[:1]) + if err != nil { + t.Errorf("can't read anything before stream closed: %v", err) + } _ = toBeClosed.Close() - _, err = io.ReadFull(toBeClosed, recvBuf) + _, err = io.ReadFull(toBeClosed, recvBuf[1:]) if err != nil { t.Errorf("can't read residual data on stream: %v", err) } diff --git a/internal/multiplex/session_test.go b/internal/multiplex/session_test.go index 5e9ee16..b280895 100644 --- a/internal/multiplex/session_test.go +++ b/internal/multiplex/session_test.go @@ -3,6 +3,7 @@ package multiplex import ( "bytes" "github.com/cbeuw/connutil" + "github.com/stretchr/testify/assert" "math/rand" "strconv" "sync" @@ -408,10 +409,10 @@ func TestSession_timeoutAfter(t *testing.T) { seshConfigOrdered.Obfuscator = obfuscator seshConfigOrdered.InactivityTimeout = 100 * time.Millisecond sesh := MakeSession(0, seshConfigOrdered) - time.Sleep(5 * seshConfigOrdered.InactivityTimeout) - if !sesh.IsClosed() { - t.Error("session should have timed out") - } + + assert.Eventually(t, func() bool { + return sesh.IsClosed() + }, 5*seshConfigOrdered.InactivityTimeout, seshConfigOrdered.InactivityTimeout, "session should have timed out") } func BenchmarkRecvDataFromRemote_Ordered(b *testing.B) { diff --git a/internal/multiplex/stream_test.go b/internal/multiplex/stream_test.go index 9527034..6ce16d9 100644 --- a/internal/multiplex/stream_test.go +++ b/internal/multiplex/stream_test.go @@ -3,6 +3,7 @@ package multiplex import ( "bytes" "github.com/cbeuw/Cloak/internal/common" + "github.com/stretchr/testify/assert" "io" "io/ioutil" "math/rand" @@ -230,11 +231,11 @@ func TestStream_Close(t *testing.T) { if err != nil { t.Errorf("can't read residual data %v", err) } - time.Sleep(eventualConsistencyTolerance) - if sI, _ := sesh.streams.Load(stream.(*Stream).id); sI != nil { - t.Error("stream still exists") - return - } + + assert.Eventually(t, func() bool { + sI, _ := sesh.streams.Load(stream.(*Stream).id) + return sI == nil + }, time.Second, 10*time.Millisecond, "streams still exists") }) } diff --git a/internal/multiplex/switchboard_test.go b/internal/multiplex/switchboard_test.go index 01d55ae..2c3f36f 100644 --- a/internal/multiplex/switchboard_test.go +++ b/internal/multiplex/switchboard_test.go @@ -2,6 +2,7 @@ package multiplex import ( "github.com/cbeuw/connutil" + "github.com/stretchr/testify/assert" "math/rand" "sync" "testing" @@ -145,11 +146,11 @@ func TestSwitchboard_CloseOnOneDisconn(t *testing.T) { sesh.AddConnection(conn1client) conn0server.Close() - time.Sleep(eventualConsistencyTolerance) - if !sesh.IsClosed() { - t.Error("session not closed after one conn is disconnected") - return - } + + assert.Eventually(t, func() bool { + return sesh.IsClosed() + }, time.Second, 10*time.Millisecond, "session not closed after one conn is disconnected") + if _, err := conn1client.Write([]byte{0x00}); err == nil { t.Error("the other conn is still connected") return @@ -178,9 +179,7 @@ func TestSwitchboard_ConnsCount(t *testing.T) { sesh.sb.closeAll() - time.Sleep(eventualConsistencyTolerance) - if sesh.sb.connsCount() != 0 { - t.Error("connsCount incorrect") - } - + assert.Eventuallyf(t, func() bool { + return sesh.sb.connsCount() == 0 + }, time.Second, 10*time.Millisecond, "connsCount incorrect: %v", sesh.sb.connsCount()) } diff --git a/internal/test/integration_test.go b/internal/test/integration_test.go index 1a3d0fb..db58b39 100644 --- a/internal/test/integration_test.go +++ b/internal/test/integration_test.go @@ -10,6 +10,7 @@ import ( mux "github.com/cbeuw/Cloak/internal/multiplex" "github.com/cbeuw/Cloak/internal/server" "github.com/cbeuw/connutil" + "github.com/stretchr/testify/assert" "io" "io/ioutil" "math/rand" @@ -356,17 +357,9 @@ func TestTCPSingleplex(t *testing.T) { proxyConn1.Close() - retries := 0 -retry: - time.Sleep(delayBeforeTestingConnClose) - if user.NumSession() != 1 { - retries++ - if retries > connCloseRetries { - t.Error("first session was not closed on connection close") - } else { - goto retry - } - } + assert.Eventually(t, func() bool { + return user.NumSession() == 1 + }, time.Second, 10*time.Millisecond, "first session was not closed on connection close") // conn2 should still work runEchoTest(t, []net.Conn{proxyConn2}, 65536) @@ -479,17 +472,10 @@ func TestClosingStreamsFromProxy(t *testing.T) { serverConn, _ := proxyFromCkServerL.Accept() serverConn.Close() - retries := 0 - retry: - time.Sleep(delayBeforeTestingConnClose) - if _, err := clientConn.Read(make([]byte, 16)); err == nil { - retries++ - if retries > connCloseRetries { - t.Errorf("closing stream on server side is not reflected to the client: %v", err) - } else { - goto retry - } - } + assert.Eventually(t, func() bool { + _, err := clientConn.Read(make([]byte, 16)) + return err != nil + }, time.Second, 10*time.Millisecond, "closing stream on server side is not reflected to the client") }) t.Run("closing from client", func(t *testing.T) { @@ -499,17 +485,10 @@ func TestClosingStreamsFromProxy(t *testing.T) { serverConn, _ := proxyFromCkServerL.Accept() clientConn.Close() - retries := 0 - retry: - time.Sleep(delayBeforeTestingConnClose) - if _, err := serverConn.Read(make([]byte, 16)); err == nil { - retries++ - if retries > 3 { - t.Errorf("closing stream on client side is not reflected to the server: %v", err) - } else { - goto retry - } - } + assert.Eventually(t, func() bool { + _, err := serverConn.Read(make([]byte, 16)) + return err != nil + }, time.Second, 10*time.Millisecond, "closing stream on client side is not reflected to the server") }) t.Run("send then close", func(t *testing.T) {