refactor: improve test coverage for help_inspector and device_manager_view

bec_widgets/applications/views/device_manager_view/device_manager_view.py

@@ -2,7 +2,7 @@ from __future__ import annotations
 
 import os
 from functools import partial
-from typing import List
+from typing import List, Literal
 
 import PySide6QtAds as QtAds
 import yaml
@@ -443,10 +443,20 @@ class DeviceManagerView(BECWidget, QWidget):
 
         # Implement the file loading logic here
         start_dir = os.path.abspath(config_path)
-        file_path, _ = QFileDialog.getOpenFileName(
-            self, caption="Select Config File", dir=start_dir
-        )
-        self._load_config_from_file(file_path)
+        file_path = self._get_file_path(start_dir, "open_file")
+        if file_path:
+            self._load_config_from_file(file_path)
+
+    def _get_file_path(self, start_dir: str, mode: Literal["open_file", "save_file"]) -> str:
+        if mode == "open_file":
+            file_path, _ = QFileDialog.getOpenFileName(
+                self, caption="Select Config File", dir=start_dir
+            )
+        else:
+            file_path, _ = QFileDialog.getSaveFileName(
+                self, caption="Save Config File", dir=start_dir
+            )
+        return file_path
 
     def _load_config_from_file(self, file_path: str):
         """
@@ -460,6 +470,15 @@ class DeviceManagerView(BECWidget, QWidget):
         except Exception as e:
             logger.error(f"Failed to load config from file {file_path}. Error: {e}")
             return
+        self._open_config_choice_dialog(config)
+
+    def _open_config_choice_dialog(self, config: List[dict]):
+        """
+        Open a dialog to choose whether to replace or add the loaded config.
+
+        Args:
+            config (List[dict]): List of device configurations loaded from the file.
+        """
         dialog = ConfigChoiceDialog(self)
         if dialog.exec():
             if dialog.result() == ConfigChoiceDialog.REPLACE:
@@ -484,7 +503,7 @@ class DeviceManagerView(BECWidget, QWidget):
             return
 
     @SafeSlot()
-    def _update_redis_action(self):
+    def _update_redis_action(self) -> None | QMessageBox.StandardButton:
         """Action to push the current composition to Redis"""
         reply = _yes_no_question(
             self,
@@ -521,9 +540,7 @@ class DeviceManagerView(BECWidget, QWidget):
             logger.warning(f"Failed to find recovery config path, fallback to: {config_path}")
 
         # Implement the file loading logic here
-        file_path, _ = QFileDialog.getSaveFileName(
-            self, caption="Save Config File", dir=config_path
-        )
+        file_path = self._get_file_path(config_path, "save_file")
         if file_path:
             config = {cfg.pop("name"): cfg for cfg in self.device_table_view.get_device_config()}
             with open(file_path, "w") as file:

bec_widgets/utils/help_inspector/help_inspector.py

@@ -128,7 +128,8 @@ class HelpInspector(BECWidget, QtWidgets.QWidget):
                 # Get BECWidget ancestor
                 # TODO check what happens if the HELP Inspector itself is embedded in another BECWidget
                 # I suppose we would like to get the first ancestor that is a BECWidget, not the topmost one
-                widget = WidgetHierarchy._get_becwidget_ancestor(widget)
+                if not isinstance(widget, BECWidget):
+                    widget = WidgetHierarchy._get_becwidget_ancestor(widget)
                 if widget:
                     if widget is self:
                         self._toggle_mode(False)

tests/unit_tests/test_device_manager_view.py

@@ -1,11 +1,18 @@
 """Unit tests for the device manager view"""
 
+# pylint: disable=protected-access,redefined-outer-name
+
 from unittest import mock
 
 import pytest
 from qtpy import QtCore
+from qtpy.QtWidgets import QFileDialog, QMessageBox
 
-from bec_widgets.applications.views.device_manager_view.device_manager_view import DeviceManagerView
+from bec_widgets.applications.views.device_manager_view.device_manager_view import (
+    ConfigChoiceDialog,
+    DeviceManagerView,
+)
+from bec_widgets.utils.help_inspector.help_inspector import HelpInspector
 from bec_widgets.widgets.control.device_manager.components import (
     DeviceTableView,
     DMConfigView,
@@ -23,70 +30,195 @@ def dm_view(qtbot):
     yield widget
 
 
-def test_device_manager_view_initialization(dm_view):
-    """Test the basic layout of QtAds DockManager."""
-    assert isinstance(dm_view.dock_manager.centralWidget().widget(), DeviceTableView)
-    assert any(
-        isinstance(dock.widget(), DMConfigView) for dock in dm_view.dock_manager.dockWidgets()
-    )
-    assert any(
-        isinstance(dock.widget(), DMOphydTest) for dock in dm_view.dock_manager.dockWidgets()
-    )
-    assert any(
-        isinstance(dock.widget(), DocstringView) for dock in dm_view.dock_manager.dockWidgets()
+@pytest.fixture
+def config_choice_dialog(qtbot, dm_view):
+    """Fixture for ConfigChoiceDialog."""
+    dialog = ConfigChoiceDialog(dm_view)
+    qtbot.addWidget(dialog)
+    qtbot.waitExposed(dialog)
+    yield dialog
+
+
+def test_device_manager_view_config_choice_dialog(qtbot, dm_view, config_choice_dialog):
+    """Test the configuration choice dialog."""
+    assert config_choice_dialog is not None
+    assert config_choice_dialog.parent() == dm_view
+
+    # Test dialog components
+    with (
+        mock.patch.object(config_choice_dialog, "accept") as mock_accept,
+        mock.patch.object(config_choice_dialog, "reject") as mock_reject,
+    ):
+
+        # Replace
+        qtbot.mouseClick(config_choice_dialog.replace_btn, QtCore.Qt.LeftButton)
+        mock_accept.assert_called_once()
+        mock_reject.assert_not_called()
+        mock_accept.reset_mock()
+        assert config_choice_dialog.result() == config_choice_dialog.REPLACE
+        # Add
+        qtbot.mouseClick(config_choice_dialog.add_btn, QtCore.Qt.LeftButton)
+        mock_accept.assert_called_once()
+        mock_reject.assert_not_called()
+        mock_accept.reset_mock()
+        assert config_choice_dialog.result() == config_choice_dialog.ADD
+        # Cancel
+        qtbot.mouseClick(config_choice_dialog.cancel_btn, QtCore.Qt.LeftButton)
+        mock_accept.assert_not_called()
+        mock_reject.assert_called_once()
+        assert config_choice_dialog.result() == config_choice_dialog.CANCEL
+
+
+class TestDeviceManagerViewInitialization:
+    """Test class for DeviceManagerView initialization and basic components."""
+
+    def test_dock_manager_initialization(self, dm_view):
+        """Test that the QtAds DockManager is properly initialized."""
+        assert dm_view.dock_manager is not None
+        assert dm_view.dock_manager.centralWidget() is not None
+
+    def test_central_widget_is_device_table_view(self, dm_view):
+        """Test that the central widget is DeviceTableView."""
+        central_widget = dm_view.dock_manager.centralWidget().widget()
+        assert isinstance(central_widget, DeviceTableView)
+        assert central_widget is dm_view.device_table_view
+
+    def test_dock_widgets_exist(self, dm_view):
+        """Test that all required dock widgets are created."""
+        dock_widgets = dm_view.dock_manager.dockWidgets()
+
+        # Check that we have the expected number of dock widgets
+        assert len(dock_widgets) >= 4
+
+        # Check for specific widget types
+        widget_types = [dock.widget().__class__ for dock in dock_widgets]
+
+        assert DMConfigView in widget_types
+        assert DMOphydTest in widget_types
+        assert DocstringView in widget_types
+
+    def test_toolbar_initialization(self, dm_view):
+        """Test that the toolbar is properly initialized with expected bundles."""
+        assert dm_view.toolbar is not None
+        assert "IO" in dm_view.toolbar.bundles
+        assert "Table" in dm_view.toolbar.bundles
+
+    def test_toolbar_components_exist(self, dm_view):
+        """Test that all expected toolbar components exist."""
+        expected_components = [
+            "load",
+            "save_to_disk",
+            "load_redis",
+            "update_config_redis",
+            "reset_composed",
+            "add_device",
+            "remove_device",
+            "rerun_validation",
+        ]
+
+        for component in expected_components:
+            assert dm_view.toolbar.components.exists(component)
+
+    def test_signal_connections(self, dm_view):
+        """Test that signals are properly connected between components."""
+        # Test that device_table_view signals are connected
+        assert dm_view.device_table_view.selected_devices is not None
+        assert dm_view.device_table_view.device_configs_changed is not None
+
+        # Test that ophyd_test_view signals are connected
+        assert dm_view.ophyd_test_view.device_validated is not None
+
+
+class TestDeviceManagerViewIOBundle:
+    """Test class for DeviceManagerView IO bundle actions."""
+
+    def test_io_bundle_exists(self, dm_view):
+        """Test that IO bundle exists and contains expected actions."""
+        assert "IO" in dm_view.toolbar.bundles
+        io_actions = ["load", "save_to_disk", "load_redis", "update_config_redis"]
+        for action in io_actions:
+            assert dm_view.toolbar.components.exists(action)
+
+    def test_load_file_action_triggered(self, tmp_path, dm_view):
+        """Test load file action trigger mechanism."""
+
+        with (
+            mock.patch.object(dm_view, "_get_file_path", return_value=tmp_path),
+            mock.patch(
+                "bec_widgets.applications.views.device_manager_view.device_manager_view.yaml_load"
+            ) as mock_yaml_load,
+            mock.patch.object(dm_view, "_open_config_choice_dialog") as mock_open_dialog,
+        ):
+            mock_yaml_data = {"device1": {"param1": "value1"}}
+            mock_yaml_load.return_value = mock_yaml_data
+
+            # Setup dialog mock
+            dm_view.toolbar.components._components["load"].action.action.triggered.emit()
+            mock_yaml_load.assert_called_once_with(tmp_path)
+            mock_open_dialog.assert_called_once_with([{"name": "device1", "param1": "value1"}])
+
+    def test_save_config_to_file(self, tmp_path, dm_view):
+        """Test saving config to file."""
+        yaml_path = tmp_path / "test_save.yaml"
+        mock_config = [{"name": "device1", "param1": "value1"}]
+        with (
+            mock.patch.object(dm_view, "_get_file_path", return_value=tmp_path),
+            mock.patch.object(dm_view, "_get_recovery_config_path", return_value=tmp_path),
+            mock.patch.object(dm_view, "_get_file_path", return_value=yaml_path),
+            mock.patch.object(
+                dm_view.device_table_view, "get_device_config", return_value=mock_config
+            ),
+        ):
+            dm_view.toolbar.components._components["save_to_disk"].action.action.triggered.emit()
+            assert yaml_path.exists()
+
+
+class TestDeviceManagerViewTableBundle:
+    """Test class for DeviceManagerView Table bundle actions."""
+
+    def test_table_bundle_exists(self, dm_view):
+        """Test that Table bundle exists and contains expected actions."""
+        assert "Table" in dm_view.toolbar.bundles
+        table_actions = ["reset_composed", "add_device", "remove_device", "rerun_validation"]
+        for action in table_actions:
+            assert dm_view.toolbar.components.exists(action)
+
+    @mock.patch(
+        "bec_widgets.applications.views.device_manager_view.device_manager_view._yes_no_question"
     )
+    def test_reset_composed_view(self, mock_question, dm_view):
+        """Test reset composed view when user confirms."""
+        with mock.patch.object(dm_view.device_table_view, "clear_device_configs") as mock_clear:
+            mock_question.return_value = QMessageBox.StandardButton.Yes
+            dm_view.toolbar.components._components["reset_composed"].action.action.triggered.emit()
+            mock_clear.assert_called_once()
+            mock_clear.reset_mock()
+            mock_question.return_value = QMessageBox.StandardButton.No
+            dm_view.toolbar.components._components["reset_composed"].action.action.triggered.emit()
+            mock_clear.assert_not_called()
 
+    def test_add_device_action_connected(self, dm_view):
+        """Test add device action opens dialog correctly."""
+        with mock.patch.object(dm_view, "_add_device_action") as mock_add:
+            dm_view.toolbar.components._components["add_device"].action.action.triggered.emit()
+            mock_add.assert_called_once()
 
-def test_device_manager_view_toolbar_components(qtbot, dm_view):
-    """Test that the toolbar components exist for the device_manager_view."""
-    # Load from disk action
-    for bundle_name in ["IO", "Table"]:
-        assert bundle_name in dm_view.toolbar.bundles
+    def test_remove_device_action(self, dm_view):
+        """Test remove device action."""
+        with mock.patch.object(dm_view.device_table_view, "remove_selected_rows") as mock_remove:
+            dm_view.toolbar.components._components["remove_device"].action.action.triggered.emit()
+            mock_remove.assert_called_once()
 
-    # Load File action
-    assert dm_view.toolbar.components.exists("load")
-    with mock.patch.object(dm_view, "_load_file_action") as mock_load_action:
-        dm_view.toolbar.components._components["load"].action.action.triggered.emit()
-        mock_load_action.assert_called_once()
-
-    # Save file action
-    assert dm_view.toolbar.components.exists("save_to_disk")
-    with mock.patch.object(dm_view, "_save_to_disk_action") as mock_save_action:
-        dm_view.toolbar.components._components["save_to_disk"].action.action.triggered.emit()
-        mock_save_action.assert_called_once()
-
-    # Load Redis action
-    assert dm_view.toolbar.components.exists("load_redis")
-    with mock.patch.object(dm_view, "_load_redis_action") as mock_load_redis:
-        dm_view.toolbar.components._components["load_redis"].action.action.triggered.emit()
-        mock_load_redis.assert_called_once()
-
-    # Update Config
-    assert dm_view.toolbar.components.exists("update_config_redis")
-    with mock.patch.object(dm_view, "_update_redis_action") as mock_update_redis:
-        dm_view.toolbar.components._components["update_config_redis"].action.action.triggered.emit()
-        mock_update_redis.assert_called_once()
-
-    # Reset Composed View
-    assert dm_view.toolbar.components.exists("reset_composed")
-    with mock.patch.object(dm_view, "_reset_composed_view") as mock_reset:
-        dm_view.toolbar.components._components["reset_composed"].action.action.triggered.emit()
-        mock_reset.assert_called_once()
-
-    # Add Device
-    assert dm_view.toolbar.components.exists("add_device")
-    with mock.patch.object(dm_view, "_add_device_action") as mock_add_device:
-        dm_view.toolbar.components._components["add_device"].action.action.triggered.emit()
-        mock_add_device.assert_called_once()
-
-    # Remove Device
-    assert dm_view.toolbar.components.exists("remove_device")
-    with mock.patch.object(dm_view, "_remove_device_action") as mock_remove_device:
-        dm_view.toolbar.components._components["remove_device"].action.action.triggered.emit()
-        mock_remove_device.assert_called_once()
-
-    # Rerun Validation
-    assert dm_view.toolbar.components.exists("rerun_validation")
-    with mock.patch.object(dm_view, "_rerun_validation_action") as mock_rerun:
-        dm_view.toolbar.components._components["rerun_validation"].action.action.triggered.emit()
-        mock_rerun.assert_called_once()
+    def test_rerun_device_validation(self, dm_view):
+        """Test rerun device validation action."""
+        cfgs = [{"name": "device1", "param1": "value1"}]
+        with (
+            mock.patch.object(dm_view.ophyd_test_view, "change_device_configs") as mock_change,
+            mock.patch.object(
+                dm_view.device_table_view.table, "selected_configs", return_value=cfgs
+            ),
+        ):
+            dm_view.toolbar.components._components[
+                "rerun_validation"
+            ].action.action.triggered.emit()
+            mock_change.assert_called_once_with(cfgs, True, True)

tests/unit_tests/test_help_inspector.py

@@ -1,9 +1,12 @@
 # pylint: disable=missing-function-docstring, missing-module-docstring, unused-import
 
+from unittest import mock
+
 import pytest
 from qtpy import QtCore, QtWidgets
 
 from bec_widgets.utils.help_inspector.help_inspector import HelpInspector
+from bec_widgets.utils.widget_io import WidgetHierarchy
 from bec_widgets.widgets.control.buttons.button_abort.button_abort import AbortButton
 
 from .client_mocks import mocked_client
@@ -79,3 +82,51 @@ def test_help_inspector_escape_key(qtbot, help_inspector):
     assert not help_inspector._active
     assert not help_inspector._button.isChecked()
     assert QtWidgets.QApplication.overrideCursor() is None
+
+
+def test_help_inspector_event_filter(help_inspector, abort_button):
+    """Test the event filter of the HelpInspector."""
+    # Test nothing happens when not active
+    obj = mock.MagicMock(spec=QtWidgets.QWidget)
+    event = mock.MagicMock(spec=QtCore.QEvent)
+    assert help_inspector._active is False
+    with mock.patch.object(
+        QtWidgets.QWidget, "eventFilter", return_value=False
+    ) as super_event_filter:
+        help_inspector.eventFilter(obj, event)  # should do nothing and return False
+        super_event_filter.assert_called_once_with(obj, event)
+        super_event_filter.reset_mock()
+
+        help_inspector._active = True
+        with mock.patch.object(help_inspector, "_toggle_mode") as mock_toggle:
+            # Key press Escape
+            event.type = mock.MagicMock(return_value=QtCore.QEvent.KeyPress)
+            event.key = mock.MagicMock(return_value=QtCore.Qt.Key.Key_Escape)
+            help_inspector.eventFilter(obj, event)
+            mock_toggle.assert_called_once_with(False)
+            mock_toggle.reset_mock()
+
+            # Click on itself
+            event.type = mock.MagicMock(return_value=QtCore.QEvent.MouseButtonPress)
+            event.button = mock.MagicMock(return_value=QtCore.Qt.LeftButton)
+            event.globalPos = mock.MagicMock(return_value=QtCore.QPoint(1, 1))
+            with mock.patch.object(
+                help_inspector._app, "widgetAt", side_effect=[help_inspector, abort_button]
+            ):
+                # Return for self call
+                help_inspector.eventFilter(obj, event)
+                mock_toggle.assert_called_once_with(False)
+                mock_toggle.reset_mock()
+                # Run Callback for abort_button
+                callback_data = []
+
+                def _my_callback(widget):
+                    callback_data.append(widget)
+
+                help_inspector.register_callback(_my_callback)
+
+                help_inspector.eventFilter(obj, event)
+                mock_toggle.assert_not_called()
+                assert len(callback_data) == 1
+                assert callback_data[0] == abort_button
+                callback_data.clear()