Avoid creating sockets in homekit port available tests (#55668)

* Avoid creating sockets in homekit port available tests

* prevent new bridge from being setup -- its too fast now that the executor job is gone and it revealed an unpatched setup
This commit is contained in:
J. Nick Koston 2021-09-03 17:15:28 -10:00 committed by GitHub
parent 0dc8fb497b
commit 195ee2a188
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 105 additions and 34 deletions

View file

@ -103,9 +103,9 @@ from .const import (
from .type_triggers import DeviceTriggerAccessory from .type_triggers import DeviceTriggerAccessory
from .util import ( from .util import (
accessory_friendly_name, accessory_friendly_name,
async_port_is_available,
dismiss_setup_message, dismiss_setup_message,
get_persist_fullpath_for_entry_id, get_persist_fullpath_for_entry_id,
port_is_available,
remove_state_files_for_entry_id, remove_state_files_for_entry_id,
show_setup_message, show_setup_message,
state_needs_accessory_mode, state_needs_accessory_mode,
@ -330,7 +330,7 @@ async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
logged_shutdown_wait = False logged_shutdown_wait = False
for _ in range(0, SHUTDOWN_TIMEOUT): for _ in range(0, SHUTDOWN_TIMEOUT):
if await hass.async_add_executor_job(port_is_available, entry.data[CONF_PORT]): if async_port_is_available(entry.data[CONF_PORT]):
break break
if not logged_shutdown_wait: if not logged_shutdown_wait:

View file

@ -172,9 +172,7 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
async def async_step_pairing(self, user_input=None): async def async_step_pairing(self, user_input=None):
"""Pairing instructions.""" """Pairing instructions."""
if user_input is not None: if user_input is not None:
port = await async_find_next_available_port( port = async_find_next_available_port(self.hass, DEFAULT_CONFIG_FLOW_PORT)
self.hass, DEFAULT_CONFIG_FLOW_PORT
)
await self._async_add_entries_for_accessory_mode_entities(port) await self._async_add_entries_for_accessory_mode_entities(port)
self.hk_data[CONF_PORT] = port self.hk_data[CONF_PORT] = port
include_domains_filter = self.hk_data[CONF_FILTER][CONF_INCLUDE_DOMAINS] include_domains_filter = self.hk_data[CONF_FILTER][CONF_INCLUDE_DOMAINS]
@ -205,7 +203,7 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
for entity_id in accessory_mode_entity_ids: for entity_id in accessory_mode_entity_ids:
if entity_id in exiting_entity_ids_accessory_mode: if entity_id in exiting_entity_ids_accessory_mode:
continue continue
port = await async_find_next_available_port(self.hass, next_port_to_check) port = async_find_next_available_port(self.hass, next_port_to_check)
next_port_to_check = port + 1 next_port_to_check = port + 1
self.hass.async_create_task( self.hass.async_create_task(
self.hass.config_entries.flow.async_init( self.hass.config_entries.flow.async_init(

View file

@ -27,7 +27,7 @@ from homeassistant.const import (
CONF_TYPE, CONF_TYPE,
TEMP_CELSIUS, TEMP_CELSIUS,
) )
from homeassistant.core import HomeAssistant, split_entity_id from homeassistant.core import HomeAssistant, callback, split_entity_id
import homeassistant.helpers.config_validation as cv import homeassistant.helpers.config_validation as cv
from homeassistant.helpers.storage import STORAGE_DIR from homeassistant.helpers.storage import STORAGE_DIR
import homeassistant.util.temperature as temp_util import homeassistant.util.temperature as temp_util
@ -433,34 +433,32 @@ def _get_test_socket():
return test_socket return test_socket
def port_is_available(port: int) -> bool: @callback
def async_port_is_available(port: int) -> bool:
"""Check to see if a port is available.""" """Check to see if a port is available."""
test_socket = _get_test_socket()
try: try:
test_socket.bind(("", port)) _get_test_socket().bind(("", port))
except OSError: except OSError:
return False return False
return True return True
async def async_find_next_available_port(hass: HomeAssistant, start_port: int) -> int: @callback
def async_find_next_available_port(hass: HomeAssistant, start_port: int) -> int:
"""Find the next available port not assigned to a config entry.""" """Find the next available port not assigned to a config entry."""
exclude_ports = { exclude_ports = {
entry.data[CONF_PORT] entry.data[CONF_PORT]
for entry in hass.config_entries.async_entries(DOMAIN) for entry in hass.config_entries.async_entries(DOMAIN)
if CONF_PORT in entry.data if CONF_PORT in entry.data
} }
return _async_find_next_available_port(start_port, exclude_ports)
return await hass.async_add_executor_job(
_find_next_available_port, start_port, exclude_ports
)
def _find_next_available_port(start_port: int, exclude_ports: set) -> int: @callback
def _async_find_next_available_port(start_port: int, exclude_ports: set) -> int:
"""Find the next available port starting with the given port.""" """Find the next available port starting with the given port."""
test_socket = _get_test_socket() test_socket = _get_test_socket()
for port in range(start_port, MAX_PORT): for port in range(start_port, MAX_PORT + 1):
if port in exclude_ports: if port in exclude_ports:
continue continue
try: try:

View file

@ -951,10 +951,15 @@ async def test_converting_bridge_to_accessory_mode(hass, hk_driver, mock_get_sou
assert result2["type"] == data_entry_flow.RESULT_TYPE_FORM assert result2["type"] == data_entry_flow.RESULT_TYPE_FORM
assert result2["step_id"] == "cameras" assert result2["step_id"] == "cameras"
with patch(
"homeassistant.components.homekit.async_setup_entry",
return_value=True,
) as mock_setup_entry:
result3 = await hass.config_entries.options.async_configure( result3 = await hass.config_entries.options.async_configure(
result2["flow_id"], result2["flow_id"],
user_input={"camera_copy": ["camera.tv"]}, user_input={"camera_copy": ["camera.tv"]},
) )
await hass.async_block_till_done()
assert result3["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY assert result3["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY
assert config_entry.options == { assert config_entry.options == {
@ -968,6 +973,7 @@ async def test_converting_bridge_to_accessory_mode(hass, hk_driver, mock_get_sou
"include_entities": ["camera.tv"], "include_entities": ["camera.tv"],
}, },
} }
assert len(mock_setup_entry.mock_calls) == 1
def _get_schema_default(schema, key_name): def _get_schema_default(schema, key_name):

View file

@ -1308,7 +1308,7 @@ async def test_homekit_uses_system_zeroconf(hass, hk_driver, mock_zeroconf):
with patch("pyhap.accessory_driver.AccessoryDriver.async_start"), patch( with patch("pyhap.accessory_driver.AccessoryDriver.async_start"), patch(
f"{PATH_HOMEKIT}.HomeKit.async_stop" f"{PATH_HOMEKIT}.HomeKit.async_stop"
), patch(f"{PATH_HOMEKIT}.port_is_available"): ), patch(f"{PATH_HOMEKIT}.async_port_is_available"):
entry.add_to_hass(hass) entry.add_to_hass(hass)
assert await hass.config_entries.async_setup(entry.entry_id) assert await hass.config_entries.async_setup(entry.entry_id)
await hass.async_block_till_done() await hass.async_block_till_done()
@ -1701,7 +1701,7 @@ async def test_wait_for_port_to_free(hass, hk_driver, mock_zeroconf, caplog):
with patch("pyhap.accessory_driver.AccessoryDriver.async_start"), patch( with patch("pyhap.accessory_driver.AccessoryDriver.async_start"), patch(
f"{PATH_HOMEKIT}.HomeKit.async_stop" f"{PATH_HOMEKIT}.HomeKit.async_stop"
), patch(f"{PATH_HOMEKIT}.port_is_available", return_value=True) as port_mock: ), patch(f"{PATH_HOMEKIT}.async_port_is_available", return_value=True) as port_mock:
assert await hass.config_entries.async_setup(entry.entry_id) assert await hass.config_entries.async_setup(entry.entry_id)
await hass.async_block_till_done() await hass.async_block_till_done()
assert await hass.config_entries.async_unload(entry.entry_id) assert await hass.config_entries.async_unload(entry.entry_id)
@ -1712,7 +1712,7 @@ async def test_wait_for_port_to_free(hass, hk_driver, mock_zeroconf, caplog):
with patch("pyhap.accessory_driver.AccessoryDriver.async_start"), patch( with patch("pyhap.accessory_driver.AccessoryDriver.async_start"), patch(
f"{PATH_HOMEKIT}.HomeKit.async_stop" f"{PATH_HOMEKIT}.HomeKit.async_stop"
), patch.object(homekit_base, "PORT_CLEANUP_CHECK_INTERVAL_SECS", 0), patch( ), patch.object(homekit_base, "PORT_CLEANUP_CHECK_INTERVAL_SECS", 0), patch(
f"{PATH_HOMEKIT}.port_is_available", return_value=False f"{PATH_HOMEKIT}.async_port_is_available", return_value=False
) as port_mock: ) as port_mock:
assert await hass.config_entries.async_setup(entry.entry_id) assert await hass.config_entries.async_setup(entry.entry_id)
await hass.async_block_till_done() await hass.async_block_till_done()

View file

@ -1,5 +1,5 @@
"""Test HomeKit util module.""" """Test HomeKit util module."""
from unittest.mock import Mock from unittest.mock import MagicMock, Mock, patch
import pytest import pytest
import voluptuous as vol import voluptuous as vol
@ -26,12 +26,12 @@ from homeassistant.components.homekit.const import (
from homeassistant.components.homekit.util import ( from homeassistant.components.homekit.util import (
accessory_friendly_name, accessory_friendly_name,
async_find_next_available_port, async_find_next_available_port,
async_port_is_available,
cleanup_name_for_homekit, cleanup_name_for_homekit,
convert_to_float, convert_to_float,
density_to_air_quality, density_to_air_quality,
dismiss_setup_message, dismiss_setup_message,
format_sw_version, format_sw_version,
port_is_available,
show_setup_message, show_setup_message,
state_needs_accessory_mode, state_needs_accessory_mode,
temperature_to_homekit, temperature_to_homekit,
@ -61,6 +61,25 @@ from .util import async_init_integration
from tests.common import MockConfigEntry, async_mock_service from tests.common import MockConfigEntry, async_mock_service
def _mock_socket(failure_attempts: int = 0) -> MagicMock:
"""Mock a socket that fails to bind failure_attempts amount of times."""
mock_socket = MagicMock()
attempts = 0
def _simulate_bind(*_):
import pprint
pprint.pprint("Calling bind")
nonlocal attempts
attempts += 1
if attempts <= failure_attempts:
raise OSError
return
mock_socket.bind = Mock(side_effect=_simulate_bind)
return mock_socket
def test_validate_entity_config(): def test_validate_entity_config():
"""Test validate entities.""" """Test validate entities."""
configs = [ configs = [
@ -257,11 +276,35 @@ async def test_dismiss_setup_msg(hass):
async def test_port_is_available(hass): async def test_port_is_available(hass):
"""Test we can get an available port and it is actually available.""" """Test we can get an available port and it is actually available."""
next_port = await async_find_next_available_port(hass, DEFAULT_CONFIG_FLOW_PORT) with patch(
"homeassistant.components.homekit.util.socket.socket",
return_value=_mock_socket(0),
):
next_port = async_find_next_available_port(hass, DEFAULT_CONFIG_FLOW_PORT)
assert next_port assert next_port
with patch(
"homeassistant.components.homekit.util.socket.socket",
return_value=_mock_socket(0),
):
assert async_port_is_available(next_port)
assert await hass.async_add_executor_job(port_is_available, next_port) with patch(
"homeassistant.components.homekit.util.socket.socket",
return_value=_mock_socket(5),
):
next_port = async_find_next_available_port(hass, DEFAULT_CONFIG_FLOW_PORT)
assert next_port == DEFAULT_CONFIG_FLOW_PORT + 5
with patch(
"homeassistant.components.homekit.util.socket.socket",
return_value=_mock_socket(0),
):
assert async_port_is_available(next_port)
with patch(
"homeassistant.components.homekit.util.socket.socket",
return_value=_mock_socket(1),
):
assert not async_port_is_available(next_port)
async def test_port_is_available_skips_existing_entries(hass): async def test_port_is_available_skips_existing_entries(hass):
@ -273,12 +316,38 @@ async def test_port_is_available_skips_existing_entries(hass):
) )
entry.add_to_hass(hass) entry.add_to_hass(hass)
next_port = await async_find_next_available_port(hass, DEFAULT_CONFIG_FLOW_PORT) with patch(
"homeassistant.components.homekit.util.socket.socket",
return_value=_mock_socket(),
):
next_port = async_find_next_available_port(hass, DEFAULT_CONFIG_FLOW_PORT)
assert next_port assert next_port == DEFAULT_CONFIG_FLOW_PORT + 1
assert next_port != DEFAULT_CONFIG_FLOW_PORT
assert await hass.async_add_executor_job(port_is_available, next_port) with patch(
"homeassistant.components.homekit.util.socket.socket",
return_value=_mock_socket(),
):
assert async_port_is_available(next_port)
with patch(
"homeassistant.components.homekit.util.socket.socket",
return_value=_mock_socket(4),
):
next_port = async_find_next_available_port(hass, DEFAULT_CONFIG_FLOW_PORT)
assert next_port == DEFAULT_CONFIG_FLOW_PORT + 5
with patch(
"homeassistant.components.homekit.util.socket.socket",
return_value=_mock_socket(),
):
assert async_port_is_available(next_port)
with pytest.raises(OSError), patch(
"homeassistant.components.homekit.util.socket.socket",
return_value=_mock_socket(10),
):
async_find_next_available_port(hass, 65530)
async def test_format_sw_version(): async def test_format_sw_version():