Clean up client connection (#93)

Co-authored-by: Martin Hjelmare <[email protected]>

Author: balloob

pylintrc

@@ -23,6 +23,7 @@ disable=
   too-few-public-methods,
   too-many-public-methods,
   too-many-instance-attributes,
+  too-many-branches,
   no-self-use
 
 [REPORTS]

test/conftest.py

@@ -8,7 +8,7 @@
 from aiohttp import ClientSession, ClientWebSocketResponse
 from aiohttp.http_websocket import WSMessage, WSMsgType
 
-from zwave_js_server.client import STATE_CONNECTED, Client
+from zwave_js_server.client import Client
 from zwave_js_server.const import MIN_SERVER_VERSION
 from zwave_js_server.model.controller import Controller
 from zwave_js_server.model.driver import Driver
@@ -48,35 +48,35 @@ def client_session_fixture(ws_client):
 
 
 @pytest.fixture(name="ws_client")
-async def ws_client_fixture(loop, version_data, ws_message):
+async def ws_client_fixture(loop, version_data, ws_message, result):
     """Mock a websocket client.
 
     This fixture only allows a single message to be received.
     """
     ws_client = AsyncMock(spec_set=ClientWebSocketResponse, closed=False)
-    ws_client.receive_json.return_value = version_data
-    receive_event = asyncio.Event()
+    ws_client.receive_json.side_effect = (version_data, result)
 
     async def receive():
         """Return a websocket message."""
         await asyncio.sleep(0)
-        await receive_event.wait()
+        ws_client.closed = True
         return ws_message
 
     ws_client.receive.side_effect = receive
 
-    async def close_client(*args):
+    async def close_client(msg):
         """Close the client."""
+        if msg["command"] == "start_listening":
+            return
+
         await asyncio.sleep(0)
         ws_client.closed = True
-        receive_event.set()
 
     ws_client.send_json.side_effect = close_client
 
     async def reset_close():
         """Reset the websocket client close method."""
-        ws_client.closed = False
-        receive_event.clear()
+        ws_client.closed = True
 
     ws_client.close.side_effect = reset_close
 
@@ -155,8 +155,7 @@ async def client_fixture(loop, client_session, ws_client, uuid4):
     when creating the client.
     """
     client = Client("ws://test.org", client_session)
-    client.state = STATE_CONNECTED
-    client.client = ws_client
+    client._client = ws_client
     return client
 
 

test/test_client.py

@@ -10,7 +10,6 @@
 from zwave_js_server.client import Client
 from zwave_js_server.exceptions import (
     CannotConnect,
-    ConnectionClosed,
     ConnectionFailed,
     FailedCommand,
     InvalidMessage,
@@ -55,7 +54,11 @@ async def test_invalid_server_version(client_session, url, version_data, caplog)
 
     assert not client.connected
 
+
+async def test_newer_server_version(client_session, url, version_data, caplog):
+    """Test client connect with invalid server version."""
     version_data["serverVersion"] = "99999.0.0"
+    client = Client(url, client_session)
 
     await client.connect()
 
@@ -73,33 +76,7 @@ async def test_send_json_when_disconnected(client_session, url):
         await client._send_json_message({"test": None})
 
 
-async def test_connect_with_existing_driver(
-    client_session, url, ws_client, driver_ready, await_other
-):
-    """Test connecting again with an existing driver raises."""
-    client = Client(url, client_session)
-    assert not client.connected
-    assert not client.driver
-
-    await client.connect()
-
-    assert client.connected
-
-    await client.listen(driver_ready)
-    await await_other(asyncio.current_task())
-
-    assert client.driver
-
-    ws_client.receive.reset_mock()
-    ws_client.closed = False
-
-    with pytest.raises(InvalidState):
-        await client.connect()
-
-    ws_client.receive.assert_not_awaited()
-
-
-async def test_listen(client_session, url, driver_ready, await_other):
+async def test_listen(client_session, url, driver_ready):
     """Test client listen."""
     client = Client(url, client_session)
 
@@ -109,13 +86,11 @@ async def test_listen(client_session, url, driver_ready, await_other):
 
     assert client.connected
 
-    await client.listen(driver_ready)
-    await await_other(asyncio.current_task())
-
+    asyncio.create_task(client.listen(driver_ready))
+    await driver_ready.wait()
     assert client.driver
 
     await client.disconnect()
-
     assert not client.connected
 
 
@@ -132,7 +107,8 @@ async def test_listen_client_error(
     ws_client.closed = False
 
     # This should break out of the listen loop before any message is received.
-    await client.listen(driver_ready)
+    with pytest.raises(asyncio.CancelledError):
+        await client.listen(driver_ready)
 
     assert not ws_message.json.called
 
@@ -142,7 +118,6 @@ async def test_listen_client_error(
     [
         (WSMsgType.ERROR, ConnectionFailed),
         (WSMsgType.BINARY, InvalidMessage),
-        (WSMsgType.CLOSE, ConnectionClosed),
     ],
 )
 async def test_listen_error_message_types(
@@ -168,12 +143,10 @@ async def test_listen_disconnect_message_types(
     async with Client(url, client_session) as client:
         assert client.connected
         ws_message.type = message_type
-        # ws_client.closed = False
 
         # This should break out of the listen loop before handling the received message.
         # Otherwise there will be an error.
         await client.listen(driver_ready)
-        await await_other(asyncio.current_task())
 
     # Assert that we received a message.
     ws_client.receive.assert_awaited()
@@ -194,9 +167,7 @@ async def test_listen_invalid_message_data(
         await client.listen(driver_ready)
 
 
-async def test_listen_not_success(
-    client_session, url, result, driver_ready, await_other
-):
+async def test_listen_not_success(client_session, url, result, driver_ready):
     """Test receive result message with success False on listen."""
     result["success"] = False
     result["errorCode"] = "error_code"
@@ -205,38 +176,8 @@ async def test_listen_not_success(
 
     with pytest.raises(FailedCommand):
         await client.listen(driver_ready)
-        await await_other(asyncio.current_task())
-
-    assert client.connected
-
 
-async def test_listen_invalid_state(
-    client_session, url, result, driver_ready, await_other
-):
-    """Test missing driver state on listen."""
-    result["type"] = "event"
-    result["event"] = {
-        "source": "node",
-        "event": "value updated",
-        "nodeId": 52,
-        "args": {
-            "commandClassName": "Basic",
-            "commandClass": 32,
-            "endpoint": 0,
-            "property": "currentValue",
-            "newValue": 255,
-            "prevValue": 255,
-            "propertyName": "currentValue",
-        },
-    }
-    client = Client(url, client_session)
-    await client.connect()
-
-    with pytest.raises(InvalidState):
-        await client.listen(driver_ready)
-        await await_other(asyncio.current_task())
-
-    assert client.connected
+    assert not client.connected
 
 
 async def test_listen_without_connect(client_session, url, driver_ready):
@@ -256,11 +197,7 @@ async def test_listen_event(
     await client.connect()
 
     assert client.connected
-    await client.listen(driver_ready)
-    await await_other(asyncio.current_task())
-    assert client.driver
 
-    result.clear()
     result["type"] = "event"
     result["event"] = {
         "source": "node",
@@ -277,18 +214,7 @@ async def test_listen_event(
         },
     }
 
-    ws_client.receive.reset_mock()
-    ws_client.closed = False
-
-    async def receive():
-        """Return a websocket message."""
-        ws_client.closed = True
-        return ws_message
-
-    ws_client.receive.side_effect = receive
-
     await client.listen(driver_ready)
-
     ws_client.receive.assert_awaited()