From 6e5cd103997b4176dbacaafbb51a545a3f736895 Mon Sep 17 00:00:00 2001
From: Emmanuel Odeke <emm.odeke@gmail.com>
Date: Wed, 25 Oct 2017 19:20:33 -0700
Subject: [PATCH] rpc/lib/client: jitter test updates and only to-be run on
 releases

* Updated code with feedback from @melekes, @ebuchman and @silasdavis.
* Added Makefile clause `release` to only run the test on seeing tag
`release` during releases i.e
```shell
make release
```
which will run the comprehensive and long integration-ish tests.
---
 Makefile                           |  3 ++
 rpc/lib/client/integration_test.go | 66 ++++++++++++++++++++++++++++++
 rpc/lib/client/ws_client.go        |  7 ++--
 rpc/lib/client/ws_client_test.go   | 55 -------------------------
 4 files changed, 73 insertions(+), 58 deletions(-)
 create mode 100644 rpc/lib/client/integration_test.go

diff --git a/Makefile b/Makefile
index dfb0dc3a9..2271abebf 100644
--- a/Makefile
+++ b/Makefile
@@ -35,6 +35,9 @@ test_race:
 test_integrations:
 	@bash ./test/test.sh
 
+release:
+	@go test -tags release $(PACKAGES)
+
 test100:
 	@for i in {1..100}; do make test; done
 
diff --git a/rpc/lib/client/integration_test.go b/rpc/lib/client/integration_test.go
new file mode 100644
index 000000000..d3d993374
--- /dev/null
+++ b/rpc/lib/client/integration_test.go
@@ -0,0 +1,66 @@
+// +build release
+
+// The code in here is comprehensive as an integration
+// test and is long, hence is only run before releases.
+
+package rpcclient
+
+import (
+	"bytes"
+	"errors"
+	"net"
+	"regexp"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+	"github.com/tendermint/tmlibs/log"
+)
+
+func TestWSClientReconnectWithJitter(t *testing.T) {
+	n := 8
+	maxReconnectAttempts := 3
+	// Max wait time is ceil(1+0.999) + ceil(2+0.999) + ceil(4+0.999) + ceil(...) = 2 + 3 + 5 = 10s + ...
+	maxSleepTime := time.Second * time.Duration(((1<<uint(maxReconnectAttempts))-1)+maxReconnectAttempts)
+
+	var errNotConnected = errors.New("not connected")
+	clientMap := make(map[int]*WSClient)
+	buf := new(bytes.Buffer)
+	logger := log.NewTMLogger(buf)
+	for i := 0; i < n; i++ {
+		c := NewWSClient("tcp://foo", "/websocket")
+		c.Dialer = func(string, string) (net.Conn, error) {
+			return nil, errNotConnected
+		}
+		c.SetLogger(logger)
+		c.maxReconnectAttempts = maxReconnectAttempts
+		// Not invoking defer c.Stop() because
+		// after all the reconnect attempts have been
+		// exhausted, c.Stop is implicitly invoked.
+		clientMap[i] = c
+		// Trigger the reconnect routine that performs exponential backoff.
+		go c.reconnect()
+	}
+
+	stopCount := 0
+	time.Sleep(maxSleepTime)
+	for key, c := range clientMap {
+		if !c.IsActive() {
+			delete(clientMap, key)
+			stopCount += 1
+		}
+	}
+	require.Equal(t, stopCount, n, "expecting all clients to have been stopped")
+
+	// Next we have to examine the logs to ensure that no single time was repeated
+	backoffDurRegexp := regexp.MustCompile(`backoff_duration=(.+)`)
+	matches := backoffDurRegexp.FindAll(buf.Bytes(), -1)
+	seenMap := make(map[string]int)
+	for i, match := range matches {
+		if origIndex, seen := seenMap[string(match)]; seen {
+			t.Errorf("Match #%d (%q) was seen originally at log entry #%d", i, match, origIndex)
+		} else {
+			seenMap[string(match)] = i
+		}
+	}
+}
diff --git a/rpc/lib/client/ws_client.go b/rpc/lib/client/ws_client.go
index 3a8673818..48db1eaa5 100644
--- a/rpc/lib/client/ws_client.go
+++ b/rpc/lib/client/ws_client.go
@@ -254,10 +254,11 @@ func (c *WSClient) reconnect() error {
 		c.mtx.Unlock()
 	}()
 
-	_1sAsNs := float64(time.Second.Nanoseconds())
+	// 1s == (1e9 ns) == (1 Billion ns)
+	billionNs := float64(time.Second.Nanoseconds())
 	for {
-		jitter := time.Duration(rand.Float64() * _1sAsNs)
-		backoffDuration := jitter + ((1 << uint(attempt)) * time.Second)
+		jitterSeconds := time.Duration(rand.Float64() * billionNs)
+		backoffDuration := jitterSeconds + ((1 << uint(attempt)) * time.Second)
 
 		c.Logger.Info("reconnecting", "attempt", attempt+1, "backoff_duration", backoffDuration)
 		time.Sleep(backoffDuration)
diff --git a/rpc/lib/client/ws_client_test.go b/rpc/lib/client/ws_client_test.go
index fecbee7ad..f5aa027fe 100644
--- a/rpc/lib/client/ws_client_test.go
+++ b/rpc/lib/client/ws_client_test.go
@@ -1,14 +1,11 @@
 package rpcclient
 
 import (
-	"bytes"
 	"context"
 	"encoding/json"
-	"errors"
 	"net"
 	"net/http"
 	"net/http/httptest"
-	"regexp"
 	"sync"
 	"testing"
 	"time"
@@ -194,55 +191,3 @@ func callWgDoneOnResult(t *testing.T, c *WSClient, wg *sync.WaitGroup) {
 		}
 	}
 }
-
-func TestWSClientReconnectWithJitter(t *testing.T) {
-	if testing.Short() {
-		t.Skipf("This is a potentially long test")
-	}
-
-	n := 8
-	maxReconnectAttempts := 3
-	// Max wait time is ceil(1+0.999) + ceil(2+0.999) + ceil(4+0.999) + ceil(...) = 2 + 3 + 5 = 10s + ...
-	maxSleepTime := time.Second * time.Duration(((1<<uint(maxReconnectAttempts))-1)+maxReconnectAttempts)
-
-	var errNotConnected = errors.New("not connected")
-	clientMap := make(map[int]*WSClient)
-	buf := new(bytes.Buffer)
-	logger := log.NewTMLogger(buf)
-	for i := 0; i < n; i++ {
-		c := NewWSClient("tcp://foo", "/websocket")
-		c.Dialer = func(string, string) (net.Conn, error) {
-			return nil, errNotConnected
-		}
-		c.SetLogger(logger)
-		c.maxReconnectAttempts = maxReconnectAttempts
-		// Not invoking defer c.Stop() because
-		// after all the reconnect attempts have been
-		// exhausted, c.Stop is implicitly invoked.
-		clientMap[i] = c
-		// Trigger the reconnect routine that performs exponential backoff.
-		go c.reconnect()
-	}
-
-	stopCount := 0
-	time.Sleep(maxSleepTime)
-	for key, c := range clientMap {
-		if !c.IsActive() {
-			delete(clientMap, key)
-			stopCount += 1
-		}
-	}
-	require.Equal(t, stopCount, n, "expecting all clients to have been stopped")
-
-	// Next we have to examine the logs to ensure that no single time was repeated
-	backoffDurRegexp := regexp.MustCompile(`backoff_duration=(.+)`)
-	matches := backoffDurRegexp.FindAll(buf.Bytes(), -1)
-	seenMap := make(map[string]int)
-	for i, match := range matches {
-		if origIndex, seen := seenMap[string(match)]; seen {
-			t.Errorf("Match #%d (%q) was seen originally at log entry #%d", i, match, origIndex)
-		} else {
-			seenMap[string(match)] = i
-		}
-	}
-}