From c31849cbc757f7346b87e1365735be6fc3829710 Mon Sep 17 00:00:00 2001 From: Vladyslav Vildanov <117659936+vladvildanov@users.noreply.github.com> Date: Wed, 2 Oct 2024 18:06:38 +0300 Subject: [PATCH] Fix bug with Redis Set commands returns List instead of Set (#3399) * Fix bug with Redis Set commands returns List instead of Set in RESP2 * Removed fixture, codestyle fixes * Fixed tests for async * Fixed asyncio cluster test cases * Added Sets alignment for RESP2 and RESP3 * Updated doctests --- doctests/dt_set.py | 34 ++++++++++++++--------------- redis/_parsers/helpers.py | 6 +++++ tests/test_asyncio/test_cluster.py | 20 ++++++++--------- tests/test_asyncio/test_commands.py | 20 ++++++++--------- tests/test_cache.py | 2 +- tests/test_cluster.py | 24 ++++++++++---------- tests/test_commands.py | 20 ++++++++--------- 7 files changed, 66 insertions(+), 60 deletions(-) diff --git a/doctests/dt_set.py b/doctests/dt_set.py index 0c0562ac80..fc66410b45 100644 --- a/doctests/dt_set.py +++ b/doctests/dt_set.py @@ -58,11 +58,11 @@ r.sadd("bikes:racing:usa", "bike:1", "bike:4") # HIDE_END res7 = r.sinter("bikes:racing:france", "bikes:racing:usa") -print(res7) # >>> ['bike:1'] +print(res7) # >>> {'bike:1'} # STEP_END # REMOVE_START -assert res7 == ["bike:1"] +assert res7 == {"bike:1"} # REMOVE_END # STEP_START scard @@ -83,12 +83,12 @@ print(res9) # >>> 3 res10 = r.smembers("bikes:racing:france") -print(res10) # >>> ['bike:1', 'bike:2', 'bike:3'] +print(res10) # >>> {'bike:1', 'bike:2', 'bike:3'} # STEP_END # REMOVE_START assert res9 == 3 -assert res10 == ['bike:1', 'bike:2', 'bike:3'] +assert res10 == {'bike:1', 'bike:2', 'bike:3'} # REMOVE_END # STEP_START smismember @@ -109,11 +109,11 @@ r.sadd("bikes:racing:usa", "bike:1", "bike:4") res13 = r.sdiff("bikes:racing:france", "bikes:racing:usa") -print(res13) # >>> ['bike:2', 'bike:3'] +print(res13) # >>> {'bike:2', 'bike:3'} # STEP_END # REMOVE_START -assert res13 == ['bike:2', 'bike:3'] +assert res13 == {'bike:2', 'bike:3'} r.delete("bikes:racing:france") r.delete("bikes:racing:usa") # REMOVE_END @@ -124,27 +124,27 @@ r.sadd("bikes:racing:italy", "bike:1", "bike:2", "bike:3", "bike:4") res13 = r.sinter("bikes:racing:france", "bikes:racing:usa", "bikes:racing:italy") -print(res13) # >>> ['bike:1'] +print(res13) # >>> {'bike:1'} res14 = r.sunion("bikes:racing:france", "bikes:racing:usa", "bikes:racing:italy") -print(res14) # >>> ['bike:1', 'bike:2', 'bike:3', 'bike:4'] +print(res14) # >>> {'bike:1', 'bike:2', 'bike:3', 'bike:4'} res15 = r.sdiff("bikes:racing:france", "bikes:racing:usa", "bikes:racing:italy") -print(res15) # >>> [] +print(res15) # >>> {} res16 = r.sdiff("bikes:racing:usa", "bikes:racing:france") -print(res16) # >>> ['bike:4'] +print(res16) # >>> {'bike:4'} res17 = r.sdiff("bikes:racing:france", "bikes:racing:usa") -print(res17) # >>> ['bike:2', 'bike:3'] +print(res17) # >>> {'bike:2', 'bike:3'} # STEP_END # REMOVE_START -assert res13 == ['bike:1'] -assert res14 == ['bike:1', 'bike:2', 'bike:3', 'bike:4'] -assert res15 == [] -assert res16 == ['bike:4'] -assert res17 == ['bike:2', 'bike:3'] +assert res13 == {'bike:1'} +assert res14 == {'bike:1', 'bike:2', 'bike:3', 'bike:4'} +assert res15 == {} +assert res16 == {'bike:4'} +assert res17 == {'bike:2', 'bike:3'} r.delete("bikes:racing:france") r.delete("bikes:racing:usa") r.delete("bikes:racing:italy") @@ -160,7 +160,7 @@ print(res19) # >>> bike:3 res20 = r.smembers("bikes:racing:france") -print(res20) # >>> ['bike:2', 'bike:4', 'bike:5'] +print(res20) # >>> {'bike:2', 'bike:4', 'bike:5'} res21 = r.srandmember("bikes:racing:france") print(res21) # >>> bike:4 diff --git a/redis/_parsers/helpers.py b/redis/_parsers/helpers.py index 7494c79210..6832100bb6 100644 --- a/redis/_parsers/helpers.py +++ b/redis/_parsers/helpers.py @@ -785,6 +785,9 @@ def string_keys_to_dict(key_string, callback): _RedisCallbacksRESP2 = { + **string_keys_to_dict( + "SDIFF SINTER SMEMBERS SUNION", lambda r: r and set(r) or set() + ), **string_keys_to_dict( "ZDIFF ZINTER ZPOPMAX ZPOPMIN ZRANGE ZRANGEBYSCORE ZRANK ZREVRANGE " "ZREVRANGEBYSCORE ZREVRANK ZUNION", @@ -829,6 +832,9 @@ def string_keys_to_dict(key_string, callback): _RedisCallbacksRESP3 = { + **string_keys_to_dict( + "SDIFF SINTER SMEMBERS SUNION", lambda r: r and set(r) or set() + ), **string_keys_to_dict( "ZRANGE ZINTER ZPOPMAX ZPOPMIN ZRANGEBYSCORE ZREVRANGE ZREVRANGEBYSCORE " "ZUNION HGETALL XREADGROUP", diff --git a/tests/test_asyncio/test_cluster.py b/tests/test_asyncio/test_cluster.py index e480db332b..f3b76b80c9 100644 --- a/tests/test_asyncio/test_cluster.py +++ b/tests/test_asyncio/test_cluster.py @@ -1752,38 +1752,38 @@ async def test_cluster_rpoplpush(self, r: RedisCluster) -> None: async def test_cluster_sdiff(self, r: RedisCluster) -> None: await r.sadd("{foo}a", "1", "2", "3") - assert set(await r.sdiff("{foo}a", "{foo}b")) == {b"1", b"2", b"3"} + assert await r.sdiff("{foo}a", "{foo}b") == {b"1", b"2", b"3"} await r.sadd("{foo}b", "2", "3") - assert await r.sdiff("{foo}a", "{foo}b") == [b"1"] + assert await r.sdiff("{foo}a", "{foo}b") == {b"1"} async def test_cluster_sdiffstore(self, r: RedisCluster) -> None: await r.sadd("{foo}a", "1", "2", "3") assert await r.sdiffstore("{foo}c", "{foo}a", "{foo}b") == 3 - assert set(await r.smembers("{foo}c")) == {b"1", b"2", b"3"} + assert await r.smembers("{foo}c") == {b"1", b"2", b"3"} await r.sadd("{foo}b", "2", "3") assert await r.sdiffstore("{foo}c", "{foo}a", "{foo}b") == 1 - assert await r.smembers("{foo}c") == [b"1"] + assert await r.smembers("{foo}c") == {b"1"} async def test_cluster_sinter(self, r: RedisCluster) -> None: await r.sadd("{foo}a", "1", "2", "3") - assert await r.sinter("{foo}a", "{foo}b") == [] + assert await r.sinter("{foo}a", "{foo}b") == set() await r.sadd("{foo}b", "2", "3") - assert set(await r.sinter("{foo}a", "{foo}b")) == {b"2", b"3"} + assert await r.sinter("{foo}a", "{foo}b") == {b"2", b"3"} async def test_cluster_sinterstore(self, r: RedisCluster) -> None: await r.sadd("{foo}a", "1", "2", "3") assert await r.sinterstore("{foo}c", "{foo}a", "{foo}b") == 0 - assert await r.smembers("{foo}c") == [] + assert await r.smembers("{foo}c") == set() await r.sadd("{foo}b", "2", "3") assert await r.sinterstore("{foo}c", "{foo}a", "{foo}b") == 2 - assert set(await r.smembers("{foo}c")) == {b"2", b"3"} + assert await r.smembers("{foo}c") == {b"2", b"3"} async def test_cluster_smove(self, r: RedisCluster) -> None: await r.sadd("{foo}a", "a1", "a2") await r.sadd("{foo}b", "b1", "b2") assert await r.smove("{foo}a", "{foo}b", "a1") - assert await r.smembers("{foo}a") == [b"a2"] - assert set(await r.smembers("{foo}b")) == {b"b1", b"b2", b"a1"} + assert await r.smembers("{foo}a") == {b"a2"} + assert await r.smembers("{foo}b") == {b"b1", b"b2", b"a1"} async def test_cluster_sunion(self, r: RedisCluster) -> None: await r.sadd("{foo}a", "1", "2") diff --git a/tests/test_asyncio/test_commands.py b/tests/test_asyncio/test_commands.py index 28c3094cdb..f6ed07fab5 100644 --- a/tests/test_asyncio/test_commands.py +++ b/tests/test_asyncio/test_commands.py @@ -1415,34 +1415,34 @@ async def test_scard(self, r: redis.Redis): @pytest.mark.onlynoncluster async def test_sdiff(self, r: redis.Redis): await r.sadd("a", "1", "2", "3") - assert set(await r.sdiff("a", "b")) == {b"1", b"2", b"3"} + assert await r.sdiff("a", "b") == {b"1", b"2", b"3"} await r.sadd("b", "2", "3") - assert await r.sdiff("a", "b") == [b"1"] + assert await r.sdiff("a", "b") == {b"1"} @pytest.mark.onlynoncluster async def test_sdiffstore(self, r: redis.Redis): await r.sadd("a", "1", "2", "3") assert await r.sdiffstore("c", "a", "b") == 3 - assert set(await r.smembers("c")) == {b"1", b"2", b"3"} + assert await r.smembers("c") == {b"1", b"2", b"3"} await r.sadd("b", "2", "3") assert await r.sdiffstore("c", "a", "b") == 1 - assert await r.smembers("c") == [b"1"] + assert await r.smembers("c") == {b"1"} @pytest.mark.onlynoncluster async def test_sinter(self, r: redis.Redis): await r.sadd("a", "1", "2", "3") - assert await r.sinter("a", "b") == [] + assert await r.sinter("a", "b") == set() await r.sadd("b", "2", "3") - assert set(await r.sinter("a", "b")) == {b"2", b"3"} + assert await r.sinter("a", "b") == {b"2", b"3"} @pytest.mark.onlynoncluster async def test_sinterstore(self, r: redis.Redis): await r.sadd("a", "1", "2", "3") assert await r.sinterstore("c", "a", "b") == 0 - assert await r.smembers("c") == [] + assert await r.smembers("c") == set() await r.sadd("b", "2", "3") assert await r.sinterstore("c", "a", "b") == 2 - assert set(await r.smembers("c")) == {b"2", b"3"} + assert await r.smembers("c") == {b"2", b"3"} async def test_sismember(self, r: redis.Redis): await r.sadd("a", "1", "2", "3") @@ -1460,8 +1460,8 @@ async def test_smove(self, r: redis.Redis): await r.sadd("a", "a1", "a2") await r.sadd("b", "b1", "b2") assert await r.smove("a", "b", "a1") - assert await r.smembers("a") == [b"a2"] - assert set(await r.smembers("b")) == {b"b1", b"b2", b"a1"} + assert await r.smembers("a") == {b"a2"} + assert await r.smembers("b") == {b"b1", b"b2", b"a1"} async def test_spop(self, r: redis.Redis): s = [b"1", b"2", b"3"] diff --git a/tests/test_cache.py b/tests/test_cache.py index 1803646094..67733dc9af 100644 --- a/tests/test_cache.py +++ b/tests/test_cache.py @@ -41,7 +41,7 @@ def r(request): @pytest.mark.skipif(HIREDIS_AVAILABLE, reason="PythonParser only") @pytest.mark.onlynoncluster -# @skip_if_resp_version(2) +@skip_if_resp_version(2) @skip_if_server_version_lt("7.4.0") class TestCache: @pytest.mark.parametrize( diff --git a/tests/test_cluster.py b/tests/test_cluster.py index c4b3188050..fe5852d1fb 100644 --- a/tests/test_cluster.py +++ b/tests/test_cluster.py @@ -1865,49 +1865,49 @@ def test_cluster_rpoplpush(self, r): def test_cluster_sdiff(self, r): r.sadd("{foo}a", "1", "2", "3") - assert set(r.sdiff("{foo}a", "{foo}b")) == {b"1", b"2", b"3"} + assert r.sdiff("{foo}a", "{foo}b") == {b"1", b"2", b"3"} r.sadd("{foo}b", "2", "3") - assert r.sdiff("{foo}a", "{foo}b") == [b"1"] + assert r.sdiff("{foo}a", "{foo}b") == {b"1"} def test_cluster_sdiffstore(self, r): r.sadd("{foo}a", "1", "2", "3") assert r.sdiffstore("{foo}c", "{foo}a", "{foo}b") == 3 - assert set(r.smembers("{foo}c")) == {b"1", b"2", b"3"} + assert r.smembers("{foo}c") == {b"1", b"2", b"3"} r.sadd("{foo}b", "2", "3") assert r.sdiffstore("{foo}c", "{foo}a", "{foo}b") == 1 - assert r.smembers("{foo}c") == [b"1"] + assert r.smembers("{foo}c") == {b"1"} def test_cluster_sinter(self, r): r.sadd("{foo}a", "1", "2", "3") - assert r.sinter("{foo}a", "{foo}b") == [] + assert r.sinter("{foo}a", "{foo}b") == set() r.sadd("{foo}b", "2", "3") - assert set(r.sinter("{foo}a", "{foo}b")) == {b"2", b"3"} + assert r.sinter("{foo}a", "{foo}b") == {b"2", b"3"} def test_cluster_sinterstore(self, r): r.sadd("{foo}a", "1", "2", "3") assert r.sinterstore("{foo}c", "{foo}a", "{foo}b") == 0 - assert r.smembers("{foo}c") == [] + assert r.smembers("{foo}c") == set() r.sadd("{foo}b", "2", "3") assert r.sinterstore("{foo}c", "{foo}a", "{foo}b") == 2 - assert set(r.smembers("{foo}c")) == {b"2", b"3"} + assert r.smembers("{foo}c") == {b"2", b"3"} def test_cluster_smove(self, r): r.sadd("{foo}a", "a1", "a2") r.sadd("{foo}b", "b1", "b2") assert r.smove("{foo}a", "{foo}b", "a1") - assert r.smembers("{foo}a") == [b"a2"] - assert set(r.smembers("{foo}b")) == {b"b1", b"b2", b"a1"} + assert r.smembers("{foo}a") == {b"a2"} + assert r.smembers("{foo}b") == {b"b1", b"b2", b"a1"} def test_cluster_sunion(self, r): r.sadd("{foo}a", "1", "2") r.sadd("{foo}b", "2", "3") - assert set(r.sunion("{foo}a", "{foo}b")) == {b"1", b"2", b"3"} + assert r.sunion("{foo}a", "{foo}b") == {b"1", b"2", b"3"} def test_cluster_sunionstore(self, r): r.sadd("{foo}a", "1", "2") r.sadd("{foo}b", "2", "3") assert r.sunionstore("{foo}c", "{foo}a", "{foo}b") == 3 - assert set(r.smembers("{foo}c")) == {b"1", b"2", b"3"} + assert r.smembers("{foo}c") == {b"1", b"2", b"3"} @skip_if_server_version_lt("6.2.0") def test_cluster_zdiff(self, r): diff --git a/tests/test_commands.py b/tests/test_commands.py index 74e9c1c88e..4cad4c14b6 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -2247,25 +2247,25 @@ def test_scard(self, r): @pytest.mark.onlynoncluster def test_sdiff(self, r): r.sadd("a", "1", "2", "3") - assert set(r.sdiff("a", "b")) == {b"1", b"2", b"3"} + assert r.sdiff("a", "b") == {b"1", b"2", b"3"} r.sadd("b", "2", "3") - assert r.sdiff("a", "b") == [b"1"] + assert r.sdiff("a", "b") == {b"1"} @pytest.mark.onlynoncluster def test_sdiffstore(self, r): r.sadd("a", "1", "2", "3") assert r.sdiffstore("c", "a", "b") == 3 - assert set(r.smembers("c")) == {b"1", b"2", b"3"} + assert r.smembers("c") == {b"1", b"2", b"3"} r.sadd("b", "2", "3") assert r.sdiffstore("c", "a", "b") == 1 - assert r.smembers("c") == [b"1"] + assert r.smembers("c") == {b"1"} @pytest.mark.onlynoncluster def test_sinter(self, r): r.sadd("a", "1", "2", "3") - assert r.sinter("a", "b") == [] + assert r.sinter("a", "b") == set() r.sadd("b", "2", "3") - assert set(r.sinter("a", "b")) == {b"2", b"3"} + assert r.sinter("a", "b") == {b"2", b"3"} @pytest.mark.onlynoncluster @skip_if_server_version_lt("7.0.0") @@ -2280,10 +2280,10 @@ def test_sintercard(self, r): def test_sinterstore(self, r): r.sadd("a", "1", "2", "3") assert r.sinterstore("c", "a", "b") == 0 - assert r.smembers("c") == [] + assert r.smembers("c") == set() r.sadd("b", "2", "3") assert r.sinterstore("c", "a", "b") == 2 - assert set(r.smembers("c")) == {b"2", b"3"} + assert r.smembers("c") == {b"2", b"3"} def test_sismember(self, r): r.sadd("a", "1", "2", "3") @@ -2308,8 +2308,8 @@ def test_smove(self, r): r.sadd("a", "a1", "a2") r.sadd("b", "b1", "b2") assert r.smove("a", "b", "a1") - assert r.smembers("a") == [b"a2"] - assert set(r.smembers("b")) == {b"b1", b"b2", b"a1"} + assert r.smembers("a") == {b"a2"} + assert r.smembers("b") == {b"b1", b"b2", b"a1"} def test_spop(self, r): s = [b"1", b"2", b"3"]