add unit tests

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."""

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

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,13 +1172,52 @@ 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()
     strategy.installed_pkgs = mock_path
     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(

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,100 +721,33 @@ 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")
 
     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,42 +831,44 @@ 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):
 
     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 = {