Skip to content

Commit b023e92

Browse files
committed
Do a proper full comparison for events in tests
Only the types of the events were checked by the tests, not their state, which doesn't really validate everything works properly. We couldn't do a proper full comparison because we were using mocks inside the error object, and mocks have non-deterministic representations, as they have a unique ID. To fix this we just create a full gRPC error instance. This is not enough though, we also need to use a specific instance because 2 gRPC error created the same are not equal. Finally, we also need to set the retry strategy jitter to 0 so we can make sure we have a predictable retry_time. Signed-off-by: Leandro Lucarella <[email protected]>
1 parent c40283b commit b023e92

File tree

1 file changed

+20
-22
lines changed

1 file changed

+20
-22
lines changed

tests/streaming/test_grpc_stream_broadcaster.py

Lines changed: 20 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
from datetime import timedelta
1111
from unittest import mock
1212

13+
import grpc
1314
import grpc.aio
1415
import pytest
1516
from frequenz.channels import Receiver
@@ -44,12 +45,12 @@ def no_retry() -> mock.MagicMock:
4445
return mock_retry
4546

4647

47-
def mock_error() -> grpc.aio.AioRpcError:
48+
def make_error() -> grpc.aio.AioRpcError:
4849
"""Mock error for testing."""
4950
return grpc.aio.AioRpcError(
50-
code=mock.MagicMock(name="mock grpc code"),
51-
initial_metadata=mock.MagicMock(),
52-
trailing_metadata=mock.MagicMock(),
51+
code=grpc.StatusCode.UNAVAILABLE,
52+
initial_metadata=grpc.aio.Metadata(),
53+
trailing_metadata=grpc.aio.Metadata(),
5354
details="mock details",
5455
debug_error_string="mock debug_error_string",
5556
)
@@ -222,7 +223,7 @@ async def test_streaming_error( # pylint: disable=too-many-arguments
222223
"""Test streaming errors."""
223224
caplog.set_level(logging.INFO)
224225

225-
error = mock_error()
226+
error = make_error()
226227

227228
helper = streaming.GrpcStreamBroadcaster(
228229
stream_name="test_helper",
@@ -269,7 +270,7 @@ async def test_retry_next_interval_zero( # pylint: disable=too-many-arguments
269270
) -> None:
270271
"""Test retry logic when next_interval returns 0."""
271272
caplog.set_level(logging.WARNING)
272-
error = mock_error()
273+
error = make_error()
273274
mock_retry = mock.MagicMock(spec=retry.Strategy)
274275
mock_retry.next_interval.side_effect = [0, None]
275276
mock_retry.copy.return_value = mock_retry
@@ -311,17 +312,19 @@ async def test_messages_on_retry(
311312
receiver_ready_event: asyncio.Event, # pylint: disable=redefined-outer-name
312313
) -> None:
313314
"""Test that messages are sent on retry."""
315+
# We need to use a specific instance for all the test here because 2 errors created
316+
# with the same arguments don't compare equal (grpc.aio.AioRpcError doesn't seem to
317+
# provide a __eq__ method).
318+
error = make_error()
319+
314320
helper = streaming.GrpcStreamBroadcaster(
315321
stream_name="test_helper",
316322
stream_method=lambda: _ErroringAsyncIter(
317-
mock_error(),
323+
error,
318324
receiver_ready_event,
319325
),
320326
transform=_transformer,
321-
retry_strategy=retry.LinearBackoff(
322-
limit=1,
323-
interval=0.01,
324-
),
327+
retry_strategy=retry.LinearBackoff(limit=1, interval=0.01, jitter=0.0),
325328
retry_on_exhausted_stream=True,
326329
)
327330

@@ -335,15 +338,10 @@ async def test_messages_on_retry(
335338
items, events = await _split_message(receiver)
336339

337340
assert items == []
338-
assert [type(e) for e in events] == [
339-
type(e)
340-
for e in [
341-
StreamStarted(),
342-
StreamStopped(
343-
exception=mock_error(), retry_time=timedelta(seconds=0.01)
344-
),
345-
StreamStarted(),
346-
StreamStopped(exception=mock_error(), retry_time=None),
347-
StreamFatalError(mock_error()),
348-
]
341+
assert events == [
342+
StreamStarted(),
343+
StreamStopped(timedelta(seconds=0.01), error),
344+
StreamStarted(),
345+
StreamStopped(None, error),
346+
StreamFatalError(error),
349347
]

0 commit comments

Comments
 (0)