Skip to content

Use SsdpServiceInfo for ssdp tests (part 3) #60334

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

Merged
bdraco merged 15 commits into from
Nov 25, 2021
Merged

Use SsdpServiceInfo for ssdp tests (part 3) #60334

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
9e4cdf2
Add get method to SsdpServiceInfo
epenet Nov 25, 2021
ae79fde
Update deconz
epenet Nov 25, 2021
19f8526
Update directv
epenet Nov 25, 2021
2993c55
Update dlna_dmr
epenet Nov 25, 2021
219568b
Update fritz
epenet Nov 25, 2021
ec7c4d6
Update fritzbox
epenet Nov 25, 2021
80bc55d
Update huawei_lte
epenet Nov 25, 2021
03323a3
Update hyperion
epenet Nov 25, 2021
4b78934
Update keenetic_ndms2
epenet Nov 25, 2021
44e3871
Update netgear
epenet Nov 25, 2021
bdbf510
Update samsungtv
epenet Nov 25, 2021
9157dc0
Update syncthru
epenet Nov 25, 2021
f20b164
Update yamaha_musiccast
epenet Nov 25, 2021
24de5ff
Update zha
epenet Nov 25, 2021
8c2523b
Location should be optional
epenet Nov 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions homeassistant/components/ssdp/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,24 @@ def __getitem__(self, name: str) -> Any:
return getattr(self, name)
return self.upnp.get(name)

def get(self, name: str, default: Any = None) -> Any:
Copy link
Copy Markdown
Contributor Author

@epenet epenet Nov 25, 2021

Choose a reason for hiding this comment

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

@MartinHjelmare / @bdraco / @frenck
Should I add this also to the other xxxServiceInfo?
When I migrated the others using a TypedDict in the first pass, and then using a dataclass in the second pass it didn't seem required.
Now with hindsight I think it might cause issues with custom components (ie. breaking change before 2022.6)

Copy link
Copy Markdown
Member

@bdraco bdraco Nov 25, 2021

Choose a reason for hiding this comment

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

Where is the code that actually needs this?

Copy link
Copy Markdown
Contributor Author

@epenet epenet Nov 25, 2021

Choose a reason for hiding this comment

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

grep "discovery_info.get(" homeassistant/components/**/**.py

As an example:

core/homeassistant/components/deconz/config_flow.py

Lines 194 to 197 in 2468724

if (
discovery_info.get(ssdp.ATTR_UPNP_MANUFACTURER_URL)
!= DECONZ_MANUFACTURERURL
):

Copy link
Copy Markdown
Contributor Author

@epenet epenet Nov 25, 2021

Choose a reason for hiding this comment

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

For zeroconf (and dhcp/usb/mqtt) they were fixed as I bumped into them.
I don't have example but I assume there will be some similar code in custom components.

Copy link
Copy Markdown
Member

@bdraco bdraco Nov 25, 2021

Choose a reason for hiding this comment

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

As much as I hate to have more temporary code, I think we should implement it in all places since someone will be doing it that way.

Copy link
Copy Markdown
Contributor Author

@epenet epenet Nov 25, 2021

Choose a reason for hiding this comment

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

I'll open a separate PR for the other discovery components.
This probably also applies for the __iter__ implementation in #60339

"""
Allow property access by name for compatibility reason.

Deprecated, and will be removed in version 2022.6.
"""
if not self._warning_logged:
report(
f"accessed discovery_info.get('{name}') instead of discovery_info.{name}; this will fail in version 2022.6",
exclude_integrations={"ssdp"},
error_if_core=False,
level=logging.DEBUG,
)
self._warning_logged = True
if hasattr(self, name):
return getattr(self, name)
return self.upnp.get(name, default)


@bind_hass
async def async_register_callback(
Expand Down
69 changes: 43 additions & 26 deletions tests/components/deconz/test_config_flow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import pydeconz

from homeassistant.components import ssdp
from homeassistant.components.deconz.config_flow import (
CONF_MANUAL_INPUT,
CONF_SERIAL,
Expand All @@ -17,11 +18,7 @@
CONF_MASTER_GATEWAY,
DOMAIN as DECONZ_DOMAIN,
)
from homeassistant.components.ssdp import (
ATTR_SSDP_LOCATION,
ATTR_UPNP_MANUFACTURER_URL,
ATTR_UPNP_SERIAL,
)
from homeassistant.components.ssdp import ATTR_UPNP_MANUFACTURER_URL, ATTR_UPNP_SERIAL
from homeassistant.config_entries import (
SOURCE_HASSIO,
SOURCE_REAUTH,
Expand Down Expand Up @@ -412,11 +409,15 @@ async def test_flow_ssdp_discovery(hass, aioclient_mock):
"""Test that config flow for one discovered bridge works."""
result = await hass.config_entries.flow.async_init(
DECONZ_DOMAIN,
data={
ATTR_SSDP_LOCATION: "http://1.2.3.4:80/",
ATTR_UPNP_MANUFACTURER_URL: DECONZ_MANUFACTURERURL,
ATTR_UPNP_SERIAL: BRIDGEID,
},
data=ssdp.SsdpServiceInfo(
ssdp_usn="mock_usn",
ssdp_st="mock_st",
ssdp_location="http://1.2.3.4:80/",
upnp={
ATTR_UPNP_MANUFACTURER_URL: DECONZ_MANUFACTURERURL,
ATTR_UPNP_SERIAL: BRIDGEID,
},
),
context={"source": SOURCE_SSDP},
)

Expand Down Expand Up @@ -446,7 +447,11 @@ async def test_flow_ssdp_bad_discovery(hass, aioclient_mock):
"""Test that SSDP discovery aborts if manufacturer URL is wrong."""
result = await hass.config_entries.flow.async_init(
DECONZ_DOMAIN,
data={ATTR_UPNP_MANUFACTURER_URL: "other"},
data=ssdp.SsdpServiceInfo(
ssdp_usn="mock_usn",
ssdp_st="mock_st",
upnp={ATTR_UPNP_MANUFACTURER_URL: "other"},
),
context={"source": SOURCE_SSDP},
)

Expand All @@ -464,11 +469,15 @@ async def test_ssdp_discovery_update_configuration(hass, aioclient_mock):
) as mock_setup_entry:
result = await hass.config_entries.flow.async_init(
DECONZ_DOMAIN,
data={
ATTR_SSDP_LOCATION: "http://2.3.4.5:80/",
ATTR_UPNP_MANUFACTURER_URL: DECONZ_MANUFACTURERURL,
ATTR_UPNP_SERIAL: BRIDGEID,
},
data=ssdp.SsdpServiceInfo(
ssdp_usn="mock_usn",
ssdp_st="mock_st",
ssdp_location="http://2.3.4.5:80/",
upnp={
ATTR_UPNP_MANUFACTURER_URL: DECONZ_MANUFACTURERURL,
ATTR_UPNP_SERIAL: BRIDGEID,
},
),
context={"source": SOURCE_SSDP},
)
await hass.async_block_till_done()
Expand All @@ -485,11 +494,15 @@ async def test_ssdp_discovery_dont_update_configuration(hass, aioclient_mock):

result = await hass.config_entries.flow.async_init(
DECONZ_DOMAIN,
data={
ATTR_SSDP_LOCATION: "http://1.2.3.4:80/",
ATTR_UPNP_MANUFACTURER_URL: DECONZ_MANUFACTURERURL,
ATTR_UPNP_SERIAL: BRIDGEID,
},
data=ssdp.SsdpServiceInfo(
ssdp_usn="mock_usn",
ssdp_st="mock_st",
ssdp_location="http://1.2.3.4:80/",
upnp={
ATTR_UPNP_MANUFACTURER_URL: DECONZ_MANUFACTURERURL,
ATTR_UPNP_SERIAL: BRIDGEID,
},
),
context={"source": SOURCE_SSDP},
)

Expand All @@ -508,11 +521,15 @@ async def test_ssdp_discovery_dont_update_existing_hassio_configuration(

result = await hass.config_entries.flow.async_init(
DECONZ_DOMAIN,
data={
ATTR_SSDP_LOCATION: "http://1.2.3.4:80/",
ATTR_UPNP_MANUFACTURER_URL: DECONZ_MANUFACTURERURL,
ATTR_UPNP_SERIAL: BRIDGEID,
},
data=ssdp.SsdpServiceInfo(
ssdp_usn="mock_usn",
ssdp_st="mock_st",
ssdp_location="http://1.2.3.4:80/",
upnp={
ATTR_UPNP_MANUFACTURER_URL: DECONZ_MANUFACTURERURL,
ATTR_UPNP_SERIAL: BRIDGEID,
},
),
context={"source": SOURCE_SSDP},
)

Expand Down
9 changes: 7 additions & 2 deletions tests/components/directv/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
"""Tests for the DirecTV component."""
from http import HTTPStatus

from homeassistant.components import ssdp
from homeassistant.components.directv.const import CONF_RECEIVER_ID, DOMAIN
from homeassistant.components.ssdp import ATTR_SSDP_LOCATION
from homeassistant.const import CONF_HOST, CONTENT_TYPE_JSON
from homeassistant.core import HomeAssistant

Expand All @@ -15,7 +15,12 @@
UPNP_SERIAL = "RID-028877455858"

MOCK_CONFIG = {DOMAIN: [{CONF_HOST: HOST}]}
MOCK_SSDP_DISCOVERY_INFO = {ATTR_SSDP_LOCATION: SSDP_LOCATION}
MOCK_SSDP_DISCOVERY_INFO = ssdp.SsdpServiceInfo(
ssdp_usn="mock_usn",
ssdp_st="mock_st",
ssdp_location=SSDP_LOCATION,
upnp={},
)
MOCK_USER_INPUT = {CONF_HOST: HOST}


Expand Down
19 changes: 10 additions & 9 deletions tests/components/directv/test_config_flow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
"""Test the DirecTV config flow."""
import dataclasses
from unittest.mock import patch

from aiohttp import ClientError as HTTPClientError
Expand Down Expand Up @@ -43,7 +44,7 @@ async def test_show_ssdp_form(
"""Test that the ssdp confirmation form is served."""
mock_connection(aioclient_mock)

discovery_info = MOCK_SSDP_DISCOVERY_INFO.copy()
discovery_info = dataclasses.replace(MOCK_SSDP_DISCOVERY_INFO)
result = await hass.config_entries.flow.async_init(
DOMAIN, context={CONF_SOURCE: SOURCE_SSDP}, data=discovery_info
)
Expand Down Expand Up @@ -77,7 +78,7 @@ async def test_ssdp_cannot_connect(
"""Test we abort SSDP flow on connection error."""
aioclient_mock.get("http://127.0.0.1:8080/info/getVersion", exc=HTTPClientError)

discovery_info = MOCK_SSDP_DISCOVERY_INFO.copy()
discovery_info = dataclasses.replace(MOCK_SSDP_DISCOVERY_INFO)
result = await hass.config_entries.flow.async_init(
DOMAIN,
context={CONF_SOURCE: SOURCE_SSDP},
Expand All @@ -94,7 +95,7 @@ async def test_ssdp_confirm_cannot_connect(
"""Test we abort SSDP flow on connection error."""
aioclient_mock.get("http://127.0.0.1:8080/info/getVersion", exc=HTTPClientError)

discovery_info = MOCK_SSDP_DISCOVERY_INFO.copy()
discovery_info = dataclasses.replace(MOCK_SSDP_DISCOVERY_INFO)
result = await hass.config_entries.flow.async_init(
DOMAIN,
context={CONF_SOURCE: SOURCE_SSDP, CONF_HOST: HOST, CONF_NAME: HOST},
Expand Down Expand Up @@ -128,7 +129,7 @@ async def test_ssdp_device_exists_abort(
"""Test we abort SSDP flow if DirecTV receiver already configured."""
await setup_integration(hass, aioclient_mock, skip_entry_setup=True)

discovery_info = MOCK_SSDP_DISCOVERY_INFO.copy()
discovery_info = dataclasses.replace(MOCK_SSDP_DISCOVERY_INFO)
result = await hass.config_entries.flow.async_init(
DOMAIN,
context={CONF_SOURCE: SOURCE_SSDP},
Expand All @@ -145,8 +146,8 @@ async def test_ssdp_with_receiver_id_device_exists_abort(
"""Test we abort SSDP flow if DirecTV receiver already configured."""
await setup_integration(hass, aioclient_mock, skip_entry_setup=True)

discovery_info = MOCK_SSDP_DISCOVERY_INFO.copy()
discovery_info[ATTR_UPNP_SERIAL] = UPNP_SERIAL
discovery_info = dataclasses.replace(MOCK_SSDP_DISCOVERY_INFO)
discovery_info.upnp[ATTR_UPNP_SERIAL] = UPNP_SERIAL
result = await hass.config_entries.flow.async_init(
DOMAIN,
context={CONF_SOURCE: SOURCE_SSDP},
Expand Down Expand Up @@ -180,7 +181,7 @@ async def test_ssdp_unknown_error(
hass: HomeAssistant, aioclient_mock: AiohttpClientMocker
) -> None:
"""Test we abort SSDP flow on unknown error."""
discovery_info = MOCK_SSDP_DISCOVERY_INFO.copy()
discovery_info = dataclasses.replace(MOCK_SSDP_DISCOVERY_INFO)
with patch(
"homeassistant.components.directv.config_flow.DIRECTV.update",
side_effect=Exception,
Expand All @@ -199,7 +200,7 @@ async def test_ssdp_confirm_unknown_error(
hass: HomeAssistant, aioclient_mock: AiohttpClientMocker
) -> None:
"""Test we abort SSDP flow on unknown error."""
discovery_info = MOCK_SSDP_DISCOVERY_INFO.copy()
discovery_info = dataclasses.replace(MOCK_SSDP_DISCOVERY_INFO)
with patch(
"homeassistant.components.directv.config_flow.DIRECTV.update",
side_effect=Exception,
Expand Down Expand Up @@ -249,7 +250,7 @@ async def test_full_ssdp_flow_implementation(
"""Test the full SSDP flow from start to finish."""
mock_connection(aioclient_mock)

discovery_info = MOCK_SSDP_DISCOVERY_INFO.copy()
discovery_info = dataclasses.replace(MOCK_SSDP_DISCOVERY_INFO)
result = await hass.config_entries.flow.async_init(
DOMAIN, context={CONF_SOURCE: SOURCE_SSDP}, data=discovery_info
)
Expand Down
Loading