addressed comments

Signed-off-by: Lior Sventitzky <[email protected]>

Author: liorsve

python/tests/async_tests/conftest.py

@@ -3,6 +3,7 @@
 from typing import AsyncGenerator, List, Optional, Union
 
 import pytest
+
 from glide.config import (
     BackoffStrategy,
     GlideClientConfiguration,
@@ -15,14 +16,14 @@
 from glide.exceptions import ClosingError
 from glide.glide_client import GlideClient, GlideClusterClient, TGlideClient
 from glide.logger import Logger
-from glide.routes import AllNodes
-
 from tests.utils.cluster import ValkeyCluster
 from tests.utils.utils import (
     DEFAULT_TEST_LOG_LEVEL,
     INITIAL_PASSWORD,
     NEW_PASSWORD,
     USERNAME,
+    auth_client,
+    config_set_new_password,
     create_client_config,
     set_new_acl_username_with_password,
 )
@@ -134,41 +135,6 @@ async def create_client(
         return await GlideClient.create(config)
 
 
-async def auth_client(client: TGlideClient, password: str, username: str = "default"):
-    """
-    Authenticates the given TGlideClient server connected. If no username is provided, uses the 'default' user.
-    """
-    if isinstance(client, GlideClient):
-        return await client.custom_command(["AUTH", username, password])
-    elif isinstance(client, GlideClusterClient):
-        return await client.custom_command(
-            ["AUTH", username, password], route=AllNodes()
-        )
-
-
-async def config_set_new_password(client: TGlideClient, password):
-    """
-    Sets a new password for the given TGlideClient server connected.
-    This function updates the server to require a new password.
-    """
-    if isinstance(client, GlideClient):
-        await client.config_set({"requirepass": password})
-    elif isinstance(client, GlideClusterClient):
-        await client.config_set({"requirepass": password}, route=AllNodes())
-
-
-async def kill_connections(client: TGlideClient):
-    """
-    Kills all connections to the given TGlideClient server connected.
-    """
-    if isinstance(client, GlideClient):
-        await client.custom_command(["CLIENT", "KILL", "TYPE", "normal"])
-    elif isinstance(client, GlideClusterClient):
-        await client.custom_command(
-            ["CLIENT", "KILL", "TYPE", "normal"], route=AllNodes()
-        )
-
-
 async def test_teardown(request, cluster_mode: bool, protocol: ProtocolVersion):
     """
     Perform teardown tasks such as flushing all data from the cluster.

python/tests/async_tests/test_async_client.py

@@ -84,7 +84,6 @@
     SlotKeyRoute,
     SlotType,
 )
-
 from tests.async_tests.conftest import create_client
 from tests.utils.utils import (
     check_function_list_response,

python/tests/async_tests/test_auth.py

@@ -8,17 +8,14 @@
 from glide.constants import OK
 from glide.exceptions import RequestError
 from glide.glide_client import TGlideClient
-
-from tests.async_tests.conftest import (
-    auth_client,
-    config_set_new_password,
-    kill_connections,
-)
 from tests.utils.utils import (
     NEW_PASSWORD,
     USERNAME,
     WRONG_PASSWORD,
+    auth_client,
+    config_set_new_password,
     delete_acl_username_and_password,
+    kill_connections,
     set_new_acl_username_with_password,
 )
 

python/tests/async_tests/test_batch.py

@@ -61,7 +61,6 @@
 from glide.constants import OK, TResult, TSingleNodeRoute
 from glide.glide_client import GlideClient, GlideClusterClient, TGlideClient
 from glide.routes import AllNodes, SlotIdRoute, SlotKeyRoute, SlotType
-
 from tests.async_tests.conftest import create_client
 from tests.utils.utils import (
     check_if_server_version_lt,

python/tests/async_tests/test_pubsub.py

@@ -17,7 +17,6 @@
 from glide.constants import OK
 from glide.exceptions import ConfigurationError
 from glide.glide_client import GlideClient, GlideClusterClient, TGlideClient
-
 from tests.async_tests.conftest import create_client
 from tests.utils.utils import check_if_server_version_lt, get_random_string
 

python/tests/async_tests/test_read_from_strategy.py

@@ -10,7 +10,6 @@
 from glide.constants import OK
 from glide.glide_client import GlideClusterClient
 from glide.routes import AllNodes, SlotIdRoute, SlotType
-
 from tests.async_tests.conftest import create_client
 from tests.utils.utils import get_first_result
 

python/tests/async_tests/test_scan.py

@@ -9,7 +9,6 @@
 from glide.exceptions import RequestError
 from glide.glide import ClusterScanCursor
 from glide.glide_client import GlideClient, GlideClusterClient
-
 from tests.async_tests.conftest import create_client
 from tests.utils.cluster import ValkeyCluster
 from tests.utils.utils import get_random_string

python/tests/async_tests/tests_server_modules/test_json.py

@@ -21,7 +21,6 @@
 from glide.constants import OK
 from glide.exceptions import RequestError
 from glide.glide_client import GlideClusterClient, TGlideClient
-
 from tests.async_tests.test_async_client import get_random_string
 
 

python/tests/conftest.py

@@ -241,6 +241,8 @@ def anyio_backend(request):
         )
 
     return request.param[0]
+
+
 @pytest.fixture(autouse=True)
 def skip_if_version_below(request):
     """
@@ -254,7 +256,7 @@ async def test_meow_meow(...):
     if request.node.get_closest_marker("skip_if_version_below"):
         min_version = request.node.get_closest_marker("skip_if_version_below").args[0]
         client = create_sync_client(request, False)
-        try: 
+        try:
             if sync_check_if_server_version_lt(client, min_version):
                 pytest.skip(
                     reason=f"This feature added in version {min_version}",

python/tests/sync_tests/conftest.py

@@ -3,6 +3,7 @@
 from typing import Generator, List, Optional
 
 import pytest
+
 from glide.config import (
     BackoffStrategy,
     GlideClientConfiguration,
@@ -13,15 +14,18 @@
     ServerCredentials,
 )
 from glide.exceptions import ClosingError
-from glide.glide_client import GlideClient, GlideClusterClient
 from glide.logger import Logger
-from glide.routes import AllNodes
 from glide.sync import GlideClient as SyncGlideClient
 from glide.sync import GlideClusterClient as SyncGlideClusterClient
 from glide.sync import TGlideClient as TSyncGlideClient
-
 from tests.utils.cluster import ValkeyCluster
-from tests.utils.utils import DEFAULT_TEST_LOG_LEVEL, NEW_PASSWORD, create_client_config
+from tests.utils.utils import (
+    DEFAULT_TEST_LOG_LEVEL,
+    NEW_PASSWORD,
+    auth_client,
+    config_set_new_password,
+    create_client_config,
+)
 
 Logger.set_logger_config(DEFAULT_TEST_LOG_LEVEL)
 
@@ -32,7 +36,7 @@ def glide_sync_client(
     cluster_mode: bool,
     protocol: ProtocolVersion,
 ) -> Generator[TSyncGlideClient, None, None]:
-    "Get async socket client for tests"
+    "Get sync socket client for tests"
     client = create_sync_client(request, cluster_mode, protocol=protocol)
     yield client
     sync_test_teardown(request, cluster_mode, protocol)
@@ -86,37 +90,6 @@ def create_sync_client(
         return SyncGlideClient.create(config)
 
 
-def sync_auth_client(client: TSyncGlideClient, password):
-    """
-    Authenticates the given TGlideClient server connected.
-    """
-    if isinstance(client, GlideClient):
-        client.custom_command(["AUTH", password])
-    elif isinstance(client, GlideClusterClient):
-        client.custom_command(["AUTH", password], route=AllNodes())
-
-
-def sync_config_set_new_password(client: TSyncGlideClient, password):
-    """
-    Sets a new password for the given TGlideClient server connected.
-    This function updates the server to require a new password.
-    """
-    if isinstance(client, GlideClient):
-        client.config_set({"requirepass": password})
-    elif isinstance(client, GlideClusterClient):
-        client.config_set({"requirepass": password}, route=AllNodes())
-
-
-def sync_kill_connections(client: TSyncGlideClient):
-    """
-    Kills all connections to the given TGlideClient server connected.
-    """
-    if isinstance(client, GlideClient):
-        client.custom_command(["CLIENT", "KILL", "TYPE", "normal"])
-    elif isinstance(client, GlideClusterClient):
-        client.custom_command(["CLIENT", "KILL", "TYPE", "normal"], route=AllNodes())
-
-
 def sync_test_teardown(request, cluster_mode: bool, protocol: ProtocolVersion):
     """
     Perform teardown tasks such as flushing all data from the cluster.
@@ -145,9 +118,9 @@ def sync_test_teardown(request, cluster_mode: bool, protocol: ProtocolVersion):
                 credentials=credentials,
             )
             try:
-                sync_auth_client(client, NEW_PASSWORD)
+                auth_client(client, NEW_PASSWORD)
                 # Reset the server password back to empty
-                sync_config_set_new_password(client, "")
+                config_set_new_password(client, "")
                 client.update_connection_password(None)
                 # Perform the teardown
                 client.custom_command(["FLUSHALL"])

python/tests/sync_tests/test_sync_client.py

@@ -81,7 +81,6 @@
     SlotType,
 )
 from glide.sync.glide_client import GlideClient, GlideClusterClient, TGlideClient
-
 from tests.sync_tests.conftest import create_sync_client
 from tests.utils.utils import (
     check_function_list_response,

python/tests/test_utils.py

@@ -1,7 +1,6 @@
 # Copyright Valkey GLIDE Project Contributors - SPDX Identifier: Apache-2.0
 
 from glide.logger import Level, Logger
-
 from tests.utils.utils import DEFAULT_TEST_LOG_LEVEL, compare_maps
 
 

python/tests/utils/utils.py

@@ -39,8 +39,12 @@
 from glide.glide_client import GlideClient, GlideClusterClient, TGlideClient
 from glide.logger import Level as logLevel
 from glide.routes import AllNodes
+from glide.sync import GlideClient as SyncGlideClient
+from glide.sync import GlideClusterClient as SyncGlideClusterClient
 from glide.sync import TGlideClient as TSyncGlideClient
 
+TAnyGlideClient = Union[TGlideClient, TSyncGlideClient]
+
 from tests.utils.cluster import ValkeyCluster
 
 T = TypeVar("T")
@@ -447,6 +451,7 @@ async def delete_acl_username_and_password(client: TGlideClient, username: str):
             ["ACL", "DELUSER", username], route=AllNodes()
         )
 
+
 def create_client_config(
     request,
     cluster_mode: bool,
@@ -510,6 +515,7 @@ def create_client_config(
         )
     return config
 
+
 def run_sync_func_with_timeout_in_thread(
     func: Callable, timeout: float, on_timeout=None
 ):
@@ -534,3 +540,42 @@ def run_sync_func_with_timeout_in_thread(
     finally:
         # Shutdown with cancel_futures=True to prevent hanging
         executor.shutdown(wait=False, cancel_futures=True)
+
+
+def auth_client(client: TAnyGlideClient, password: str, username: str = "default"):
+    """
+    Authenticates the given TGlideClient server connected.
+    When passing a sync client, this returns the result of the AUTH command.
+    When passing an async client, this returns a coroutine that should be awaited.
+    """
+    if isinstance(client, (GlideClient, SyncGlideClient)):
+        return client.custom_command(["AUTH", username, password])
+    elif isinstance(client, (GlideClusterClient, SyncGlideClusterClient)):
+        return client.custom_command(["AUTH", username, password], route=AllNodes())
+
+
+def config_set_new_password(client: TAnyGlideClient, password):
+    """
+    Sets a new password for the given TGlideClient server connected.
+    This function updates the server to require a new password.
+    When passing a sync client, this returns the reuslt of the CONFIG SET command.
+    When passing an async client, this returns a coroutine that should be awaited.
+    """
+    if isinstance(client, (GlideClient, SyncGlideClient)):
+        return client.config_set({"requirepass": password})
+    elif isinstance(client, (GlideClusterClient, SyncGlideClusterClient)):
+        return client.config_set({"requirepass": password}, route=AllNodes())
+
+
+def kill_connections(client: TAnyGlideClient):
+    """
+    Kills all connections to the given TGlideClient server connected.
+    When passing a sync client, this returns the reuslt of the CLIENT KILL command.
+    When passing an async client, this returns a coroutine that should be awaited.
+    """
+    if isinstance(client, (GlideClient, SyncGlideClient)):
+        return client.custom_command(["CLIENT", "KILL", "TYPE", "normal"])
+    elif isinstance(client, (GlideClusterClient, SyncGlideClusterClient)):
+        return client.custom_command(
+            ["CLIENT", "KILL", "TYPE", "normal"], route=AllNodes()
+        )