From c65725d3275faae5836758733e6e3b3c71618def Mon Sep 17 00:00:00 2001 From: Gabriel Cocenza Date: Mon, 7 Oct 2024 16:57:56 -0300 Subject: [PATCH] add unit tests --- src/hw_tools.py | 8 +- src/service.py | 4 +- tests/unit/test_hw_tools.py | 48 +++++++++++- tests/unit/test_service.py | 148 +++++++++++------------------------- 4 files changed, 97 insertions(+), 111 deletions(-) diff --git a/src/hw_tools.py b/src/hw_tools.py index b9db254c..b83cd565 100644 --- a/src/hw_tools.py +++ b/src/hw_tools.py @@ -197,6 +197,11 @@ def snap_name(self) -> str: """Snap name.""" return self._name.value + @property + def snap_common(self) -> Path: + """Snap common directory.""" + return Path(f"/var/snap/{self.snap_name}/common/") + @property def snap_client(self) -> snap.Snap: """Return the snap client.""" @@ -239,8 +244,7 @@ class DCGMExporterStrategy(SnapStrategy): """DCGM exporter strategy class.""" _name = HWTool.DCGM - metric_file = Path().parent / "/gpu_metrics/dcgm_metrics.csv" - snap_common: Path = Path("/var/snap/dcgm/common/") + metric_file = Path.cwd() / "src/gpu_metrics/dcgm_metrics.csv" def __init__(self, channel: str) -> None: """Init.""" diff --git a/src/service.py b/src/service.py index e56d71d7..38b30a0a 100644 --- a/src/service.py +++ b/src/service.py @@ -325,7 +325,6 @@ class SnapExporter(BaseExporter): def __init__(self, config: ConfigData): """Init.""" self.config = config - self.strategies = [] @property def snap_client(self) -> snap.Snap: @@ -407,7 +406,8 @@ def configure(self) -> bool: for strategy in self.strategies: if isinstance(strategy, SnapStrategy): try: - return self.install() + # refresh the snap for a new channel if necessary + strategy.install() except Exception as err: # pylint: disable=broad-except logger.error("Failed to configure %s: %s", self.exporter_name, err) return False diff --git a/tests/unit/test_hw_tools.py b/tests/unit/test_hw_tools.py index f544e247..37eac304 100644 --- a/tests/unit/test_hw_tools.py +++ b/tests/unit/test_hw_tools.py @@ -22,6 +22,7 @@ from config import SNAP_COMMON, TOOLS_DIR, TPR_RESOURCES, HWTool, StorageVendor, SystemVendor from hw_tools import ( APTStrategyABC, + DCGMExporterStrategy, HWToolHelper, IPMIDCMIStrategy, IPMISELStrategy, @@ -1039,7 +1040,7 @@ def mock_snap_lib(): def test_snap_strategy_name(snap_exporter): - assert snap_exporter.snap == "my-snap" + assert snap_exporter.snap_name == "my-snap" def test_snap_strategy_channel(snap_exporter): @@ -1048,7 +1049,9 @@ def test_snap_strategy_channel(snap_exporter): def test_snap_strategy_install_success(snap_exporter, mock_snap_lib): snap_exporter.install() - mock_snap_lib.add.assert_called_once_with(snap_exporter.snap, channel=snap_exporter.channel) + mock_snap_lib.add.assert_called_once_with( + snap_exporter.snap_name, channel=snap_exporter.channel + ) def test_snap_strategy_install_fail(snap_exporter, mock_snap_lib): @@ -1060,7 +1063,7 @@ def test_snap_strategy_install_fail(snap_exporter, mock_snap_lib): def test_snap_strategy_remove_success(snap_exporter, mock_snap_lib): snap_exporter.remove() - mock_snap_lib.remove.assert_called_once_with([snap_exporter.snap]) + mock_snap_lib.remove.assert_called_once_with([snap_exporter.snap_name]) def test_snap_strategy_remove_fail(snap_exporter, mock_snap_lib): @@ -1169,6 +1172,12 @@ def mock_path(): yield mocked_path +@pytest.fixture +def mock_shutil_copy(): + with mock.patch("hw_tools.shutil.copy") as mocked_copy: + yield mocked_copy + + @pytest.fixture def nvidia_driver_strategy(mock_check_output, mock_apt_lib, mock_path, mock_check_call): strategy = NVIDIADriverStrategy() @@ -1176,6 +1185,39 @@ def nvidia_driver_strategy(mock_check_output, mock_apt_lib, mock_path, mock_chec yield strategy +@pytest.fixture +def dcgm_exporter_strategy(mock_snap_lib, mock_shutil_copy): + yield DCGMExporterStrategy("latest/stable") + + +@mock.patch("hw_tools.DCGMExporterStrategy._create_custom_metrics") +def test_dcgm_exporter_install(mock_custom_metrics, dcgm_exporter_strategy): + assert dcgm_exporter_strategy.install() is None + mock_custom_metrics.assert_called_once() + + +def test_dcgm_create_custom_metrics(dcgm_exporter_strategy, mock_shutil_copy, mock_snap_lib): + assert dcgm_exporter_strategy._create_custom_metrics() is None + mock_shutil_copy.assert_called_once_with( + Path.cwd() / "src/gpu_metrics/dcgm_metrics.csv", Path("/var/snap/dcgm/common") + ) + dcgm_exporter_strategy.snap_client.set.assert_called_once_with( + {"dcgm-exporter-metrics-file": "dcgm_metrics.csv"} + ) + dcgm_exporter_strategy.snap_client.restart.assert_called_once_with(reload=True) + + +def test_dcgm_create_custom_metrics_copy_fail( + dcgm_exporter_strategy, mock_shutil_copy, mock_snap_lib +): + mock_shutil_copy.side_effect = FileNotFoundError + with pytest.raises(FileNotFoundError): + dcgm_exporter_strategy._create_custom_metrics() + + dcgm_exporter_strategy.snap_client.set.assert_not_called() + dcgm_exporter_strategy.snap_client.restart.assert_not_called() + + @mock.patch("hw_tools.NVIDIADriverStrategy._install_nvidia_drivers") @mock.patch("hw_tools.NVIDIADriverStrategy._install_nvidia_utils") def test_nvidia_driver_strategy_install( diff --git a/tests/unit/test_service.py b/tests/unit/test_service.py index 7965233d..3c75577b 100644 --- a/tests/unit/test_service.py +++ b/tests/unit/test_service.py @@ -4,7 +4,6 @@ import pathlib import tempfile import unittest -from subprocess import CalledProcessError from unittest import mock import pytest @@ -722,21 +721,18 @@ class TestDCGMSnapExporter(unittest.TestCase): def setUp(self) -> None: """Set up harness for each test case.""" snap_lib_patcher = mock.patch.object(service, "snap") - shutil_lib_patcher = mock.patch.object(service, "shutil") self.mock_snap = snap_lib_patcher.start() - self.mock_shutil = shutil_lib_patcher.start() self.addCleanup(snap_lib_patcher.stop) - self.addCleanup(shutil_lib_patcher.stop) - search_path = pathlib.Path(f"{__file__}/../../..").resolve() - self.mock_config = { - "dcgm-snap-channel": "latest/stable", - } - - self.exporter = service.DCGMExporter(search_path, self.mock_config) - self.exporter.strategy = mock.MagicMock() - self.exporter.nvidia_driver_strategy = mock.MagicMock() + self.exporter = service.DCGMExporter( + { + "dcgm-snap-channel": "latest/stable", + } + ) + self.snap_strategy = mock.MagicMock(spec=service.DCGMExporterStrategy) + self.nvidia_strategy = mock.MagicMock(spec=service.NVIDIADriverStrategy) + self.exporter.strategies = [self.snap_strategy, self.nvidia_strategy] def test_exporter_name(self): self.assertEqual(self.exporter.exporter_name, "dcgm") @@ -744,78 +740,14 @@ def test_exporter_name(self): def test_hw_tools(self): self.assertEqual(self.exporter.hw_tools(), {HWTool.DCGM}) - @mock.patch("service.DCGMExporter._create_custom_metrics") - @mock.patch("service.DCGMExporter._install_nvidia_drivers") - @mock.patch("service.SnapExporter.install") - def test_install_dcgm( - self, - mock_super_install, - mock_nvidia, - mock_custom_metrics, - ): - test_cases = [ - (True, True, True, True), - (False, True, True, False), - (True, False, True, False), - (True, True, False, False), - (False, False, False, False), - ] - for super_install, nvidia_drivers, custom_metrics, expected in test_cases: - with self.subTest( - super_install=super_install, - nvidia_drivers=nvidia_drivers, - custom_metrics=custom_metrics, - expected=expected, - ): - mock_super_install.return_value = super_install - mock_nvidia.return_value = nvidia_drivers - mock_custom_metrics.return_value = custom_metrics - - result = self.exporter.install() - - self.assertEqual(result, expected) - - def test_create_custom_metrics(self): - - result = self.exporter._create_custom_metrics() - - self.mock_shutil.copy.assert_called_with( - self.exporter.metrics_file, self.exporter.snap_common - ) - self.exporter.snap_client.set.assert_called_with( - {self.exporter.metric_config: self.exporter.metric_config_value} - ) - self.exporter.snap_client.restart.assert_called_with(reload=True) - self.assertTrue(result) - - def test_install_metrics_copy_fail(self): - self.exporter.snap_client.present = True - self.mock_shutil.copy.side_effect = FileNotFoundError - - result = self.exporter.install() - - self.exporter.snap_client.set.assert_not_called() - self.exporter.snap_client.restart.assert_not_called() - self.assertFalse(result) - - def test_install_nvidia_drivers_success(self): - result = self.exporter._install_nvidia_drivers() - self.assertTrue(result) - self.exporter.nvidia_driver_strategy.install.assert_called_once() - - def test_install_nvidia_drivers_fails(self): - self.exporter.nvidia_driver_strategy.install.side_effect = CalledProcessError(1, []) - result = self.exporter._install_nvidia_drivers() - self.assertFalse(result) - - def test_validate_exporter_configs_success(self): - self.exporter.nvidia_driver_strategy.check.return_value = True + @mock.patch("service.NVIDIADriverStrategy.check", return_value=True) + def test_validate_exporter_configs_success(self, _): valid, msg = self.exporter.validate_exporter_configs() self.assertTrue(valid) self.assertEqual(msg, "Exporter config is valid.") - def test_validate_exporter_configs_fails(self): - self.exporter.nvidia_driver_strategy.check.return_value = False + @mock.patch("service.NVIDIADriverStrategy.check", return_value=False) + def test_validate_exporter_configs_fails(self, _): valid, msg = self.exporter.validate_exporter_configs() self.assertFalse(valid) self.assertEqual( @@ -824,7 +756,6 @@ def test_validate_exporter_configs_fails(self): @mock.patch.object(service.BaseExporter, "validate_exporter_configs") def test_validate_exporter_configs_fails_parent(self, mock_parent_validate): - self.exporter.nvidia_driver_strategy.check.return_value = True mock_parent_validate.return_value = False, "Invalid config: exporter's port" valid, msg = self.exporter.validate_exporter_configs() self.assertFalse(valid) @@ -900,25 +831,27 @@ def test_write_to_file_not_a_directory_error(self, mock_open): @pytest.fixture def snap_exporter(): - my_strategy = mock.MagicMock(spec=service.SnapStrategy) + my_snap_strategy = mock.MagicMock(spec=service.SnapStrategy) + my_apt_strategy = mock.MagicMock(spec=service.APTStrategyABC) class MySnapExporter(service.SnapExporter): exporter_name = "my-exporter" channel = "my-channel" - strategy = my_strategy - - mock_config = { - "dcgm-snap-channel": "latest/stable", - } + strategies = [my_snap_strategy, my_apt_strategy] with mock.patch("service.snap.SnapCache"): - exporter = MySnapExporter(mock_config) + exporter = MySnapExporter( + { + "dcgm-snap-channel": "latest/stable", + } + ) exporter.snap_client.services = {"service1": {}, "service2": {}} yield exporter - my_strategy.reset_mock() + my_snap_strategy.reset_mock() + my_apt_strategy.reset_mock() def test_snap_exporter_hw_tools(snap_exporter): @@ -926,16 +859,16 @@ def test_snap_exporter_hw_tools(snap_exporter): assert snap_exporter.hw_tools() == set() -def test_snap_exporter_install(snap_exporter): - snap_exporter.strategy.install.return_value = True +def test_snap_exporter_install_success(snap_exporter): snap_exporter.snap_client.present = True assert snap_exporter.install() is True - snap_exporter.strategy.install.assert_called_once() + for strategy in snap_exporter.strategies: + strategy.install.assert_called_once() def test_snap_exporter_install_fail(snap_exporter): - snap_exporter.strategy.install.side_effect = ValueError + snap_exporter.strategies[0].install.side_effect = ValueError assert snap_exporter.install() is False @@ -944,11 +877,12 @@ def test_snap_exporter_uninstall(snap_exporter): snap_exporter.snap_client.present = False assert snap_exporter.uninstall() is True - snap_exporter.strategy.remove.assert_called_once() + for strategy in snap_exporter.strategies: + strategy.remove.assert_called_once() def test_snap_exporter_uninstall_fail(snap_exporter): - snap_exporter.strategy.remove.side_effect = ValueError + snap_exporter.strategies[0].remove.side_effect = ValueError assert snap_exporter.uninstall() is False @@ -957,7 +891,8 @@ def test_snap_exporter_uninstall_present(snap_exporter): snap_exporter.snap_client.present = True assert snap_exporter.uninstall() is False - snap_exporter.strategy.remove.assert_called_once() + for strategy in snap_exporter.strategies: + strategy.remove.assert_called_once() def test_snap_exporter_enable_and_start(snap_exporter): @@ -990,20 +925,25 @@ def test_snap_exporter_set_failed(snap_exporter): def test_snap_exporter_check_health(snap_exporter): snap_exporter.check_health() - snap_exporter.strategy.check.assert_called_once() + for strategy in snap_exporter.strategies: + strategy.check.assert_called_once() + +@mock.patch("service.isinstance", return_value=True) +def test_snap_exporter_configure(_, snap_exporter): + assert snap_exporter.configure() is True + for strategy in snap_exporter.strategies: + strategy.install.assert_called_once() -@pytest.mark.parametrize("install_result, expected_result", [(True, True), (False, False)]) -@mock.patch("service.SnapExporter.install") -def test_snap_exporter_configure(mock_install, snap_exporter, install_result, expected_result): - mock_install.return_value = install_result - assert snap_exporter.configure() is expected_result - mock_install.assert_called_once() +@mock.patch("service.isinstance", return_value=True) +def test_snap_exporter_configure_exception(_, snap_exporter): + snap_exporter.strategies[0].install.side_effect = snap.SnapError + assert snap_exporter.configure() is False @pytest.mark.parametrize("result, expected_result", [(True, True), (False, False)]) -@mock.patch("service.SnapExporter.install") +@mock.patch("service.SmartCtlExporterStrategy.install") @mock.patch("service.SnapExporter.set") def test_smartctl_exporter_configure(mock_set, mock_install, result, expected_result): mock_config = {