From ab2de18f8f0ecf431c43a366305c55a49cb15247 Mon Sep 17 00:00:00 2001
From: Erik Montnemery <erik@montnemery.com>
Date: Tue, 3 Oct 2023 19:21:27 +0200
Subject: [PATCH] Refactor frame.get_integration_frame (#101322)
---
homeassistant/helpers/deprecation.py | 10 ++--
homeassistant/helpers/frame.py | 38 +++++++-----
tests/conftest.py | 27 ---------
tests/helpers/test_deprecation.py | 86 +++++++++++++++++++++++++++-
tests/helpers/test_frame.py | 62 +++++++++++++++-----
5 files changed, 163 insertions(+), 60 deletions(-)
diff --git a/homeassistant/helpers/deprecation.py b/homeassistant/helpers/deprecation.py
index 08803aaded6..307a297272c 100644
--- a/homeassistant/helpers/deprecation.py
+++ b/homeassistant/helpers/deprecation.py
@@ -132,24 +132,24 @@ def deprecated_function(
def _print_deprecation_warning(obj: Any, replacement: str, description: str) -> None:
logger = logging.getLogger(obj.__module__)
try:
- _, integration, path = get_integration_frame()
- if path == "custom_components/":
+ integration_frame = get_integration_frame()
+ if integration_frame.custom_integration:
logger.warning(
(
"%s was called from %s, this is a deprecated %s. Use %s instead,"
" please report this to the maintainer of %s"
),
obj.__name__,
- integration,
+ integration_frame.integration,
description,
replacement,
- integration,
+ integration_frame.integration,
)
else:
logger.warning(
"%s was called from %s, this is a deprecated %s. Use %s instead",
obj.__name__,
- integration,
+ integration_frame.integration,
description,
replacement,
)
diff --git a/homeassistant/helpers/frame.py b/homeassistant/helpers/frame.py
index 988db411a6b..084a781bf62 100644
--- a/homeassistant/helpers/frame.py
+++ b/homeassistant/helpers/frame.py
@@ -3,6 +3,7 @@ from __future__ import annotations
import asyncio
from collections.abc import Callable
+from dataclasses import dataclass
import functools
import logging
from traceback import FrameSummary, extract_stack
@@ -18,9 +19,17 @@ _REPORTED_INTEGRATIONS: set[str] = set()
_CallableT = TypeVar("_CallableT", bound=Callable)
-def get_integration_frame(
- exclude_integrations: set | None = None,
-) -> tuple[FrameSummary, str, str]:
+@dataclass
+class IntegrationFrame:
+ """Integration frame container."""
+
+ custom_integration: bool
+ filename: str
+ frame: FrameSummary
+ integration: str
+
+
+def get_integration_frame(exclude_integrations: set | None = None) -> IntegrationFrame:
"""Return the frame, integration and integration path of the current stack frame."""
found_frame = None
if not exclude_integrations:
@@ -46,7 +55,12 @@ def get_integration_frame(
if found_frame is None:
raise MissingIntegrationFrame
- return found_frame, integration, path
+ return IntegrationFrame(
+ path == "custom_components/",
+ found_frame.filename[index:],
+ found_frame,
+ integration,
+ )
class MissingIntegrationFrame(HomeAssistantError):
@@ -74,28 +88,26 @@ def report(
_LOGGER.warning(msg, stack_info=True)
return
- report_integration(what, integration_frame, level)
+ _report_integration(what, integration_frame, level)
-def report_integration(
+def _report_integration(
what: str,
- integration_frame: tuple[FrameSummary, str, str],
+ integration_frame: IntegrationFrame,
level: int = logging.WARNING,
) -> None:
"""Report incorrect usage in an integration.
Async friendly.
"""
- found_frame, integration, path = integration_frame
-
+ found_frame = integration_frame.frame
# Keep track of integrations already reported to prevent flooding
key = f"{found_frame.filename}:{found_frame.lineno}"
if key in _REPORTED_INTEGRATIONS:
return
_REPORTED_INTEGRATIONS.add(key)
- index = found_frame.filename.index(path)
- if path == "custom_components/":
+ if integration_frame.custom_integration:
extra = " to the custom integration author"
else:
extra = ""
@@ -108,8 +120,8 @@ def report_integration(
),
what,
extra,
- integration,
- found_frame.filename[index:],
+ integration_frame.integration,
+ integration_frame.filename,
found_frame.lineno,
(found_frame.line or "?").strip(),
)
diff --git a/tests/conftest.py b/tests/conftest.py
index f743a2fe96a..015cae17205 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -1515,33 +1515,6 @@ async def recorder_mock(
return await async_setup_recorder_instance(hass, recorder_config)
-@pytest.fixture
-def mock_integration_frame() -> Generator[Mock, None, None]:
- """Mock as if we're calling code from inside an integration."""
- correct_frame = Mock(
- filename="/home/paulus/homeassistant/components/hue/light.py",
- lineno="23",
- line="self.light.is_on",
- )
- with patch(
- "homeassistant.helpers.frame.extract_stack",
- return_value=[
- Mock(
- filename="/home/paulus/homeassistant/core.py",
- lineno="23",
- line="do_something()",
- ),
- correct_frame,
- Mock(
- filename="/home/paulus/aiohue/lights.py",
- lineno="2",
- line="something()",
- ),
- ],
- ):
- yield correct_frame
-
-
@pytest.fixture(name="enable_bluetooth")
async def mock_enable_bluetooth(
hass: HomeAssistant,
diff --git a/tests/helpers/test_deprecation.py b/tests/helpers/test_deprecation.py
index 380801123b0..1128f7d43c6 100644
--- a/tests/helpers/test_deprecation.py
+++ b/tests/helpers/test_deprecation.py
@@ -1,5 +1,5 @@
"""Test deprecation helpers."""
-from unittest.mock import MagicMock, patch
+from unittest.mock import MagicMock, Mock, patch
import pytest
@@ -87,7 +87,10 @@ def test_config_get_deprecated_new(mock_get_logger) -> None:
def test_deprecated_function(caplog: pytest.LogCaptureFixture) -> None:
- """Test deprecated_function decorator."""
+ """Test deprecated_function decorator.
+
+ This tests the behavior when the calling integration is not known.
+ """
@deprecated_function("new_function")
def mock_deprecated_function():
@@ -98,3 +101,82 @@ def test_deprecated_function(caplog: pytest.LogCaptureFixture) -> None:
"mock_deprecated_function is a deprecated function. Use new_function instead"
in caplog.text
)
+
+
+def test_deprecated_function_called_from_built_in_integration(
+ caplog: pytest.LogCaptureFixture,
+) -> None:
+ """Test deprecated_function decorator.
+
+ This tests the behavior when the calling integration is built-in.
+ """
+
+ @deprecated_function("new_function")
+ def mock_deprecated_function():
+ pass
+
+ with patch(
+ "homeassistant.helpers.frame.extract_stack",
+ return_value=[
+ Mock(
+ filename="/home/paulus/homeassistant/core.py",
+ lineno="23",
+ line="do_something()",
+ ),
+ Mock(
+ filename="/home/paulus/homeassistant/components/hue/light.py",
+ lineno="23",
+ line="await session.close()",
+ ),
+ Mock(
+ filename="/home/paulus/aiohue/lights.py",
+ lineno="2",
+ line="something()",
+ ),
+ ],
+ ):
+ mock_deprecated_function()
+ assert (
+ "mock_deprecated_function was called from hue, this is a deprecated function. "
+ "Use new_function instead" in caplog.text
+ )
+
+
+def test_deprecated_function_called_from_custom_integration(
+ caplog: pytest.LogCaptureFixture,
+) -> None:
+ """Test deprecated_function decorator.
+
+ This tests the behavior when the calling integration is custom.
+ """
+
+ @deprecated_function("new_function")
+ def mock_deprecated_function():
+ pass
+
+ with patch(
+ "homeassistant.helpers.frame.extract_stack",
+ return_value=[
+ Mock(
+ filename="/home/paulus/homeassistant/core.py",
+ lineno="23",
+ line="do_something()",
+ ),
+ Mock(
+ filename="/home/paulus/config/custom_components/hue/light.py",
+ lineno="23",
+ line="await session.close()",
+ ),
+ Mock(
+ filename="/home/paulus/aiohue/lights.py",
+ lineno="2",
+ line="something()",
+ ),
+ ],
+ ):
+ mock_deprecated_function()
+ assert (
+ "mock_deprecated_function was called from hue, this is a deprecated function. "
+ "Use new_function instead, please report this to the maintainer of hue"
+ in caplog.text
+ )
diff --git a/tests/helpers/test_frame.py b/tests/helpers/test_frame.py
index 3086bebe09d..53d799a0400 100644
--- a/tests/helpers/test_frame.py
+++ b/tests/helpers/test_frame.py
@@ -1,5 +1,6 @@
"""Test the frame helper."""
+from collections.abc import Generator
from unittest.mock import Mock, patch
import pytest
@@ -7,15 +8,41 @@ import pytest
from homeassistant.helpers import frame
+@pytest.fixture
+def mock_integration_frame() -> Generator[Mock, None, None]:
+ """Mock as if we're calling code from inside an integration."""
+ correct_frame = Mock(
+ filename="/home/paulus/homeassistant/components/hue/light.py",
+ lineno="23",
+ line="self.light.is_on",
+ )
+ with patch(
+ "homeassistant.helpers.frame.extract_stack",
+ return_value=[
+ Mock(
+ filename="/home/paulus/homeassistant/core.py",
+ lineno="23",
+ line="do_something()",
+ ),
+ correct_frame,
+ Mock(
+ filename="/home/paulus/aiohue/lights.py",
+ lineno="2",
+ line="something()",
+ ),
+ ],
+ ):
+ yield correct_frame
+
+
async def test_extract_frame_integration(
caplog: pytest.LogCaptureFixture, mock_integration_frame: Mock
) -> None:
"""Test extracting the current frame from integration context."""
- found_frame, integration, path = frame.get_integration_frame()
-
- assert integration == "hue"
- assert path == "homeassistant/components/"
- assert found_frame == mock_integration_frame
+ integration_frame = frame.get_integration_frame()
+ assert integration_frame == frame.IntegrationFrame(
+ False, "homeassistant/components/hue/light.py", mock_integration_frame, "hue"
+ )
async def test_extract_frame_integration_with_excluded_integration(
@@ -48,13 +75,13 @@ async def test_extract_frame_integration_with_excluded_integration(
),
],
):
- found_frame, integration, path = frame.get_integration_frame(
+ integration_frame = frame.get_integration_frame(
exclude_integrations={"zeroconf"}
)
- assert integration == "mdns"
- assert path == "homeassistant/components/"
- assert found_frame == correct_frame
+ assert integration_frame == frame.IntegrationFrame(
+ False, "homeassistant/components/mdns/light.py", correct_frame, "mdns"
+ )
async def test_extract_frame_no_integration(caplog: pytest.LogCaptureFixture) -> None:
@@ -77,23 +104,32 @@ async def test_extract_frame_no_integration(caplog: pytest.LogCaptureFixture) ->
frame.get_integration_frame()
-@pytest.mark.usefixtures("mock_integration_frame")
@patch.object(frame, "_REPORTED_INTEGRATIONS", set())
-async def test_prevent_flooding(caplog: pytest.LogCaptureFixture) -> None:
+async def test_prevent_flooding(
+ caplog: pytest.LogCaptureFixture, mock_integration_frame: Mock
+) -> None:
"""Test to ensure a report is only written once to the log."""
what = "accessed hi instead of hello"
key = "/home/paulus/homeassistant/components/hue/light.py:23"
+ integration = "hue"
+ filename = "homeassistant/components/hue/light.py"
+
+ expected_message = (
+ f"Detected integration that {what}. Please report issue for {integration} using"
+ f" this method at {filename}, line "
+ f"{mock_integration_frame.lineno}: {mock_integration_frame.line}"
+ )
frame.report(what, error_if_core=False)
- assert what in caplog.text
+ assert expected_message in caplog.text
assert key in frame._REPORTED_INTEGRATIONS
assert len(frame._REPORTED_INTEGRATIONS) == 1
caplog.clear()
frame.report(what, error_if_core=False)
- assert what not in caplog.text
+ assert expected_message not in caplog.text
assert key in frame._REPORTED_INTEGRATIONS
assert len(frame._REPORTED_INTEGRATIONS) == 1