Merge remote-tracking branch 'origin/main' into feature/interactive-dashboard-ux

Author: claude

custom_components/cellar_tracker/cellar_data.py

@@ -269,7 +269,6 @@ class WineCellarData(DataUpdateCoordinator[CellarData]):
 
     def __init__(self, hass: HomeAssistant, entry: ConfigEntry) -> None:
         """Initialize the data coordinator."""
-        self._hass = hass
         self._username = entry.data[CONF_USERNAME]
         self._password = entry.data[CONF_PASSWORD]
         self._currency = normalize_currency(
@@ -514,7 +513,7 @@ def _process_inventory(
     async def _fetch_payload(self) -> str:
         """Fetch the raw inventory export for this entry's account."""
         return await async_fetch_inventory_payload(
-            self._hass, self._username, self._password
+            self.hass, self._username, self._password
         )
 
     async def _async_update_data(self) -> CellarData:
@@ -546,7 +545,7 @@ async def _async_update_data(self) -> CellarData:
         # means splitting 66 columns per row, hashing each one and copying every
         # dict. self.data is the last successful result, or None on first poll.
         try:
-            data = await self._hass.async_add_executor_job(
+            data = await self.hass.async_add_executor_job(
                 self._parse_and_process, payload, self.data
             )
         except UpdateFailed:

tests/test_async_fetch.py

@@ -54,7 +54,7 @@ def update(coordinator):
 def test_the_http_call_does_not_go_through_an_executor():
     coordinator = build(text=TWO_BOTTLES)
     update(coordinator)
-    assert "get_inventory" not in coordinator._hass.executor_jobs, (
+    assert "get_inventory" not in coordinator.hass.executor_jobs, (
         "the blocking client is still being used"
     )
 
@@ -63,7 +63,7 @@ def test_parsing_still_runs_in_the_executor():
     """CPU-bound work stays off the event loop even though I/O no longer needs it."""
     coordinator = build(text=TWO_BOTTLES)
     update(coordinator)
-    assert coordinator._hass.executor_jobs == ["_parse_and_process"], (
+    assert coordinator.hass.executor_jobs == ["_parse_and_process"], (
         "parsing must be the only executor job, and must still be one"
     )
 
@@ -74,13 +74,13 @@ def test_parsing_still_runs_in_the_executor():
 def test_the_request_targets_the_library_endpoint():
     coordinator = build(text=TWO_BOTTLES)
     update(coordinator)
-    assert coordinator._hass.session.requests[0]["url"] == BASE_URL
+    assert coordinator.hass.session.requests[0]["url"] == BASE_URL
 
 
 def test_the_request_carries_the_expected_query():
     coordinator = build(text=TWO_BOTTLES)
     update(coordinator)
-    params = coordinator._hass.session.requests[0]["params"]
+    params = coordinator.hass.session.requests[0]["params"]
     assert params["User"] == "alice"
     assert params["Password"] == "s3cret"
     assert params["Table"] == "Inventory"

tests/test_coordinator_config_entry.py

@@ -39,3 +39,31 @@ def test_the_entry_is_not_smuggled_through_kwargs():
     """It has to be the named parameter, not an extra the base class ignores."""
     coordinator, _ = build()
     assert "config_entry" not in coordinator.init_kwargs
+
+
+# --------------------------------------------------------------------------
+# P3-2: no private copy of the base class's hass
+# --------------------------------------------------------------------------
+def test_the_base_class_hass_is_the_one_it_was_given():
+    """DataUpdateCoordinator.__init__ assigns it; nothing else needs to."""
+    hass = FakeHass()
+    hass.session = FakeSession()
+    coordinator = WineCellarData(hass, ConfigEntry(entry_id="e", data=ENTRY_DATA))
+
+    assert coordinator.hass is hass
+
+
+def test_it_keeps_no_private_copy_of_hass():
+    """`_hass` shadowed `self.hass`, which the base class already provides.
+
+    Two names for one object, and the underscore claimed an ownership this
+    code does not have: `hass` belongs to Home Assistant, which exposes it
+    publicly and reads it in its own methods. Harmless while `hass` stays a
+    plain attribute - if it ever became a property, the private copy would
+    quietly bypass whatever that property did.
+    """
+    coordinator, _ = build()
+
+    assert not hasattr(coordinator, "_hass"), (
+        "self._hass duplicates the base class's self.hass; use that instead"
+    )

tests/test_inventory_performance.py

@@ -120,7 +120,7 @@ def test_totals_are_unaffected_by_the_refactor():
 def test_parsing_is_handed_to_the_executor():
     coordinator = build_coordinator(returns=identical_bottles(10))
     asyncio.run(coordinator._async_update_data())
-    assert "_parse_and_process" in coordinator._hass.executor_jobs, (
+    assert "_parse_and_process" in coordinator.hass.executor_jobs, (
         "parsing ran on the event loop"
     )
 

tests/test_parse_error_taxonomy.py

@@ -70,7 +70,7 @@ def test_a_deliberate_update_failure_is_not_rewrapped():
     assert coordinator.data["total_bottles"] == 1
 
     # An empty response after the cellar held stock: rejected on the first poll.
-    coordinator._hass.session = FakeSession(text=HEADER)
+    coordinator.hass.session = FakeSession(text=HEADER)
     with pytest.raises(UpdateFailed) as caught:
         asyncio.run(coordinator._async_update_data())
 

tests/test_rate_limit.py

@@ -94,7 +94,7 @@ def test_the_interval_is_restored_after_a_successful_poll():
     refresh(coordinator)
     assert coordinator.update_interval == timedelta(seconds=1800)
 
-    coordinator._hass.session = FakeSession(text=GOOD)
+    coordinator.hass.session = FakeSession(text=GOOD)
     asyncio.run(coordinator._async_update_data())
 
     assert coordinator.update_interval == timedelta(seconds=SCAN_INTERVAL)