From c2ff5b8b67975d4a185b505e5f8d3d9d272a5bf5 Mon Sep 17 00:00:00 2001 From: Gabriel Erzse Date: Wed, 19 Jun 2024 15:32:55 +0300 Subject: [PATCH 1/6] Re-enable Graph tests Although the Graph module was deprecated, we want to have tests running as long as we keep the client code. So re-enable the Graph tests, by adding to the docker-compose stack a redis-stack-server container with an older version. Tweak the CI so that it runs against the latest RC release for now, in order to have all tests executed, including the ones for hash field expiration. Take the opportunity to mark all tests related to TimeSeries as modules tests. --- .github/workflows/integration.yaml | 3 ++- docker-compose.yml | 19 ++++++++++++++----- dockers/create_cluster.sh | 6 +++--- tests/test_asyncio/test_graph.py | 21 +-------------------- tests/test_graph.py | 26 +++----------------------- tests/test_timeseries.py | 7 +++++++ 6 files changed, 30 insertions(+), 52 deletions(-) diff --git a/.github/workflows/integration.yaml b/.github/workflows/integration.yaml index 6bddc1cf4e..f6186b8271 100644 --- a/.github/workflows/integration.yaml +++ b/.github/workflows/integration.yaml @@ -24,8 +24,9 @@ permissions: contents: read # to fetch code (actions/checkout) env: - REDIS_STACK_IMAGE: redis/redis-stack-server:7.4.0-rc1 CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} + REDIS_IMAGE: redis/redis-stack-server:7.4.0-rc1 + REDIS_STACK_IMAGE: redis/redis-stack-server:7.4.0-rc1 jobs: dependency-audit: diff --git a/docker-compose.yml b/docker-compose.yml index 72c43c2252..c8528a7d58 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -7,7 +7,7 @@ services: redis: image: ${REDIS_IMAGE:-redis:latest} container_name: redis-standalone - command: redis-server --enable-debug-command yes + command: redis-server --enable-debug-command yes --protected-mode no ports: - 6379:6379 profiles: @@ -21,7 +21,7 @@ services: container_name: redis-replica depends_on: - redis - command: redis-server --replicaof redis 6379 + command: redis-server --replicaof redis 6379 --protected-mode no ports: - 6380:6379 profiles: @@ -67,7 +67,7 @@ services: container_name: redis-sentinel depends_on: - redis - entrypoint: "/usr/local/bin/redis-sentinel /redis.conf --port 26379" + entrypoint: "redis-sentinel /redis.conf --port 26379" ports: - 26379:26379 volumes: @@ -81,7 +81,7 @@ services: container_name: redis-sentinel2 depends_on: - redis - entrypoint: "/usr/local/bin/redis-sentinel /redis.conf --port 26380" + entrypoint: "redis-sentinel /redis.conf --port 26380" ports: - 26380:26380 volumes: @@ -95,7 +95,7 @@ services: container_name: redis-sentinel3 depends_on: - redis - entrypoint: "/usr/local/bin/redis-sentinel /redis.conf --port 26381" + entrypoint: "redis-sentinel /redis.conf --port 26381" ports: - 26381:26381 volumes: @@ -114,3 +114,12 @@ services: profiles: - standalone - all + + redis-stack-graph: + image: redis/redis-stack-server:6.2.6-v15 + container_name: redis-stack-graph + ports: + - 6480:6379 + profiles: + - standalone + - all diff --git a/dockers/create_cluster.sh b/dockers/create_cluster.sh index af41192585..dc17c7c4d9 100644 --- a/dockers/create_cluster.sh +++ b/dockers/create_cluster.sh @@ -31,7 +31,7 @@ dir /nodes/$PORT EOF set -x - /usr/local/bin/redis-server /nodes/$PORT/redis.conf + redis-server /nodes/$PORT/redis.conf sleep 1 if [ $? -ne 0 ]; then echo "Redis failed to start, exiting." @@ -40,8 +40,8 @@ EOF echo 127.0.0.1:$PORT >> /nodes/nodemap done if [ -z "${REDIS_PASSWORD}" ]; then - echo yes | /usr/local/bin/redis-cli --cluster create `seq -f 127.0.0.1:%g ${START_PORT} ${END_PORT}` --cluster-replicas 1 + echo yes | redis-cli --cluster create `seq -f 127.0.0.1:%g ${START_PORT} ${END_PORT}` --cluster-replicas 1 else - echo yes | /usr/local/bin/redis-cli -a ${REDIS_PASSWORD} --cluster create `seq -f 127.0.0.1:%g ${START_PORT} ${END_PORT}` --cluster-replicas 1 + echo yes | redis-cli -a ${REDIS_PASSWORD} --cluster create `seq -f 127.0.0.1:%g ${START_PORT} ${END_PORT}` --cluster-replicas 1 fi tail -f /redis.log diff --git a/tests/test_asyncio/test_graph.py b/tests/test_asyncio/test_graph.py index ab8bf17618..c51508e78e 100644 --- a/tests/test_asyncio/test_graph.py +++ b/tests/test_asyncio/test_graph.py @@ -9,7 +9,7 @@ @pytest_asyncio.fixture() async def decoded_r(create_redis, stack_url): - return await create_redis(decode_responses=True, url=stack_url) + return await create_redis(decode_responses=True, url="redis://localhost:6480") @pytest.mark.redismod @@ -20,7 +20,6 @@ async def test_bulk(decoded_r): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") async def test_graph_creation(decoded_r: redis.Redis): graph = decoded_r.graph() @@ -66,7 +65,6 @@ async def test_graph_creation(decoded_r: redis.Redis): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") async def test_array_functions(decoded_r: redis.Redis): graph = decoded_r.graph() @@ -90,7 +88,6 @@ async def test_array_functions(decoded_r: redis.Redis): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") async def test_path(decoded_r: redis.Redis): node0 = Node(node_id=0, label="L1") node1 = Node(node_id=1, label="L1") @@ -111,7 +108,6 @@ async def test_path(decoded_r: redis.Redis): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") async def test_param(decoded_r: redis.Redis): params = [1, 2.3, "str", True, False, None, [0, 1, 2]] query = "RETURN $param" @@ -122,7 +118,6 @@ async def test_param(decoded_r: redis.Redis): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") async def test_map(decoded_r: redis.Redis): query = "RETURN {a:1, b:'str', c:NULL, d:[1,2,3], e:True, f:{x:1, y:2}}" @@ -140,7 +135,6 @@ async def test_map(decoded_r: redis.Redis): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") async def test_point(decoded_r: redis.Redis): query = "RETURN point({latitude: 32.070794860, longitude: 34.820751118})" expected_lat = 32.070794860 @@ -158,7 +152,6 @@ async def test_point(decoded_r: redis.Redis): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") async def test_index_response(decoded_r: redis.Redis): result_set = await decoded_r.graph().query("CREATE INDEX ON :person(age)") assert 1 == result_set.indices_created @@ -174,7 +167,6 @@ async def test_index_response(decoded_r: redis.Redis): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") async def test_stringify_query_result(decoded_r: redis.Redis): graph = decoded_r.graph() @@ -229,7 +221,6 @@ async def test_stringify_query_result(decoded_r: redis.Redis): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") async def test_optional_match(decoded_r: redis.Redis): # Build a graph of form (a)-[R]->(b) node0 = Node(node_id=0, label="L1", properties={"value": "a"}) @@ -255,7 +246,6 @@ async def test_optional_match(decoded_r: redis.Redis): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") async def test_cached_execution(decoded_r: redis.Redis): await decoded_r.graph().query("CREATE ()") @@ -276,7 +266,6 @@ async def test_cached_execution(decoded_r: redis.Redis): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") async def test_slowlog(decoded_r: redis.Redis): create_query = """CREATE (:Rider {name:'Valentino Rossi'})-[:rides]->(:Team {name:'Yamaha'}), @@ -291,7 +280,6 @@ async def test_slowlog(decoded_r: redis.Redis): @pytest.mark.xfail(strict=False) @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") async def test_query_timeout(decoded_r: redis.Redis): # Build a sample graph with 1000 nodes. await decoded_r.graph().query("UNWIND range(0,1000) as val CREATE ({v: val})") @@ -306,7 +294,6 @@ async def test_query_timeout(decoded_r: redis.Redis): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") async def test_read_only_query(decoded_r: redis.Redis): with pytest.raises(Exception): # Issue a write query, specifying read-only true, @@ -316,7 +303,6 @@ async def test_read_only_query(decoded_r: redis.Redis): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") async def test_profile(decoded_r: redis.Redis): q = """UNWIND range(1, 3) AS x CREATE (p:Person {v:x})""" profile = (await decoded_r.graph().profile(q)).result_set @@ -333,7 +319,6 @@ async def test_profile(decoded_r: redis.Redis): @skip_if_redis_enterprise() @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") async def test_config(decoded_r: redis.Redis): config_name = "RESULTSET_SIZE" config_value = 3 @@ -366,7 +351,6 @@ async def test_config(decoded_r: redis.Redis): @pytest.mark.onlynoncluster @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") async def test_list_keys(decoded_r: redis.Redis): result = await decoded_r.graph().list_keys() assert result == [] @@ -390,7 +374,6 @@ async def test_list_keys(decoded_r: redis.Redis): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") async def test_multi_label(decoded_r: redis.Redis): redis_graph = decoded_r.graph("g") @@ -417,7 +400,6 @@ async def test_multi_label(decoded_r: redis.Redis): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") async def test_execution_plan(decoded_r: redis.Redis): redis_graph = decoded_r.graph("execution_plan") create_query = """CREATE @@ -437,7 +419,6 @@ async def test_execution_plan(decoded_r: redis.Redis): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") async def test_explain(decoded_r: redis.Redis): redis_graph = decoded_r.graph("execution_plan") # graph creation / population diff --git a/tests/test_graph.py b/tests/test_graph.py index 7f46a538ff..fd169c9fa4 100644 --- a/tests/test_graph.py +++ b/tests/test_graph.py @@ -25,13 +25,14 @@ @pytest.fixture def client(request, stack_url): - r = _get_client(Redis, request, decode_responses=True, from_url=stack_url) + r = _get_client( + Redis, request, decode_responses=True, from_url="redis://localhost:6480" + ) r.flushdb() return r @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") def test_bulk(client): with pytest.raises(NotImplementedError): client.graph().bulk() @@ -39,7 +40,6 @@ def test_bulk(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") def test_graph_creation(client): graph = client.graph() @@ -85,7 +85,6 @@ def test_graph_creation(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") def test_array_functions(client): query = """CREATE (p:person{name:'a',age:32, array:[0,1,2]})""" client.graph().query(query) @@ -107,7 +106,6 @@ def test_array_functions(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") def test_path(client): node0 = Node(node_id=0, label="L1") node1 = Node(node_id=1, label="L1") @@ -128,7 +126,6 @@ def test_path(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") def test_param(client): params = [1, 2.3, "str", True, False, None, [0, 1, 2], r"\" RETURN 1337 //"] query = "RETURN $param" @@ -139,7 +136,6 @@ def test_param(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") def test_map(client): query = "RETURN {a:1, b:'str', c:NULL, d:[1,2,3], e:True, f:{x:1, y:2}}" @@ -157,7 +153,6 @@ def test_map(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") def test_point(client): query = "RETURN point({latitude: 32.070794860, longitude: 34.820751118})" expected_lat = 32.070794860 @@ -175,7 +170,6 @@ def test_point(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") def test_index_response(client): result_set = client.graph().query("CREATE INDEX ON :person(age)") assert 1 == result_set.indices_created @@ -191,7 +185,6 @@ def test_index_response(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") def test_stringify_query_result(client): graph = client.graph() @@ -246,7 +239,6 @@ def test_stringify_query_result(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") def test_optional_match(client): # Build a graph of form (a)-[R]->(b) node0 = Node(node_id=0, label="L1", properties={"value": "a"}) @@ -272,7 +264,6 @@ def test_optional_match(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") def test_cached_execution(client): client.graph().query("CREATE ()") @@ -291,7 +282,6 @@ def test_cached_execution(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") def test_slowlog(client): create_query = """CREATE (:Rider {name:'Valentino Rossi'})-[:rides]->(:Team {name:'Yamaha'}), @@ -306,7 +296,6 @@ def test_slowlog(client): @pytest.mark.redismod @pytest.mark.xfail(strict=False) -@pytest.mark.skip(reason="Graph module removed from Redis Stack") def test_query_timeout(client): # Build a sample graph with 1000 nodes. client.graph().query("UNWIND range(0,1000) as val CREATE ({v: val})") @@ -321,7 +310,6 @@ def test_query_timeout(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") def test_read_only_query(client): with pytest.raises(Exception): # Issue a write query, specifying read-only true, @@ -331,7 +319,6 @@ def test_read_only_query(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") def test_profile(client): q = """UNWIND range(1, 3) AS x CREATE (p:Person {v:x})""" profile = client.graph().profile(q).result_set @@ -348,7 +335,6 @@ def test_profile(client): @pytest.mark.redismod @skip_if_redis_enterprise() -@pytest.mark.skip(reason="Graph module removed from Redis Stack") def test_config(client): config_name = "RESULTSET_SIZE" config_value = 3 @@ -381,7 +367,6 @@ def test_config(client): @pytest.mark.onlynoncluster @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") def test_list_keys(client): result = client.graph().list_keys() assert result == [] @@ -405,7 +390,6 @@ def test_list_keys(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") def test_multi_label(client): redis_graph = client.graph("g") @@ -432,7 +416,6 @@ def test_multi_label(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") def test_cache_sync(client): pass return @@ -506,7 +489,6 @@ def test_cache_sync(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") def test_execution_plan(client): redis_graph = client.graph("execution_plan") create_query = """CREATE @@ -526,7 +508,6 @@ def test_execution_plan(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") def test_explain(client): redis_graph = client.graph("execution_plan") # graph creation / population @@ -616,7 +597,6 @@ def test_explain(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") def test_resultset_statistics(client): with patch.object(target=QueryResult, attribute="_get_stat") as mock_get_stats: result = client.graph().query("RETURN 1") diff --git a/tests/test_timeseries.py b/tests/test_timeseries.py index 95402a1cee..90e627ef6e 100644 --- a/tests/test_timeseries.py +++ b/tests/test_timeseries.py @@ -1024,6 +1024,7 @@ def test_uncompressed(client): assert compressed_info["memoryUsage"] < uncompressed_info["memoryUsage"] +@pytest.mark.redismod @skip_ifmodversion_lt("1.12.0", "timeseries") def test_create_with_insertion_filters(client): client.ts().create( @@ -1047,6 +1048,7 @@ def test_create_with_insertion_filters(client): ) +@pytest.mark.redismod @skip_ifmodversion_lt("1.12.0", "timeseries") def test_create_with_insertion_filters_other_duplicate_policy(client): client.ts().create( @@ -1068,6 +1070,7 @@ def test_create_with_insertion_filters_other_duplicate_policy(client): ) +@pytest.mark.redismod @skip_ifmodversion_lt("1.12.0", "timeseries") def test_alter_with_insertion_filters(client): assert 1000 == client.ts().add("time-series-1", 1000, 1.0) @@ -1092,6 +1095,7 @@ def test_alter_with_insertion_filters(client): ) +@pytest.mark.redismod @skip_ifmodversion_lt("1.12.0", "timeseries") def test_add_with_insertion_filters(client): assert 1000 == client.ts().add( @@ -1109,6 +1113,7 @@ def test_add_with_insertion_filters(client): assert_resp_response(client, data_points, [(1000, 1.0)], [[1000, 1.0]]) +@pytest.mark.redismod @skip_ifmodversion_lt("1.12.0", "timeseries") def test_incrby_with_insertion_filters(client): assert 1000 == client.ts().incrby( @@ -1131,6 +1136,7 @@ def test_incrby_with_insertion_filters(client): assert_resp_response(client, data_points, [(1000, 11.1)], [[1000, 11.1]]) +@pytest.mark.redismod @skip_ifmodversion_lt("1.12.0", "timeseries") def test_decrby_with_insertion_filters(client): assert 1000 == client.ts().decrby( @@ -1153,6 +1159,7 @@ def test_decrby_with_insertion_filters(client): assert_resp_response(client, data_points, [(1000, -11.1)], [[1000, -11.1]]) +@pytest.mark.redismod @skip_ifmodversion_lt("1.12.0", "timeseries") def test_madd_with_insertion_filters(client): client.ts().create( From c518bb4b692cf0da4a1f85c1126a0e3b3b8e9d1f Mon Sep 17 00:00:00 2001 From: Gabriel Erzse Date: Wed, 19 Jun 2024 17:27:39 +0300 Subject: [PATCH 2/6] Skip Graph tests with RESP3 --- tests/conftest.py | 7 +++++++ tests/test_asyncio/test_graph.py | 22 +++++++++++++++++++++- tests/test_graph.py | 24 +++++++++++++++++++++++- 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 6df6875845..732129e2f0 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -135,6 +135,8 @@ def _get_info(redis_url): def pytest_sessionstart(session): # during test discovery, e.g. with VS Code, we may not # have a server running. + protocol = session.config.getoption("--protocol") + REDIS_INFO["resp_version"] = protocol redis_url = session.config.getoption("--redis-url") try: info = _get_info(redis_url) @@ -279,6 +281,11 @@ def skip_if_cryptography() -> _TestDecorator: return pytest.mark.skipif(False, reason="No cryptography dependency") +def skip_if_resp_version(resp_version) -> _TestDecorator: + check = REDIS_INFO.get("resp_version", None) != resp_version + return pytest.mark.skipif(check, reason=f"RESP version required != {resp_version}") + + def _get_client( cls, request, single_connection_client=True, flushdb=True, from_url=None, **kwargs ): diff --git a/tests/test_asyncio/test_graph.py b/tests/test_asyncio/test_graph.py index c51508e78e..2014ea38b6 100644 --- a/tests/test_asyncio/test_graph.py +++ b/tests/test_asyncio/test_graph.py @@ -4,7 +4,7 @@ from redis.commands.graph import Edge, Node, Path from redis.commands.graph.execution_plan import Operation from redis.exceptions import ResponseError -from tests.conftest import skip_if_redis_enterprise +from tests.conftest import skip_if_redis_enterprise, skip_if_resp_version @pytest_asyncio.fixture() @@ -13,6 +13,7 @@ async def decoded_r(create_redis, stack_url): @pytest.mark.redismod +@skip_if_resp_version(3) async def test_bulk(decoded_r): with pytest.raises(NotImplementedError): await decoded_r.graph().bulk() @@ -20,6 +21,7 @@ async def test_bulk(decoded_r): @pytest.mark.redismod +@skip_if_resp_version(3) async def test_graph_creation(decoded_r: redis.Redis): graph = decoded_r.graph() @@ -65,6 +67,7 @@ async def test_graph_creation(decoded_r: redis.Redis): @pytest.mark.redismod +@skip_if_resp_version(3) async def test_array_functions(decoded_r: redis.Redis): graph = decoded_r.graph() @@ -88,6 +91,7 @@ async def test_array_functions(decoded_r: redis.Redis): @pytest.mark.redismod +@skip_if_resp_version(3) async def test_path(decoded_r: redis.Redis): node0 = Node(node_id=0, label="L1") node1 = Node(node_id=1, label="L1") @@ -108,6 +112,7 @@ async def test_path(decoded_r: redis.Redis): @pytest.mark.redismod +@skip_if_resp_version(3) async def test_param(decoded_r: redis.Redis): params = [1, 2.3, "str", True, False, None, [0, 1, 2]] query = "RETURN $param" @@ -118,6 +123,7 @@ async def test_param(decoded_r: redis.Redis): @pytest.mark.redismod +@skip_if_resp_version(3) async def test_map(decoded_r: redis.Redis): query = "RETURN {a:1, b:'str', c:NULL, d:[1,2,3], e:True, f:{x:1, y:2}}" @@ -135,6 +141,7 @@ async def test_map(decoded_r: redis.Redis): @pytest.mark.redismod +@skip_if_resp_version(3) async def test_point(decoded_r: redis.Redis): query = "RETURN point({latitude: 32.070794860, longitude: 34.820751118})" expected_lat = 32.070794860 @@ -152,6 +159,7 @@ async def test_point(decoded_r: redis.Redis): @pytest.mark.redismod +@skip_if_resp_version(3) async def test_index_response(decoded_r: redis.Redis): result_set = await decoded_r.graph().query("CREATE INDEX ON :person(age)") assert 1 == result_set.indices_created @@ -167,6 +175,7 @@ async def test_index_response(decoded_r: redis.Redis): @pytest.mark.redismod +@skip_if_resp_version(3) async def test_stringify_query_result(decoded_r: redis.Redis): graph = decoded_r.graph() @@ -221,6 +230,7 @@ async def test_stringify_query_result(decoded_r: redis.Redis): @pytest.mark.redismod +@skip_if_resp_version(3) async def test_optional_match(decoded_r: redis.Redis): # Build a graph of form (a)-[R]->(b) node0 = Node(node_id=0, label="L1", properties={"value": "a"}) @@ -246,6 +256,7 @@ async def test_optional_match(decoded_r: redis.Redis): @pytest.mark.redismod +@skip_if_resp_version(3) async def test_cached_execution(decoded_r: redis.Redis): await decoded_r.graph().query("CREATE ()") @@ -266,6 +277,7 @@ async def test_cached_execution(decoded_r: redis.Redis): @pytest.mark.redismod +@skip_if_resp_version(3) async def test_slowlog(decoded_r: redis.Redis): create_query = """CREATE (:Rider {name:'Valentino Rossi'})-[:rides]->(:Team {name:'Yamaha'}), @@ -280,6 +292,7 @@ async def test_slowlog(decoded_r: redis.Redis): @pytest.mark.xfail(strict=False) @pytest.mark.redismod +@skip_if_resp_version(3) async def test_query_timeout(decoded_r: redis.Redis): # Build a sample graph with 1000 nodes. await decoded_r.graph().query("UNWIND range(0,1000) as val CREATE ({v: val})") @@ -294,6 +307,7 @@ async def test_query_timeout(decoded_r: redis.Redis): @pytest.mark.redismod +@skip_if_resp_version(3) async def test_read_only_query(decoded_r: redis.Redis): with pytest.raises(Exception): # Issue a write query, specifying read-only true, @@ -303,6 +317,7 @@ async def test_read_only_query(decoded_r: redis.Redis): @pytest.mark.redismod +@skip_if_resp_version(3) async def test_profile(decoded_r: redis.Redis): q = """UNWIND range(1, 3) AS x CREATE (p:Person {v:x})""" profile = (await decoded_r.graph().profile(q)).result_set @@ -319,6 +334,7 @@ async def test_profile(decoded_r: redis.Redis): @skip_if_redis_enterprise() @pytest.mark.redismod +@skip_if_resp_version(3) async def test_config(decoded_r: redis.Redis): config_name = "RESULTSET_SIZE" config_value = 3 @@ -351,6 +367,7 @@ async def test_config(decoded_r: redis.Redis): @pytest.mark.onlynoncluster @pytest.mark.redismod +@skip_if_resp_version(3) async def test_list_keys(decoded_r: redis.Redis): result = await decoded_r.graph().list_keys() assert result == [] @@ -374,6 +391,7 @@ async def test_list_keys(decoded_r: redis.Redis): @pytest.mark.redismod +@skip_if_resp_version(3) async def test_multi_label(decoded_r: redis.Redis): redis_graph = decoded_r.graph("g") @@ -400,6 +418,7 @@ async def test_multi_label(decoded_r: redis.Redis): @pytest.mark.redismod +@skip_if_resp_version(3) async def test_execution_plan(decoded_r: redis.Redis): redis_graph = decoded_r.graph("execution_plan") create_query = """CREATE @@ -419,6 +438,7 @@ async def test_execution_plan(decoded_r: redis.Redis): @pytest.mark.redismod +@skip_if_resp_version(3) async def test_explain(decoded_r: redis.Redis): redis_graph = decoded_r.graph("execution_plan") # graph creation / population diff --git a/tests/test_graph.py b/tests/test_graph.py index fd169c9fa4..680b8af645 100644 --- a/tests/test_graph.py +++ b/tests/test_graph.py @@ -20,7 +20,7 @@ QueryResult, ) from redis.exceptions import ResponseError -from tests.conftest import _get_client, skip_if_redis_enterprise +from tests.conftest import _get_client, skip_if_redis_enterprise, skip_if_resp_version @pytest.fixture @@ -33,6 +33,7 @@ def client(request, stack_url): @pytest.mark.redismod +@skip_if_resp_version(3) def test_bulk(client): with pytest.raises(NotImplementedError): client.graph().bulk() @@ -40,6 +41,7 @@ def test_bulk(client): @pytest.mark.redismod +@skip_if_resp_version(3) def test_graph_creation(client): graph = client.graph() @@ -85,6 +87,7 @@ def test_graph_creation(client): @pytest.mark.redismod +@skip_if_resp_version(3) def test_array_functions(client): query = """CREATE (p:person{name:'a',age:32, array:[0,1,2]})""" client.graph().query(query) @@ -106,6 +109,7 @@ def test_array_functions(client): @pytest.mark.redismod +@skip_if_resp_version(3) def test_path(client): node0 = Node(node_id=0, label="L1") node1 = Node(node_id=1, label="L1") @@ -126,6 +130,7 @@ def test_path(client): @pytest.mark.redismod +@skip_if_resp_version(3) def test_param(client): params = [1, 2.3, "str", True, False, None, [0, 1, 2], r"\" RETURN 1337 //"] query = "RETURN $param" @@ -136,6 +141,7 @@ def test_param(client): @pytest.mark.redismod +@skip_if_resp_version(3) def test_map(client): query = "RETURN {a:1, b:'str', c:NULL, d:[1,2,3], e:True, f:{x:1, y:2}}" @@ -153,6 +159,7 @@ def test_map(client): @pytest.mark.redismod +@skip_if_resp_version(3) def test_point(client): query = "RETURN point({latitude: 32.070794860, longitude: 34.820751118})" expected_lat = 32.070794860 @@ -170,6 +177,7 @@ def test_point(client): @pytest.mark.redismod +@skip_if_resp_version(3) def test_index_response(client): result_set = client.graph().query("CREATE INDEX ON :person(age)") assert 1 == result_set.indices_created @@ -185,6 +193,7 @@ def test_index_response(client): @pytest.mark.redismod +@skip_if_resp_version(3) def test_stringify_query_result(client): graph = client.graph() @@ -239,6 +248,7 @@ def test_stringify_query_result(client): @pytest.mark.redismod +@skip_if_resp_version(3) def test_optional_match(client): # Build a graph of form (a)-[R]->(b) node0 = Node(node_id=0, label="L1", properties={"value": "a"}) @@ -264,6 +274,7 @@ def test_optional_match(client): @pytest.mark.redismod +@skip_if_resp_version(3) def test_cached_execution(client): client.graph().query("CREATE ()") @@ -282,6 +293,7 @@ def test_cached_execution(client): @pytest.mark.redismod +@skip_if_resp_version(3) def test_slowlog(client): create_query = """CREATE (:Rider {name:'Valentino Rossi'})-[:rides]->(:Team {name:'Yamaha'}), @@ -295,6 +307,7 @@ def test_slowlog(client): @pytest.mark.redismod +@skip_if_resp_version(3) @pytest.mark.xfail(strict=False) def test_query_timeout(client): # Build a sample graph with 1000 nodes. @@ -310,6 +323,7 @@ def test_query_timeout(client): @pytest.mark.redismod +@skip_if_resp_version(3) def test_read_only_query(client): with pytest.raises(Exception): # Issue a write query, specifying read-only true, @@ -319,6 +333,7 @@ def test_read_only_query(client): @pytest.mark.redismod +@skip_if_resp_version(3) def test_profile(client): q = """UNWIND range(1, 3) AS x CREATE (p:Person {v:x})""" profile = client.graph().profile(q).result_set @@ -334,6 +349,7 @@ def test_profile(client): @pytest.mark.redismod +@skip_if_resp_version(3) @skip_if_redis_enterprise() def test_config(client): config_name = "RESULTSET_SIZE" @@ -367,6 +383,7 @@ def test_config(client): @pytest.mark.onlynoncluster @pytest.mark.redismod +@skip_if_resp_version(3) def test_list_keys(client): result = client.graph().list_keys() assert result == [] @@ -390,6 +407,7 @@ def test_list_keys(client): @pytest.mark.redismod +@skip_if_resp_version(3) def test_multi_label(client): redis_graph = client.graph("g") @@ -416,6 +434,7 @@ def test_multi_label(client): @pytest.mark.redismod +@skip_if_resp_version(3) def test_cache_sync(client): pass return @@ -489,6 +508,7 @@ def test_cache_sync(client): @pytest.mark.redismod +@skip_if_resp_version(3) def test_execution_plan(client): redis_graph = client.graph("execution_plan") create_query = """CREATE @@ -508,6 +528,7 @@ def test_execution_plan(client): @pytest.mark.redismod +@skip_if_resp_version(3) def test_explain(client): redis_graph = client.graph("execution_plan") # graph creation / population @@ -597,6 +618,7 @@ def test_explain(client): @pytest.mark.redismod +@skip_if_resp_version(3) def test_resultset_statistics(client): with patch.object(target=QueryResult, attribute="_get_stat") as mock_get_stats: result = client.graph().query("RETURN 1") From 2a9f95d444bee6bedcc5351c845de110bd8eefe3 Mon Sep 17 00:00:00 2001 From: Gabriel Erzse Date: Wed, 19 Jun 2024 17:57:44 +0300 Subject: [PATCH 3/6] Fix invalidations push handler in RESP3 --- redis/_parsers/resp3.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/redis/_parsers/resp3.py b/redis/_parsers/resp3.py index 7afa43a0c2..cc210b9df5 100644 --- a/redis/_parsers/resp3.py +++ b/redis/_parsers/resp3.py @@ -15,7 +15,7 @@ class _RESP3Parser(_RESPBase): def __init__(self, socket_read_size): super().__init__(socket_read_size) self.pubsub_push_handler_func = self.handle_pubsub_push_response - self.invalidations_push_handler_func = None + self.invalidation_push_handler_func = None def handle_pubsub_push_response(self, response): logger = getLogger("push_response") @@ -129,7 +129,10 @@ def _read_response(self, disable_decoding=False, push_request=False): def handle_push_response(self, response, disable_decoding, push_request): if response[0] in _INVALIDATION_MESSAGE: - res = self.invalidation_push_handler_func(response) + if self.invalidation_push_handler_func: + res = self.invalidation_push_handler_func(response) + else: + res = None else: res = self.pubsub_push_handler_func(response) if not push_request: @@ -142,15 +145,15 @@ def handle_push_response(self, response, disable_decoding, push_request): def set_pubsub_push_handler(self, pubsub_push_handler_func): self.pubsub_push_handler_func = pubsub_push_handler_func - def set_invalidation_push_handler(self, invalidations_push_handler_func): - self.invalidation_push_handler_func = invalidations_push_handler_func + def set_invalidation_push_handler(self, invalidation_push_handler_func): + self.invalidation_push_handler_func = invalidation_push_handler_func class _AsyncRESP3Parser(_AsyncRESPBase): def __init__(self, socket_read_size): super().__init__(socket_read_size) self.pubsub_push_handler_func = self.handle_pubsub_push_response - self.invalidations_push_handler_func = None + self.invalidation_push_handler_func = None def handle_pubsub_push_response(self, response): logger = getLogger("push_response") @@ -273,7 +276,10 @@ async def _read_response( async def handle_push_response(self, response, disable_decoding, push_request): if response[0] in _INVALIDATION_MESSAGE: - res = self.invalidation_push_handler_func(response) + if self.invalidation_push_handler_func: + res = self.invalidation_push_handler_func(response) + else: + res = None else: res = self.pubsub_push_handler_func(response) if not push_request: @@ -286,5 +292,5 @@ async def handle_push_response(self, response, disable_decoding, push_request): def set_pubsub_push_handler(self, pubsub_push_handler_func): self.pubsub_push_handler_func = pubsub_push_handler_func - def set_invalidation_push_handler(self, invalidations_push_handler_func): - self.invalidation_push_handler_func = invalidations_push_handler_func + def set_invalidation_push_handler(self, invalidation_push_handler_func): + self.invalidation_push_handler_func = invalidation_push_handler_func From 5e87cc7eb1303f85165678c56dde7f143dacd5e4 Mon Sep 17 00:00:00 2001 From: Gabriel Erzse Date: Wed, 19 Jun 2024 18:52:29 +0300 Subject: [PATCH 4/6] Improve client kill maxage test --- tests/test_commands.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/test_commands.py b/tests/test_commands.py index aab2a227ed..b059949387 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -712,11 +712,15 @@ def test_client_kill_filter_by_user(self, r, request): @skip_if_redis_enterprise() @pytest.mark.onlynoncluster def test_client_kill_filter_by_maxage(self, r, request): - _get_client(redis.Redis, request, flushdb=False) + r2 = _get_client(redis.Redis, request, flushdb=False) + name = f"target-foobar" + r2.client_setname(name) time.sleep(4) - assert len(r.client_list()) >= 2 + initial_clients = [c["name"] for c in r.client_list()] + assert name in initial_clients r.client_kill_filter(maxage=2) - assert len(r.client_list()) == 1 + final_clients = [c["name"] for c in r.client_list()] + assert name not in final_clients @pytest.mark.onlynoncluster @skip_if_server_version_lt("2.9.50") From 5da33f2888115f51e183d243aa1d75078bf70586 Mon Sep 17 00:00:00 2001 From: Gabriel Erzse Date: Wed, 19 Jun 2024 19:26:36 +0300 Subject: [PATCH 5/6] Fix linter errors --- tests/test_commands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_commands.py b/tests/test_commands.py index b059949387..42376b50d8 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -713,7 +713,7 @@ def test_client_kill_filter_by_user(self, r, request): @pytest.mark.onlynoncluster def test_client_kill_filter_by_maxage(self, r, request): r2 = _get_client(redis.Redis, request, flushdb=False) - name = f"target-foobar" + name = "target-foobar" r2.client_setname(name) time.sleep(4) initial_clients = [c["name"] for c in r.client_list()] From 5eedb13310153f90fdfe73990cbe7612823c36ca Mon Sep 17 00:00:00 2001 From: Gabriel Erzse Date: Wed, 19 Jun 2024 23:40:31 +0300 Subject: [PATCH 6/6] Fix skip based on RESP version condition --- tests/conftest.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 732129e2f0..e3326f85d5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -136,7 +136,7 @@ def pytest_sessionstart(session): # during test discovery, e.g. with VS Code, we may not # have a server running. protocol = session.config.getoption("--protocol") - REDIS_INFO["resp_version"] = protocol + REDIS_INFO["resp_version"] = int(protocol) if protocol else None redis_url = session.config.getoption("--redis-url") try: info = _get_info(redis_url) @@ -282,7 +282,7 @@ def skip_if_cryptography() -> _TestDecorator: def skip_if_resp_version(resp_version) -> _TestDecorator: - check = REDIS_INFO.get("resp_version", None) != resp_version + check = REDIS_INFO.get("resp_version", None) == resp_version return pytest.mark.skipif(check, reason=f"RESP version required != {resp_version}")