From a5d2b971a16dc382c8676c7291e1bbfffdb0b684 Mon Sep 17 00:00:00 2001 From: Kevin Wan Date: Sat, 2 Mar 2024 21:58:13 +0800 Subject: [PATCH] chore: add more tests (#3958) --- core/stores/redis/redis.go | 95 ++++++--------------- core/stores/redis/redis_test.go | 145 +++++++++++++++++++++++++------- 2 files changed, 140 insertions(+), 100 deletions(-) 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) { 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 }