test: fix RuntimeWarnings from improper AsyncMock usage in tests (#1344)

* test: fix RuntimeWarnings from improper AsyncMock usage in tests

Fixed RuntimeWarnings caused by using AsyncMock() for synchronous methods
across multiple test files. The issue occurred when tests mocked sync methods
(like db.commit(), response.raise_for_status(), Redis.from_url()) with
AsyncMock, creating unawaited coroutines.

Changes:
- Use Mock()/MagicMock() for synchronous methods instead of AsyncMock()
- Fixed SQLAlchemy session methods (commit, refresh, close)
- Fixed HTTP response methods (raise_for_status)
- Fixed Redis client methods (from_url, pubsub, close)
- Fixed asyncio methods (create_task return values)
- Fixed event loop methods (call_soon_threadsafe)
- Removed unused event_loop fixture from conftest.py

Files modified:
- tests/unit/mcpgateway/test_reverse_proxy.py: Fixed process/task mocks
- tests/unit/mcpgateway/cache/test_session_registry.py: Fixed db session mocks
- tests/unit/mcpgateway/cache/test_session_registry_extended.py: Fixed Redis mocks
- tests/unit/mcpgateway/services/test_tool_service.py: Fixed db/HTTP mocks
- tests/unit/mcpgateway/services/test_logging_service_comprehensive.py: Fixed event loop mocks
- tests/unit/mcpgateway/services/test_role_service.py: Fixed patch.object usage
- tests/conftest.py: Removed deprecated event_loop fixture

Test results:
- 3108 tests passed
- Reduced RuntimeWarnings from many to 4 false-positive attributions
- All individual tests pass with no RuntimeWarnings

Signed-off-by: Jonathan Springer <[email protected]>

* chore: remove accidentally committed .orig file

Remove test_translate.py.orig which was accidentally included in the
previous commit. This is a merge/rebase artifact that should not be
tracked in version control.

Signed-off-by: Mihai Criveti <[email protected]>

---------

Signed-off-by: Jonathan Springer <[email protected]>
Signed-off-by: Mihai Criveti <[email protected]>
Co-authored-by: Mihai Criveti <[email protected]>

Author: jonpspri

mcpgateway/reverse_proxy.py

@@ -258,7 +258,7 @@ def __init__(
         token: Optional[str] = None,
         reconnect_delay: float = DEFAULT_RECONNECT_DELAY,
         max_retries: int = DEFAULT_MAX_RETRIES,
-        keepalive_interval: int = DEFAULT_KEEPALIVE_INTERVAL,
+        keepalive_interval: float = DEFAULT_KEEPALIVE_INTERVAL,
     ):
         """Initialize reverse proxy client.
 

tests/conftest.py

@@ -23,20 +23,11 @@
 from mcpgateway.db import Base
 
 # Local
-# Test utilities - import before mcpgateway modules
 
 # Skip session-level RBAC patching for now - let individual tests handle it
 # _session_rbac_originals = patch_rbac_decorators()
 
 
-@pytest.fixture(scope="session")
-def event_loop():
-    """Create an instance of the default event loop for each test session."""
-    loop = asyncio.get_event_loop_policy().new_event_loop()
-    yield loop
-    loop.close()
-
-
 @pytest.fixture(scope="session")
 def test_db_url():
     """Return the URL for the test database."""
@@ -143,17 +134,6 @@ def mock_websocket():
     return mock
 
 
-# @pytest.fixture(scope="session", autouse=True)
-# def _patch_stdio_first():
-#     """
-#     Runs once, *before* the test session collects other modules,
-#     so no rogue coroutine can be created.
-#     """
-#     import mcpgateway.translate as translate
-#     translate._run_stdio_to_sse = AsyncMock(return_value=None)
-#     translate._run_sse_to_stdio = AsyncMock(return_value=None)
-
-
 @pytest.fixture(scope="module")  # one DB per test module is usually fine
 def app_with_temp_db():
     """Return a FastAPI app wired to a fresh SQLite database."""

tests/unit/mcpgateway/cache/test_session_registry.py

@@ -33,7 +33,7 @@
 import re
 import sys
 from typing import Any, Dict, List
-from unittest.mock import AsyncMock, Mock, patch
+from unittest.mock import AsyncMock, MagicMock, Mock, patch
 
 # Third-Party
 from fastapi import HTTPException
@@ -284,7 +284,7 @@ async def test_broadcast_database_input(monkeypatch, registry: SessionRegistry,
     monkeypatch.setattr("mcpgateway.cache.session_registry.SQLALCHEMY_AVAILABLE", True)
     registry._backend = "database"
 
-    mock_db = AsyncMock()
+    mock_db = MagicMock()  # Use MagicMock for sync SQLAlchemy session methods
     monkeypatch.setattr("mcpgateway.cache.session_registry.SQLALCHEMY_AVAILABLE", True)
     monkeypatch.setattr("mcpgateway.cache.session_registry.get_db", lambda: iter([mock_db]), raising=True)
 

tests/unit/mcpgateway/cache/test_session_registry_extended.py

@@ -857,10 +857,12 @@ class TestRedisSessionRefresh:
     async def test_refresh_redis_sessions_general_error(self, monkeypatch, caplog):
         """Test _refresh_redis_sessions handles general errors."""
         mock_redis = AsyncMock()
+        mock_redis.close = Mock()  # Redis close() is synchronous
+        mock_redis.pubsub = Mock(return_value=AsyncMock())  # pubsub() is synchronous, returns async object
 
         with patch("mcpgateway.cache.session_registry.REDIS_AVAILABLE", True):
             with patch("mcpgateway.cache.session_registry.Redis") as MockRedis:
-                MockRedis.from_url.return_value = mock_redis
+                MockRedis.from_url = Mock(return_value=mock_redis)  # from_url is synchronous
 
                 registry = SessionRegistry(backend="redis", redis_url="redis://localhost")
 
@@ -929,7 +931,7 @@ async def test_redis_initialization_subscribe(self, monkeypatch):
 
         with patch("mcpgateway.cache.session_registry.REDIS_AVAILABLE", True):
             with patch("mcpgateway.cache.session_registry.Redis") as MockRedis:
-                MockRedis.from_url.return_value = mock_redis
+                MockRedis.from_url = Mock(return_value=mock_redis)  # from_url is synchronous
 
                 registry = SessionRegistry(backend="redis", redis_url="redis://localhost")
                 await registry.initialize()

tests/unit/mcpgateway/services/test_logging_service_comprehensive.py

@@ -581,6 +581,7 @@ async def test_storage_handler_emit_format_error():
 
     # Mock the event loop
     mock_loop = MagicMock()
+    mock_loop.call_soon_threadsafe = MagicMock()  # Ensure call_soon_threadsafe is sync
     handler.loop = mock_loop
 
     # Should not raise

tests/unit/mcpgateway/services/test_role_service.py

@@ -540,7 +540,8 @@ async def test_assign_team_role_without_scope_id(self, role_service, sample_role
         """Test assigning team-scoped role without scope_id."""
         sample_role.scope = "team"
 
-        with patch.object(role_service, "get_role_by_id", new=AsyncMock(return_value=sample_role)):
+        mock_get_role = AsyncMock(return_value=sample_role)
+        with patch.object(role_service, "get_role_by_id", new=mock_get_role):
             with pytest.raises(ValueError, match="scope_id required"):
                 await role_service.assign_role_to_user(user_email="[email protected]", role_id="role-123", scope="team", scope_id=None, granted_by="[email protected]")
 

tests/unit/mcpgateway/services/test_tool_service.py

@@ -1114,8 +1114,8 @@ async def test_update_tool_extra_fields(self, tool_service, mock_tool, test_db):
         # Mock DB get to return None
         mock_tool.id = "999"
         test_db.get = Mock(return_value=mock_tool)
-        test_db.commit = AsyncMock()
-        test_db.refresh = AsyncMock()
+        test_db.commit = Mock()  # SQLAlchemy commit is synchronous
+        test_db.refresh = Mock()  # SQLAlchemy refresh is synchronous
 
         # Create update request
         tool_update = ToolUpdate(integration_type="REST", request_type="POST", headers={"key": "value"}, input_schema={"key2": "value2"}, annotations={"key3": "value3"}, jsonpath_filter="test_filter")
@@ -1136,8 +1136,8 @@ async def test_update_tool_basic_auth(self, tool_service, mock_tool, test_db):
         # Mock DB get to return None
         mock_tool.id = "999"
         test_db.get = Mock(return_value=mock_tool)
-        test_db.commit = AsyncMock()
-        test_db.refresh = AsyncMock()
+        test_db.commit = Mock()  # SQLAlchemy commit is synchronous
+        test_db.refresh = Mock()  # SQLAlchemy refresh is synchronous
 
         # Basic auth_value
         # Create auth_value with the following values
@@ -1162,8 +1162,8 @@ async def test_update_tool_bearer_auth(self, tool_service, mock_tool, test_db):
         # Mock DB get to return None
         mock_tool.id = "999"
         test_db.get = Mock(return_value=mock_tool)
-        test_db.commit = AsyncMock()
-        test_db.refresh = AsyncMock()
+        test_db.commit = Mock()  # SQLAlchemy commit is synchronous
+        test_db.refresh = Mock()  # SQLAlchemy refresh is synchronous
 
         # Bearer auth_value
         # Create auth_value with the following values
@@ -1183,8 +1183,8 @@ async def test_update_tool_empty_auth(self, tool_service, mock_tool, test_db):
         # Mock DB get to return None
         mock_tool.id = "999"
         test_db.get = Mock(return_value=mock_tool)
-        test_db.commit = AsyncMock()
-        test_db.refresh = AsyncMock()
+        test_db.commit = Mock()  # SQLAlchemy commit is synchronous
+        test_db.refresh = Mock()  # SQLAlchemy refresh is synchronous
 
         # Create update request
         tool_update = ToolUpdate(auth=AuthenticationValues())
@@ -1243,7 +1243,7 @@ async def test_invoke_tool_rest_get(self, tool_service, mock_tool, test_db):
 
         # --------------- HTTP ------------------
         mock_response = AsyncMock()
-        mock_response.raise_for_status = AsyncMock()
+        mock_response.raise_for_status = Mock()  # HTTP response raise_for_status is synchronous
         mock_response.status_code = 200
         # <-- make json() *synchronous*
         mock_response.json = Mock(return_value={"result": "REST tool response"})
@@ -1268,7 +1268,7 @@ async def test_invoke_tool_rest_get(self, tool_service, mock_tool, test_db):
 
         # Test 204 status
         mock_response = AsyncMock()
-        mock_response.raise_for_status = AsyncMock()
+        mock_response.raise_for_status = Mock()  # HTTP response raise_for_status is synchronous
         mock_response.status_code = 204
         mock_response.json = Mock(return_value=ToolResult(content=[TextContent(type="text", text="Request completed successfully (No Content)")]))
 
@@ -1284,7 +1284,7 @@ async def test_invoke_tool_rest_get(self, tool_service, mock_tool, test_db):
 
         # Test 205 status
         mock_response = AsyncMock()
-        mock_response.raise_for_status = AsyncMock()
+        mock_response.raise_for_status = Mock()  # HTTP response raise_for_status is synchronous
         mock_response.status_code = 205
         mock_response.json = Mock(return_value=ToolResult(content=[TextContent(type="text", text="Tool error encountered")]))
 
@@ -1314,7 +1314,7 @@ async def test_invoke_tool_rest_post(self, tool_service, mock_tool, test_db):
 
         # Mock HTTP client response
         mock_response = AsyncMock()
-        mock_response.raise_for_status = AsyncMock()
+        mock_response.raise_for_status = Mock()  # HTTP response raise_for_status is synchronous
         mock_response.status_code = 200
         mock_response.json = Mock(return_value={"result": "REST tool response"})  # Make json() synchronous
         tool_service._http_client.request.return_value = mock_response
@@ -2038,7 +2038,7 @@ async def test_invoke_tool_rest_oauth_success(self, tool_service, mock_tool, tes
 
         # Mock HTTP client response
         mock_response = AsyncMock()
-        mock_response.raise_for_status = AsyncMock()
+        mock_response.raise_for_status = Mock()  # HTTP response raise_for_status is synchronous
         mock_response.status_code = 200
         mock_response.json = Mock(return_value={"result": "OAuth success"})
         tool_service._http_client.request.return_value = mock_response
@@ -2150,7 +2150,7 @@ async def test_invoke_tool_with_passthrough_headers_rest(self, tool_service, moc
 
         # Mock HTTP client response
         mock_response = AsyncMock()
-        mock_response.raise_for_status = AsyncMock()
+        mock_response.raise_for_status = Mock()  # HTTP response raise_for_status is synchronous
         mock_response.status_code = 200
         mock_response.json = Mock(return_value={"result": "success with headers"})
         tool_service._http_client.request.return_value = mock_response
@@ -2242,7 +2242,7 @@ async def test_invoke_tool_with_plugin_post_invoke_success(self, tool_service, m
 
         # Mock HTTP client response
         mock_response = AsyncMock()
-        mock_response.raise_for_status = AsyncMock()
+        mock_response.raise_for_status = Mock()  # HTTP response raise_for_status is synchronous
         mock_response.status_code = 200
         mock_response.json = Mock(return_value={"result": "original response"})
         tool_service._http_client.request.return_value = mock_response
@@ -2283,7 +2283,7 @@ async def test_invoke_tool_with_plugin_post_invoke_modified_payload(self, tool_s
 
         # Mock HTTP client response
         mock_response = AsyncMock()
-        mock_response.raise_for_status = AsyncMock()
+        mock_response.raise_for_status = Mock()  # HTTP response raise_for_status is synchronous
         mock_response.status_code = 200
         mock_response.json = Mock(return_value={"result": "original response"})
         tool_service._http_client.request.return_value = mock_response
@@ -2327,7 +2327,7 @@ async def test_invoke_tool_with_plugin_post_invoke_invalid_modified_payload(self
 
         # Mock HTTP client response
         mock_response = AsyncMock()
-        mock_response.raise_for_status = AsyncMock()
+        mock_response.raise_for_status = Mock()  # HTTP response raise_for_status is synchronous
         mock_response.status_code = 200
         mock_response.json = Mock(return_value={"result": "original response"})
         tool_service._http_client.request.return_value = mock_response
@@ -2371,7 +2371,7 @@ async def test_invoke_tool_with_plugin_post_invoke_error_fail_on_error(self, too
 
         # Mock HTTP client response
         mock_response = AsyncMock()
-        mock_response.raise_for_status = AsyncMock()
+        mock_response.raise_for_status = Mock()  # HTTP response raise_for_status is synchronous
         mock_response.status_code = 200
         mock_response.json = Mock(return_value={"result": "original response"})
         tool_service._http_client.request.return_value = mock_response
@@ -2414,7 +2414,7 @@ async def test_invoke_tool_with_plugin_metadata_rest(self, tool_service, mock_to
 
         # Mock HTTP client response
         mock_response = AsyncMock()
-        mock_response.raise_for_status = AsyncMock()
+        mock_response.raise_for_status = Mock()  # HTTP response raise_for_status is synchronous
         mock_response.status_code = 200
         mock_response.json = Mock(return_value={"result": "original response"})
         tool_service._http_client.request.return_value = mock_response

tests/unit/mcpgateway/test_cli_export_import_coverage.py

@@ -12,7 +12,7 @@
 import os
 from pathlib import Path
 import tempfile
-from unittest.mock import MagicMock, patch
+from unittest.mock import AsyncMock, MagicMock, patch
 
 # Third-Party
 import pytest
@@ -179,8 +179,7 @@ def test_main_with_subcommands_export():
     from mcpgateway.cli_export_import import main_with_subcommands
 
     with patch.object(sys, "argv", ["mcpgateway", "export", "--help"]):
-        with patch("mcpgateway.cli_export_import.asyncio.run") as mock_run:
-            mock_run.side_effect = SystemExit(0)  # Simulate help exit
+        with patch("mcpgateway.cli_export_import.export_command", new_callable=AsyncMock, side_effect=SystemExit(0)):
             with pytest.raises(SystemExit):
                 main_with_subcommands()
 
@@ -194,8 +193,7 @@ def test_main_with_subcommands_import():
     from mcpgateway.cli_export_import import main_with_subcommands
 
     with patch.object(sys, "argv", ["mcpgateway", "import", "--help"]):
-        with patch("mcpgateway.cli_export_import.asyncio.run") as mock_run:
-            mock_run.side_effect = SystemExit(0)  # Simulate help exit
+        with patch("mcpgateway.cli_export_import.import_command", new_callable=AsyncMock, side_effect=SystemExit(0)):
             with pytest.raises(SystemExit):
                 main_with_subcommands()
 
@@ -665,19 +663,18 @@ def test_main_with_subcommands_keyboard_interrupt():
 
     mock_parser = MagicMock()
     mock_args = MagicMock()
-    mock_args.func = MagicMock()
+    mock_args.func = AsyncMock(side_effect=KeyboardInterrupt())
     mock_args.include_dependencies = True
     mock_parser.parse_args.return_value = mock_args
 
     with patch.object(sys, "argv", ["mcpgateway", "import", "test.json"]):
         with patch("mcpgateway.cli_export_import.create_parser", return_value=mock_parser):
-            with patch("mcpgateway.cli_export_import.asyncio.run", side_effect=KeyboardInterrupt()):
-                with patch("builtins.print") as mock_print:
-                    with pytest.raises(SystemExit) as exc_info:
-                        main_with_subcommands()
+            with patch("builtins.print") as mock_print:
+                with pytest.raises(SystemExit) as exc_info:
+                    main_with_subcommands()
 
-                    assert exc_info.value.code == 1
-                    mock_print.assert_called_with("\n❌ Operation cancelled by user", file=sys.stderr)
+                assert exc_info.value.code == 1
+                mock_print.assert_called_with("\n❌ Operation cancelled by user", file=sys.stderr)
 
 
 def test_main_with_subcommands_include_dependencies_handling():
@@ -690,15 +687,14 @@ def test_main_with_subcommands_include_dependencies_handling():
 
     mock_parser = MagicMock()
     mock_args = MagicMock()
-    mock_args.func = MagicMock()
+    mock_args.func = AsyncMock()
     mock_args.no_dependencies = True  # This should set include_dependencies to False
     mock_parser.parse_args.return_value = mock_args
 
     with patch.object(sys, "argv", ["mcpgateway", "export", "--no-dependencies"]):
         with patch("mcpgateway.cli_export_import.create_parser", return_value=mock_parser):
-            with patch("mcpgateway.cli_export_import.asyncio.run") as mock_run:
-                main_with_subcommands()
+            main_with_subcommands()
 
-                # Verify include_dependencies was set to False (opposite of no_dependencies)
-                assert mock_args.include_dependencies is False
-                mock_run.assert_called_once_with(mock_args.func(mock_args))
+            # Verify include_dependencies was set to False (opposite of no_dependencies)
+            assert mock_args.include_dependencies is False
+            mock_args.func.assert_called_once_with(mock_args)