From aac241ba3a30c3ce5e271832600f381f64687c2d Mon Sep 17 00:00:00 2001 From: vladvildanov Date: Fri, 14 Mar 2025 10:05:58 +0200 Subject: [PATCH 01/14] (fix): Separated retries for read and write operations --- redis/asyncio/client.py | 25 ++++++------ redis/client.py | 25 ++++++------ tests/conftest.py | 14 ++++++- tests/test_asyncio/conftest.py | 15 +++++++- tests/test_asyncio/test_connection.py | 55 ++++++++++++++++++++++++++- tests/test_connection.py | 55 ++++++++++++++++++++++++++- 6 files changed, 156 insertions(+), 33 deletions(-) diff --git a/redis/asyncio/client.py b/redis/asyncio/client.py index 412d5a24b3..59d500934e 100644 --- a/redis/asyncio/client.py +++ b/redis/asyncio/client.py @@ -619,13 +619,6 @@ async def close(self, close_connection_pool: Optional[bool] = None) -> None: """ await self.aclose(close_connection_pool) - async def _send_command_parse_response(self, conn, command_name, *args, **options): - """ - Send a command and parse the response - """ - await conn.send_command(*args) - return await self.parse_response(conn, command_name, **options) - async def _disconnect_raise(self, conn: Connection, error: Exception): """ Close the connection and raise an exception @@ -650,10 +643,12 @@ async def execute_command(self, *args, **options): if self.single_connection_client: await self._single_conn_lock.acquire() try: + await conn.retry.call_with_retry( + lambda: conn.send_command(*args, **options), + lambda error: self._disconnect_raise(conn, error), + ) return await conn.retry.call_with_retry( - lambda: self._send_command_parse_response( - conn, command_name, *args, **options - ), + lambda: self.parse_response(conn, command_name, **options), lambda error: self._disconnect_raise(conn, error), ) finally: @@ -1378,11 +1373,13 @@ async def immediate_execute_command(self, *args, **options): conn = await self.connection_pool.get_connection() self.connection = conn + await conn.retry.call_with_retry( + lambda: conn.send_command(*args, **options), + lambda error: self._disconnect_raise(conn, error), + ) return await conn.retry.call_with_retry( - lambda: self._send_command_parse_response( - conn, command_name, *args, **options - ), - lambda error: self._disconnect_reset_raise(conn, error), + lambda: self.parse_response(conn, command_name, **options), + lambda error: self._disconnect_raise(conn, error), ) def pipeline_execute_command(self, *args, **options): diff --git a/redis/client.py b/redis/client.py index 2c4a1fadff..d3d5e31c9d 100755 --- a/redis/client.py +++ b/redis/client.py @@ -590,13 +590,6 @@ def close(self) -> None: if self.auto_close_connection_pool: self.connection_pool.disconnect() - def _send_command_parse_response(self, conn, command_name, *args, **options): - """ - Send a command and parse the response - """ - conn.send_command(*args, **options) - return self.parse_response(conn, command_name, **options) - def _disconnect_raise(self, conn, error): """ Close the connection and raise an exception @@ -623,10 +616,12 @@ def _execute_command(self, *args, **options): if self._single_connection_client: self.single_connection_lock.acquire() try: + conn.retry.call_with_retry( + lambda: conn.send_command(*args, **options), + lambda error: self._disconnect_raise(conn, error), + ) return conn.retry.call_with_retry( - lambda: self._send_command_parse_response( - conn, command_name, *args, **options - ), + lambda: self.parse_response(conn, command_name, **options), lambda error: self._disconnect_raise(conn, error), ) finally: @@ -1408,11 +1403,13 @@ def immediate_execute_command(self, *args, **options): conn = self.connection_pool.get_connection() self.connection = conn + conn.retry.call_with_retry( + lambda: conn.send_command(*args, **options), + lambda error: self._disconnect_raise(conn, error), + ) return conn.retry.call_with_retry( - lambda: self._send_command_parse_response( - conn, command_name, *args, **options - ), - lambda error: self._disconnect_reset_raise(conn, error), + lambda: self.parse_response(conn, command_name, **options), + lambda error: self._disconnect_raise(conn, error), ) def pipeline_execute_command(self, *args, **options) -> "Pipeline": diff --git a/tests/conftest.py b/tests/conftest.py index 7eaccb1acb..89f16046c1 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -23,7 +23,13 @@ CacheKey, EvictionPolicy, ) -from redis.connection import Connection, ConnectionInterface, SSLConnection, parse_url +from redis.connection import ( + Connection, + ConnectionInterface, + SSLConnection, + parse_url, + ConnectionPool, +) from redis.credentials import CredentialProvider from redis.exceptions import RedisClusterException from redis.retry import Retry @@ -582,6 +588,12 @@ def mock_connection() -> ConnectionInterface: return mock_connection +@pytest.fixture() +def mock_pool() -> ConnectionPool: + mock_pool = Mock(spec=ConnectionPool) + return mock_pool + + @pytest.fixture() def cache_key(request) -> CacheKey: command = request.param.get("command") diff --git a/tests/test_asyncio/conftest.py b/tests/test_asyncio/conftest.py index 340d146ea3..fd7a816cd7 100644 --- a/tests/test_asyncio/conftest.py +++ b/tests/test_asyncio/conftest.py @@ -1,6 +1,7 @@ import random from contextlib import asynccontextmanager as _asynccontextmanager from typing import Union +from unittest.mock import Mock import pytest import pytest_asyncio @@ -8,7 +9,7 @@ from packaging.version import Version from redis.asyncio import Sentinel from redis.asyncio.client import Monitor -from redis.asyncio.connection import Connection, parse_url +from redis.asyncio.connection import Connection, parse_url, ConnectionPool from redis.asyncio.retry import Retry from redis.backoff import NoBackoff from redis.credentials import CredentialProvider @@ -219,6 +220,18 @@ async def mock_cluster_resp_slaves(create_redis, **kwargs): yield mocked +@pytest_asyncio.fixture() +def mock_connection() -> Connection: + mock_connection = Mock(spec=Connection) + return mock_connection + + +@pytest_asyncio.fixture() +def mock_pool() -> ConnectionPool: + mock_pool = Mock(spec=ConnectionPool) + return mock_pool + + @pytest_asyncio.fixture() async def credential_provider(request) -> CredentialProvider: return get_credential_provider(request) diff --git a/tests/test_asyncio/test_connection.py b/tests/test_asyncio/test_connection.py index 38764d30cd..c928a82945 100644 --- a/tests/test_asyncio/test_connection.py +++ b/tests/test_asyncio/test_connection.py @@ -2,7 +2,7 @@ import socket import types from errno import ECONNREFUSED -from unittest.mock import patch +from unittest.mock import patch, call import pytest import redis @@ -20,7 +20,7 @@ parse_url, ) from redis.asyncio.retry import Retry -from redis.backoff import NoBackoff +from redis.backoff import NoBackoff, ExponentialBackoff from redis.exceptions import ConnectionError, InvalidResponse, TimeoutError from redis.utils import HIREDIS_AVAILABLE from tests.conftest import skip_if_server_version_lt @@ -315,6 +315,57 @@ async def get_redis_connection(): await r1.aclose() +@pytest.mark.onlynoncluster +async def test_client_do_not_retry_write_on_read_failure(mock_connection, mock_pool): + mock_connection.send_command.return_value = True + mock_connection.read_response.side_effect = [ + ConnectionError, + ConnectionError, + b"OK", + ] + mock_connection.retry = Retry(ExponentialBackoff(), 3) + mock_connection.retry_on_error = (ConnectionError,) + mock_pool.get_connection.return_value = mock_connection + mock_pool.connection_kwargs = {} + + r = Redis(connection_pool=mock_pool, retry=Retry(ExponentialBackoff(), 3)) + await r.set("key", "value") + + # If read from socket fails, writes won't be executed. + mock_connection.send_command.assert_has_calls( + [ + call("SET", "key", "value"), + ] + ) + + +@pytest.mark.onlynoncluster +async def test_pipeline_immediate_do_not_retry_write_on_read_failure( + mock_connection, mock_pool +): + mock_connection.send_command.return_value = True + mock_connection.read_response.side_effect = [ + ConnectionError, + ConnectionError, + b"OK", + ] + mock_connection.retry = Retry(ExponentialBackoff(), 3) + mock_connection.retry_on_error = (ConnectionError,) + mock_pool.get_connection.return_value = mock_connection + mock_pool.connection_kwargs = {} + + r = Redis(connection_pool=mock_pool, retry=Retry(ExponentialBackoff(), 3)) + pipe = r.pipeline(transaction=False) + await pipe.immediate_execute_command("SET", "key", "value") + + # If read from socket fails, writes won't be executed. + mock_connection.send_command.assert_has_calls( + [ + call("SET", "key", "value"), + ] + ) + + async def test_close_is_aclose(request): """Verify close() calls aclose()""" calls = 0 diff --git a/tests/test_connection.py b/tests/test_connection.py index 9664146ce5..f67dd86fa4 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -10,10 +10,12 @@ from unittest.mock import call, patch import pytest +from requests.cookies import MockResponse + import redis from redis import ConnectionPool, Redis from redis._parsers import _HiredisParser, _RESP2Parser, _RESP3Parser -from redis.backoff import NoBackoff +from redis.backoff import NoBackoff, ExponentialBackoff from redis.cache import ( CacheConfig, CacheEntry, @@ -249,6 +251,57 @@ def get_redis_connection(): r1.close() +@pytest.mark.onlynoncluster +def test_client_do_not_retry_write_on_read_failure(mock_connection, mock_pool): + mock_connection.send_command.return_value = True + mock_connection.read_response.side_effect = [ + ConnectionError, + ConnectionError, + b"OK", + ] + mock_connection.retry = Retry(ExponentialBackoff(), 3) + mock_connection.retry_on_error = (ConnectionError,) + mock_pool.get_connection.return_value = mock_connection + mock_pool.connection_kwargs = {} + + r = Redis(connection_pool=mock_pool, retry=Retry(ExponentialBackoff(), 3)) + r.set("key", "value") + + # If read from socket fails, writes won't be executed. + mock_connection.send_command.assert_has_calls( + [ + call("SET", "key", "value"), + ] + ) + + +@pytest.mark.onlynoncluster +def test_pipeline_immediate_do_not_retry_write_on_read_failure( + mock_connection, mock_pool +): + mock_connection.send_command.return_value = True + mock_connection.read_response.side_effect = [ + ConnectionError, + ConnectionError, + b"OK", + ] + mock_connection.retry = Retry(ExponentialBackoff(), 3) + mock_connection.retry_on_error = (ConnectionError,) + mock_pool.get_connection.return_value = mock_connection + mock_pool.connection_kwargs = {} + + r = Redis(connection_pool=mock_pool, retry=Retry(ExponentialBackoff(), 3)) + pipe = r.pipeline(transaction=False) + pipe.immediate_execute_command("SET", "key", "value") + + # If read from socket fails, writes won't be executed. + mock_connection.send_command.assert_has_calls( + [ + call("SET", "key", "value"), + ] + ) + + @pytest.mark.skipif(sys.version_info == (3, 9), reason="Flacky test on Python 3.9") @pytest.mark.parametrize("from_url", (True, False), ids=("from_url", "from_args")) def test_redis_connection_pool(request, from_url): From 5cad7d45e7751e16107b6001db1204d961667f04 Mon Sep 17 00:00:00 2001 From: vladvildanov Date: Fri, 14 Mar 2025 10:11:40 +0200 Subject: [PATCH 02/14] Removed unused import --- tests/test_connection.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_connection.py b/tests/test_connection.py index f67dd86fa4..a6fb5b3c98 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -10,7 +10,6 @@ from unittest.mock import call, patch import pytest -from requests.cookies import MockResponse import redis from redis import ConnectionPool, Redis From bc7aaf1a016691b2f342009c030a9523a55c8b8b Mon Sep 17 00:00:00 2001 From: vladvildanov Date: Fri, 14 Mar 2025 10:42:27 +0200 Subject: [PATCH 03/14] Fixed incorrect on error callback --- redis/asyncio/client.py | 8 ++++---- redis/client.py | 4 ++-- tests/test_asyncio/conftest.py | 6 +++--- tests/test_asyncio/test_connection.py | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/redis/asyncio/client.py b/redis/asyncio/client.py index 59d500934e..d2acc87b19 100644 --- a/redis/asyncio/client.py +++ b/redis/asyncio/client.py @@ -644,7 +644,7 @@ async def execute_command(self, *args, **options): await self._single_conn_lock.acquire() try: await conn.retry.call_with_retry( - lambda: conn.send_command(*args, **options), + lambda: conn.send_command(*args), lambda error: self._disconnect_raise(conn, error), ) return await conn.retry.call_with_retry( @@ -1374,12 +1374,12 @@ async def immediate_execute_command(self, *args, **options): self.connection = conn await conn.retry.call_with_retry( - lambda: conn.send_command(*args, **options), - lambda error: self._disconnect_raise(conn, error), + lambda: conn.send_command(*args), + lambda error: self._disconnect_reset_raise(conn, error), ) return await conn.retry.call_with_retry( lambda: self.parse_response(conn, command_name, **options), - lambda error: self._disconnect_raise(conn, error), + lambda error: self._disconnect_reset_raise(conn, error), ) def pipeline_execute_command(self, *args, **options): diff --git a/redis/client.py b/redis/client.py index d3d5e31c9d..d16334e1fa 100755 --- a/redis/client.py +++ b/redis/client.py @@ -1405,11 +1405,11 @@ def immediate_execute_command(self, *args, **options): conn.retry.call_with_retry( lambda: conn.send_command(*args, **options), - lambda error: self._disconnect_raise(conn, error), + lambda error: self._disconnect_reset_raise(conn, error), ) return conn.retry.call_with_retry( lambda: self.parse_response(conn, command_name, **options), - lambda error: self._disconnect_raise(conn, error), + lambda error: self._disconnect_reset_raise(conn, error), ) def pipeline_execute_command(self, *args, **options) -> "Pipeline": diff --git a/tests/test_asyncio/conftest.py b/tests/test_asyncio/conftest.py index fd7a816cd7..93357ac128 100644 --- a/tests/test_asyncio/conftest.py +++ b/tests/test_asyncio/conftest.py @@ -1,7 +1,7 @@ import random from contextlib import asynccontextmanager as _asynccontextmanager from typing import Union -from unittest.mock import Mock +from unittest.mock import Mock, AsyncMock import pytest import pytest_asyncio @@ -222,13 +222,13 @@ async def mock_cluster_resp_slaves(create_redis, **kwargs): @pytest_asyncio.fixture() def mock_connection() -> Connection: - mock_connection = Mock(spec=Connection) + mock_connection = AsyncMock(spec=Connection) return mock_connection @pytest_asyncio.fixture() def mock_pool() -> ConnectionPool: - mock_pool = Mock(spec=ConnectionPool) + mock_pool = AsyncMock(spec=ConnectionPool) return mock_pool diff --git a/tests/test_asyncio/test_connection.py b/tests/test_asyncio/test_connection.py index c928a82945..2cdfb59a5d 100644 --- a/tests/test_asyncio/test_connection.py +++ b/tests/test_asyncio/test_connection.py @@ -91,7 +91,7 @@ async def get_conn(): await asyncio.gather(r.set("a", "b"), r.set("c", "d")) assert init_call_count == 1 - assert command_call_count == 2 + assert command_call_count == 4 r.connection = None # it was a Mock await r.aclose() From ebbb44bc240b887a987d70479ce7dde04818db48 Mon Sep 17 00:00:00 2001 From: vladvildanov Date: Fri, 14 Mar 2025 10:44:36 +0200 Subject: [PATCH 04/14] Removed unused import --- tests/test_asyncio/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_asyncio/conftest.py b/tests/test_asyncio/conftest.py index 93357ac128..2b56f73dba 100644 --- a/tests/test_asyncio/conftest.py +++ b/tests/test_asyncio/conftest.py @@ -1,7 +1,7 @@ import random from contextlib import asynccontextmanager as _asynccontextmanager from typing import Union -from unittest.mock import Mock, AsyncMock +from unittest.mock import AsyncMock import pytest import pytest_asyncio From f55e0bc2b1add5e2a779181b3b65b5e53fab5c41 Mon Sep 17 00:00:00 2001 From: vladvildanov Date: Fri, 14 Mar 2025 10:51:57 +0200 Subject: [PATCH 05/14] Fixed broken tests --- tests/test_asyncio/test_connection.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/test_asyncio/test_connection.py b/tests/test_asyncio/test_connection.py index 2cdfb59a5d..92a9cf7ff2 100644 --- a/tests/test_asyncio/test_connection.py +++ b/tests/test_asyncio/test_connection.py @@ -328,7 +328,11 @@ async def test_client_do_not_retry_write_on_read_failure(mock_connection, mock_p mock_pool.get_connection.return_value = mock_connection mock_pool.connection_kwargs = {} - r = Redis(connection_pool=mock_pool, retry=Retry(ExponentialBackoff(), 3)) + r = Redis( + connection_pool=mock_pool, + retry=Retry(ExponentialBackoff(), 3), + single_connection_client=True, + ) await r.set("key", "value") # If read from socket fails, writes won't be executed. @@ -354,7 +358,11 @@ async def test_pipeline_immediate_do_not_retry_write_on_read_failure( mock_pool.get_connection.return_value = mock_connection mock_pool.connection_kwargs = {} - r = Redis(connection_pool=mock_pool, retry=Retry(ExponentialBackoff(), 3)) + r = Redis( + connection_pool=mock_pool, + retry=Retry(ExponentialBackoff(), 3), + single_connection_client=True, + ) pipe = r.pipeline(transaction=False) await pipe.immediate_execute_command("SET", "key", "value") From 93da5b1e75cbaf1373c4226eca8b9d4761abf9f1 Mon Sep 17 00:00:00 2001 From: vladvildanov Date: Fri, 14 Mar 2025 11:12:20 +0200 Subject: [PATCH 06/14] Added hack to make mocks awaitable --- tests/test_asyncio/conftest.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/test_asyncio/conftest.py b/tests/test_asyncio/conftest.py index 2b56f73dba..38a83c35f4 100644 --- a/tests/test_asyncio/conftest.py +++ b/tests/test_asyncio/conftest.py @@ -223,14 +223,19 @@ async def mock_cluster_resp_slaves(create_redis, **kwargs): @pytest_asyncio.fixture() def mock_connection() -> Connection: mock_connection = AsyncMock(spec=Connection) + mock_connection.__await__ = dummy_awaitable return mock_connection @pytest_asyncio.fixture() def mock_pool() -> ConnectionPool: mock_pool = AsyncMock(spec=ConnectionPool) + mock_pool.__await__ = dummy_awaitable return mock_pool +def dummy_awaitable(): + pass + @pytest_asyncio.fixture() async def credential_provider(request) -> CredentialProvider: From 7c9fd86a86155021cb454c2e831f23deb9512b09 Mon Sep 17 00:00:00 2001 From: vladvildanov Date: Fri, 14 Mar 2025 11:15:54 +0200 Subject: [PATCH 07/14] Convert hack into class --- tests/test_asyncio/conftest.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/test_asyncio/conftest.py b/tests/test_asyncio/conftest.py index 38a83c35f4..1055591ff2 100644 --- a/tests/test_asyncio/conftest.py +++ b/tests/test_asyncio/conftest.py @@ -18,6 +18,11 @@ from .compat import mock +class AwaitableMock(AsyncMock): + def __await__(self): + pass + + async def _get_info(redis_url): client = redis.Redis.from_url(redis_url) info = await client.info() @@ -222,20 +227,15 @@ async def mock_cluster_resp_slaves(create_redis, **kwargs): @pytest_asyncio.fixture() def mock_connection() -> Connection: - mock_connection = AsyncMock(spec=Connection) - mock_connection.__await__ = dummy_awaitable + mock_connection = AwaitableMock(spec=Connection) return mock_connection @pytest_asyncio.fixture() def mock_pool() -> ConnectionPool: - mock_pool = AsyncMock(spec=ConnectionPool) - mock_pool.__await__ = dummy_awaitable + mock_pool = AwaitableMock(spec=ConnectionPool) return mock_pool -def dummy_awaitable(): - pass - @pytest_asyncio.fixture() async def credential_provider(request) -> CredentialProvider: From de0e4d7e41d61f265d9ca14c40f1c9092813a94c Mon Sep 17 00:00:00 2001 From: vladvildanov Date: Fri, 14 Mar 2025 11:26:46 +0200 Subject: [PATCH 08/14] Changed assertion to assert called once --- tests/test_asyncio/test_connection.py | 12 ++---------- tests/test_connection.py | 12 ++---------- 2 files changed, 4 insertions(+), 20 deletions(-) diff --git a/tests/test_asyncio/test_connection.py b/tests/test_asyncio/test_connection.py index 92a9cf7ff2..6051377535 100644 --- a/tests/test_asyncio/test_connection.py +++ b/tests/test_asyncio/test_connection.py @@ -336,11 +336,7 @@ async def test_client_do_not_retry_write_on_read_failure(mock_connection, mock_p await r.set("key", "value") # If read from socket fails, writes won't be executed. - mock_connection.send_command.assert_has_calls( - [ - call("SET", "key", "value"), - ] - ) + mock_connection.send_command.assert_called_once_with("SET", "key", "value") @pytest.mark.onlynoncluster @@ -367,11 +363,7 @@ async def test_pipeline_immediate_do_not_retry_write_on_read_failure( await pipe.immediate_execute_command("SET", "key", "value") # If read from socket fails, writes won't be executed. - mock_connection.send_command.assert_has_calls( - [ - call("SET", "key", "value"), - ] - ) + mock_connection.send_command.assert_called_once_with("SET", "key", "value") async def test_close_is_aclose(request): diff --git a/tests/test_connection.py b/tests/test_connection.py index a6fb5b3c98..777cc1a03a 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -267,11 +267,7 @@ def test_client_do_not_retry_write_on_read_failure(mock_connection, mock_pool): r.set("key", "value") # If read from socket fails, writes won't be executed. - mock_connection.send_command.assert_has_calls( - [ - call("SET", "key", "value"), - ] - ) + mock_connection.send_command.assert_called_once_with("SET", "key", "value") @pytest.mark.onlynoncluster @@ -294,11 +290,7 @@ def test_pipeline_immediate_do_not_retry_write_on_read_failure( pipe.immediate_execute_command("SET", "key", "value") # If read from socket fails, writes won't be executed. - mock_connection.send_command.assert_has_calls( - [ - call("SET", "key", "value"), - ] - ) + mock_connection.send_command.assert_called_once_with("SET", "key", "value") @pytest.mark.skipif(sys.version_info == (3, 9), reason="Flacky test on Python 3.9") From add64c56b8331698475c7e33f928cda941096606 Mon Sep 17 00:00:00 2001 From: vladvildanov Date: Fri, 14 Mar 2025 11:28:56 +0200 Subject: [PATCH 09/14] Removed unused import --- tests/test_asyncio/test_connection.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_asyncio/test_connection.py b/tests/test_asyncio/test_connection.py index 6051377535..bfbb3dd66b 100644 --- a/tests/test_asyncio/test_connection.py +++ b/tests/test_asyncio/test_connection.py @@ -2,7 +2,7 @@ import socket import types from errno import ECONNREFUSED -from unittest.mock import patch, call +from unittest.mock import patch import pytest import redis From 5cf8e1420c3eee189d65e0209129dad7e0ab8a18 Mon Sep 17 00:00:00 2001 From: vladvildanov Date: Fri, 14 Mar 2025 11:35:28 +0200 Subject: [PATCH 10/14] Added additional call count check for read_response --- tests/test_asyncio/conftest.py | 7 ++++--- tests/test_asyncio/test_connection.py | 2 ++ tests/test_connection.py | 2 ++ 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/test_asyncio/conftest.py b/tests/test_asyncio/conftest.py index 1055591ff2..18fff4965d 100644 --- a/tests/test_asyncio/conftest.py +++ b/tests/test_asyncio/conftest.py @@ -1,6 +1,6 @@ import random from contextlib import asynccontextmanager as _asynccontextmanager -from typing import Union +from typing import Union, Iterator, Any from unittest.mock import AsyncMock import pytest @@ -19,8 +19,9 @@ class AwaitableMock(AsyncMock): - def __await__(self): - pass + def __await__(self) -> Iterator[Any]: + self.await_count += 1 + return iter([]) async def _get_info(redis_url): diff --git a/tests/test_asyncio/test_connection.py b/tests/test_asyncio/test_connection.py index bfbb3dd66b..0476cb0bad 100644 --- a/tests/test_asyncio/test_connection.py +++ b/tests/test_asyncio/test_connection.py @@ -337,6 +337,7 @@ async def test_client_do_not_retry_write_on_read_failure(mock_connection, mock_p # If read from socket fails, writes won't be executed. mock_connection.send_command.assert_called_once_with("SET", "key", "value") + mock_connection.read_response.call_count = 3 @pytest.mark.onlynoncluster @@ -364,6 +365,7 @@ async def test_pipeline_immediate_do_not_retry_write_on_read_failure( # If read from socket fails, writes won't be executed. mock_connection.send_command.assert_called_once_with("SET", "key", "value") + mock_connection.read_response.call_count = 3 async def test_close_is_aclose(request): diff --git a/tests/test_connection.py b/tests/test_connection.py index 777cc1a03a..3260a2ae34 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -268,6 +268,7 @@ def test_client_do_not_retry_write_on_read_failure(mock_connection, mock_pool): # If read from socket fails, writes won't be executed. mock_connection.send_command.assert_called_once_with("SET", "key", "value") + mock_connection.read_response.call_count = 3 @pytest.mark.onlynoncluster @@ -291,6 +292,7 @@ def test_pipeline_immediate_do_not_retry_write_on_read_failure( # If read from socket fails, writes won't be executed. mock_connection.send_command.assert_called_once_with("SET", "key", "value") + mock_connection.read_response.call_count = 3 @pytest.mark.skipif(sys.version_info == (3, 9), reason="Flacky test on Python 3.9") From 12e6b49b864281cd47f54b8f6a0910e3c8639ee0 Mon Sep 17 00:00:00 2001 From: vladvildanov Date: Fri, 14 Mar 2025 11:45:31 +0200 Subject: [PATCH 11/14] Restrict async tests to > 3.8 --- tests/test_asyncio/conftest.py | 12 +++--------- tests/test_asyncio/test_connection.py | 3 +++ 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/tests/test_asyncio/conftest.py b/tests/test_asyncio/conftest.py index 18fff4965d..2b56f73dba 100644 --- a/tests/test_asyncio/conftest.py +++ b/tests/test_asyncio/conftest.py @@ -1,6 +1,6 @@ import random from contextlib import asynccontextmanager as _asynccontextmanager -from typing import Union, Iterator, Any +from typing import Union from unittest.mock import AsyncMock import pytest @@ -18,12 +18,6 @@ from .compat import mock -class AwaitableMock(AsyncMock): - def __await__(self) -> Iterator[Any]: - self.await_count += 1 - return iter([]) - - async def _get_info(redis_url): client = redis.Redis.from_url(redis_url) info = await client.info() @@ -228,13 +222,13 @@ async def mock_cluster_resp_slaves(create_redis, **kwargs): @pytest_asyncio.fixture() def mock_connection() -> Connection: - mock_connection = AwaitableMock(spec=Connection) + mock_connection = AsyncMock(spec=Connection) return mock_connection @pytest_asyncio.fixture() def mock_pool() -> ConnectionPool: - mock_pool = AwaitableMock(spec=ConnectionPool) + mock_pool = AsyncMock(spec=ConnectionPool) return mock_pool diff --git a/tests/test_asyncio/test_connection.py b/tests/test_asyncio/test_connection.py index 0476cb0bad..9afff2074e 100644 --- a/tests/test_asyncio/test_connection.py +++ b/tests/test_asyncio/test_connection.py @@ -1,5 +1,6 @@ import asyncio import socket +import sys import types from errno import ECONNREFUSED from unittest.mock import patch @@ -316,6 +317,7 @@ async def get_redis_connection(): @pytest.mark.onlynoncluster +@pytest.mark.skipif(sys.version_info < (3, 8), reason="requires python > 3.8") async def test_client_do_not_retry_write_on_read_failure(mock_connection, mock_pool): mock_connection.send_command.return_value = True mock_connection.read_response.side_effect = [ @@ -341,6 +343,7 @@ async def test_client_do_not_retry_write_on_read_failure(mock_connection, mock_p @pytest.mark.onlynoncluster +@pytest.mark.skipif(sys.version_info < (3, 8), reason="requires python > 3.8") async def test_pipeline_immediate_do_not_retry_write_on_read_failure( mock_connection, mock_pool ): From 0855a565387aa4161ec1a82c13c40cbc02ac1aed Mon Sep 17 00:00:00 2001 From: vladvildanov Date: Fri, 14 Mar 2025 11:55:34 +0200 Subject: [PATCH 12/14] Changed version restrictions --- tests/test_asyncio/test_connection.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_asyncio/test_connection.py b/tests/test_asyncio/test_connection.py index 9afff2074e..7597149dda 100644 --- a/tests/test_asyncio/test_connection.py +++ b/tests/test_asyncio/test_connection.py @@ -317,7 +317,7 @@ async def get_redis_connection(): @pytest.mark.onlynoncluster -@pytest.mark.skipif(sys.version_info < (3, 8), reason="requires python > 3.8") +@pytest.mark.skipif(sys.version_info < (3, 9), reason="requires python > 3.8") async def test_client_do_not_retry_write_on_read_failure(mock_connection, mock_pool): mock_connection.send_command.return_value = True mock_connection.read_response.side_effect = [ @@ -343,7 +343,7 @@ async def test_client_do_not_retry_write_on_read_failure(mock_connection, mock_p @pytest.mark.onlynoncluster -@pytest.mark.skipif(sys.version_info < (3, 8), reason="requires python > 3.8") +@pytest.mark.skipif(sys.version_info < (3, 9), reason="requires python > 3.8") async def test_pipeline_immediate_do_not_retry_write_on_read_failure( mock_connection, mock_pool ): From 311a223538ff4e3b5456b58faa10c586cc11cb33 Mon Sep 17 00:00:00 2001 From: vladvildanov Date: Mon, 17 Mar 2025 09:29:14 +0200 Subject: [PATCH 13/14] Fixed error with awaitable tests --- tests/test_asyncio/conftest.py | 6 +++--- tests/test_asyncio/test_connection.py | 8 +++----- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/tests/test_asyncio/conftest.py b/tests/test_asyncio/conftest.py index 2b56f73dba..fd7a816cd7 100644 --- a/tests/test_asyncio/conftest.py +++ b/tests/test_asyncio/conftest.py @@ -1,7 +1,7 @@ import random from contextlib import asynccontextmanager as _asynccontextmanager from typing import Union -from unittest.mock import AsyncMock +from unittest.mock import Mock import pytest import pytest_asyncio @@ -222,13 +222,13 @@ async def mock_cluster_resp_slaves(create_redis, **kwargs): @pytest_asyncio.fixture() def mock_connection() -> Connection: - mock_connection = AsyncMock(spec=Connection) + mock_connection = Mock(spec=Connection) return mock_connection @pytest_asyncio.fixture() def mock_pool() -> ConnectionPool: - mock_pool = AsyncMock(spec=ConnectionPool) + mock_pool = Mock(spec=ConnectionPool) return mock_pool diff --git a/tests/test_asyncio/test_connection.py b/tests/test_asyncio/test_connection.py index 7597149dda..5a968c35a2 100644 --- a/tests/test_asyncio/test_connection.py +++ b/tests/test_asyncio/test_connection.py @@ -3,7 +3,7 @@ import sys import types from errno import ECONNREFUSED -from unittest.mock import patch +from unittest.mock import patch, AsyncMock import pytest import redis @@ -317,7 +317,6 @@ async def get_redis_connection(): @pytest.mark.onlynoncluster -@pytest.mark.skipif(sys.version_info < (3, 9), reason="requires python > 3.8") async def test_client_do_not_retry_write_on_read_failure(mock_connection, mock_pool): mock_connection.send_command.return_value = True mock_connection.read_response.side_effect = [ @@ -327,7 +326,7 @@ async def test_client_do_not_retry_write_on_read_failure(mock_connection, mock_p ] mock_connection.retry = Retry(ExponentialBackoff(), 3) mock_connection.retry_on_error = (ConnectionError,) - mock_pool.get_connection.return_value = mock_connection + mock_pool.get_connection = AsyncMock(return_value=mock_connection) mock_pool.connection_kwargs = {} r = Redis( @@ -343,7 +342,6 @@ async def test_client_do_not_retry_write_on_read_failure(mock_connection, mock_p @pytest.mark.onlynoncluster -@pytest.mark.skipif(sys.version_info < (3, 9), reason="requires python > 3.8") async def test_pipeline_immediate_do_not_retry_write_on_read_failure( mock_connection, mock_pool ): @@ -355,7 +353,7 @@ async def test_pipeline_immediate_do_not_retry_write_on_read_failure( ] mock_connection.retry = Retry(ExponentialBackoff(), 3) mock_connection.retry_on_error = (ConnectionError,) - mock_pool.get_connection.return_value = mock_connection + mock_pool.get_connection = AsyncMock(return_value=mock_connection) mock_pool.connection_kwargs = {} r = Redis( From 8612a59c3701d692ebd120d5e1b062c27ad8ef60 Mon Sep 17 00:00:00 2001 From: vladvildanov Date: Mon, 17 Mar 2025 09:30:46 +0200 Subject: [PATCH 14/14] Removed unused import --- tests/test_asyncio/test_connection.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_asyncio/test_connection.py b/tests/test_asyncio/test_connection.py index 5a968c35a2..3de70737e3 100644 --- a/tests/test_asyncio/test_connection.py +++ b/tests/test_asyncio/test_connection.py @@ -1,6 +1,5 @@ import asyncio import socket -import sys import types from errno import ECONNREFUSED from unittest.mock import patch, AsyncMock