more fixes

Author: amilcarlucas

ardupilot_methodic_configurator/frontend_tkinter_component_editor_base.py

@@ -70,7 +70,7 @@ def argument_parser() -> Namespace:
 VEICLE_IMAGE_WIDTH_PIX = 100
 
 
-class ComponentEditorWindowBase(BaseWindow):
+class ComponentEditorWindowBase(BaseWindow):  # pylint: disable=too-many-instance-attributes
     """
     A class for editing JSON files in the ArduPilot methodic configurator.
 
@@ -298,33 +298,55 @@ def _add_widget(self, parent: tk.Widget, key: str, value: ComponentValue, path:
         else:  # JSON leaf elements, add Entry widget
             self._add_leaf_widget(parent, key, value, path)
 
-    def _add_non_leaf_widget(self, parent: tk.Widget, key: str, value: dict, path: list[str]) -> None:
-        """Add a non-leaf widget (frame containing other widgets) to the UI."""
-        is_toplevel = parent == self.scroll_frame.view_port
-        pady = 5 if is_toplevel else 3
-
+    def _prepare_non_leaf_widget_config(self, key: str, value: dict, path: list[str]) -> dict:
+        """Prepare configuration for non-leaf widget creation. Pure function for easy testing."""
+        is_toplevel = len(path) == 0  # More explicit than checking parent type
         current_path = (*path, key)
         description, is_optional = self.local_filesystem.get_component_property_description(current_path)
         description = _(description) if description else ""
 
-        if is_optional:
-            frame = ttk.LabelFrame(parent, text=_(key), style="Optional.TLabelframe")
+        # Enhance tooltip for optional fields
+        if is_optional and description:
+            description += _("\nThis is optional and can be left blank")
+
+        return {
+            "key": key,
+            "value": value,
+            "path": current_path,
+            "description": description,
+            "is_optional": is_optional,
+            "is_toplevel": is_toplevel,
+            "pady": 5 if is_toplevel else 3,
+        }
+
+    def _create_non_leaf_widget_ui(self, parent: tk.Widget, config: dict) -> ttk.LabelFrame:
+        """Create the UI elements for a non-leaf widget. Separated for better testability."""
+        if config["is_optional"]:
+            frame = ttk.LabelFrame(parent, text=_(config["key"]), style="Optional.TLabelframe")
         else:
-            frame = ttk.LabelFrame(parent, text=_(key))
+            frame = ttk.LabelFrame(parent, text=_(config["key"]))
 
         frame.pack(
-            fill=tk.X, side=tk.TOP if is_toplevel else tk.LEFT, pady=pady, padx=5, anchor=tk.NW if is_toplevel else tk.N
+            fill=tk.X,
+            side=tk.TOP if config["is_toplevel"] else tk.LEFT,
+            pady=config["pady"],
+            padx=5,
+            anchor=tk.NW if config["is_toplevel"] else tk.N,
         )
 
-        # Enhance tooltip for optional fields
-        if is_optional and description:
-            description += _("\nThis is optional and can be left blank")
-
         # Add tooltip based on schema description
-        if description:
-            show_tooltip(frame, description, position_below=False)
+        if config["description"]:
+            show_tooltip(frame, config["description"], position_below=False)
 
-        if is_toplevel and key in self.data_model.get_all_components():
+        return frame
+
+    def _add_non_leaf_widget(self, parent: tk.Widget, key: str, value: dict, path: list[str]) -> None:
+        """Add a non-leaf widget (frame containing other widgets) to the UI."""
+        # Extract business logic to separate method for better testability
+        widget_config = self._prepare_non_leaf_widget_config(key, value, path)
+        frame = self._create_non_leaf_widget_ui(parent, widget_config)
+
+        if widget_config["is_toplevel"] and key in self.data_model.get_all_components():
             self._add_template_controls(frame, key)
 
         for sub_key, sub_value in value.items():
@@ -459,7 +481,7 @@ def create_for_testing(
         version: str = "test",
         local_filesystem: Optional[LocalFilesystem] = None,
         data_model: Optional[ComponentDataModel] = None,
-        root_tk: Optional[tk.Tk] = None,
+        root_tk: Optional[tk.Tk] = None,  # pylint: disable=unused-argument
     ) -> "ComponentEditorWindowBase":
         """
         Factory method for creating instances suitable for testing.
@@ -478,7 +500,7 @@ def create_for_testing(
 
         """
         # Import MagicMock at the method level to avoid import issues
-        from unittest.mock import MagicMock
+        from unittest.mock import MagicMock  # pylint: disable=import-outside-toplevel
 
         if local_filesystem is None:
             # Create a minimal mock filesystem for testing

tests/test_frontend_tkinter_component_editor_base.py

@@ -16,8 +16,10 @@
 from unittest.mock import MagicMock, patch
 
 import pytest
+from test_data_model_vehicle_components_common import REALISTIC_VEHICLE_DATA
 
 from ardupilot_methodic_configurator.backend_filesystem import LocalFilesystem
+from ardupilot_methodic_configurator.backend_filesystem_vehicle_components import VehicleComponents
 from ardupilot_methodic_configurator.data_model_vehicle_components import ComponentDataModel
 from ardupilot_methodic_configurator.frontend_tkinter_component_editor_base import (
     VEICLE_IMAGE_WIDTH_PIX,
@@ -30,6 +32,106 @@
 # pylint: disable=protected-access
 
 
+def setup_common_editor_mocks(editor) -> ComponentEditorWindowBase:
+    """Set up common mock attributes and methods for editor fixtures."""
+    # Set up all required attributes manually
+    editor.root = MagicMock()
+    editor.main_frame = MagicMock()
+    editor.scroll_frame = MagicMock()
+    editor.scroll_frame.view_port = MagicMock()
+    editor.version = "1.0.0"
+
+    # Mock filesystem and methods with proper schema loading
+    editor.local_filesystem = MagicMock(spec=LocalFilesystem)
+    editor.local_filesystem.vehicle_dir = "dummy_vehicle_dir"
+    editor.local_filesystem.get_component_property_description = MagicMock(return_value=("Test description", False))
+    editor.local_filesystem.vehicle_image_exists = MagicMock(return_value=False)
+    editor.local_filesystem.vehicle_image_filepath = MagicMock(return_value="test.jpg")
+    editor.local_filesystem.save_component_to_system_templates = MagicMock()
+
+    # Mock the vehicle_components attribute
+    mock_vehicle_components = MagicMock()
+    # Make schema loading return a valid empty schema
+    mock_vehicle_components.load_schema.return_value = {"properties": {}}
+    mock_vehicle_components.get_component_property_description = MagicMock(return_value=("Test description", False))
+    editor.local_filesystem.vehicle_components = mock_vehicle_components
+
+    # Setup test data and data model
+    editor.entry_widgets = {}
+
+    # Create data model with realistic test data
+    vehicle_components = VehicleComponents()
+    component_datatypes = vehicle_components.get_all_value_datatypes()
+    editor.data_model = ComponentDataModel(REALISTIC_VEHICLE_DATA, component_datatypes)
+
+    # Mock specific methods that are used in tests
+    editor.data_model.set_component_value = MagicMock()
+    editor.data_model.update_component = MagicMock()
+
+    # Override methods that might cause UI interactions in tests
+    # Mock _add_widget completely to avoid UI creation
+    editor._add_widget = MagicMock()
+    editor.put_image_in_label = MagicMock(return_value=MagicMock())
+    editor.add_entry_or_combobox = MagicMock(return_value=MagicMock())
+
+    return editor
+
+
+def add_editor_helper_methods(editor) -> None:
+    """Add common helper methods for testing that bypass UI operations."""
+
+    # Use add_widget as a public proxy for _add_widget for easier testing
+    def add_widget_proxy(parent, key, value, path) -> None:
+        return editor._add_widget(parent, key, value, path)
+
+    editor.add_widget = add_widget_proxy
+
+    # Add helper methods for testing that bypass UI operations
+    def test_populate_frames() -> None:
+        # This is a test-friendly version that doesn't involve actual UI widgets
+        components = editor.data_model.get_all_components()
+        for key, value in components.items():
+            editor._add_widget(editor.scroll_frame.view_port, key, value, [])
+
+    editor.populate_frames = test_populate_frames
+
+
+class SharedTestArgumentParser:
+    """Shared test cases for the argument_parser function to avoid duplication."""
+
+    def test_argument_parser(self) -> None:
+        """Test argument_parser function."""
+        with patch("sys.argv", ["test_script", "--vehicle-dir", "test_dir", "--vehicle-type", "ArduCopter"]):
+            args = argument_parser()
+
+            assert hasattr(args, "vehicle_dir")
+            assert hasattr(args, "vehicle_type")
+            assert hasattr(args, "skip_component_editor")
+
+    def test_argument_parser_with_skip_component_editor(self) -> None:
+        """Test argument_parser with skip-component-editor flag."""
+        with patch(
+            "sys.argv", ["test_script", "--vehicle-dir", "test_dir", "--vehicle-type", "ArduCopter", "--skip-component-editor"]
+        ):
+            args = argument_parser()
+
+            assert args.skip_component_editor is True
+
+
+@pytest.fixture
+def editor_with_mocked_root() -> ComponentEditorWindowBase:
+    """Create a mock ComponentEditorWindowBase for testing."""
+    # Create the class without initialization
+    with patch.object(ComponentEditorWindowBase, "__init__", return_value=None):
+        editor = ComponentEditorWindowBase()  # pylint: disable=no-value-for-parameter
+
+        # Set up common mocks and helper methods
+        setup_common_editor_mocks(editor)
+        add_editor_helper_methods(editor)
+
+        yield editor
+
+
 class TestArgumentParserBehavior:
     """Test argument parser behavior and functionality."""