Add tests for System Monitor #107891

gjohansson-ST · 2024-01-12T16:14:25Z

Proposed change

Adds test coverage for System Monitor

---------- coverage: platform linux, python 3.11.7-final-0 -----------
Name                                                    Stmts   Miss  Cover   Missing
-------------------------------------------------------------------------------------
homeassistant/components/systemmonitor/__init__.py         12      0   100%
homeassistant/components/systemmonitor/config_flow.py      55      0   100%
homeassistant/components/systemmonitor/const.py             4      0   100%
homeassistant/components/systemmonitor/sensor.py          291      2    99%   548-555
homeassistant/components/systemmonitor/util.py             36      0   100%
-------------------------------------------------------------------------------------
TOTAL                                                     398      2    99%

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Deprecation (breaking change to happen in the future)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Additional information

This PR fixes or closes issue: fixes #
This PR is related to issue:
Link to documentation pull request:

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
I have followed the perfect PR recommendations
The code has been formatted using Ruff (ruff format homeassistant tests)
Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

gjohansson-ST · 2024-01-12T16:14:45Z

homeassistant/components/systemmonitor/sensor.py

@@ -70,6 +66,13 @@
 SIGNAL_SYSTEMMONITOR_UPDATE = "systemmonitor_update"


+def get_cpu_icon() -> Literal["mdi:cpu-64-bit", "mdi:cpu-32-bit"]:


Has been added to test the icons in an easier manner

maybe decorate with @cache since it never changes

diff --git a/homeassistant/components/systemmonitor/sensor.py b/homeassistant/components/systemmonitor/sensor.py index ad770eae762..cdab34f71a9 100644 --- a/homeassistant/components/systemmonitor/sensor.py +++ b/homeassistant/components/systemmonitor/sensor.py @@ -10,6 +10,7 @@ import os import socket import sys from typing import Any, Literal +from functools import cache import psutil import voluptuous as vol @@ -66,11 +67,10 @@ SENSOR_TYPE_MANDATORY_ARG = 4 SIGNAL_SYSTEMMONITOR_UPDATE = "systemmonitor_update" +@cache def get_cpu_icon() -> Literal["mdi:cpu-64-bit", "mdi:cpu-32-bit"]: """Return cpu icon.""" - if sys.maxsize > 2**32: - return "mdi:cpu-64-bit" - return "mdi:cpu-32-bit" + return "mdi:cpu-64-bit" if sys.maxsize > 2**32 else "mdi:cpu-32-bit" @dataclass(frozen=True)

bdraco · 2024-01-13T21:19:39Z

tests/components/systemmonitor/test_sensor.py

+async def test_sensor_icon_32bit() -> None:
+    """Test the sensor icon for 32bit system."""
+
+    with patch("sys.maxsize", 2**32):
+        assert get_cpu_icon() == "mdi:cpu-32-bit"
+    with patch("sys.maxsize", 2**64):
+        assert get_cpu_icon() == "mdi:cpu-64-bit"
+


Ideally we would patch sys.maxsize and write state to verify the icon instead of testing this directly (and i there is a cache it would be ok to clear the cache with .clear_cache https://docs.python.org/3/library/functools.html#functools.lru_cache

The problem is that the icon is evaluated when the file is loaded so before the test even starts.
No idea how to patch sys.maxsize in such case and I don't think clearing a cache helps as the icon has already been set in the entity descriptions then.

I can push a suggestion if you like

diff --git a/tests/components/systemmonitor/test_sensor.py b/tests/components/systemmonitor/test_sensor.py index 25b6fd901fe..85ea38ddb70 100644 --- a/tests/components/systemmonitor/test_sensor.py +++ b/tests/components/systemmonitor/test_sensor.py @@ -5,7 +5,7 @@ from unittest.mock import Mock, patch from freezegun.api import FrozenDateTimeFactory from psutil._common import shwtemp, snetio, snicaddr -from psutil._pslinux import svmem + import pytest from syrupy.assertion import SnapshotAssertion @@ -16,7 +16,7 @@ from homeassistant.core import HomeAssistant from homeassistant.helpers import entity_registry as er from homeassistant.setup import async_setup_component -from .conftest import MockProcess +from .conftest import MockProcess, MockSvMem from tests.common import MockConfigEntry, async_fire_time_changed @@ -69,9 +69,10 @@ async def test_sensor_not_loading_veth_networks( async def test_sensor_icon() -> None: """Test the sensor icon for 32bit/64bit system.""" - + get_cpu_icon.cache_clear() with patch("sys.maxsize", 2**32): assert get_cpu_icon() == "mdi:cpu-32-bit" + get_cpu_icon.cache_clear() with patch("sys.maxsize", 2**64): assert get_cpu_icon() == "mdi:cpu-64-bit"

But better to patch sys.maxsize and clear the cache, move time forward, and wait for the update and verify the icon

with patch("sys.maxsize", 2**32): get_cpu_icon.cache_clear() mock_config_entry.add_to_hass(hass) await hass.config_entries.async_setup(mock_config_entry.entry_id) await hass.async_block_till_done() process_sensor = hass.states.get("sensor.system_monitor_process_python3") assert process_sensor is not None assert process_sensor.attributes["icon"] == "mdi:cpu-32-bit"

Not helping as it still comes out as 64-bit given my system.
entity descriptions are also a constant so not sure how the cache would help here. I only see it would work by moving it down to the entity but I would prefer to skip that.

Feel free to push a change if you find a workable solution 👍
I'm not going to continue now until tomorrow.

bdraco · 2024-01-13T22:13:48Z

tests/components/systemmonitor/conftest.py


+from psutil import NoSuchProcess, Process
+from psutil._common import sdiskpart, sdiskusage, shwtemp, snetio, snicaddr, sswap
+from psutil._pslinux import svmem


This fails to import when testing on macos

Maybe something like

@dataclass class MockSvMem: total: int available: int percent: float

FAILED tests/components/systemmonitor/test_init.py::test_load_unload_entry - AttributeError: Mock object has no attribute 'sensors_temperatures'

sensors_temperatures also doesn't exist on macos

sensors_temperatures also doesn't exist on macos

According to psutil it should https://github.com/giampaolo/psutil/blob/master/psutil/__init__.py#L2284

ImportError while loading conftest '/Users/bdraco/home-assistant/tests/components/systemmonitor/conftest.py'. tests/components/systemmonitor/conftest.py:10: in <module> from psutil._pslinux import svmem venv/lib/python3.12/site-packages/psutil/_pslinux.py:25: in <module> from . import _psutil_linux as cext E ImportError: cannot import name '_psutil_linux' from 'psutil' (/Users/bdraco/home-assistant/venv/lib/python3.12/site-packages/psutil/__init__.py). Did you mean: '_psutil_osx'?

Not sure if this helps

@pytest.fixture(autouse=True) def mock_sys_platform() -> Generator[None, None, None]: """Mock sys platform to Linux.""" with patch("sys.platform", "linux"): yield

bdraco · 2024-01-14T18:29:24Z

mock_process = [tests.components.systemmonitor.conftest.MockProcess(pid=1, name='python3', status='running', started='2023-12-19 20:5...ests.components.systemmonitor.conftest.MockProcess(pid=1, name='pip', status='running', started='2023-12-19 20:52:42')]

    @pytest.fixture
    def mock_psutil(mock_process: list[MockProcess]) -> Mock:
        """Mock psutil."""
        with patch(
            "homeassistant.components.systemmonitor.sensor.psutil",
            autospec=True,
        ) as mock_psutil:
            mock_psutil.disk_usage.return_value = sdiskusage(
                500 * 1024**2, 300 * 1024**2, 200 * 1024**2, 60.0
            )
            mock_psutil.swap_memory.return_value = sswap(
                100 * 1024**2, 60 * 1024**2, 40 * 1024**2, 60.0, 1, 1
            )
            mock_psutil.virtual_memory.return_value = svmem(
                100 * 1024**2,
                40 * 1024**2,
                40.0,
                60 * 1024**2,
                30 * 1024**2,
                1,
                1,
                1,
                1,
                1,
                1,
            )
            mock_psutil.net_io_counters.return_value = {
                "eth0": snetio(100 * 1024**2, 100 * 1024**2, 50, 50, 0, 0, 0, 0),
                "eth1": snetio(200 * 1024**2, 200 * 1024**2, 150, 150, 0, 0, 0, 0),
                "vethxyzxyz": snetio(300 * 1024**2, 300 * 1024**2, 150, 150, 0, 0, 0, 0),
            }
            mock_psutil.net_if_addrs.return_value = {
                "eth0": [
                    snicaddr(
                        socket.AF_INET,
                        "192.168.1.1",
                        "255.255.255.0",
                        "255.255.255.255",
                        None,
                    )
                ],
                "eth1": [
                    snicaddr(
                        socket.AF_INET,
                        "192.168.10.1",
                        "255.255.255.0",
                        "255.255.255.255",
                        None,
                    )
                ],
                "vethxyzxyz": [
                    snicaddr(
                        socket.AF_INET,
                        "172.16.10.1",
                        "255.255.255.0",
                        "255.255.255.255",
                        None,
                    )
                ],
            }
            mock_psutil.cpu_percent.return_value = 10.0
            mock_psutil.boot_time.return_value = 1703973338.0
            mock_psutil.process_iter.return_value = mock_process
>           mock_psutil.sensors_temperatures.return_value = {
                "cpu0-thermal": [shwtemp("cpu0-thermal", 50.0, 60.0, 70.0)]
            }

tests/components/systemmonitor/conftest.py:177: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <NonCallableMagicMock name='psutil' spec='module' id='4391436512'>, name = 'sensors_temperatures'

    def __getattr__(self, name):
        if name in {'_mock_methods', '_mock_unsafe'}:
            raise AttributeError(name)
        elif self._mock_methods is not None:
            if name not in self._mock_methods or name in _all_magics:
>               raise AttributeError("Mock object has no attribute %r" % name)
E               AttributeError: Mock object has no attribute 'sensors_temperatures'

/opt/homebrew/Cellar/python@3.12/3.12.1/Frameworks/Python.framework/Versions/3.12/lib/python3.12/unittest/mock.py:658: AttributeError

bdraco · 2024-01-14T18:33:00Z

diff --git a/tests/components/systemmonitor/conftest.py b/tests/components/systemmonitor/conftest.py
index ef29e54bb34..06bdfcf404b 100644
--- a/tests/components/systemmonitor/conftest.py
+++ b/tests/components/systemmonitor/conftest.py
@@ -174,6 +174,9 @@ def mock_psutil(mock_process: list[MockProcess]) -> Mock:
         mock_psutil.cpu_percent.return_value = 10.0
         mock_psutil.boot_time.return_value = 1703973338.0
         mock_psutil.process_iter.return_value = mock_process
+        # sensors_temperatures not available on MacOS so we
+        # need to override the spec
+        mock_psutil.__setattr__("sensors_temperatures", Mock())
         mock_psutil.sensors_temperatures.return_value = {
             "cpu0-thermal": [shwtemp("cpu0-thermal", 50.0, 60.0, 70.0)]
         }
@@ -218,6 +221,9 @@ def mock_util(mock_process) -> Mock:
         }
         mock_process = [MockProcess("python3")]
         mock_util.process_iter.return_value = mock_process
+        # sensors_temperatures not available on MacOS so we
+        # need to override the spec
+        mock_util.__setattr__("sensors_temperatures", Mock())
         mock_util.sensors_temperatures.return_value = {
             "cpu0-thermal": [shwtemp("cpu0-thermal", 50.0, 60.0, 70.0)]
         }

The tests pass with this change

bdraco · 2024-01-14T18:34:43Z

tests/components/systemmonitor/test_sensor.py

+
+    process_sensor = hass.states.get("sensor.system_monitor_process_python3")
+    assert process_sensor is not None
+    # assert process_sensor.state == STATE_ON


old comment?

Forgot this one, addressed in new PR

bdraco · 2024-01-14T18:35:36Z

---------- coverage: platform darwin, python 3.12.1-final-0 ----------
Name                                                    Stmts   Miss  Cover   Missing
-------------------------------------------------------------------------------------
homeassistant/components/systemmonitor/__init__.py         12      0   100%
homeassistant/components/systemmonitor/config_flow.py      55      0   100%
homeassistant/components/systemmonitor/const.py             4      0   100%
homeassistant/components/systemmonitor/sensor.py          295      2    99%   549-556
homeassistant/components/systemmonitor/util.py             40      0   100%
-------------------------------------------------------------------------------------
TOTAL                                                     406      2    99%

locally on macos

gjohansson-ST · 2024-01-14T18:39:19Z

locally on macos

👍
Same for me. Haven't figured out how to test the slow updating so I might just leave that for later to move on with the coordinators.

home-assistant bot added cla-signed code-quality has-tests integration: systemmonitor small-pr PRs with less than 30 lines. by-code-owner Quality Scale: No score labels Jan 12, 2024

gjohansson-ST commented Jan 12, 2024

View reviewed changes

gjohansson-ST marked this pull request as ready for review January 13, 2024 12:11

gjohansson-ST added 8 commits January 13, 2024 21:05

Add tests

540b489

no coordinator

067a27a

Coverage

a12ff24

processes

412f8ba

test init

890e342

util

53bc41b

test icon

b39eb81

Add tests

73c34d2

gjohansson-ST force-pushed the systemmonitor-tests branch from aa53fe8 to 73c34d2 Compare January 13, 2024 21:05

Mod tests

5b0de51

gjohansson-ST mentioned this pull request Jan 13, 2024

Create update coordinator for Systemmonitor #106693

Merged

20 tasks

bdraco reviewed Jan 13, 2024

View reviewed changes

bdraco approved these changes Jan 13, 2024

View reviewed changes

gjohansson-ST added 4 commits January 13, 2024 21:35

Add tests

c40709f

Test attributes

0af7717

snapshots

f7a05ae

icon

780c549

bdraco reviewed Jan 13, 2024

View reviewed changes

gjohansson-ST added 3 commits January 13, 2024 22:23

test disk mounts

2645f6f

fixes

3e8a2c2

svmem

e45fe47

gjohansson-ST added 4 commits January 13, 2024 22:53

cache_clear

db09141

test icon

d7bd49f

reset icon test

fa87713

test_processor_temperature

84ad481

fix tests on macos

5462521

bdraco reviewed Jan 14, 2024

View reviewed changes

gjohansson-ST merged commit 5b3e130 into dev Jan 15, 2024
23 checks passed

gjohansson-ST deleted the systemmonitor-tests branch January 15, 2024 17:26

gjohansson-ST mentioned this pull request Jan 15, 2024

Fix test_sensor_process_fails test in System Monitor #108110

Merged

20 tasks

github-actions bot locked and limited conversation to collaborators Jan 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add tests for System Monitor #107891

Add tests for System Monitor #107891

gjohansson-ST commented Jan 12, 2024 •

edited

gjohansson-ST Jan 12, 2024

bdraco Jan 13, 2024

bdraco Jan 13, 2024

bdraco Jan 13, 2024

gjohansson-ST Jan 13, 2024

bdraco Jan 13, 2024

bdraco Jan 13, 2024

gjohansson-ST Jan 13, 2024

gjohansson-ST Jan 13, 2024

bdraco Jan 13, 2024

bdraco Jan 13, 2024

bdraco Jan 13, 2024

gjohansson-ST Jan 13, 2024

bdraco Jan 13, 2024

gjohansson-ST Jan 13, 2024

bdraco commented Jan 14, 2024

bdraco commented Jan 14, 2024

bdraco Jan 14, 2024

gjohansson-ST Jan 15, 2024

bdraco commented Jan 14, 2024

gjohansson-ST commented Jan 14, 2024

		@@ -70,6 +66,13 @@
		SIGNAL_SYSTEMMONITOR_UPDATE = "systemmonitor_update"


		def get_cpu_icon() -> Literal["mdi:cpu-64-bit", "mdi:cpu-32-bit"]:

Add tests for System Monitor #107891

Add tests for System Monitor #107891

Conversation

gjohansson-ST commented Jan 12, 2024 • edited

Proposed change

Type of change

Additional information

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bdraco commented Jan 14, 2024

bdraco commented Jan 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bdraco commented Jan 14, 2024

gjohansson-ST commented Jan 14, 2024

gjohansson-ST commented Jan 12, 2024 •

edited