Close MockServer (Fake server) gracefully in tests

Previosly only one of 2 methods (stop, wait_for_termination)
was called to shut down the server.
But both should be called. Otherwise server
will continue working.
Server will be destroyed by GC in different test causing
missleading warnings.

`stop` method stops the server from servicing new RPCs.
`wait_for_termination` Block current coroutine until the
server stops.

Signed-off-by: ela-kotulska-frequenz <[email protected]>

Author: ela-kotulska-frequenz

tests/actor/test_data_sourcing.py

@@ -93,5 +93,5 @@ async def test_data_sourcing_actor(self) -> None:
             assert sample is not None
             assert 100.0 == sample.value
 
-        await server.stop(0.1)
+        assert await server.graceful_shutdown()
         _microgrid._MICROGRID = None  # pylint: disable=protected-access

tests/microgrid/mock_api.py

@@ -10,6 +10,8 @@
 
 from __future__ import annotations
 
+import asyncio
+
 # pylint: disable=invalid-name,no-name-in-module,unused-import
 from concurrent import futures
 from typing import Iterable, Iterator, List, Optional, Tuple
@@ -268,10 +270,31 @@ async def start(self) -> None:
         """Start the server."""
         await self._server.start()
 
-    async def stop(self, grace: Optional[float]) -> None:
+    async def _stop(self, grace: Optional[float]) -> None:
         """Stop the server."""
         await self._server.stop(grace)
 
-    async def wait_for_termination(self, timeout: Optional[float] = None) -> None:
+    async def _wait_for_termination(self, timeout: Optional[float] = None) -> None:
         """Wait for termination."""
         await self._server.wait_for_termination(timeout)
+
+    async def graceful_shutdown(
+        self, stop_timeout: float = 0.1, terminate_timeout: float = 0.2
+    ) -> bool:
+        """Shutdown server gracefully.
+
+        Args:
+            stop_timeout: Argument for self.stop method
+            terminate_timeout: Argument for self.wait_for_termination method.
+
+        Returns:
+            True if server was stopped in given timeout. False otherwise.
+        """
+        await self._stop(stop_timeout)
+        try:
+            await asyncio.wait_for(
+                self._wait_for_termination(None), timeout=terminate_timeout
+            )
+        except TimeoutError:
+            return False
+        return True

tests/microgrid/test_client.py

@@ -130,7 +130,7 @@ async def test_components(self) -> None:
             }
 
         finally:
-            await server.stop(grace=1.0)
+            assert await server.graceful_shutdown()
 
     async def test_connections(self) -> None:
         servicer = mock_api.MockMicrogridServicer()
@@ -284,7 +284,7 @@ async def test_connections(self) -> None:
             }
 
         finally:
-            await server.stop(grace=1.0)
+            assert await server.graceful_shutdown()
 
     async def test_bad_connections(self) -> None:
         """Validate that the client does not apply connection filters itself."""
@@ -355,7 +355,7 @@ def ListAllComponents(
             )
 
         finally:
-            await server.stop(grace=1.0)
+            assert await server.graceful_shutdown()
 
     async def test_meter_data(self) -> None:
         servicer = mock_api.MockMicrogridServicer()
@@ -383,7 +383,7 @@ async def test_meter_data(self) -> None:
             await asyncio.sleep(0.2)
 
         finally:
-            await server.stop(0.1)
+            assert await server.graceful_shutdown()
 
         latest = peekable.peek()
         assert isinstance(latest, MeterData)
@@ -415,7 +415,7 @@ async def test_battery_data(self) -> None:
             await asyncio.sleep(0.2)
 
         finally:
-            await server.stop(0.1)
+            assert await server.graceful_shutdown()
 
         latest = peekable.peek()
         assert isinstance(latest, BatteryData)
@@ -447,7 +447,7 @@ async def test_inverter_data(self) -> None:
             await asyncio.sleep(0.2)
 
         finally:
-            await server.stop(0.1)
+            assert await server.graceful_shutdown()
 
         latest = peekable.peek()
         assert isinstance(latest, InverterData)
@@ -479,7 +479,7 @@ async def test_ev_charger_data(self) -> None:
             await asyncio.sleep(0.2)
 
         finally:
-            await server.stop(0.1)
+            assert await server.graceful_shutdown()
 
         latest = peekable.peek()
         assert isinstance(latest, EVChargerData)
@@ -506,7 +506,7 @@ async def test_charge(self) -> None:
             assert servicer.latest_charge.power_w == 12
 
         finally:
-            await server.stop(0.1)
+            assert await server.graceful_shutdown()
 
     async def test_discharge(self) -> None:
         """Check if discharge is able to discharge component."""
@@ -528,7 +528,7 @@ async def test_discharge(self) -> None:
             assert servicer.latest_discharge.component_id == 73
             assert servicer.latest_discharge.power_w == 15
         finally:
-            await server.stop(0.1)
+            assert await server.graceful_shutdown()
 
     async def test_set_bounds(self) -> None:
         servicer = mock_api.MockMicrogridServicer()
@@ -558,7 +558,7 @@ async def test_set_bounds(self) -> None:
                 await asyncio.sleep(0.1)
 
         finally:
-            await server.stop(0.1)
+            assert await server.graceful_shutdown()
 
         assert len(expected_bounds) == len(servicer.get_bounds())
 

tests/microgrid/test_graph.py

@@ -766,7 +766,7 @@ async def test_refresh_from_api(self) -> None:
         }
         graph.validate()
 
-        await server.stop(1)
+        assert await server.graceful_shutdown()
 
     def test_validate(self) -> None:
         # `validate` will fail if any of the following are the case:

tests/microgrid/test_mock_api.py

@@ -239,7 +239,7 @@ async def test_MockGrpcServer() -> None:
         Connection(start=2, end=3),
     ]
 
-    await server1.stop(1)
+    await server1.graceful_shutdown()
 
     servicer2 = mock_api.MockMicrogridServicer(
         components=[
@@ -285,4 +285,4 @@ async def test_MockGrpcServer() -> None:
         Connection(start=77, end=9999),
     ]
 
-    await server2.wait_for_termination(0.1)
+    await server2.graceful_shutdown()

tests/microgrid/test_timeout.py

@@ -30,10 +30,6 @@
 # error and needs to be greater than `GRPC_CALL_TIMEOUT`.
 GRPC_SERVER_DELAY: float = 0.3
 
-# How long a mocked Microgrid server should be running for a single test function,
-# before it gets shut down so that the tests can finish.
-GRPC_SERVER_SHUTDOWN_DELAY: float = 1.0
-
 
 @patch(
     "frequenz.sdk.microgrid.client._client.DEFAULT_GRPC_CALL_TIMEOUT", GRPC_CALL_TIMEOUT
@@ -59,7 +55,7 @@ def mock_list_components(
     with pytest.raises(grpc.aio.AioRpcError) as err_ctx:
         _ = await client.components()
     assert err_ctx.value.code() == grpc.StatusCode.DEADLINE_EXCEEDED
-    await server.wait_for_termination(GRPC_SERVER_SHUTDOWN_DELAY)
+    assert await server.graceful_shutdown()
 
 
 @patch(
@@ -86,7 +82,7 @@ def mock_list_connections(
     with pytest.raises(grpc.aio.AioRpcError) as err_ctx:
         _ = await client.connections()
     assert err_ctx.value.code() == grpc.StatusCode.DEADLINE_EXCEEDED
-    await server.wait_for_termination(GRPC_SERVER_SHUTDOWN_DELAY)
+    assert await server.graceful_shutdown()
 
 
 @patch(
@@ -117,4 +113,4 @@ def mock_set_power(
             _ = await client.set_power(component_id=1, power_w=power_w)
         assert err_ctx.value.code() == grpc.StatusCode.DEADLINE_EXCEEDED
 
-    await server.stop(GRPC_SERVER_SHUTDOWN_DELAY)
+    assert await server.graceful_shutdown()

tests/timeseries/mock_microgrid.py

@@ -243,5 +243,5 @@ async def _init_client_and_actors(
 
     async def cleanup(self) -> None:
         """Clean up after a test."""
-        await self._server.stop(0.1)
+        await self._server.graceful_shutdown()
         microgrid._microgrid._MICROGRID = None  # pylint: disable=protected-access