Adopt suggestion from Fernando

Author: LiaoU3

providers/base/bin/ishtp.py

@@ -1,31 +1,17 @@
 #!/usr/bin/env python3
 import subprocess
-import sys
 import os
 import argparse
+from checkbox_support.helpers.release_info import get_release_info
 
 
-def get_release_version():
-    try:
-        output = subprocess.check_output(
-            ["lsb_release", "-rs"], text=True
-        ).strip()
-        return int(float(output))
-    except (subprocess.CalledProcessError, ValueError):
-        print("Error: Unable to determine release version.")
-        sys.exit(1)
-
-
-def is_module_loaded(module_name):
-    try:
-        output = subprocess.check_output(["lsmod"], text=True)
-        return module_name in output
-    except subprocess.CalledProcessError:
-        return False
+def get_module_list():
+    output = subprocess.check_output(["lsmod"], text=True)
+    return [line.split()[0] for line in output.splitlines() if line]
 
 
 def check_modules():
-    release = get_release_version()
+    release = int(get_release_info()["release"].split(".")[0])
 
     if release >= 24:
         expected_modules = ["intel_ishtp_hid", "intel_ish_ipc", "intel_ishtp"]
@@ -37,32 +23,33 @@ def check_modules():
             "intel_ishtp",
         ]
 
-    exit_code = 0
+    module_list = get_module_list()
     for module in expected_modules:
         print("Checking module: {}".format(module))
-        if not is_module_loaded(module):
-            print("FAIL: The '{}' module is not loaded!".format(module))
-            exit_code = 1
+        if module not in module_list:
+            raise SystemExit(
+                "FAIL: The '{}' module is not loaded!".format(module)
+            )
         else:
             print("PASS: It's loaded")
         print()
 
-    return exit_code
+    return 0
 
 
 def check_devices():
     ishtp_dir = "/sys/bus/ishtp/devices/"
 
     if not os.path.isdir(ishtp_dir):
-        print("ISHTP devices directory does not exist!")
-        print("The ISHTP folder: {}".format(ishtp_dir))
-        return 1
+        raise SystemExit(
+            "The ISHTP folder does not exist:  {}".format(ishtp_dir)
+        )
 
     devices = os.listdir(ishtp_dir)
     if not devices:
-        print("ISHTP devices directory empty - no devices found!")
-        print("The ISHTP folder: {}".format(ishtp_dir))
-        return 1
+        raise SystemExit(
+            "No devices found on the ISHTP folder:  {}".format(ishtp_dir)
+        )
 
     print("Found ishtp devices under {}:".format(ishtp_dir))
     for device in devices:
@@ -84,12 +71,8 @@ def check_devices():
     )
     args = parser.parse_args()
 
-    exit_code = 0
-
     if args.module:
-        exit_code |= check_modules()
+        check_modules()
 
     if args.device:
-        exit_code |= check_devices()
-
-    sys.exit(exit_code)
+        check_devices()

providers/base/tests/test_ishtp.py

@@ -4,39 +4,19 @@
 import os
 import subprocess
 from ishtp import (
-    get_release_version,
-    is_module_loaded,
+    get_module_list,
     check_modules,
     check_devices,
 )
 
 
 class TestISHTP(unittest.TestCase):
 
-    @patch("subprocess.check_output", return_value="24.04\n")
-    def test_get_release_version(self, mock_subproc):
-        self.assertEqual(get_release_version(), 24)
-
-    @patch("subprocess.check_output", return_value="somethingwrong\n")
-    def test_get_release_version_value_error(self, mock_subproc):
-        with self.assertRaises(SystemExit) as cm:
-            get_release_version()
-        self.assertEqual(cm.exception.code, 1)
-
-    @patch("subprocess.check_output", return_value="intel_ishtp 123 0\n")
-    def test_is_module_loaded(self, mock_subproc):
-        self.assertTrue(is_module_loaded("intel_ishtp"))
-
-    @patch("subprocess.check_output", return_value="")
-    def test_is_module_not_loaded(self, mock_subproc):
-        self.assertFalse(is_module_loaded("intel_ishtp"))
-
     @patch(
-        "subprocess.check_output",
-        side_effect=subprocess.CalledProcessError(1, "lsmod"),
+        "subprocess.check_output", return_value="module1\nnodule2\nmodule3\n"
     )
-    def test_is_module_loaded_error(self, mock_subproc):
-        self.assertFalse(is_module_loaded("intel_ishtp"))
+    def test_get_module_list(self, mock_subproc):
+        self.assertEqual(get_module_list(), ["module1", "nodule2", "module3"])
 
     @patch("os.path.isdir", return_value=True)
     @patch("os.listdir", return_value=["device1", "device2"])
@@ -45,22 +25,61 @@ def test_check_devices_success(self, mock_listdir, mock_isdir):
 
     @patch("os.path.isdir", return_value=False)
     def test_check_devices_no_directory(self, mock_isdir):
-        self.assertEqual(check_devices(), 1)
+        self.assertRaises(SystemExit)
 
     @patch("os.path.isdir", return_value=True)
     @patch("os.listdir", return_value=[])
     def test_check_devices_empty_directory(self, mock_listdir, mock_isdir):
-        self.assertEqual(check_devices(), 1)
+        self.assertRaises(SystemExit)
 
-    @patch("ishtp.is_module_loaded", return_value=True)
-    @patch("ishtp.get_release_version", return_value=24)
-    def test_check_modules_success(self, mock_release, mock_module):
+    @patch(
+        "ishtp.get_module_list",
+        return_value=["intel_ishtp_hid", "intel_ish_ipc", "intel_ishtp"],
+    )
+    @patch(
+        "checkbox_support.helpers.release_info.get_release_info",
+        return_value="24.04",
+    )
+    def test_check_modules_success_24(self, mock_release, mock_module):
         self.assertEqual(check_modules(), 0)
 
-    @patch("ishtp.is_module_loaded", return_value=False)
-    @patch("ishtp.get_release_version", return_value=24)
-    def test_check_modules_fail(self, mock_release, mock_module):
-        self.assertEqual(check_modules(), 1)
+    @patch(
+        "ishtp.get_module_list",
+        return_value=["intel_ishtp_hid", "intel_ish_ipc"],
+    )
+    @patch(
+        "checkbox_support.helpers.release_info.get_release_info",
+        return_value="24.04",
+    )
+    def test_check_modules_fail_24(self, mock_release, mock_module):
+        self.assertRaises(SystemExit)
+
+    @patch(
+        "ishtp.get_module_list",
+        return_value=[
+            "intel_ishtp_loader",
+            "intel_ishtp_hid",
+            "intel_ish_ipc",
+            "intel_ishtp",
+        ],
+    )
+    @patch(
+        "checkbox_support.helpers.release_info.get_release_info",
+        return_value="22.04",
+    )
+    def test_check_modules_success_22(self, mock_release, mock_module):
+        self.assertEqual(check_modules(), 0)
+
+    @patch(
+        "ishtp.get_module_list",
+        return_value=["intel_ishtp_hid", "intel_ish_ipc"],
+    )
+    @patch(
+        "checkbox_support.helpers.release_info.get_release_info",
+        return_value="22.04",
+    )
+    def test_check_modules_fail_22(self, mock_release, mock_module):
+        self.assertRaises(SystemExit)
 
 
 if __name__ == "__main__":