Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix some handle leaks in rainforest_raven #113035

Merged
merged 1 commit into from Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 20 additions & 10 deletions homeassistant/components/rainforest_raven/coordinator.py
Expand Up @@ -133,32 +133,42 @@ def device_info(self) -> DeviceInfo | None:
)
return None

async def async_shutdown(self) -> None:
"""Shutdown the coordinator."""
await self._cleanup_device()
await super().async_shutdown()

async def _async_update_data(self) -> dict[str, Any]:
try:
device = await self._get_device()
async with asyncio.timeout(5):
return await _get_all_data(device, self.entry.data[CONF_MAC])
except RAVEnConnectionError as err:
if self._raven_device:
await self._raven_device.close()
self._raven_device = None
await self._cleanup_device()
raise UpdateFailed(f"RAVEnConnectionError: {err}") from err
except TimeoutError:
await self._cleanup_device()
raise

async def _cleanup_device(self) -> None:
device, self._raven_device = self._raven_device, None
if device is not None:
await device.close()

async def _get_device(self) -> RAVEnSerialDevice:
if self._raven_device is not None:
return self._raven_device

device = RAVEnSerialDevice(self.entry.data[CONF_DEVICE])

async with asyncio.timeout(5):
await device.open()

try:
try:
async with asyncio.timeout(5):
await device.open()
await device.synchronize()
self._device_info = await device.get_device_info()
except Exception:
await device.close()
raise
except:
await device.close()
raise

self._raven_device = device
return device
16 changes: 16 additions & 0 deletions tests/components/rainforest_raven/test_coordinator.py
@@ -1,5 +1,8 @@
"""Tests for the Rainforest RAVEn data coordinator."""

import asyncio
import functools

from aioraven.device import RAVEnConnectionError
import pytest

Expand Down Expand Up @@ -84,6 +87,19 @@ async def test_coordinator_device_error_update(hass: HomeAssistant, mock_device)
assert coordinator.last_update_success is False


async def test_coordinator_device_timeout_update(hass: HomeAssistant, mock_device):
"""Test handling of a device timeout during an update."""
entry = create_mock_entry()
coordinator = RAVEnDataCoordinator(hass, entry)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set up the integration instead. We shouldn't interact with the coordinator directly as that's an integration detail.

https://developers.home-assistant.io/docs/development_testing#writing-tests-for-integrations


await coordinator.async_config_entry_first_refresh()
assert coordinator.last_update_success is True

mock_device.get_network_info.side_effect = functools.partial(asyncio.sleep, 10)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides this, I'd make the timeout time a constant so we can easily patch it to 0. We don't want slow tests.

await coordinator.async_refresh()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move time forward instead to trigger a refresh. See other integrations that use a coordinator with tests for examples.

assert coordinator.last_update_success is False
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should assert that the mock device is closed. We can also assert that a coordinator entity have its state set to unavailable.



async def test_coordinator_comm_error(hass: HomeAssistant, mock_device):
"""Test handling of an error parsing or reading raw device data."""
entry = create_mock_entry()
Expand Down