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.

Sign up for GitHub

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

Jump to bottom

Fix Reolink DHCP IP update #103654

Merged
merged 6 commits into from Nov 10, 2023
Merged

Fix Reolink DHCP IP update #103654

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9b26bce
fix DHCP
starkillerOG Nov 8, 2023
98ba29b
fix Mappingproxy to normal dict
starkillerOG Nov 8, 2023
d433133
Add tests
starkillerOG Nov 9, 2023
b5c443a
Simplify
starkillerOG Nov 9, 2023
3b91103
use call_args_list
starkillerOG Nov 10, 2023
964d66a
remove duplicate test
starkillerOG Nov 10, 2023
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
4 changes: 3 additions & 1 deletion homeassistant/components/reolink/config_flow.py
View file
Open in desktop
Expand Up @@ -113,7 +113,9 @@ async def async_step_dhcp(self, discovery_info: dhcp.DhcpServiceInfo) -> FlowRes
raise AbortFlow("already_configured")

# check if the camera is reachable at the new IP
host = ReolinkHost(self.hass, existing_entry.data, existing_entry.options)
new_config = dict(existing_entry.data)
Copy link
Member

Choose a reason for hiding this comment

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

It seems like this is missing test coverage if it can be changed without updating tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well there is a check that goes over this code, but since the test controlls if a ConnectionError occurs or not it does not mather if the IP is valid or not.
The IP that is eventually saved to the config is checked, but that is correct, see line 142:
self._abort_if_unique_id_configured(updates={CONF_HOST: discovery_info.ip})
Where the correct discovery_info.ip is used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just added some extra assertions (assert_called_with) in the tests so that it would now fail if this would be changed in the future.

new_config[CONF_HOST] = discovery_info.ip
host = ReolinkHost(self.hass, new_config, existing_entry.options)
try:
await host.api.get_state("GetLocalLink")
await host.api.logout()
Expand Down
16 changes: 13 additions & 3 deletions tests/components/reolink/conftest.py
View file
Open in desktop
Expand Up @@ -34,8 +34,10 @@ def mock_setup_entry() -> Generator[AsyncMock, None, None]:


@pytest.fixture
def reolink_connect(mock_get_source_ip: None) -> Generator[MagicMock, None, None]:
"""Mock reolink connection."""
def reolink_connect_class(
mock_get_source_ip: None,
) -> Generator[MagicMock, None, None]:
"""Mock reolink connection and return both the host_mock and host_mock_class."""
with patch(
"homeassistant.components.reolink.host.webhook.async_register",
return_value=True,
Expand Down Expand Up @@ -65,7 +67,15 @@ def reolink_connect(mock_get_source_ip: None) -> Generator[MagicMock, None, None
host_mock.session_active = True
host_mock.timeout = 60
host_mock.renewtimer.return_value = 600
yield host_mock
yield host_mock_class


@pytest.fixture
def reolink_connect(
reolink_connect_class: MagicMock,
) -> Generator[MagicMock, None, None]:
"""Mock reolink connection."""
return reolink_connect_class.return_value


@pytest.fixture
Expand Down
23 changes: 23 additions & 0 deletions tests/components/reolink/test_config_flow.py
View file
Open in desktop
Expand Up @@ -12,6 +12,7 @@
from homeassistant.components.reolink import DEVICE_UPDATE_INTERVAL, const
from homeassistant.components.reolink.config_flow import DEFAULT_PROTOCOL
from homeassistant.components.reolink.exceptions import ReolinkWebhookException
from homeassistant.components.reolink.host import DEFAULT_TIMEOUT
from homeassistant.config_entries import ConfigEntryState
from homeassistant.const import CONF_HOST, CONF_PASSWORD, CONF_PORT, CONF_USERNAME
from homeassistant.core import HomeAssistant
Expand Down Expand Up @@ -410,6 +411,7 @@ async def test_dhcp_flow(hass: HomeAssistant, mock_setup_entry: MagicMock) -> No
)
async def test_dhcp_ip_update(
hass: HomeAssistant,
reolink_connect_class: MagicMock,
reolink_connect: MagicMock,
last_update_success: bool,
attr: str,
Expand Down Expand Up @@ -438,6 +440,16 @@ async def test_dhcp_ip_update(
await hass.async_block_till_done()
assert config_entry.state == ConfigEntryState.LOADED

reolink_connect_class.assert_called_with(
TEST_HOST,
TEST_USERNAME,
TEST_PASSWORD,
port=TEST_PORT,
use_https=TEST_USE_HTTPS,
protocol=DEFAULT_PROTOCOL,
timeout=DEFAULT_TIMEOUT,
)

if not last_update_success:
# ensure the last_update_succes is False for the device_coordinator.
reolink_connect.get_states = AsyncMock(side_effect=ReolinkError("Test error"))
Expand All @@ -459,6 +471,17 @@ async def test_dhcp_ip_update(
const.DOMAIN, context={"source": config_entries.SOURCE_DHCP}, data=dhcp_data
)

if not last_update_success:
MartinHjelmare marked this conversation as resolved.
Show resolved Hide resolved
reolink_connect_class.assert_called_with(
TEST_HOST2,
TEST_USERNAME,
TEST_PASSWORD,
port=TEST_PORT,
use_https=TEST_USE_HTTPS,
protocol=DEFAULT_PROTOCOL,
timeout=DEFAULT_TIMEOUT,
)

assert result["type"] is data_entry_flow.FlowResultType.ABORT
assert result["reason"] == "already_configured"

Expand Down