Add the missing sort for lsusb in reboot_check_test.py and fix unused…

… device comparison result (BugFix) (#1607)

* fix: missing sort in reboot_check_test.py

* fix: detailed diff

* fix: device comp result never used

* test: increase coverage

* feat: use -q to suppress fwts progress messages

* fix: restore stderr

providers/base/bin/reboot_check_test.py

@@ -54,15 +54,17 @@ def get_wireless_info(self) -> str:
         return "\n".join(map(lambda line: line.strip(), lines_to_write))
 
     def get_usb_info(self) -> str:
-        return sp.check_output(
+        out = sp.check_output(
             [
                 "checkbox-support-lsusb",
                 "-f",
                 '"{}"/var/lib/usbutils/usb.ids'.format(RUNTIME_ROOT),
                 "-s",
             ],
             universal_newlines=True,
-        )
+        ).splitlines()
+        out.sort()
+        return "\n".join(out)
 
     def get_pci_info(self) -> str:
         return sp.check_output(
@@ -97,6 +99,7 @@ def compare_device_lists(
                     "[ ERR ] The output of {} differs!".format(device),
                     file=sys.stderr,
                 )
+                self.print_diff(device, expected, actual)
                 return False
 
         for device in devices["optional"]:
@@ -107,6 +110,7 @@ def compare_device_lists(
                     "[ WARN ] Items under {} have changed.".format(actual),
                     file=sys.stderr,
                 )
+                self.print_diff(device, expected, actual)
 
         return True
 
@@ -131,6 +135,16 @@ def dump(
 
         os.sync()
 
+    def print_diff(self, name: str, expected_path: str, actual_path: str):
+        with open(expected_path) as file_expected, open(
+            actual_path
+        ) as file_actual:
+            print("Expected {} output:".format(name), file=sys.stderr)
+            print(file_expected.read(), file=sys.stderr)
+            print("Actual {} output:".format(name), file=sys.stderr)
+            print(file_actual.read(), file=sys.stderr)
+            print("End of {} diff".format(name), file=sys.stderr)
+
     def __init__(self) -> None:
         self.dump_function = {
             self.Device.PCI: self.get_pci_info,
@@ -159,7 +173,7 @@ def fwts_log_check_passed(
         log_file_path = "{}/fwts_{}.log".format(
             output_directory, "_".join(fwts_arguments)
         )
-        sp.run(["fwts", "-r", log_file_path, *fwts_arguments])
+        sp.run(["fwts", "-r", log_file_path, "-q", *fwts_arguments])
         result = sp.run(
             [
                 "sleep_test_log_check.py",
@@ -248,8 +262,9 @@ def is_hardware_renderer_available(self) -> bool:
         )
         if unity_support_output.returncode != 0:
             print(
-                "[ ERR ] unity support test returned {}".format(
-                    unity_support_output.returncode
+                "[ ERR ] unity support test returned {}. Error is: {}".format(
+                    unity_support_output.returncode,
+                    unity_support_output.stdout,
                 ),
                 file=sys.stderr,
             )
@@ -390,11 +405,10 @@ def main() -> int:
     if args.comparison_directory is not None:
         if args.output_directory is None:
             print(
-                "[ ERR ] Please specify an output directory with the -d flag.",
-                file=sys.stderr,
+                "[ ERR ] Please specify an output directory with the -d flag."
             )
             raise ValueError(
-                "Cmoparison directory is specified, but output directory isn't"
+                "Comparison directory is specified, but output directory isn't"
             )
         else:
             collector = DeviceInfoCollector()
@@ -403,6 +417,8 @@ def main() -> int:
                 args.comparison_directory, args.output_directory
             ):
                 print("[ OK ] Devices match!")
+            else:
+                device_comparison_passed = False
 
     # dump (no checks) if only output_directory is specified
     if args.output_directory is not None and args.comparison_directory is None:

providers/base/tests/test_reboot_check_test.py

@@ -302,13 +302,16 @@ def test_partial_main(self, mock_run: MagicMock):
         ), patch(
             "reboot_check_test.DeviceInfoCollector.compare_device_lists"
         ) as mock_compare:
-            RCT.main()
+            mock_compare.return_value = False
+
+            rv = RCT.main()
 
             self.assertEqual(
                 mock_run.call_count,
                 len(RCT.DeviceInfoCollector.DEFAULT_DEVICES["required"]),
             )  # only lspci, lsusb, iw calls
             self.assertEqual(mock_compare.call_count, 1)
+            self.assertEqual(rv, 1)
 
     @patch("subprocess.run")
     def test_main_function_full(self, mock_run: MagicMock):