From 78e255f9bd2ae9c9885793a751f42f5698a5da8c Mon Sep 17 00:00:00 2001
From: Steven Hartland <steven.hartland@multiplay.co.uk>
Date: Fri, 12 May 2023 00:11:01 +0100
Subject: [PATCH] fix: test goroutine leaks (#643)

Ensure that goroutines started by tests are cleaned up on termination.

Also:
* make TestLatencyHistories compatible with -count=X.
* Update to the supported versions of go 1.19 and 1.20.
* Update golangci-lint to v1.15.2

Fixes #641
---
 .github/workflows/go-test.yml       |  4 ++--
 .github/workflows/golangci-lint.yml |  2 +-
 redis/conn_test.go                  | 34 ++++++++++++++++++++++++++---
 redis/reply_test.go                 |  6 ++++-
 4 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml
index 1086bf3a..5363fb31 100644
--- a/.github/workflows/go-test.yml
+++ b/.github/workflows/go-test.yml
@@ -12,8 +12,8 @@ jobs:
     strategy:
       matrix:
         go-version:
-        - '1.17.x'
-        - '1.18.x'
+        - '1.19.x'
+        - '1.20.x'
         os:
         - 'ubuntu-latest'
         redis:
diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml
index eb7a22f8..177259b0 100644
--- a/.github/workflows/golangci-lint.yml
+++ b/.github/workflows/golangci-lint.yml
@@ -16,7 +16,7 @@ jobs:
         uses: golangci/golangci-lint-action@v2
         with:
           # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version.
-          version: v1.46.2
+          version: v1.51.2
 
           # Optional: working directory, useful for monorepos
           # working-directory: somedir
diff --git a/redis/conn_test.go b/redis/conn_test.go
index b33cc41a..0c942ce2 100644
--- a/redis/conn_test.go
+++ b/redis/conn_test.go
@@ -23,6 +23,7 @@ import (
 	"io"
 	"math"
 	"net"
+	"sync"
 	"os"
 	"reflect"
 	"strings"
@@ -479,22 +480,49 @@ func TestError(t *testing.T) {
 }
 
 func TestReadTimeout(t *testing.T) {
+	done := make(chan struct{})
+	errs := make(chan error, 2)
+	defer func() {
+		close(done)
+		for err := range errs {
+			require.NoError(t, err)
+		}
+	}()
+
+	var wg sync.WaitGroup
+	defer func() {
+		wg.Wait()
+		close(errs)
+	}()
+
 	l, err := net.Listen("tcp", "127.0.0.1:0")
 	if err != nil {
 		t.Fatalf("net.Listen returned %v", err)
 	}
 	defer l.Close()
 
+	wg.Add(1)
 	go func() {
+		defer wg.Done()
+
 		for {
 			c, err := l.Accept()
 			if err != nil {
 				return
 			}
+			wg.Add(1)
 			go func() {
-				time.Sleep(time.Second)
+				defer wg.Done()
+
+				to := time.NewTimer(time.Second)
+				defer to.Stop()
+				select {
+				case <-to.C:
+				case <-done:
+					return
+				}
 				_, err := c.Write([]byte("+OK\r\n"))
-				require.NoError(t, err)
+				errs <- err
 				c.Close()
 			}()
 		}
@@ -719,7 +747,7 @@ type blockedReader struct {
 
 func (b blockedReader) Read(p []byte) (n int, err error) {
 	<-b.ch
-	return 0, nil
+	return 0, io.EOF
 }
 
 func dialTestBlockedConn(ch chan struct{}, w io.Writer) redis.DialOption {
diff --git a/redis/reply_test.go b/redis/reply_test.go
index 7362a36c..5ff3b5f0 100644
--- a/redis/reply_test.go
+++ b/redis/reply_test.go
@@ -16,7 +16,6 @@ package redis_test
 
 import (
 	"fmt"
-	"github.com/stretchr/testify/require"
 	"math"
 	"reflect"
 	"strconv"
@@ -24,6 +23,7 @@ import (
 	"time"
 
 	"github.com/gomodule/redigo/redis"
+	"github.com/stretchr/testify/require"
 )
 
 var (
@@ -288,6 +288,10 @@ func TestLatencyHistories(t *testing.T) {
 	latencyMonitorThresholdOldCfg, err := strconv.Atoi(res[1])
 	require.NoError(t, err)
 
+	// Reset so we're compatible with -count=X
+	_, err = c.Do("LATENCY", "RESET", "command")
+	require.NoError(t, err)
+
 	// Enable latency monitoring for events that take 1ms or longer
 	result, err := c.Do("CONFIG", "SET", "latency-monitor-threshold", "1")
 	// reset the old configuration after test.