refactor: Split IPMIStrategy to sensor, sel, and dcmi strategies (#167)

Fix #124

src/hw_tools.py

@@ -308,12 +308,6 @@ def check(self) -> bool:
 class IPMIStrategy(APTStrategyABC):
     """Strategy for installing ipmi."""
 
-    # Because IPMISTrategy now encompasses all of
-    # HWTool.IPMI_SENSOR, HWTool.IPMI_SEL and HWTool.IPMI_DCMI,
-    # we will need some refactoring here to avoid misleading log
-    # messages. The installation should be good since all of these
-    # tools require the same `freeipmi-tools` to be installed.
-    _name = HWTool.IPMI_SENSOR
     pkg = "freeipmi-tools"
 
     def install(self) -> None:
@@ -322,13 +316,31 @@ def install(self) -> None:
     def remove(self) -> None:
         # Skip removing because we afriad this cause dependency error
         # for other services on the same machine.
-        logger.info("IPMIStrategy skip removing %s", self.pkg)
+        logger.info("%s skip removing %s", self._name, self.pkg)
 
     def check(self) -> bool:
         """Check package status."""
         return check_deb_pkg_installed(self.pkg)
 
 
+class IPMISENSORStrategy(IPMIStrategy):
+    """Strategy for installing ipmi."""
+
+    _name = HWTool.IPMI_SENSOR
+
+
+class IPMISELStrategy(IPMIStrategy):
+    """Strategy for installing ipmi."""
+
+    _name = HWTool.IPMI_SEL
+
+
+class IPMIDCMIStrategy(IPMIStrategy):
+    """Strategy for installing ipmi."""
+
+    _name = HWTool.IPMI_DCMI
+
+
 class RedFishStrategy(StrategyABC):  # pylint: disable=R0903
     """Install strategy for redfish.
 
@@ -490,7 +502,9 @@ def strategies(self) -> List[StrategyABC]:
             SAS2IRCUStrategy(),
             SAS3IRCUStrategy(),
             SSACLIStrategy(),
-            IPMIStrategy(),
+            IPMISELStrategy(),
+            IPMIDCMIStrategy(),
+            IPMISENSORStrategy(),
             RedFishStrategy(),
         ]
 

tests/unit/test_hw_tools.py

@@ -22,7 +22,9 @@
     APTStrategyABC,
     HWToolHelper,
     InvalidCredentialsError,
-    IPMIStrategy,
+    IPMIDCMIStrategy,
+    IPMISELStrategy,
+    IPMISENSORStrategy,
     PercCLIStrategy,
     ResourceChecksumError,
     ResourceFileSizeZeroError,
@@ -673,11 +675,11 @@ def test_remove(self, mock_apt):
         mock_repos.disable.assert_not_called()
 
 
-class TestIPMIStrategy(unittest.TestCase):
+class TestIPMISENSORStrategy(unittest.TestCase):
     @mock.patch("apt_helpers.get_candidate_version")
     @mock.patch("apt_helpers.apt")
     def test_install(self, mock_apt, mock_candidate_version):
-        strategy = IPMIStrategy()
+        strategy = IPMISENSORStrategy()
         mock_candidate_version.return_value = "some-candidate-version"
         strategy.install()
 
@@ -687,7 +689,47 @@ def test_install(self, mock_apt, mock_candidate_version):
 
     @mock.patch("hw_tools.apt")
     def test_remove(self, mock_apt):
-        strategy = IPMIStrategy()
+        strategy = IPMISENSORStrategy()
+        strategy.remove()
+
+        mock_apt.remove_package.assert_not_called()
+
+
+class TestIPMISELStrategy(unittest.TestCase):
+    @mock.patch("apt_helpers.get_candidate_version")
+    @mock.patch("apt_helpers.apt")
+    def test_install(self, mock_apt, mock_candidate_version):
+        strategy = IPMISELStrategy()
+        mock_candidate_version.return_value = "some-candidate-version"
+        strategy.install()
+
+        mock_apt.add_package.assert_called_with(
+            "freeipmi-tools", version="some-candidate-version", update_cache=False
+        )
+
+    @mock.patch("hw_tools.apt")
+    def test_remove(self, mock_apt):
+        strategy = IPMISELStrategy()
+        strategy.remove()
+
+        mock_apt.remove_package.assert_not_called()
+
+
+class TestIPMIDCMIStrategy(unittest.TestCase):
+    @mock.patch("apt_helpers.get_candidate_version")
+    @mock.patch("apt_helpers.apt")
+    def test_install(self, mock_apt, mock_candidate_version):
+        strategy = IPMIDCMIStrategy()
+        mock_candidate_version.return_value = "some-candidate-version"
+        strategy.install()
+
+        mock_apt.add_package.assert_called_with(
+            "freeipmi-tools", version="some-candidate-version", update_cache=False
+        )
+
+    @mock.patch("hw_tools.apt")
+    def test_remove(self, mock_apt):
+        strategy = IPMIDCMIStrategy()
         strategy.remove()
 
         mock_apt.remove_package.assert_not_called()