Repository: zeromicro/go-zero
Pull request #3958 (2024-03-02 13:47:02)

chore: add more tests

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/zeromicro/go-zero/pull/3958?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zeromicro) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 94.68%. Comparing base [(`9763c8b`)](https://app.codecov.io/gh/zeromicro/go-zero/commit/9763c8b143afbf42ed6df988f67a885ddce8bd52?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zeromicro) to head [(`45ff527`)](https://app.codecov.io/gh/zeromicro/go-zero/pull/3958?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zeromicro).

<details><summary>Additional details and impacted files</summary>



| [Files](https://app.codecov.io/gh/zeromicro/go-zero/pull/3958?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zeromicro) | Coverage Δ | |
|---|---|---|
| [core/stores/redis/redis.go](https://app.codecov.io/gh/zeromicro/go-zero/pull/3958?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zeromicro#diff-Y29yZS9zdG9yZXMvcmVkaXMvcmVkaXMuZ28=) | `99.75% <100.00%> (+1.41%)` | :arrow_up: |

... and [3 files with indirect coverage changes](https://app.codecov.io/gh/zeromicro/go-zero/pull/3958/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zeromicro)

</details>

Patch:
diff --git a/core/stores/redis/redis.go b/core/stores/redis/redis.go
index bbad891c..8f2ee9f2 100644
--- a/core/stores/redis/redis.go
+++ b/core/stores/redis/redis.go
@@ -483,13 +483,8 @@ func (s *Redis) ExistsManyCtx(ctx context.Context, keys ...string) (val int64, e
 			return err
 		}
 
-		v, err := conn.Exists(ctx, keys...).Result()
-		if err != nil {
-			return err
-		}
-
-		val = v
-		return nil
+		val, err = conn.Exists(ctx, keys...).Result()
+		return err
 	}, acceptable)
 
 	return
@@ -543,13 +538,8 @@ func (s *Redis) GeoAddCtx(ctx context.Context, key string, geoLocation ...*GeoLo
 			return err
 		}
 
-		v, err := conn.GeoAdd(ctx, key, geoLocation...).Result()
-		if err != nil {
-			return err
-		}
-
-		val = v
-		return nil
+		val, err = conn.GeoAdd(ctx, key, geoLocation...).Result()
+		return err
 	}, acceptable)
 
 	return
@@ -569,13 +559,8 @@ func (s *Redis) GeoDistCtx(ctx context.Context, key, member1, member2, unit stri
 			return err
 		}
 
-		v, err := conn.GeoDist(ctx, key, member1, member2, unit).Result()
-		if err != nil {
-			return err
-		}
-
-		val = v
-		return nil
+		val, err = conn.GeoDist(ctx, key, member1, member2, unit).Result()
+		return err
 	}, acceptable)
 
 	return
@@ -595,13 +580,8 @@ func (s *Redis) GeoHashCtx(ctx context.Context, key string, members ...string) (
 			return err
 		}
 
-		v, err := conn.GeoHash(ctx, key, members...).Result()
-		if err != nil {
-			return err
-		}
-
-		val = v
-		return nil
+		val, err = conn.GeoHash(ctx, key, members...).Result()
+		return err
 	}, acceptable)
 
 	return
@@ -622,13 +602,8 @@ func (s *Redis) GeoRadiusCtx(ctx context.Context, key string, longitude, latitud
 			return err
 		}
 
-		v, err := conn.GeoRadius(ctx, key, longitude, latitude, query).Result()
-		if err != nil {
-			return err
-		}
-
-		val = v
-		return nil
+		val, err = conn.GeoRadius(ctx, key, longitude, latitude, query).Result()
+		return err
 	}, acceptable)
 
 	return
@@ -648,13 +623,8 @@ func (s *Redis) GeoRadiusByMemberCtx(ctx context.Context, key, member string,
 			return err
 		}
 
-		v, err := conn.GeoRadiusByMember(ctx, key, member, query).Result()
-		if err != nil {
-			return err
-		}
-
-		val = v
-		return nil
+		val, err = conn.GeoRadiusByMember(ctx, key, member, query).Result()
+		return err
 	}, acceptable)
 
 	return
@@ -674,13 +644,8 @@ func (s *Redis) GeoPosCtx(ctx context.Context, key string, members ...string) (
 			return err
 		}
 
-		v, err := conn.GeoPos(ctx, key, members...).Result()
-		if err != nil {
-			return err
-		}
-
-		val = v
-		return nil
+		val, err = conn.GeoPos(ctx, key, members...).Result()
+		return err
 	}, acceptable)
 
 	return
@@ -699,7 +664,7 @@ func (s *Redis) GetCtx(ctx context.Context, key string) (val string, err error)
 			return err
 		}
 
-		if val, err = conn.Get(ctx, key).Result(); err == red.Nil {
+		if val, err = conn.Get(ctx, key).Result(); errors.Is(err, red.Nil) {
 			return nil
 		} else if err != nil {
 			return err
@@ -749,7 +714,7 @@ func (s *Redis) GetSetCtx(ctx context.Context, key, value string) (val string, e
 			return err
 		}
 
-		if val, err = conn.GetSet(ctx, key, value).Result(); err == red.Nil {
+		if val, err = conn.GetSet(ctx, key, value).Result(); errors.Is(err, red.Nil) {
 			return nil
 		}
 
@@ -875,17 +840,16 @@ func (s *Redis) HincrbyFloat(key, field string, increment float64) (float64, err
 }
 
 // HincrbyFloatCtx is the implementation of redis hincrbyfloat command.
-func (s *Redis) HincrbyFloatCtx(ctx context.Context, key, field string, increment float64) (val float64, err error) {
+func (s *Redis) HincrbyFloatCtx(ctx context.Context, key, field string, increment float64) (
+	val float64, err error) {
 	err = s.brk.DoWithAcceptable(func() error {
 		conn, err := getRedis(s)
 		if err != nil {
 			return err
 		}
+
 		val, err = conn.HIncrByFloat(ctx, key, field, increment).Result()
-		if err != nil {
-			return err
-		}
-		return nil
+		return err
 	}, acceptable)
 
 	return
@@ -1353,11 +1317,7 @@ func (s *Redis) MsetCtx(ctx context.Context, fieldsAndValues ...any) (val string
 		}
 
 		val, err = conn.MSet(ctx, fieldsAndValues...).Result()
-		if err != nil {
-			return err
-		}
-
-		return nil
+		return err
 	}, acceptable)
 
 	return
@@ -2052,6 +2012,7 @@ func (s *Redis) TtlCtx(ctx context.Context, key string) (val int, err error) {
 			// -1 means key exists but has no expire
 			val = int(duration)
 		}
+
 		return nil
 	}, acceptable)
 
@@ -2157,13 +2118,8 @@ func (s *Redis) ZaddsCtx(ctx context.Context, key string, ps ...Pair) (val int64
 			zs = append(zs, z)
 		}
 
-		v, err := conn.ZAdd(ctx, key, zs...).Result()
-		if err != nil {
-			return err
-		}
-
-		val = v
-		return nil
+		val, err = conn.ZAdd(ctx, key, zs...).Result()
+		return err
 	}, acceptable)
 
 	return
@@ -2283,6 +2239,7 @@ func (s *Redis) ZscoreByFloatCtx(ctx context.Context, key, value string) (val fl
 		if err != nil {
 			return err
 		}
+
 		val, err = conn.ZScore(ctx, key, value).Result()
 		return err
 	}, acceptable)
@@ -2947,7 +2904,7 @@ func withHook(hook red.Hook) Option {
 }
 
 func acceptable(err error) bool {
-	return err == nil || err == red.Nil || errors.Is(err, context.Canceled)
+	return err == nil || errors.Is(err, red.Nil) || errors.Is(err, context.Canceled)
 }
 
 func getRedis(r *Redis) (RedisNode, error) {


Test patch:
diff --git a/core/stores/redis/redis_test.go b/core/stores/redis/redis_test.go
index 29cc5417..cdb082ab 100644
--- a/core/stores/redis/redis_test.go
+++ b/core/stores/redis/redis_test.go
@@ -911,9 +911,11 @@ func TestRedis_Persist(t *testing.T) {
 }
 
 func TestRedis_Ping(t *testing.T) {
-	runOnRedis(t, func(client *Redis) {
-		ok := client.Ping()
-		assert.True(t, ok)
+	t.Run("ping", func(t *testing.T) {
+		runOnRedis(t, func(client *Redis) {
+			ok := client.Ping()
+			assert.True(t, ok)
+		})
 	})
 }
 
@@ -1070,24 +1072,40 @@ func TestRedis_Set(t *testing.T) {
 }
 
 func TestRedis_GetSet(t *testing.T) {
-	runOnRedis(t, func(client *Redis) {
-		_, err := newRedis(client.Addr, badType()).GetSet("hello", "world")
-		assert.NotNil(t, err)
-		val, err := client.GetSet("hello", "world")
-		assert.Nil(t, err)
-		assert.Equal(t, "", val)
-		val, err = client.Get("hello")
-		assert.Nil(t, err)
-		assert.Equal(t, "world", val)
-		val, err = client.GetSet("hello", "newworld")
-		assert.Nil(t, err)
-		assert.Equal(t, "world", val)
-		val, err = client.Get("hello")
-		assert.Nil(t, err)
-		assert.Equal(t, "newworld", val)
-		ret, err := client.Del("hello")
-		assert.Nil(t, err)
-		assert.Equal(t, 1, ret)
+	t.Run("set_get", func(t *testing.T) {
+		runOnRedis(t, func(client *Redis) {
+			_, err := newRedis(client.Addr, badType()).GetSet("hello", "world")
+			assert.NotNil(t, err)
+			val, err := client.GetSet("hello", "world")
+			assert.Nil(t, err)
+			assert.Equal(t, "", val)
+			val, err = client.Get("hello")
+			assert.Nil(t, err)
+			assert.Equal(t, "world", val)
+			val, err = client.GetSet("hello", "newworld")
+			assert.Nil(t, err)
+			assert.Equal(t, "world", val)
+			val, err = client.Get("hello")
+			assert.Nil(t, err)
+			assert.Equal(t, "newworld", val)
+			ret, err := client.Del("hello")
+			assert.Nil(t, err)
+			assert.Equal(t, 1, ret)
+		})
+	})
+
+	t.Run("set_get with notexists", func(t *testing.T) {
+		runOnRedis(t, func(client *Redis) {
+			_, err := client.Get("hello")
+			assert.NoError(t, err)
+		})
+	})
+
+	t.Run("set_get with error", func(t *testing.T) {
+		runOnRedisWithError(t, func(client *Redis) {
+			_, err := client.Get("hello")
+			assert.Error(t, err)
+		})
 	})
 }
 
@@ -1823,9 +1841,9 @@ func TestRedisBlpop(t *testing.T) {
 		client.Ping()
 		var node mockedNode
 		_, err := client.Blpop(nil, "foo")
-		assert.NotNil(t, err)
+		assert.Error(t, err)
 		_, err = client.Blpop(node, "foo")
-		assert.NotNil(t, err)
+		assert.NoError(t, err)
 	})
 }
 
@@ -1837,19 +1855,60 @@ func TestRedisBlpopEx(t *testing.T) {
 			_, _, err := client.BlpopEx(nil, "foo")
 			assert.Error(t, err)
 			_, _, err = client.BlpopEx(node, "foo")
+			assert.NoError(t, err)
+		})
+	})
+
+	t.Run("blpopex with expire", func(t *testing.T) {
+		runOnRedis(t, func(client *Redis) {
+			client.Ping()
+			node, err := getRedis(client)
+			assert.NoError(t, err)
+			assert.NoError(t, client.Set("foo", "bar"))
+			_, _, err = client.BlpopEx(node, "foo")
+			assert.Error(t, err)
+		})
+	})
+
+	t.Run("blpopex with bad return", func(t *testing.T) {
+		runOnRedis(t, func(client *Redis) {
+			client.Ping()
+			node := &mockedNode{args: []string{"bar"}}
+			_, _, err := client.BlpopEx(node, "foo")
 			assert.Error(t, err)
 		})
 	})
 }
 
 func TestRedisBlpopWithTimeout(t *testing.T) {
-	runOnRedis(t, func(client *Redis) {
-		client.Ping()
-		var node mockedNode
-		_, err := client.BlpopWithTimeout(nil, 10*time.Second, "foo")
-		assert.NotNil(t, err)
-		_, err = client.BlpopWithTimeout(node, 10*time.Second, "foo")
-		assert.NotNil(t, err)
+	t.Run("blpop_withTimeout", func(t *testing.T) {
+		runOnRedis(t, func(client *Redis) {
+			client.Ping()
+			var node mockedNode
+			_, err := client.BlpopWithTimeout(nil, 10*time.Second, "foo")
+			assert.Error(t, err)
+			_, err = client.BlpopWithTimeout(node, 10*time.Second, "foo")
+			assert.NoError(t, err)
+		})
+	})
+
+	t.Run("blpop_withTimeout_error", func(t *testing.T) {
+		runOnRedis(t, func(client *Redis) {
+			client.Ping()
+			node, err := getRedis(client)
+			assert.NoError(t, err)
+			assert.NoError(t, client.Set("foo", "bar"))
+			_, err = client.BlpopWithTimeout(node, time.Millisecond, "foo")
+			assert.Error(t, err)
+		})
+	})
+
+	t.Run("blpop_with_bad_return", func(t *testing.T) {
+		runOnRedis(t, func(client *Redis) {
+			node := &mockedNode{args: []string{"foo"}}
+			_, err := client.Blpop(node, "foo")
+			assert.Error(t, err)
+		})
 	})
 }
 
@@ -1944,6 +2003,22 @@ func TestRedis_WithPass(t *testing.T) {
 	})
 }
 
+func TestRedis_checkConnection(t *testing.T) {
+	t.Run("checkConnection", func(t *testing.T) {
+		runOnRedis(t, func(client *Redis) {
+			client.Ping()
+			assert.NoError(t, client.checkConnection(time.Millisecond))
+		})
+	})
+
+	t.Run("checkConnection error", func(t *testing.T) {
+		runOnRedisWithError(t, func(client *Redis) {
+			assert.Error(t, newRedis(client.Addr, badType()).checkConnection(time.Millisecond))
+			assert.Error(t, client.checkConnection(time.Millisecond))
+		})
+	})
+}
+
 func runOnRedis(t *testing.T, fn func(client *Redis)) {
 	logx.Disable()
 
@@ -1992,8 +2067,16 @@ func badType() Option {
 
 type mockedNode struct {
 	RedisNode
+	args []string
 }
 
 func (n mockedNode) BLPop(_ context.Context, _ time.Duration, _ ...string) *red.StringSliceCmd {
-	return red.NewStringSliceCmd(context.Background(), "foo", "bar")
+	cmd := red.NewStringSliceCmd(context.Background())
+	if len(n.args) == 0 {
+		cmd.SetVal([]string{"foo", "bar"})
+	} else {
+		cmd.SetVal(n.args)
+	}
+
+	return cmd
 }