Rename ManifestBrowser.get_flattened_values, apply the same fix for _save_manifest in RemoteController, make tests more readable.

Author: gntzio

checkbox-ng/checkbox_ng/launcher/controller.py

@@ -628,24 +628,20 @@ def _save_manifest(self, interactive):
         if not manifest_repr:
             _logger.info("Skipping saving of the manifest")
             return
-        if interactive:
+
+        if interactive and ManifestBrowser.has_visible_manifests(
+            manifest_repr
+        ):
             # Ask the user the values
             to_save_manifest = ManifestBrowser(
                 "System Manifest:", manifest_repr
             ).run()
         else:
-            # Use the one provided in repr
-            # repr is question : [manifests]
-            #   manifest ex m1 is [conf_m1_1, conf_m1_2, ...]
-            # here we recover [conf_m1_1, conf_m1_2, ..., conf_m2_1, ...]
-            all_preconf = (
-                conf
-                for conf_list in manifest_repr.values()
-                for conf in conf_list
+            # Use the one provided in repr (either non-interactive or no visible manifests)
+            to_save_manifest = ManifestBrowser.get_flattened_values(
+                manifest_repr
             )
-            to_save_manifest = {
-                conf["id"]: conf["value"] for conf in all_preconf
-            }
+
         self.sa.save_manifest_json(json.dumps(to_save_manifest))
 
     def select_jobs(self, all_jobs):

checkbox-ng/checkbox_ng/launcher/subcommands.py

@@ -702,7 +702,7 @@ def _save_manifest(self, interactive):
             ).run()
         else:
             # Use the one provided in repr (either non-interactive or no visible manifests)
-            to_save_manifest = ManifestBrowser.get_default_values(
+            to_save_manifest = ManifestBrowser.get_flattened_values(
                 manifest_repr
             )
 

checkbox-ng/checkbox_ng/launcher/test_controller.py

@@ -28,7 +28,7 @@
 from checkbox_ng.launcher.controller import is_hostname_a_loopback
 
 
-class ControllerTests(TestCase):
+class TestRemoteController(TestCase):
     @mock.patch("checkbox_ng.launcher.controller.is_hostname_a_loopback")
     @mock.patch("time.time")
     @mock.patch("builtins.print")
@@ -1063,54 +1063,98 @@ def test_start_session_runtime_error(self):
         with self.assertRaises(SystemExit) as _:
             RemoteController.start_session(self_mock)
 
-    def test__save_manifest_no_repr(self):
-        self_mock = mock.MagicMock()
-        self_mock.sa.get_manifest_repr_json.return_value = "{}"
-        RemoteController._save_manifest(self_mock, False)
-
     @mock.patch("checkbox_ng.launcher.controller.ManifestBrowser")
-    def test__save_manifest_interactive(self, manifest_browser_mock):
-        self_mock = mock.MagicMock()
-        manifest_repr = {"Question 1": [{"id": "conf1", "value": "val1"}]}
-        self_mock.sa.get_manifest_repr_json.return_value = json.dumps(
-            manifest_repr
-        )
-        to_save_manifest = {"conf1": "new_val"}
-        manifest_browser_mock.return_value.run.return_value = to_save_manifest
+    def test__save_manifest_interactive_with_visible_manifests(
+        self, mock_browser_class
+    ):
+        controller = RemoteController()
+        sa_mock = mock.MagicMock()
+        controller._sa = sa_mock
 
-        RemoteController._save_manifest(self_mock, True)
+        manifest_repr = {
+            "section1": [
+                {"id": "visible1", "value": 0, "hidden": False},
+                {"id": "visible2", "value": False, "hidden": False},
+            ]
+        }
+        sa_mock.get_manifest_repr_json.return_value = json.dumps(manifest_repr)
 
-        manifest_browser_mock.assert_called_once_with(
-            "System Manifest:", manifest_repr
-        )
-        self_mock.sa.save_manifest_json.assert_called_once_with(
-            json.dumps(to_save_manifest)
+        mock_browser = mock.MagicMock()
+        mock_browser.run.return_value = {
+            "visible1": 5,
+            "visible2": True,
+        }
+        mock_browser_class.return_value = mock_browser
+        mock_browser_class.has_visible_manifests.return_value = True
+
+        controller._save_manifest(interactive=True)
+
+        sa_mock.save_manifest_json.assert_called_with(
+            json.dumps({"visible1": 5, "visible2": True})
         )
 
-    def test__save_manifest_non_interactive(self):
-        self_mock = mock.MagicMock()
+    @mock.patch("checkbox_ng.launcher.controller.ManifestBrowser")
+    def test__save_manifest_interactive_no_visible_manifests(
+        self, mock_browser_class
+    ):
+        controller = RemoteController()
+        sa_mock = mock.MagicMock()
+        controller._sa = sa_mock
+
         manifest_repr = {
-            "Question 1": [
-                {"id": "conf1", "value": "val1"},
-                {"id": "conf2", "value": "val2"},
-            ],
-            "Question 2": [{"id": "conf3", "value": "val3"}],
+            "section1": [
+                {"id": "hidden1", "value": True, "hidden": True},
+                {"id": "hidden2", "value": 2, "hidden": True},
+            ]
+        }
+        sa_mock.get_manifest_repr_json.return_value = json.dumps(manifest_repr)
+        mock_browser_class.has_visible_manifests.return_value = False
+        mock_browser_class.get_flattened_values.return_value = {
+            "hidden1": True,
+            "hidden2": 2,
         }
-        self_mock.sa.get_manifest_repr_json.return_value = json.dumps(
-            manifest_repr
+
+        controller._save_manifest(interactive=True)
+
+        self.assertEqual(mock_browser_class.call_count, 0)
+        self.assertEqual(
+            mock_browser_class.has_visible_manifests.call_count, 1
+        )
+        self.assertEqual(mock_browser_class.get_flattened_values.call_count, 1)
+        self.assertEqual(sa_mock.save_manifest_json.call_count, 1)
+        sa_mock.save_manifest_json.assert_called_with(
+            json.dumps({"hidden1": True, "hidden2": 2})
         )
 
-        RemoteController._save_manifest(self_mock, False)
+    @mock.patch("checkbox_ng.launcher.controller.ManifestBrowser")
+    def test__save_manifest_non_interactive(self, mock_browser_class):
+        controller = RemoteController()
+        sa_mock = mock.MagicMock()
+        controller._sa = sa_mock
 
-        expected_to_save = {
-            "conf1": "val1",
-            "conf2": "val2",
-            "conf3": "val3",
+        manifest_repr = {
+            "section1": [
+                {"id": "manifest1", "value": False, "hidden": False},
+                {"id": "manifest2", "value": 7, "hidden": True},
+            ]
+        }
+        sa_mock.get_manifest_repr_json.return_value = json.dumps(manifest_repr)
+        mock_browser_class.get_flattened_values.return_value = {
+            "manifest1": False,
+            "manifest2": 7,
         }
-        self_mock.sa.save_manifest_json.assert_called_once_with(
-            json.dumps(expected_to_save)
+
+        controller._save_manifest(interactive=False)
+
+        sa_mock.save_manifest_json.assert_called_with(
+            json.dumps({"manifest1": False, "manifest2": 7})
         )
 
+    def test__save_manifest_no_repr(self):
+        self_mock = mock.MagicMock()
+        self_mock.sa.get_manifest_repr_json.return_value = "{}"
+        RemoteController._save_manifest(self_mock, False)
+
     def test_select_jobs_forced_with_manifest(self):
         self_mock = mock.MagicMock()
         self_mock.launcher.get_value.return_value = True

checkbox-ng/checkbox_ng/launcher/test_subcommands.py

@@ -808,26 +808,24 @@ def test__save_manifest_interactive_with_visible_manifests(
 
         manifest_repr = {
             "section1": [
-                {"id": "visible1", "value": "default1", "hidden": False},
-                {"id": "visible2", "value": "default2", "hidden": False},
+                {"id": "visible1", "value": 0, "hidden": False},
+                {"id": "visible2", "value": False, "hidden": False},
             ]
         }
         ctx_mock.sa.get_manifest_repr.return_value = manifest_repr
 
         mock_browser = MagicMock()
         mock_browser.run.return_value = {
-            "visible1": "user_value1",
-            "visible2": "user_value2",
+            "visible1": 5,
+            "visible2": True,
         }
         mock_browser_class.return_value = mock_browser
         mock_browser_class.has_visible_manifests.return_value = True
 
         launcher._save_manifest(interactive=True)
 
-        self.assertEqual(mock_browser.run.call_count, 1)
-        self.assertEqual(ctx_mock.sa.save_manifest.call_count, 1)
         ctx_mock.sa.save_manifest.assert_called_with(
-            {"visible1": "user_value1", "visible2": "user_value2"}
+            {"visible1": 5, "visible2": True}
         )
 
     @patch("checkbox_ng.launcher.subcommands.ManifestBrowser")
@@ -840,27 +838,21 @@ def test__save_manifest_interactive_no_visible_manifests(
 
         manifest_repr = {
             "section1": [
-                {"id": "hidden1", "value": "default1", "hidden": True},
-                {"id": "hidden2", "value": "default2", "hidden": True},
+                {"id": "hidden1", "value": True, "hidden": True},
+                {"id": "hidden2", "value": 2, "hidden": True},
             ]
         }
         ctx_mock.sa.get_manifest_repr.return_value = manifest_repr
         mock_browser_class.has_visible_manifests.return_value = False
-        mock_browser_class.get_default_values.return_value = {
-            "hidden1": "default1",
-            "hidden2": "default2",
+        mock_browser_class.get_flattened_values.return_value = {
+            "hidden1": True,
+            "hidden2": 2,
         }
 
         launcher._save_manifest(interactive=True)
 
-        self.assertEqual(mock_browser_class.call_count, 0)
-        self.assertEqual(
-            mock_browser_class.has_visible_manifests.call_count, 1
-        )
-        self.assertEqual(mock_browser_class.get_default_values.call_count, 1)
-        self.assertEqual(ctx_mock.sa.save_manifest.call_count, 1)
         ctx_mock.sa.save_manifest.assert_called_with(
-            {"hidden1": "default1", "hidden2": "default2"}
+            {"hidden1": True, "hidden2": 2}
         )
 
     @patch("checkbox_ng.launcher.subcommands.ManifestBrowser")
@@ -871,27 +863,20 @@ def test__save_manifest_non_interactive(self, mock_browser_class):
 
         manifest_repr = {
             "section1": [
-                {"id": "manifest1", "value": "default1", "hidden": False},
-                {"id": "manifest2", "value": "default2", "hidden": True},
+                {"id": "manifest1", "value": False, "hidden": False},
+                {"id": "manifest2", "value": 7, "hidden": True},
             ]
         }
         ctx_mock.sa.get_manifest_repr.return_value = manifest_repr
-        mock_browser_class.get_default_values.return_value = {
-            "manifest1": "default1",
-            "manifest2": "default2",
+        mock_browser_class.get_flattened_values.return_value = {
+            "manifest1": False,
+            "manifest2": 7,
         }
 
         launcher._save_manifest(interactive=False)
 
-        self.assertEqual(mock_browser_class.call_count, 0)
-        self.assertEqual(
-            mock_browser_class.has_visible_manifests.call_count, 0
-        )
-        self.assertEqual(mock_browser_class.get_default_values.call_count, 1)
-        mock_browser_class.get_default_values.assert_called_with(manifest_repr)
-        self.assertEqual(ctx_mock.sa.save_manifest.call_count, 1)
         ctx_mock.sa.save_manifest.assert_called_with(
-            {"manifest1": "default1", "manifest2": "default2"}
+            {"manifest1": False, "manifest2": 7}
         )
 
 
@@ -1291,107 +1276,3 @@ def find_children_by_name(pattern):
         # we expect the relevant unit function to leave unfound values the same
         # and all in the same order
         self.assertEqual(found_ids, ["other2.*", *should_find, "other1.*"])
-
-
-    def setUp(self):
-        self.launcher = Launcher()
-        self.ctx_mock = MagicMock()
-        self.launcher.ctx = self.ctx_mock
-
-    def test__save_manifest_no_or_empty_manifest_repr(self):
-
-        cases = [
-            ("None", None),
-            ("Empty", {}),
-        ]
-
-        for case_name, manifest_repr in cases:
-            with self.subTest(case=case_name):
-                self.ctx_mock.sa.get_manifest_repr.return_value = manifest_repr
-                self.launcher._save_manifest(interactive=True)
-                self.assertEqual(self.ctx_mock.sa.save_manifest.call_count, 0)
-
-    @patch("checkbox_ng.launcher.subcommands.ManifestBrowser")
-    def test__save_manifest_interactive_with_visible_manifests(
-        self, mock_browser_class
-    ):
-        manifest_repr = {
-            "section1": [
-                {"id": "visible1", "value": "default1", "hidden": False},
-                {"id": "visible2", "value": "default2", "hidden": False},
-            ]
-        }
-        self.ctx_mock.sa.get_manifest_repr.return_value = manifest_repr
-
-        mock_browser = MagicMock()
-        mock_browser.run.return_value = {
-            "visible1": "user_value1",
-            "visible2": "user_value2",
-        }
-        mock_browser_class.return_value = mock_browser
-        mock_browser_class.has_visible_manifests.return_value = True
-
-        self.launcher._save_manifest(interactive=True)
-
-        self.assertEqual(mock_browser.run.call_count, 1)
-        self.assertEqual(self.ctx_mock.sa.save_manifest.call_count, 1)
-        self.ctx_mock.sa.save_manifest.assert_called_with(
-            {"visible1": "user_value1", "visible2": "user_value2"}
-        )
-
-    @patch("checkbox_ng.launcher.subcommands.ManifestBrowser")
-    def test__save_manifest_interactive_no_visible_manifests(
-        self, mock_browser_class
-    ):
-        manifest_repr = {
-            "section1": [
-                {"id": "hidden1", "value": "default1", "hidden": True},
-                {"id": "hidden2", "value": "default2", "hidden": True},
-            ]
-        }
-        self.ctx_mock.sa.get_manifest_repr.return_value = manifest_repr
-        mock_browser_class.has_visible_manifests.return_value = False
-        mock_browser_class.get_default_values.return_value = {
-            "hidden1": "default1",
-            "hidden2": "default2",
-        }
-
-        self.launcher._save_manifest(interactive=True)
-
-        self.assertEqual(mock_browser_class.call_count, 0)
-        self.assertEqual(
-            mock_browser_class.has_visible_manifests.call_count, 1
-        )
-        self.assertEqual(mock_browser_class.get_default_values.call_count, 1)
-        self.assertEqual(self.ctx_mock.sa.save_manifest.call_count, 1)
-        self.ctx_mock.sa.save_manifest.assert_called_with(
-            {"hidden1": "default1", "hidden2": "default2"}
-        )
-
-    @patch("checkbox_ng.launcher.subcommands.ManifestBrowser")
-    def test__save_manifest_non_interactive(self, mock_browser_class):
-        """Test _save_manifest in non-interactive mode."""
-        manifest_repr = {
-            "section1": [
-                {"id": "manifest1", "value": "default1", "hidden": False},
-                {"id": "manifest2", "value": "default2", "hidden": True},
-            ]
-        }
-        self.ctx_mock.sa.get_manifest_repr.return_value = manifest_repr
-        mock_browser_class.get_default_values.return_value = {
-            "manifest1": "default1",
-            "manifest2": "default2",
-        }
-
-        self.launcher._save_manifest(interactive=False)
-
-        self.assertEqual(mock_browser_class.call_count, 0)
-        self.assertEqual(
-            mock_browser_class.has_visible_manifests.call_count, 0
-        )
-        self.assertEqual(mock_browser_class.get_default_values.call_count, 1)
-        mock_browser_class.get_default_values.assert_called_with(manifest_repr)
-        self.assertEqual(self.ctx_mock.sa.save_manifest.call_count, 1)
-        self.ctx_mock.sa.save_manifest.assert_called_with(
-            {"manifest1": "default1", "manifest2": "default2"}
-        )