core: update github actions and fix tests (#657)

Update GitHub actions to use the latest versions and add go mod tidy check. This bumps go version for tests to 1.20 and 1.21 the currently supported versions. Update checks to validate against Redis 7.2 and 7.0, removing 5.0 and 4.0 which may still work but haven't been updated since 2020. Fix pubsub test instability due to unsubscribe notifications ordering not being guaranteed. Fix latency tests on Redis 7 due to DEBUG being disabled by default.
gomodule · Jan 29, 2024 · 9129745 · 9129745
1 parent a60882b
commit 9129745
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 66 deletions.
diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml
@@ -12,15 +12,15 @@ jobs:
     strategy:
       matrix:
         go-version:
-        - '1.19.x'
-        - '1.20.x'
+        - '1.20'
+        - '1.21'
         os:
         - 'ubuntu-latest'
         redis:
+        - '7.2'
+        - '7.0'
         - '6.2'
         - '6.0'
-        - '5.0'
-        - '4.0'
     name: Test go ${{ matrix.go-version }} redis ${{ matrix.redis }} on ${{ matrix.os }}
     steps:
       - name: Setup redis
@@ -29,30 +29,12 @@ jobs:
           redis-version: ${{ matrix.redis }}
 
       - name: Install Go
-        uses: actions/setup-go@v3
+        uses: actions/setup-go@v5
         with:
           go-version: ${{ matrix.go-version }}
 
-      - name: Go Cache Paths
-        id: go-cache-paths
-        run: |
-          echo "::set-output name=go-build::$(go env GOCACHE)"
-          echo "::set-output name=go-mod::$(go env GOMODCACHE)"
-
       - name: Checkout code
-        uses: actions/checkout@v3
-
-      - name: Go Build Cache
-        uses: actions/cache@v3
-        with:
-          path: ${{ steps.go-cache-paths.outputs.go-build }}
-          key: ${{ runner.os }}-go-build-${{ hashFiles('**/go.sum') }}
-
-      - name: Go Mod Cache
-        uses: actions/cache@v3
-        with:
-          path: ${{ steps.go-cache-paths.outputs.go-mod }}
-          key: ${{ runner.os }}-go-mod-${{ hashFiles('**/go.sum') }}
+        uses: actions/checkout@v4
 
       - name: Go Test
-        run: go test -v ./...
+        run: go test ./...
diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml
@@ -11,21 +11,22 @@ jobs:
     name: lint
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
-      - name: golangci-lint
-        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.51.2
-
-          # Optional: working directory, useful for monorepos
-          # working-directory: somedir
+      - name: Checkout code
+        uses: actions/checkout@v4
 
-          # Optional: golangci-lint command line arguments.
-          # args: --issues-exit-code=0
+      - name: Setup golang
+        uses: actions/setup-go@v4
+        with:
+          go-version: '1.21'
+          cache: false # Handled by golangci-lint.
 
-          # Optional: show only new issues if it's a pull request. The default value is `false`.
-          # only-new-issues: true
+      - name: Validate go mod
+        run: |
+          go mod tidy
+          git --no-pager diff && [[ 0 -eq $(git status --porcelain | wc -l) ]]
 
-          # Optional: if set to true then the action will use pre-installed Go.
-          # skip-go-installation: true
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v3
+        with:
+          version: v1.55.2
+          args: --out-format=colored-line-number
diff --git a/redis/pubsub_test.go b/redis/pubsub_test.go
@@ -17,58 +17,58 @@ package redis_test
 import (
 	"context"
 	"errors"
-	"reflect"
 	"testing"
 	"time"
 
 	"github.com/gomodule/redigo/redis"
 	"github.com/stretchr/testify/require"
 )
 
-func expectPushed(t *testing.T, c redis.PubSubConn, message string, expected interface{}) {
-	actual := c.Receive()
-	if !reflect.DeepEqual(actual, expected) {
-		t.Errorf("%s = %v, want %v", message, actual, expected)
-	}
-}
-
 func TestPushed(t *testing.T) {
 	pc, err := redis.DialDefaultServer()
-	if err != nil {
-		t.Fatalf("error connection to database, %v", err)
-	}
+	require.NoError(t, err)
 	defer pc.Close()
 
 	sc, err := redis.DialDefaultServer()
-	if err != nil {
-		t.Fatalf("error connection to database, %v", err)
-	}
+	require.NoError(t, err)
 	defer sc.Close()
 
 	c := redis.PubSubConn{Conn: sc}
 
 	require.NoError(t, c.Subscribe("c1"))
-	expectPushed(t, c, "Subscribe(c1)", redis.Subscription{Kind: "subscribe", Channel: "c1", Count: 1})
+	require.Equal(t, redis.Subscription{Kind: "subscribe", Channel: "c1", Count: 1}, c.Receive())
 	require.NoError(t, c.Subscribe("c2"))
-	expectPushed(t, c, "Subscribe(c2)", redis.Subscription{Kind: "subscribe", Channel: "c2", Count: 2})
+	require.Equal(t, redis.Subscription{Kind: "subscribe", Channel: "c2", Count: 2}, c.Receive())
 	require.NoError(t, c.PSubscribe("p1"))
-	expectPushed(t, c, "PSubscribe(p1)", redis.Subscription{Kind: "psubscribe", Channel: "p1", Count: 3})
+	require.Equal(t, redis.Subscription{Kind: "psubscribe", Channel: "p1", Count: 3}, c.Receive())
 	require.NoError(t, c.PSubscribe("p2"))
-	expectPushed(t, c, "PSubscribe(p2)", redis.Subscription{Kind: "psubscribe", Channel: "p2", Count: 4})
+	require.Equal(t, redis.Subscription{Kind: "psubscribe", Channel: "p2", Count: 4}, c.Receive())
 	require.NoError(t, c.PUnsubscribe())
-	expectPushed(t, c, "Punsubscribe(p1)", redis.Subscription{Kind: "punsubscribe", Channel: "p1", Count: 3})
-	expectPushed(t, c, "Punsubscribe()", redis.Subscription{Kind: "punsubscribe", Channel: "p2", Count: 2})
+
+	// Response can return in any order.
+	v := c.Receive()
+	require.IsType(t, redis.Subscription{}, v)
+	u := v.(redis.Subscription)
+	expected1 := redis.Subscription{Kind: "punsubscribe", Channel: "p1", Count: 3}
+	expected2 := redis.Subscription{Kind: "punsubscribe", Channel: "p2", Count: 2}
+	if u.Channel == "p2" {
+		// Order reversed.
+		expected1.Channel = "p2"
+		expected2.Channel = "p1"
+	}
+	require.Equal(t, expected1, u)
+	require.Equal(t, expected2, c.Receive())
 
 	_, err = pc.Do("PUBLISH", "c1", "hello")
 	require.NoError(t, err)
-	expectPushed(t, c, "PUBLISH c1 hello", redis.Message{Channel: "c1", Data: []byte("hello")})
+	require.Equal(t, redis.Message{Channel: "c1", Data: []byte("hello")}, c.Receive())
 
 	require.NoError(t, c.Ping("hello"))
-	expectPushed(t, c, `Ping("hello")`, redis.Pong{Data: "hello"})
+	require.Equal(t, redis.Pong{Data: "hello"}, c.Receive())
 
 	require.NoError(t, c.Conn.Send("PING"))
 	c.Conn.Flush()
-	expectPushed(t, c, `Send("PING")`, redis.Pong{})
+	require.Equal(t, redis.Pong{}, c.Receive())
 
 	require.NoError(t, c.Ping("timeout"))
 	got := c.ReceiveWithTimeout(time.Minute)
@@ -87,7 +87,7 @@ func TestPubSubReceiveContext(t *testing.T) {
 	c := redis.PubSubConn{Conn: sc}
 
 	require.NoError(t, c.Subscribe("c1"))
-	expectPushed(t, c, "Subscribe(c1)", redis.Subscription{Kind: "subscribe", Channel: "c1", Count: 1})
+	require.Equal(t, redis.Subscription{Kind: "subscribe", Channel: "c1", Count: 1}, c.Receive())
 
 	ctx, cancel := context.WithCancel(context.Background())
 	cancel()

diff --git a/redis/reply_test.go b/redis/reply_test.go
@@ -19,6 +19,7 @@ import (
 	"math"
 	"reflect"
 	"strconv"
+	"strings"
 	"testing"
 	"time"
 
@@ -226,6 +227,17 @@ func TestSlowLog(t *testing.T) {
 	}
 }
 
+// debugCheck skips t if err indicates that DEBUG is not allowed,
+// otherwise it fails if err is not nil.
+func debugCheck(t *testing.T, err error) {
+	t.Helper()
+
+	if err != nil && strings.Contains(err.Error(), "ERR DEBUG command not allowed") {
+		t.Skip("DEBUG command not allowed")
+	}
+	require.NoError(t, err)
+}
+
 func TestLatency(t *testing.T) {
 	c, err := dial()
 	require.NoError(t, err)
@@ -253,7 +265,7 @@ func TestLatency(t *testing.T) {
 
 	// Sleep for 1ms to register a slow event.
 	_, err = c.Do("DEBUG", "SLEEP", 0.001)
-	require.NoError(t, err)
+	debugCheck(t, err)
 
 	result, err = c.Do("LATENCY", "LATEST")
 	require.NoError(t, err)
@@ -305,7 +317,7 @@ func TestLatencyHistories(t *testing.T) {
 
 	// Sleep for 1ms to register a slow event
 	_, err = c.Do("DEBUG", "SLEEP", 0.001)
-	require.NoError(t, err)
+	debugCheck(t, err)
 
 	result, err = c.Do("LATENCY", "HISTORY", "command")
 	require.NoError(t, err)