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
redis · Oct 2, 2024 · c31849c · c31849c
1 parent 7215a52
commit c31849c
Show file tree

Hide file tree

Showing 7 changed files with 66 additions and 60 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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"]