Apply recommendations from roku code review (#32883)

* avoid patching integration methods

* Update config_flow.py

* apply recommendations from roku code review

* Update config_flow.py
This commit is contained in:
Chris Talkington 2020-03-17 15:15:41 -05:00 committed by GitHub
parent e97d21aec0
commit 9146f76b01
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 39 additions and 23 deletions

View File

@ -1,11 +1,11 @@
{ {
"config": { "config": {
"abort": { "abort": {
"already_configured": "Roku device is already configured" "already_configured": "Roku device is already configured",
"unknown": "Unexpected error"
}, },
"error": { "error": {
"cannot_connect": "Failed to connect, please try again", "cannot_connect": "Failed to connect, please try again"
"unknown": "Unexpected error"
}, },
"flow_title": "Roku: {name}", "flow_title": "Roku: {name}",
"step": { "step": {

View File

@ -34,8 +34,13 @@ def validate_input(data: Dict) -> Dict:
Data has the keys from DATA_SCHEMA with values provided by the user. Data has the keys from DATA_SCHEMA with values provided by the user.
""" """
roku = Roku(data["host"]) try:
device_info = roku.device_info roku = Roku(data["host"])
device_info = roku.device_info
except (SocketGIAError, RequestException, RokuException) as exception:
raise CannotConnect from exception
except Exception as exception: # pylint: disable=broad-except
raise UnknownError from exception
return { return {
"title": data["host"], "title": data["host"],
@ -74,11 +79,11 @@ class RokuConfigFlow(ConfigFlow, domain=DOMAIN):
try: try:
info = await self.hass.async_add_executor_job(validate_input, user_input) info = await self.hass.async_add_executor_job(validate_input, user_input)
except (SocketGIAError, RequestException, RokuException): except CannotConnect:
errors["base"] = ERROR_CANNOT_CONNECT errors["base"] = ERROR_CANNOT_CONNECT
return self._show_form(errors) return self._show_form(errors)
except Exception: # pylint: disable=broad-except except UnknownError:
_LOGGER.exception("Unknown error trying to connect.") _LOGGER.exception("Unknown error trying to connect")
return self.async_abort(reason=ERROR_UNKNOWN) return self.async_abort(reason=ERROR_UNKNOWN)
await self.async_set_unique_id(info["serial_num"]) await self.async_set_unique_id(info["serial_num"])
@ -119,10 +124,10 @@ class RokuConfigFlow(ConfigFlow, domain=DOMAIN):
try: try:
await self.hass.async_add_executor_job(validate_input, user_input) await self.hass.async_add_executor_job(validate_input, user_input)
return self.async_create_entry(title=name, data=user_input) return self.async_create_entry(title=name, data=user_input)
except (SocketGIAError, RequestException, RokuException): except CannotConnect:
return self.async_abort(reason=ERROR_CANNOT_CONNECT) return self.async_abort(reason=ERROR_CANNOT_CONNECT)
except Exception: # pylint: disable=broad-except except UnknownError:
_LOGGER.exception("Unknown error trying to connect.") _LOGGER.exception("Unknown error trying to connect")
return self.async_abort(reason=ERROR_UNKNOWN) return self.async_abort(reason=ERROR_UNKNOWN)
return self.async_show_form( return self.async_show_form(
@ -132,3 +137,7 @@ class RokuConfigFlow(ConfigFlow, domain=DOMAIN):
class CannotConnect(HomeAssistantError): class CannotConnect(HomeAssistantError):
"""Error to indicate we cannot connect.""" """Error to indicate we cannot connect."""
class UnknownError(HomeAssistantError):
"""Error to indicate we encountered an unknown error."""

View File

@ -17,11 +17,11 @@
} }
}, },
"error": { "error": {
"cannot_connect": "Failed to connect, please try again", "cannot_connect": "Failed to connect, please try again"
"unknown": "Unexpected error"
}, },
"abort": { "abort": {
"already_configured": "Roku device is already configured" "already_configured": "Roku device is already configured",
"unknown": "Unexpected error"
} }
} }
} }

View File

@ -124,10 +124,12 @@ async def test_form_cannot_connect(hass: HomeAssistantType) -> None:
) )
with patch( with patch(
"homeassistant.components.roku.config_flow.validate_input", "homeassistant.components.roku.config_flow.Roku._call",
side_effect=RokuException, side_effect=RokuException,
) as mock_validate_input: ) as mock_validate_input:
result = await async_configure_flow(hass, result["flow_id"], {CONF_HOST: HOST},) result = await hass.config_entries.flow.async_configure(
flow_id=result["flow_id"], user_input={CONF_HOST: HOST}
)
assert result["type"] == RESULT_TYPE_FORM assert result["type"] == RESULT_TYPE_FORM
assert result["errors"] == {"base": "cannot_connect"} assert result["errors"] == {"base": "cannot_connect"}
@ -143,10 +145,12 @@ async def test_form_cannot_connect_request(hass: HomeAssistantType) -> None:
) )
with patch( with patch(
"homeassistant.components.roku.config_flow.validate_input", "homeassistant.components.roku.config_flow.Roku._call",
side_effect=RequestException, side_effect=RequestException,
) as mock_validate_input: ) as mock_validate_input:
result = await async_configure_flow(hass, result["flow_id"], {CONF_HOST: HOST},) result = await hass.config_entries.flow.async_configure(
flow_id=result["flow_id"], user_input={CONF_HOST: HOST}
)
assert result["type"] == RESULT_TYPE_FORM assert result["type"] == RESULT_TYPE_FORM
assert result["errors"] == {"base": "cannot_connect"} assert result["errors"] == {"base": "cannot_connect"}
@ -162,10 +166,12 @@ async def test_form_cannot_connect_socket(hass: HomeAssistantType) -> None:
) )
with patch( with patch(
"homeassistant.components.roku.config_flow.validate_input", "homeassistant.components.roku.config_flow.Roku._call",
side_effect=SocketGIAError, side_effect=SocketGIAError,
) as mock_validate_input: ) as mock_validate_input:
result = await async_configure_flow(hass, result["flow_id"], {CONF_HOST: HOST},) result = await hass.config_entries.flow.async_configure(
flow_id=result["flow_id"], user_input={CONF_HOST: HOST}
)
assert result["type"] == RESULT_TYPE_FORM assert result["type"] == RESULT_TYPE_FORM
assert result["errors"] == {"base": "cannot_connect"} assert result["errors"] == {"base": "cannot_connect"}
@ -181,10 +187,11 @@ async def test_form_unknown_error(hass: HomeAssistantType) -> None:
) )
with patch( with patch(
"homeassistant.components.roku.config_flow.validate_input", "homeassistant.components.roku.config_flow.Roku._call", side_effect=Exception,
side_effect=Exception,
) as mock_validate_input: ) as mock_validate_input:
result = await async_configure_flow(hass, result["flow_id"], {CONF_HOST: HOST},) result = await hass.config_entries.flow.async_configure(
flow_id=result["flow_id"], user_input={CONF_HOST: HOST}
)
assert result["type"] == RESULT_TYPE_ABORT assert result["type"] == RESULT_TYPE_ABORT
assert result["reason"] == "unknown" assert result["reason"] == "unknown"