From 338cbfdeb8d9638c7d6d2b70ded08dce35413601 Mon Sep 17 00:00:00 2001 From: Gabriel Erzse Date: Thu, 20 Jun 2024 10:17:27 +0300 Subject: [PATCH] Re-enable Graph tests (#3287) Although the Graph module was deprecated, we want to have tests running as long as we keep the client code. Therefore re-enable the Graph tests, by adding to the docker-compose stack an older redis-stack-server image. The Graph tests run only with RESP2. Tweak the CI so that it runs against the latest RC release for now. This way we get all the features in RC release of server, so we can execute all tests, including the ones for hash field expiration. Some test failures brought to light issues with the RESP3 push message invalidation handler, align a bit the code in that area. Some more housekeeping around tests, here and there. --- .github/workflows/integration.yaml | 3 +- docker-compose.yml | 19 +++++++++--- dockers/create_cluster.sh | 6 ++-- tests/conftest.py | 7 +++++ tests/test_asyncio/test_graph.py | 24 ++++++++++++-- tests/test_commands.py | 10 ++++-- tests/test_graph.py | 50 ++++++++++++++++-------------- tests/test_timeseries.py | 7 +++++ 8 files changed, 88 insertions(+), 38 deletions(-) diff --git a/.github/workflows/integration.yaml b/.github/workflows/integration.yaml index c430d6f021..71f52cf42d 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/conftest.py b/tests/conftest.py index 74d37f0149..dd60a8ecd5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -121,6 +121,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"] = int(protocol) if protocol else None redis_url = session.config.getoption("--redis-url") try: info = _get_info(redis_url) @@ -265,6 +267,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 9cd9149dcc..2014ea38b6 100644 --- a/tests/test_asyncio/test_graph.py +++ b/tests/test_asyncio/test_graph.py @@ -4,15 +4,16 @@ 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() 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 +@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_commands.py b/tests/test_commands.py index aab2a227ed..42376b50d8 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 = "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") diff --git a/tests/test_graph.py b/tests/test_graph.py index 7f46a538ff..680b8af645 100644 --- a/tests/test_graph.py +++ b/tests/test_graph.py @@ -20,18 +20,20 @@ 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 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") +@skip_if_resp_version(3) def test_bulk(client): with pytest.raises(NotImplementedError): client.graph().bulk() @@ -39,7 +41,7 @@ def test_bulk(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") +@skip_if_resp_version(3) def test_graph_creation(client): graph = client.graph() @@ -85,7 +87,7 @@ def test_graph_creation(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") +@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) @@ -107,7 +109,7 @@ def test_array_functions(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") +@skip_if_resp_version(3) def test_path(client): node0 = Node(node_id=0, label="L1") node1 = Node(node_id=1, label="L1") @@ -128,7 +130,7 @@ def test_path(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") +@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" @@ -139,7 +141,7 @@ def test_param(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") +@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}}" @@ -157,7 +159,7 @@ def test_map(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") +@skip_if_resp_version(3) def test_point(client): query = "RETURN point({latitude: 32.070794860, longitude: 34.820751118})" expected_lat = 32.070794860 @@ -175,7 +177,7 @@ def test_point(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") +@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 @@ -191,7 +193,7 @@ def test_index_response(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") +@skip_if_resp_version(3) def test_stringify_query_result(client): graph = client.graph() @@ -246,7 +248,7 @@ def test_stringify_query_result(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") +@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"}) @@ -272,7 +274,7 @@ def test_optional_match(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") +@skip_if_resp_version(3) def test_cached_execution(client): client.graph().query("CREATE ()") @@ -291,7 +293,7 @@ def test_cached_execution(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") +@skip_if_resp_version(3) def test_slowlog(client): create_query = """CREATE (:Rider {name:'Valentino Rossi'})-[:rides]->(:Team {name:'Yamaha'}), @@ -305,8 +307,8 @@ def test_slowlog(client): @pytest.mark.redismod +@skip_if_resp_version(3) @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 +323,7 @@ def test_query_timeout(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") +@skip_if_resp_version(3) def test_read_only_query(client): with pytest.raises(Exception): # Issue a write query, specifying read-only true, @@ -331,7 +333,7 @@ def test_read_only_query(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") +@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 @@ -347,8 +349,8 @@ def test_profile(client): @pytest.mark.redismod +@skip_if_resp_version(3) @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 +383,7 @@ def test_config(client): @pytest.mark.onlynoncluster @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") +@skip_if_resp_version(3) def test_list_keys(client): result = client.graph().list_keys() assert result == [] @@ -405,7 +407,7 @@ def test_list_keys(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") +@skip_if_resp_version(3) def test_multi_label(client): redis_graph = client.graph("g") @@ -432,7 +434,7 @@ def test_multi_label(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") +@skip_if_resp_version(3) def test_cache_sync(client): pass return @@ -506,7 +508,7 @@ def test_cache_sync(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") +@skip_if_resp_version(3) def test_execution_plan(client): redis_graph = client.graph("execution_plan") create_query = """CREATE @@ -526,7 +528,7 @@ def test_execution_plan(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") +@skip_if_resp_version(3) def test_explain(client): redis_graph = client.graph("execution_plan") # graph creation / population @@ -616,7 +618,7 @@ def test_explain(client): @pytest.mark.redismod -@pytest.mark.skip(reason="Graph module removed from Redis Stack") +@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") 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(