From a57d3c6e1cd92f5c9545e1029a41d5299cfb1b24 Mon Sep 17 00:00:00 2001 From: appel_c Date: Tue, 14 Oct 2025 17:40:42 +0200 Subject: [PATCH] refactor: improve test coverage for help_inspector and device_manager_view --- .../device_manager_view.py | 35 ++- .../utils/help_inspector/help_inspector.py | 3 +- tests/unit_tests/test_device_manager_view.py | 260 +++++++++++++----- tests/unit_tests/test_help_inspector.py | 51 ++++ 4 files changed, 275 insertions(+), 74 deletions(-) diff --git a/bec_widgets/applications/views/device_manager_view/device_manager_view.py b/bec_widgets/applications/views/device_manager_view/device_manager_view.py index 47f85366..9acdb5a3 100644 --- a/bec_widgets/applications/views/device_manager_view/device_manager_view.py +++ b/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: diff --git a/bec_widgets/utils/help_inspector/help_inspector.py b/bec_widgets/utils/help_inspector/help_inspector.py index db561a9a..9a73cd34 100644 --- a/bec_widgets/utils/help_inspector/help_inspector.py +++ b/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) diff --git a/tests/unit_tests/test_device_manager_view.py b/tests/unit_tests/test_device_manager_view.py index 096cd656..a85be731 100644 --- a/tests/unit_tests/test_device_manager_view.py +++ b/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) diff --git a/tests/unit_tests/test_help_inspector.py b/tests/unit_tests/test_help_inspector.py index 75cd738b..5ab96274 100644 --- a/tests/unit_tests/test_help_inspector.py +++ b/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()