feat: Fix tests reverted by co-pilot

Author: allenporter

roborock/protocols/v1_protocol.py

@@ -120,7 +120,7 @@ def decode_rpc_response(message: RoborockMessage) -> dict[str, Any]:
         raise RoborockException("Invalid V1 message format: missing payload")
     try:
         payload = json.loads(message.payload.decode())
-    except json.JSONDecodeError as e:
+    except (json.JSONDecodeError, TypeError) as e:
         raise RoborockException(f"Invalid V1 message payload: {e} for {message.payload!r}") from e
 
     _LOGGER.debug("Decoded V1 message payload: %s", payload)

tests/devices/test_v1_channel.py

@@ -6,12 +6,11 @@
 
 import json
 import logging
-from dataclasses import dataclass
-from unittest.mock import AsyncMock, Mock, patch
+from unittest.mock import AsyncMock, Mock
 
 import pytest
 
-from roborock.containers import NetworkInfo, RoborockBase, UserData, S5MaxStatus, RoborockStateCode
+from roborock.containers import NetworkInfo, RoborockStateCode, S5MaxStatus, UserData
 from roborock.devices.local_channel import LocalChannel, LocalSession
 from roborock.devices.mqtt_channel import MqttChannel
 from roborock.devices.v1_channel import V1Channel
@@ -37,7 +36,9 @@
 )
 TEST_RESPONSE = RoborockMessage(
     protocol=RoborockMessageProtocol.RPC_RESPONSE,
-    payload=json.dumps({"dps": {"102": json.dumps({"id": 12345, "result": {"state": RoborockStateCode.cleaning}})}}).encode(),
+    payload=json.dumps(
+        {"dps": {"102": json.dumps({"id": 12345, "result": {"state": RoborockStateCode.cleaning}})}}
+    ).encode(),
 )
 TEST_NETWORK_INFO_RESPONSE = RoborockMessage(
     protocol=RoborockMessageProtocol.RPC_RESPONSE,
@@ -62,6 +63,19 @@ def setup_mock_mqtt_channel() -> Mock:
     return mock_mqtt
 
 
+@pytest.fixture(name="mqtt_responses", autouse=True)
+def setup_mqtt_responses(mock_mqtt_channel: Mock) -> list[RoborockMessage]:
+    """Fixture to provide a list of mock MQTT responses."""
+
+    responses: list[RoborockMessage] = [TEST_NETWORK_INFO_RESPONSE]
+
+    def send_command(*args) -> RoborockMessage:
+        return responses.pop(0)
+
+    mock_mqtt_channel.send_command.side_effect = send_command
+    return responses
+
+
 @pytest.fixture(name="mock_local_channel")
 def setup_mock_local_channel() -> Mock:
     """Mock Local channel for testing."""
@@ -148,9 +162,8 @@ async def test_v1_channel_subscribe_both_connections_success(
     mock_local_channel.subscribe.return_value = local_unsub
 
     # Mock network info retrieval
-    with patch.object(v1_channel, "_get_networking_info", return_value=TEST_NETWORKING_INFO):
-        callback = Mock()
-        unsub = await v1_channel.subscribe(callback)
+    callback = Mock()
+    unsub = await v1_channel.subscribe(callback)
 
     # Verify both connections established
     mock_mqtt_channel.subscribe.assert_called_once()
@@ -189,8 +202,7 @@ async def test_v1_channel_local_connection_warning_logged(
     mock_mqtt_channel.subscribe.return_value = Mock()
     mock_local_channel.connect.side_effect = RoborockException("Local connection failed")
 
-    with patch.object(v1_channel, "_get_networking_info", return_value=TEST_NETWORKING_INFO):
-        await v1_channel.subscribe(Mock())
+    await v1_channel.subscribe(Mock())
 
     assert "Could not establish local connection for device abc123" in warning_caplog.text
     assert "Local connection failed" in warning_caplog.text
@@ -205,16 +217,12 @@ async def test_v1_channel_send_decoded_command_local_preferred(
     mock_local_channel: Mock,
 ) -> None:
     """Test command sending prefers local connection when available."""
-    # Setup: both connections available
-    mock_mqtt_channel.subscribe.return_value = Mock()
-    mock_local_channel.subscribe.return_value = Mock()
-    mock_local_channel.send_command.return_value = TEST_RESPONSE
-
     # Establish connections
-    with patch.object(v1_channel, "_get_networking_info", return_value=TEST_NETWORKING_INFO):
-        await v1_channel.subscribe(Mock())
+    await v1_channel.subscribe(Mock())
+    mock_mqtt_channel.send_command.reset_mock(return_value=False)
 
     # Send command
+    mock_local_channel.send_command.return_value = TEST_RESPONSE
     result = await v1_channel.send_decoded_command(
         RoborockCommand.CHANGE_SOUND_VOLUME,
         response_type=S5MaxStatus,
@@ -230,21 +238,19 @@ async def test_v1_channel_send_decoded_command_fallback_to_mqtt(
     v1_channel: V1Channel,
     mock_mqtt_channel: Mock,
     mock_local_channel: Mock,
+    mqtt_responses: list[RoborockMessage],
 ) -> None:
     """Test command sending falls back to MQTT when local fails."""
-    # Setup: both connections available initially
-    mock_mqtt_channel.subscribe.return_value = Mock()
-    mock_local_channel.subscribe.return_value = Mock()
-    mock_mqtt_channel.send_command.return_value = TEST_RESPONSE
 
     # Establish connections
-    with patch.object(v1_channel, "_get_networking_info", return_value=TEST_NETWORKING_INFO):
-        await v1_channel.subscribe(Mock())
+    await v1_channel.subscribe(Mock())
+    mock_mqtt_channel.send_command.reset_mock(return_value=False)
 
     # Local command fails
     mock_local_channel.send_command.side_effect = RoborockException("Local failed")
 
     # Send command
+    mqtt_responses.append(TEST_RESPONSE)
     result = await v1_channel.send_decoded_command(
         RoborockCommand.CHANGE_SOUND_VOLUME,
         response_type=S5MaxStatus,
@@ -260,22 +266,19 @@ async def test_v1_channel_send_decoded_command_mqtt_only(
     v1_channel: V1Channel,
     mock_mqtt_channel: Mock,
     mock_local_channel: Mock,
+    mqtt_responses: list[RoborockMessage],
 ) -> None:
     """Test command sending works with MQTT only."""
     # Setup: only MQTT connection
-    mock_mqtt_channel.subscribe.return_value = Mock()
+    # mock_mqtt_channel.subscribe.return_value = Mock()
     mock_local_channel.connect.side_effect = RoborockException("No local")
 
-    responses = [TEST_NETWORK_INFO_RESPONSE, TEST_RESPONSE]
-
-    def send_command(*args) -> RoborockMessage:
-        return responses.pop(0)
-
-    mock_mqtt_channel.send_command.side_effect = send_command
-
     await v1_channel.subscribe(Mock())
+    mock_mqtt_channel.send_command.assert_called_once()  # network info
+    mock_mqtt_channel.send_command.reset_mock(return_value=False)
 
     # Send command
+    mqtt_responses.append(TEST_RESPONSE)
     result = await v1_channel.send_decoded_command(
         RoborockCommand.CHANGE_SOUND_VOLUME,
         response_type=S5MaxStatus,
@@ -293,17 +296,13 @@ async def test_v1_channel_send_decoded_command_with_params(
     mock_local_channel: Mock,
 ) -> None:
     """Test command sending with parameters."""
-    # Setup: local connection only
-    mock_mqtt_channel.subscribe.return_value = Mock()
-    mock_local_channel.subscribe.return_value = Mock()
-    mock_local_channel.send_command.return_value = TEST_RESPONSE
 
-    with patch.object(v1_channel, "_get_networking_info", return_value=TEST_NETWORKING_INFO):
-        await v1_channel.subscribe(Mock())
+    await v1_channel.subscribe(Mock())
 
     # Send command with params
+    mock_local_channel.send_command.return_value = TEST_RESPONSE
     test_params = {"volume": 80}
-    result = await v1_channel.send_decoded_command(
+    await v1_channel.send_decoded_command(
         RoborockCommand.CHANGE_SOUND_VOLUME,
         response_type=S5MaxStatus,
         params=test_params,
@@ -312,11 +311,17 @@ async def test_v1_channel_send_decoded_command_with_params(
     # Verify command was sent with correct params
     mock_local_channel.send_command.assert_called_once()
     call_args = mock_local_channel.send_command.call_args
-    assert call_args[1]["params"] == test_params
-    assert result.state == RoborockStateCode.cleaning
-
-
-# V1Channel message handling tests
+    sent_message = call_args[0][0]
+    assert sent_message
+    assert isinstance(sent_message, RoborockMessage)
+    assert sent_message.payload
+    payload = sent_message.payload.decode()
+    json_data = json.loads(payload)
+    assert "dps" in json_data
+    assert "101" in json_data["dps"]
+    decoded_payload = json.loads(json_data["dps"]["101"])
+    assert decoded_payload["method"] == "change_sound_volume"
+    assert decoded_payload["params"] == {"volume": 80}
 
 
 async def test_v1_channel_subscription_receives_mqtt_messages(
@@ -332,8 +337,7 @@ async def test_v1_channel_subscription_receives_mqtt_messages(
     mock_local_channel.connect.side_effect = RoborockException("Local failed")
 
     # Subscribe
-    with patch.object(v1_channel, "_get_networking_info", return_value=TEST_NETWORKING_INFO):
-        await v1_channel.subscribe(callback)
+    await v1_channel.subscribe(callback)
 
     # Get the MQTT callback that was registered
     mqtt_callback = mock_mqtt_channel.subscribe.call_args[0][0]
@@ -359,8 +363,7 @@ async def test_v1_channel_subscription_receives_local_messages(
     mock_local_channel.subscribe.return_value = Mock()
 
     # Subscribe
-    with patch.object(v1_channel, "_get_networking_info", return_value=TEST_NETWORKING_INFO):
-        await v1_channel.subscribe(callback)
+    await v1_channel.subscribe(callback)
 
     # Get the local callback that was registered
     local_callback = mock_local_channel.subscribe.call_args[0][0]
@@ -476,13 +479,11 @@ async def test_v1_channel_connection_state_properties(v1_channel: V1Channel) ->
     assert not v1_channel.is_local_connected
 
 
-# V1Channel integration tests
-
-
 async def test_v1_channel_full_subscribe_and_command_flow(
     mock_mqtt_channel: Mock,
     mock_local_session: Mock,
     mock_local_channel: Mock,
+    mqtt_responses: list[RoborockMessage],
 ) -> None:
     """Test the complete flow from subscription to command execution."""
     # Setup: successful connections and responses
@@ -501,9 +502,9 @@ async def test_v1_channel_full_subscribe_and_command_flow(
     )
 
     # Mock network info for local connection
-    with patch.object(v1_channel, "_get_networking_info", return_value=TEST_NETWORKING_INFO):
-        callback = Mock()
-        unsub = await v1_channel.subscribe(callback)
+    callback = Mock()
+    unsub = await v1_channel.subscribe(callback)
+    mock_mqtt_channel.send_command.reset_mock(return_value=False)
 
     # Verify both connections established
     assert v1_channel.is_mqtt_connected
@@ -536,31 +537,29 @@ async def test_v1_channel_graceful_degradation_local_to_mqtt(
     mock_mqtt_channel: Mock,
     mock_local_session: Mock,
     mock_local_channel: Mock,
+    mqtt_responses: list[RoborockMessage],
 ) -> None:
     """Test graceful degradation from local to MQTT during operation."""
-    # Setup: both connections succeed initially
-    mock_mqtt_channel.subscribe.return_value = Mock()
-    mock_local_channel.subscribe.return_value = Mock()
-
     v1_channel = V1Channel(
         device_uid=TEST_DEVICE_UID,
         security_data=TEST_SECURITY_DATA,
         mqtt_channel=mock_mqtt_channel,
         local_session=mock_local_session,
     )
 
-    with patch.object(v1_channel, "_get_networking_info", return_value=TEST_NETWORKING_INFO):
-        await v1_channel.subscribe(Mock())
+    await v1_channel.subscribe(Mock())
+    mock_mqtt_channel.send_command.reset_mock(return_value=False)
 
     # First command: local works
     mock_local_channel.send_command.return_value = TEST_RESPONSE
     result1 = await v1_channel.send_decoded_command(RoborockCommand.GET_STATUS, response_type=S5MaxStatus)
     assert result1.state == RoborockStateCode.cleaning
     mock_local_channel.send_command.assert_called_once()
+    mock_mqtt_channel.send_command.assert_not_called()
 
     # Second command: local fails, falls back to MQTT
     mock_local_channel.send_command.side_effect = RoborockException("Local failed")
-    mock_mqtt_channel.send_command.return_value = TEST_RESPONSE
+    mqtt_responses.append(TEST_RESPONSE)
     result2 = await v1_channel.send_decoded_command(RoborockCommand.GET_STATUS, response_type=S5MaxStatus)
     assert result2.state == RoborockStateCode.cleaning