diff --git a/adapter/redis_collection_fastpath_test.go b/adapter/redis_collection_fastpath_test.go index 42f40a6a..80c75dec 100644 --- a/adapter/redis_collection_fastpath_test.go +++ b/adapter/redis_collection_fastpath_test.go @@ -2,6 +2,7 @@ package adapter import ( "context" + "errors" "testing" "time" @@ -30,14 +31,6 @@ import ( const collectionFastPathTTL = 80 * time.Millisecond -// waitForTTLExpiry sleeps past a short TTL so the next read sees the -// key as expired. A small headroom accounts for wall-clock jitter on -// CI; tests that are sensitive to this use short TTLs (~80 ms) so the -// delay stays well under a second. -func waitForTTLExpiry() { - time.Sleep(collectionFastPathTTL + 50*time.Millisecond) -} - func TestRedis_HGET_FastPathHit(t *testing.T) { t.Parallel() nodes, _, _ := createNode(t, 3) @@ -107,10 +100,10 @@ func TestRedis_HGET_TTLExpired(t *testing.T) { require.NoError(t, rdb.HSet(ctx, "h:ttl", "field", "value").Err()) require.NoError(t, rdb.PExpire(ctx, "h:ttl", collectionFastPathTTL).Err()) - waitForTTLExpiry() - - _, err := rdb.HGet(ctx, "h:ttl", "field").Result() - require.ErrorIs(t, err, redis.Nil, "HGET on an expired hash must return nil") + eventuallyExpired(t, collectionFastPathTTL, func() bool { + _, err := rdb.HGet(ctx, "h:ttl", "field").Result() + return errors.Is(err, redis.Nil) + }, "HGET on an expired hash must return nil") } func TestRedis_HEXISTS_FastPathHit(t *testing.T) { @@ -184,11 +177,10 @@ func TestRedis_HEXISTS_TTLExpired(t *testing.T) { require.NoError(t, rdb.HSet(ctx, "he:ttl", "field", "v").Err()) require.NoError(t, rdb.PExpire(ctx, "he:ttl", collectionFastPathTTL).Err()) - waitForTTLExpiry() - - ok, err := rdb.HExists(ctx, "he:ttl", "field").Result() - require.NoError(t, err) - require.False(t, ok, "HEXISTS on an expired hash must return 0") + eventuallyExpired(t, collectionFastPathTTL, func() bool { + ok, err := rdb.HExists(ctx, "he:ttl", "field").Result() + return err == nil && !ok + }, "HEXISTS on an expired hash must return 0") } func TestRedis_SISMEMBER_FastPathHit(t *testing.T) { @@ -262,11 +254,10 @@ func TestRedis_SISMEMBER_TTLExpired(t *testing.T) { require.NoError(t, rdb.SAdd(ctx, "s:ttl", "member").Err()) require.NoError(t, rdb.PExpire(ctx, "s:ttl", collectionFastPathTTL).Err()) - waitForTTLExpiry() - - ok, err := rdb.SIsMember(ctx, "s:ttl", "member").Result() - require.NoError(t, err) - require.False(t, ok, "SISMEMBER on an expired set must return 0") + eventuallyExpired(t, collectionFastPathTTL, func() bool { + ok, err := rdb.SIsMember(ctx, "s:ttl", "member").Result() + return err == nil && !ok + }, "SISMEMBER on an expired set must return 0") } // TestRedis_HGET_FastPathGuardDualEncoding exercises the string- diff --git a/adapter/redis_lua_collection_fastpath_test.go b/adapter/redis_lua_collection_fastpath_test.go index 2494a5c4..bb461f37 100644 --- a/adapter/redis_lua_collection_fastpath_test.go +++ b/adapter/redis_lua_collection_fastpath_test.go @@ -2,6 +2,7 @@ package adapter import ( "context" + "errors" "math" "strconv" "testing" @@ -29,10 +30,6 @@ import ( const luaFastPathTTL = 80 * time.Millisecond -func waitForLuaTTL() { - time.Sleep(luaFastPathTTL + 50*time.Millisecond) -} - func TestLua_HGET_FastPathHit(t *testing.T) { t.Parallel() nodes, _, _ := createNode(t, 3) @@ -117,13 +114,13 @@ func TestLua_HGET_TTLExpired(t *testing.T) { require.NoError(t, rdb.HSet(ctx, "lua:h:ttl", "field", "v").Err()) require.NoError(t, rdb.PExpire(ctx, "lua:h:ttl", luaFastPathTTL).Err()) - waitForLuaTTL() - - _, err := rdb.Eval(ctx, - `return redis.call("HGET", KEYS[1], "field")`, - []string{"lua:h:ttl"}, - ).Result() - require.ErrorIs(t, err, redis.Nil, "HGET on an expired hash from Lua must surface as nil") + eventuallyExpired(t, luaFastPathTTL, func() bool { + _, err := rdb.Eval(ctx, + `return redis.call("HGET", KEYS[1], "field")`, + []string{"lua:h:ttl"}, + ).Result() + return errors.Is(err, redis.Nil) + }, "HGET on an expired hash from Lua must surface as nil") } func TestLua_HEXISTS_FastPathHit(t *testing.T) { @@ -190,14 +187,13 @@ func TestLua_HEXISTS_TTLExpired(t *testing.T) { require.NoError(t, rdb.HSet(ctx, "lua:he:ttl", "f", "v").Err()) require.NoError(t, rdb.PExpire(ctx, "lua:he:ttl", luaFastPathTTL).Err()) - waitForLuaTTL() - - got, err := rdb.Eval(ctx, - `return redis.call("HEXISTS", KEYS[1], "f")`, - []string{"lua:he:ttl"}, - ).Result() - require.NoError(t, err) - require.Equal(t, int64(0), got) + eventuallyExpired(t, luaFastPathTTL, func() bool { + got, err := rdb.Eval(ctx, + `return redis.call("HEXISTS", KEYS[1], "f")`, + []string{"lua:he:ttl"}, + ).Result() + return err == nil && got == int64(0) + }, "HEXISTS on an expired hash from Lua must return 0") } func TestLua_SISMEMBER_FastPathHit(t *testing.T) { @@ -553,13 +549,13 @@ func TestLua_ZSCORE_TTLExpired(t *testing.T) { require.NoError(t, rdb.ZAdd(ctx, "lua:z:ttl", redis.Z{Score: 1, Member: "m"}).Err()) require.NoError(t, rdb.PExpire(ctx, "lua:z:ttl", luaFastPathTTL).Err()) - waitForLuaTTL() - - _, err := rdb.Eval(ctx, - `return redis.call("ZSCORE", KEYS[1], "m")`, - []string{"lua:z:ttl"}, - ).Result() - require.ErrorIs(t, err, redis.Nil) + eventuallyExpired(t, luaFastPathTTL, func() bool { + _, err := rdb.Eval(ctx, + `return redis.call("ZSCORE", KEYS[1], "m")`, + []string{"lua:z:ttl"}, + ).Result() + return errors.Is(err, redis.Nil) + }, "ZSCORE on an expired zset from Lua must return nil") } func TestLua_ZSCORE_SetThenZScoreReturnsWrongType(t *testing.T) { @@ -847,14 +843,14 @@ func TestLua_ZRANGEBYSCORE_TTLExpired(t *testing.T) { require.NoError(t, rdb.ZAdd(ctx, "lua:zr:ttl", redis.Z{Score: 1, Member: "m"}).Err()) require.NoError(t, rdb.PExpire(ctx, "lua:zr:ttl", luaFastPathTTL).Err()) - waitForLuaTTL() - - got, err := rdb.Eval(ctx, - `return redis.call("ZRANGEBYSCORE", KEYS[1], "-inf", "+inf")`, - []string{"lua:zr:ttl"}, - ).Result() - require.NoError(t, err) - require.Equal(t, []any{}, got) + eventuallyExpired(t, luaFastPathTTL, func() bool { + got, err := rdb.Eval(ctx, + `return redis.call("ZRANGEBYSCORE", KEYS[1], "-inf", "+inf")`, + []string{"lua:zr:ttl"}, + ).Result() + gotArr, ok := got.([]any) + return err == nil && ok && len(gotArr) == 0 + }, "ZRANGEBYSCORE on an expired zset from Lua must return an empty array") } // --- Correctness regressions surfaced by PR #570 review --- diff --git a/adapter/redis_misskey_compat_test.go b/adapter/redis_misskey_compat_test.go index e7e7d30a..44cd998f 100644 --- a/adapter/redis_misskey_compat_test.go +++ b/adapter/redis_misskey_compat_test.go @@ -2,6 +2,7 @@ package adapter import ( "context" + "errors" "testing" "time" @@ -43,8 +44,15 @@ func TestRedis_MisskeyConnectionCompatibility(t *testing.T) { require.NoError(t, err) require.Greater(t, pttl, time.Duration(0)) - time.Sleep(1100 * time.Millisecond) - require.ErrorIs(t, rdb.Get(ctx, "lock:ap-object").Err(), redis.Nil) + // The "lock:ap-object" key was just SET with EX 1 (1-second TTL). + // Wait for it to expire via the deadline-multiplier helper rather + // than a fixed time.Sleep — the previous Sleep(1100ms) raced CI + // scheduler jitter on slow runners (the post-sleep GET could fire + // before the TTL had actually been processed by the sweeper). + const lockTTL = 1 * time.Second + eventuallyExpired(t, lockTTL, func() bool { + return errors.Is(rdb.Get(ctx, "lock:ap-object").Err(), redis.Nil) + }, "lock:ap-object must be gone after its 1-second EX TTL") } func TestRedis_MisskeyPubSubCompatibility(t *testing.T) { diff --git a/adapter/test_util.go b/adapter/test_util.go index ba88cfef..744249a6 100644 --- a/adapter/test_util.go +++ b/adapter/test_util.go @@ -707,6 +707,49 @@ func isTransientNotLeaderErr(err error) bool { return strings.Contains(s, "not leader") || strings.Contains(s, "leader not found") } +// === TTL-expiry deadline-multiplier helper ============================= +// +// Redis TTL tests that "set a short TTL, sleep past it, assert the key is +// gone" race their own wall clock under -race on slow CI runners. The +// inter-call pause (typically 50–150 ms above the TTL) does not absorb +// scheduler jitter that can push the post-sleep read past the TTL window +// itself, so the read sometimes observes a refilled/un-expired view and +// the test fails with "expected redis.Nil, got ". PR #818 +// (TestRedis_ExpiredKey_BecomesInvisible) fixed one instance by switching +// to require.Eventually with a TTL-derived deadline. eventuallyExpired +// generalises that pattern for any TTL-expiry condition. +// +// Use this helper in any new test of the form: +// +// require.NoError(t, rdb.PExpire(ctx, key, ttl).Err()) +// eventuallyExpired(t, ttl, func() bool { +// _, err := rdb.(ctx, key, ...).Result() +// return errors.Is(err, redis.Nil) +// }, "key must be gone after TTL") +// +// instead of `time.Sleep(ttl + N*time.Millisecond)` followed by a single +// assertion. The deadline = ttl + 3 s gives generous CI headroom; the +// 25 ms poll cadence matches waitForStableLeader so flake patterns stay +// uniform across the file. +func eventuallyExpired(t *testing.T, ttl time.Duration, condition func() bool, msg string) { + t.Helper() + const ( + ttlExpiryHeadroom = 3 * time.Second + ttlExpiryPoll = 25 * time.Millisecond + ) + // Sleep past the TTL before the first poll. require.Eventually + // runs the condition once IMMEDIATELY before the first tick, so + // without this gate a regression that deletes/hides the key at + // PExpire time would satisfy the first poll and the test would + // never exercise the actual expired-after-deadline state — see + // codex P2 on PR #903. The post-sleep require.Eventually then + // has ttlExpiryHeadroom (3 s) of CI-jitter slack to observe the + // expired state, with a 25 ms poll cadence that matches + // waitForStableLeader's sampler so flake patterns stay uniform. + time.Sleep(ttl) + require.Eventually(t, condition, ttlExpiryHeadroom, ttlExpiryPoll, msg) +} + // doEventually retries do() while it returns a transient "not leader" error, // giving the cluster a few seconds to re-settle leadership after startup. // Non-"not leader" errors fail the test immediately.