Address review comments for Picnic (#104732)

homeassistant/components/picnic/todo.py

@@ -12,6 +12,7 @@ from homeassistant.components.todo import (
 )
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.core import HomeAssistant
+from homeassistant.exceptions import ServiceValidationError
 from homeassistant.helpers.device_registry import DeviceEntryType, DeviceInfo
 from homeassistant.helpers.entity_platform import AddEntitiesCallback
 from homeassistant.helpers.update_coordinator import CoordinatorEntity
@@ -31,7 +32,7 @@ async def async_setup_entry(
     """Set up the Picnic shopping cart todo platform config entry."""
     picnic_coordinator = hass.data[DOMAIN][config_entry.entry_id][CONF_COORDINATOR]
 
-    async_add_entities([PicnicCart(hass, picnic_coordinator, config_entry)])
+    async_add_entities([PicnicCart(picnic_coordinator, config_entry)])
 
 
 class PicnicCart(TodoListEntity, CoordinatorEntity[PicnicUpdateCoordinator]):
@@ -44,7 +45,6 @@ class PicnicCart(TodoListEntity, CoordinatorEntity[PicnicUpdateCoordinator]):
 
     def __init__(
         self,
-        hass: HomeAssistant,
         coordinator: PicnicUpdateCoordinator,
         config_entry: ConfigEntry,
     ) -> None:
@@ -56,7 +56,6 @@ class PicnicCart(TodoListEntity, CoordinatorEntity[PicnicUpdateCoordinator]):
             manufacturer="Picnic",
             model=config_entry.unique_id,
         )
-        self.hass = hass
         self._attr_unique_id = f"{config_entry.unique_id}-cart"
 
     @property
@@ -87,7 +86,7 @@ class PicnicCart(TodoListEntity, CoordinatorEntity[PicnicUpdateCoordinator]):
         )
 
         if not product_id:
-            raise ValueError("No product found or no product ID given")
+            raise ServiceValidationError("No product found or no product ID given")
 
         await self.hass.async_add_executor_job(
             self.coordinator.picnic_api_client.add_product, product_id, 1

tests/components/picnic/conftest.py

@@ -56,31 +56,16 @@ async def init_integration(
     return mock_config_entry
 
 
-@pytest.fixture
-def ws_req_id() -> Callable[[], int]:
-    """Fixture for incremental websocket requests."""
-
-    id = 0
-
-    def next_id() -> int:
-        nonlocal id
-        id += 1
-        return id
-
-    return next_id
-
-
 @pytest.fixture
 async def get_items(
-    hass_ws_client: WebSocketGenerator, ws_req_id: Callable[[], int]
+    hass_ws_client: WebSocketGenerator
 ) -> Callable[[], Awaitable[dict[str, str]]]:
     """Fixture to fetch items from the todo websocket."""
 
     async def get() -> list[dict[str, str]]:
         # Fetch items using To-do platform
         client = await hass_ws_client()
-        id = ws_req_id()
+        await client.send_json_auto_id(
-        await client.send_json(
             {
                 "id": id,
                 "type": "todo/item/list",
@@ -88,7 +73,6 @@ async def get_items(
             }
         )
         resp = await client.receive_json()
-        assert resp.get("id") == id
         assert resp.get("success")
         return resp.get("result", {}).get("items", [])
 

tests/components/picnic/test_todo.py

@@ -7,6 +7,7 @@ from syrupy.assertion import SnapshotAssertion
 
 from homeassistant.components.todo import DOMAIN
 from homeassistant.core import HomeAssistant
+from homeassistant.exceptions import ServiceValidationError
 
 from .conftest import ENTITY_ID
 
@@ -115,7 +116,7 @@ async def test_create_todo_list_item_not_found(
     mock_picnic_api.search = Mock()
     mock_picnic_api.search.return_value = [{"items": []}]
 
-    with pytest.raises(ValueError):
+    with pytest.raises(ServiceValidationError):
         await hass.services.async_call(
             DOMAIN,
             "add_item",