✅ Enhanced test coverage for webhooks service (#748)

* 🚚 move tests to services module

* 🎨

* ✅ Test coverage for webhooks route

* 🎨 modify try-except block for readability and ensure cancelled errors are caught correctly

* 🎨

* ✅ Test coverage for disconnect check

* ✅ Test coverage for health check

* 🎨 remove unnecessary get_container method

* ✅ Broader test coverage for webhooks redis service

* 🎨

* 👷 remove unused constant

* 🎨

shared/constants.py

@@ -54,9 +54,6 @@
 SSE_TIMEOUT = int(
     os.getenv("SSE_TIMEOUT", "150")
 )  # maximum duration of an SSE connection
-QUEUE_POLL_PERIOD = float(
-    os.getenv("QUEUE_POLL_PERIOD", "0.1")
-)  # period in seconds to retry reading empty queues
 DISCONNECT_CHECK_PERIOD = float(
     os.getenv("DISCONNECT_CHECK_PERIOD", "0.2")
 )  # period in seconds to check for disconnection

webhooks/services/dependency_injection/container.py

@@ -49,13 +49,3 @@ class Container(containers.DeclarativeContainer):
         SseManager,
         redis_service=redis_service,
     )
-
-
-def get_container() -> Container:
-    """
-    Creates a configured instance of the dependency injection container for the application.
-
-    Returns:
-        An instance of the configured Container.
-    """
-    return Container()

webhooks/tests/e2e/test_sse.py

@@ -114,13 +114,10 @@ async def get_sse_stream_response(url, duration=2) -> Response:
         async with RichAsyncClient(timeout=timeout) as client:
             async with client.stream("GET", url) as response:
                 response_text = ""
-                try:
-                    async for line in response.aiter_lines():
-                        response_text += line
-                except TimeoutError:
-                    pass  # Timeout reached, return the response text read so far
-    except ReadTimeout:
-        # Closing connection gracefully, as event_stream is infinite
+                async for line in response.aiter_lines():
+                    response_text += line
+    except (TimeoutError, ReadTimeout):
+        # Closing connection gracefully, return the response text read so far
         pass
     finally:
         return response_text
@@ -144,7 +141,3 @@ async def listen_for_event(url, duration=10):
                 if line.startswith("data: "):
                     data = line[6:]
                     return json.loads(data)
-                elif line == "" or line.startswith(": ping"):
-                    pass  # ignore newlines and pings
-                else:
-                    logger.warning("Unexpected SSE line: %s", line)

webhooks/tests/routes/test_sse.py

@@ -133,6 +133,42 @@ async def test_sse_event_stream_generator_wallet_id(
     )
 
 
+@pytest.mark.anyio
+async def test_sse_event_stream_generator_wallet_id_disconnect(
+    async_generator_mock,  # pylint: disable=redefined-outer-name
+    sse_manager_mock,  # pylint: disable=redefined-outer-name
+    request_mock,  # pylint: disable=redefined-outer-name
+):
+    request_mock.is_disconnected.return_value = AsyncMock(True)
+
+    # Configure the sse_manager mock
+    sse_manager_mock.sse_event_stream.return_value = EventGeneratorWrapper(
+        generator=async_generator_mock([dummy_cloudapi_event]),
+        populate_task=Mock(),
+    )
+
+    background_tasks = BackgroundTasks()
+
+    # Convert generator to a list to force evaluation
+    events = [
+        event
+        async for event in sse_event_stream_generator(
+            sse_manager=sse_manager_mock,
+            request=request_mock,
+            background_tasks=background_tasks,
+            wallet_id=wallet_id,
+            logger=Mock(),
+        )
+    ]
+
+    # Assertions
+    assert len(events) == 0, "Expected no events to be yielded after disconnection."
+    request_mock.is_disconnected.assert_awaited()
+    assert (
+        len(background_tasks.tasks) > 0
+    ), "Expected at least one background task for disconnection check."
+
+
 @pytest.mark.anyio
 async def test_sse_event_stream_generator_wallet_id_topic(
     async_generator_mock,  # pylint: disable=redefined-outer-name

webhooks/tests/routes/test_webhooks.py

@@ -0,0 +1,87 @@
+from unittest.mock import Mock
+
+import pytest
+
+from shared.models.webhook_events import CloudApiWebhookEventGeneric
+from webhooks.web.routers.webhooks import (
+    get_webhooks_by_wallet,
+    get_webhooks_by_wallet_and_topic,
+)
+
+wallet_id = "wallet123"
+topic = "test_topic"
+cloud_api_event = CloudApiWebhookEventGeneric(
+    wallet_id=wallet_id,
+    topic=topic,
+    origin="test_origin",
+    group_id="test_group",
+    payload={"key": "value"},
+)
+
+
+@pytest.mark.anyio
+async def test_get_webhooks_by_wallet():
+    redis_service_mock = Mock()
+
+    redis_service_mock.get_cloudapi_events_by_wallet.return_value = [cloud_api_event]
+
+    result = await get_webhooks_by_wallet(
+        wallet_id=wallet_id, redis_service=redis_service_mock
+    )
+
+    assert len(result) == 1
+    assert result[0].wallet_id == wallet_id
+    redis_service_mock.get_cloudapi_events_by_wallet.assert_called_once_with(
+        wallet_id, num=100
+    )
+
+
+@pytest.mark.anyio
+async def test_get_webhooks_by_wallet_empty():
+    redis_service_mock = Mock()
+
+    redis_service_mock.get_cloudapi_events_by_wallet.return_value = []
+
+    result = await get_webhooks_by_wallet(
+        wallet_id=wallet_id, redis_service=redis_service_mock
+    )
+
+    assert result == []
+    redis_service_mock.get_cloudapi_events_by_wallet.assert_called_once_with(
+        wallet_id, num=100
+    )
+
+
+@pytest.mark.anyio
+async def test_get_webhooks_by_wallet_and_topic():
+    redis_service_mock = Mock()
+
+    redis_service_mock.get_cloudapi_events_by_wallet_and_topic.return_value = [
+        cloud_api_event
+    ]
+
+    result = await get_webhooks_by_wallet_and_topic(
+        wallet_id=wallet_id, topic=topic, redis_service=redis_service_mock
+    )
+
+    assert len(result) == 1
+    assert result[0].wallet_id == wallet_id
+    redis_service_mock.get_cloudapi_events_by_wallet_and_topic.assert_called_once_with(
+        wallet_id=wallet_id, topic=topic, num=100
+    )
+
+
+@pytest.mark.anyio
+async def test_get_webhooks_by_wallet_and_topic_empty():
+    redis_service_mock = Mock()
+
+    redis_service_mock.get_cloudapi_events_by_wallet_and_topic.return_value = []
+
+    result = await get_webhooks_by_wallet_and_topic(
+        wallet_id=wallet_id, topic=topic, redis_service=redis_service_mock
+    )
+
+    assert result == []
+    redis_service_mock.get_cloudapi_events_by_wallet_and_topic.assert_called_once_with(
+        wallet_id=wallet_id, topic=topic, num=100
+    )

webhooks/tests/test_redis_service.py → webhooks/tests/services/test_webhooks_redis_service.py

@@ -1,4 +1,5 @@
 import time
+from itertools import chain
 from unittest.mock import Mock
 
 import pytest
@@ -62,6 +63,37 @@ async def test_get_json_cloudapi_events_by_wallet():
     redis_client.zrevrangebyscore.assert_called_once()
 
 
+@pytest.mark.anyio
+async def test_get_json_cloudapi_events_by_wallet_no_events():
+    redis_client = Mock()
+    redis_client.zrevrangebyscore = Mock(
+        return_value=[e.encode() for e in json_entries]
+    )
+    redis_service = WebhooksRedisService(redis_client)
+    redis_service.match_keys = Mock(return_value=[])
+
+    events = redis_service.get_json_cloudapi_events_by_wallet(wallet_id)
+
+    assert events == []
+    redis_client.zrevrangebyscore.assert_not_called()
+
+
+@pytest.mark.anyio
+async def test_get_json_cloudapi_events_by_wallet_defaults_max():
+    redis_client = Mock()
+    redis_client.zrevrangebyscore = Mock(
+        return_value=[e.encode() for e in json_entries]
+    )
+    redis_service = WebhooksRedisService(redis_client)
+    redis_service.match_keys = Mock(return_value=[b"dummy_key"])
+
+    redis_service.get_json_cloudapi_events_by_wallet(wallet_id, num=None)
+
+    redis_client.zrevrangebyscore.assert_called_once_with(
+        name="dummy_key", max="+inf", min="-inf", start=0, num=10000  # default max num
+    )
+
+
 @pytest.mark.anyio
 async def test_get_cloudapi_events_by_wallet(mocker):
     expected_events = cloudapi_entries
@@ -150,6 +182,27 @@ async def test_get_json_cloudapi_events_by_timestamp():
     )
 
 
+@pytest.mark.anyio
+async def test_get_json_cloudapi_events_by_timestamp_no_events():
+    start_timestamp = 1609459200
+    end_timestamp = 1609545600
+
+    redis_client = Mock()
+    redis_client.zrangebyscore = Mock(return_value=[])
+    redis_service = WebhooksRedisService(redis_client)
+
+    redis_service.match_keys = Mock(return_value=[])
+
+    events = redis_service.get_json_cloudapi_events_by_timestamp(
+        group_id=group_id,
+        wallet_id=wallet_id,
+        start_timestamp=start_timestamp,
+        end_timestamp=end_timestamp,
+    )
+
+    assert events == []
+
+
 @pytest.mark.anyio
 async def test_get_cloudapi_events_by_timestamp(mocker):
     start_timestamp = 1609459200
@@ -195,6 +248,86 @@ async def test_get_all_cloudapi_wallet_ids():
     assert redis_client.scan.call_count == 2
 
 
+@pytest.mark.anyio
+async def test_get_all_cloudapi_wallet_ids_no_wallets():
+    scan_results = [({"localhost:6379": 1}, []), ({"localhost:6379": 0}, [])]
+
+    redis_client = Mock()
+    redis_client.scan = Mock(side_effect=scan_results)
+
+    redis_service = WebhooksRedisService(redis_client)
+
+    wallet_ids = redis_service.get_all_cloudapi_wallet_ids()
+
+    assert wallet_ids == []
+    assert redis_client.scan.call_count == 2
+
+
+@pytest.mark.anyio
+async def test_get_all_cloudapi_wallet_ids_handles_exception():
+    expected_wallet_ids = ["wallet1", "wallet2"]
+
+    # Adjust the mock to raise an Exception on the first call
+    redis_client = Mock()
+    redis_client.scan.side_effect = chain(
+        [
+            (
+                {"localhost:6379": 1},
+                [f"cloudapi:{wallet_id}".encode() for wallet_id in expected_wallet_ids],
+            )
+        ],
+        Exception("Test exception"),
+    )
+
+    redis_service = WebhooksRedisService(redis_client)
+
+    wallet_ids = redis_service.get_all_cloudapi_wallet_ids()
+
+    assert set(wallet_ids) == set(expected_wallet_ids)
+
+
+@pytest.mark.anyio
+async def test_add_endorsement_event():
+    event_json = '{"data": "value"}'
+    transaction_id = "transaction123"
+
+    # Mock the Redis client
+    redis_client = Mock()
+    redis_service = WebhooksRedisService(redis_client)
+
+    # Mock Redis 'set' operation to return True to simulate a successful operation
+    redis_client.set = Mock(return_value=True)
+
+    redis_service.add_endorsement_event(event_json, transaction_id)
+
+    # Verify Redis 'set' was called correctly
+    redis_client.set.assert_called_once_with(
+        f"{redis_service.endorsement_redis_prefix}:{transaction_id}",
+        value=event_json,
+    )
+
+
+@pytest.mark.anyio
+async def test_add_endorsement_event_key_exists():
+    event_json = '{"data": "value"}'
+    transaction_id = "transaction123"
+
+    # Mock the Redis client
+    redis_client = Mock()
+    redis_service = WebhooksRedisService(redis_client)
+
+    # Mock Redis 'set' operation to return False to simulate a key already exists
+    redis_client.set = Mock(return_value=False)
+
+    redis_service.add_endorsement_event(event_json, transaction_id)
+
+    # Verify Redis 'set' was called correctly
+    redis_client.set.assert_called_once_with(
+        f"{redis_service.endorsement_redis_prefix}:{transaction_id}",
+        value=event_json,
+    )
+
+
 @pytest.mark.anyio
 async def test_check_wallet_belongs_to_group():
     valid_group_id = "abc"