Fix bug with Redis Set commands returns List instead of Set in RESP2

Author: vladvildanov

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",

tests/test_asyncio/test_cluster.py

@@ -1754,26 +1754,26 @@ 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"}
         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"}
         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"}
 
     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"}
@@ -1782,7 +1782,7 @@ 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 await r.smembers("{foo}a") == {b"a2"}
         assert set(await r.smembers("{foo}b")) == {b"b1", b"b2", b"a1"}
 
     async def test_cluster_sunion(self, r: RedisCluster) -> None:

tests/test_asyncio/test_commands.py

@@ -1417,7 +1417,7 @@ 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"}
         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):
@@ -1426,20 +1426,20 @@ async def test_sdiffstore(self, r: redis.Redis):
         assert set(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"}
 
     @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"}
@@ -1460,7 +1460,7 @@ 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 await r.smembers("a") == {b"a2"}
         assert set(await r.smembers("b")) == {b"b1", b"b2", b"a1"}
 
     async def test_spop(self, r: redis.Redis):

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(

tests/test_cluster.py

@@ -48,7 +48,7 @@
     skip_if_redis_enterprise,
     skip_if_server_version_lt,
     skip_unless_arch_bits,
-    wait_for_command,
+    wait_for_command, skip_if_resp_version,
 )
 
 default_host = "127.0.0.1"
@@ -284,6 +284,19 @@ def ok_response(connection, *args, **options):
             assert prev_primary.server_type == REPLICA
 
 
+@pytest.fixture()
+def r(request):
+    protocol = 2
+
+    if hasattr(request, "param"):
+        protocol = request.param.get("protocol", 2)
+    with _get_client(
+        redis.Redis,
+        request,
+        protocol=protocol,
+    ) as client:
+        yield client
+
 @pytest.mark.onlycluster
 class TestRedisClusterObj:
     """
@@ -1864,12 +1877,30 @@ def test_cluster_rpoplpush(self, r):
         assert r.lrange("{foo}b", 0, -1) == [b"a3", b"b1", b"b2", b"b3"]
 
     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"}
+        r.sadd("{foo}b", "2", "3")
+        assert r.sdiff("{foo}a", "{foo}b") == {b"1"}
+
+    @pytest.mark.parametrize("r", [{"protocol": 3}], indirect=True)
+    @skip_if_resp_version(2)
+    def test_cluster_sdiff_resp3(self, r):
         r.sadd("{foo}a", "1", "2", "3")
         assert set(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"]
 
     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"}
+        r.sadd("{foo}b", "2", "3")
+        assert r.sdiffstore("{foo}c", "{foo}a", "{foo}b") == 1
+        assert r.smembers("{foo}c") == {b"1"}
+
+    @pytest.mark.parametrize("r", [{"protocol": 3}], indirect=True)
+    @skip_if_resp_version(2)
+    def test_cluster_sdiffstore_resp3(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"}
@@ -1878,12 +1909,30 @@ def test_cluster_sdiffstore(self, r):
         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") == set()
+        r.sadd("{foo}b", "2", "3")
+        assert set(r.sinter("{foo}a", "{foo}b")) == {b"2", b"3"}
+
+    @pytest.mark.parametrize("r", [{"protocol": 3}], indirect=True)
+    @skip_if_resp_version(2)
+    def test_cluster_sinter_resp3(self, r):
         r.sadd("{foo}a", "1", "2", "3")
         assert r.sinter("{foo}a", "{foo}b") == []
         r.sadd("{foo}b", "2", "3")
         assert set(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") == 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"}
+
+    @pytest.mark.parametrize("r", [{"protocol": 3}], indirect=True)
+    @skip_if_resp_version(2)
+    def test_cluster_sinterstore_resp3(self, r):
         r.sadd("{foo}a", "1", "2", "3")
         assert r.sinterstore("{foo}c", "{foo}a", "{foo}b") == 0
         assert r.smembers("{foo}c") == []
@@ -1892,6 +1941,15 @@ def test_cluster_sinterstore(self, r):
         assert set(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"}
+
+    @pytest.mark.parametrize("r", [{"protocol": 3}], indirect=True)
+    @skip_if_resp_version(2)
+    def test_cluster_smove_resp3(self, r):
         r.sadd("{foo}a", "a1", "a2")
         r.sadd("{foo}b", "b1", "b2")
         assert r.smove("{foo}a", "{foo}b", "a1")

tests/test_commands.py

@@ -27,9 +27,21 @@
     skip_if_redis_enterprise,
     skip_if_server_version_gte,
     skip_if_server_version_lt,
-    skip_unless_arch_bits,
+    skip_unless_arch_bits, skip_if_resp_version,
 )
 
+@pytest.fixture()
+def r(request):
+    protocol = 2
+
+    if hasattr(request, "param"):
+        protocol = request.param.get("protocol", 2)
+    with _get_client(
+        redis.Redis,
+        request,
+        protocol=protocol,
+    ) as client:
+        yield client
 
 @pytest.fixture()
 def slowlog(request, r):
@@ -2246,6 +2258,15 @@ 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"}
+        r.sadd("b", "2", "3")
+        assert r.sdiff("a", "b") == {b"1"}
+
+    @pytest.mark.parametrize("r", [{"protocol": 3}], indirect=True)
+    @pytest.mark.onlynoncluster
+    @skip_if_resp_version(2)
+    def test_sdiff_resp3(self, r):
         r.sadd("a", "1", "2", "3")
         assert set(r.sdiff("a", "b")) == {b"1", b"2", b"3"}
         r.sadd("b", "2", "3")
@@ -2258,10 +2279,30 @@ def test_sdiffstore(self, r):
         assert set(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.parametrize("r", [{"protocol": 3}], indirect=True)
+    @pytest.mark.onlynoncluster
+    @skip_if_resp_version(2)
+    def test_sdiffstore_resp3(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"}
+        r.sadd("b", "2", "3")
+        assert r.sdiffstore("c", "a", "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") == set()
+        r.sadd("b", "2", "3")
+        assert set(r.sinter("a", "b")) == {b"2", b"3"}
+
+    @pytest.mark.parametrize("r", [{"protocol": 3}], indirect=True)
+    @pytest.mark.onlynoncluster
+    @skip_if_resp_version(2)
+    def test_sinter_resp3(self, r):
         r.sadd("a", "1", "2", "3")
         assert r.sinter("a", "b") == []
         r.sadd("b", "2", "3")
@@ -2278,6 +2319,17 @@ def test_sintercard(self, r):
 
     @pytest.mark.onlynoncluster
     def test_sinterstore(self, r):
+        r.sadd("a", "1", "2", "3")
+        assert r.sinterstore("c", "a", "b") == 0
+        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"}
+
+    @pytest.mark.parametrize("r", [{"protocol": 3}], indirect=True)
+    @pytest.mark.onlynoncluster
+    @skip_if_resp_version(2)
+    def test_sinterstore_resp3(self, r):
         r.sadd("a", "1", "2", "3")
         assert r.sinterstore("c", "a", "b") == 0
         assert r.smembers("c") == []
@@ -2308,7 +2360,7 @@ 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 r.smembers("a") == {b"a2"}
         assert set(r.smembers("b")) == {b"b1", b"b2", b"a1"}
 
     def test_spop(self, r):