Replace all time-sensitive tests with assert.Eventually

Author: cbeuw

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

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=

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)
 	}

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) {

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")
 
 	})
 }

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())
 }

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) {