diff --git a/bec_widgets/applications/launch_window.py b/bec_widgets/applications/launch_window.py index 50f8e3b1..349db130 100644 --- a/bec_widgets/applications/launch_window.py +++ b/bec_widgets/applications/launch_window.py @@ -207,6 +207,7 @@ class LaunchWindow(BECMainWindow): self.app = QApplication.instance() self.tiles: dict[str, LaunchTile] = {} + self._logged_unparented_connections: set[str] = set() # Track the smallest main‑label font size chosen so far self._min_main_label_pt: int | None = None @@ -655,37 +656,51 @@ class LaunchWindow(BECMainWindow): super().showEvent(event) self.setFixedSize(self.size()) - def _launcher_is_last_widget(self, connections: dict) -> bool: + def _has_external_window(self, connections: dict) -> bool: """ - Check if the launcher is the last widget in the application. + Check if any registered non-launcher connection owns a top-level Qt window. """ - for connection in connections.values(): - if not self._connection_belongs_to_launcher(connection): - return False - return True + if self._connection_belongs_to_launcher(connection): + continue + if isinstance(connection, QWidget) and connection.isWindow(): + return True + return False + + def _log_unparented_connections(self, connections: dict) -> None: + """ + Log non-launcher RPC connections that remain without an active top-level window. + """ + for connection in connections.values(): + if self._connection_belongs_to_launcher(connection): + continue + if isinstance(connection, QWidget) and connection.isWindow(): + continue + + connection_description = ( + f"type={type(connection).__name__} objectName={connection.objectName()!r} " + f"gui_id={connection.gui_id!r}" + ) + if connection_description in self._logged_unparented_connections: + continue + self._logged_unparented_connections.add(connection_description) + logger.warning( + "Registered non-launcher RPC connection has no active top-level window: " + f"{connection_description}" + ) def _connection_belongs_to_launcher(self, connection: QObject) -> bool: """ Check whether a registered connection is the launcher itself or part of its Qt hierarchy. - - Registered top-level windows such as BECMainWindowNoRPC are expected when another GUI is - open. They are not launcher children, but they are also not an error condition. """ - try: - if connection is self or getattr(connection, "gui_id", None) == self.gui_id: - return True - if connection.objectName() == self.objectName(): - return True + if connection is self or connection.gui_id == self.gui_id: + return True - parent = connection.parent() - while parent is not None: - if parent is self: - return True - parent = parent.parent() - except Exception as e: - logger.error(f"Error checking launcher ownership of connection: {e}") - return False + parent = connection.parent() + while parent is not None: + if parent is self: + return True + parent = parent.parent() return False @@ -694,29 +709,30 @@ class LaunchWindow(BECMainWindow): If there is only one connection remaining, it is the launcher, so we show it. Once the launcher is closed as the last window, we quit the application. """ - if self._launcher_is_last_widget(connections): - self.show() - self.activateWindow() - self.raise_() + if self._has_external_window(connections): + self.hide() if self.app: - self.app.setQuitOnLastWindowClosed(True) # type: ignore + self.app.setQuitOnLastWindowClosed(False) # type: ignore return - self.hide() + self._log_unparented_connections(connections) + self.show() + self.activateWindow() + self.raise_() if self.app: - self.app.setQuitOnLastWindowClosed(False) # type: ignore + self.app.setQuitOnLastWindowClosed(True) # type: ignore def closeEvent(self, event): """ Close the launcher window. """ connections = self.register.list_all_connections() - if self._launcher_is_last_widget(connections): - event.accept() + if self._has_external_window(connections): + event.ignore() + self.hide() return - event.ignore() - self.hide() + event.accept() if __name__ == "__main__": # pragma: no cover diff --git a/bec_widgets/utils/bec_widget.py b/bec_widgets/utils/bec_widget.py index d1d99656..6f254aab 100644 --- a/bec_widgets/utils/bec_widget.py +++ b/bec_widgets/utils/bec_widget.py @@ -331,32 +331,34 @@ class BECWidget(BECConnector): # All widgets need to call super().cleanup() in their cleanup method logger.info(f"Registry cleanup for widget {self.__class__.__name__}") self.rpc_register.remove_rpc(self) - children = self.findChildren(BECWidget) - for child in children: - if not shiboken6.isValid(child): - # If the child is not valid, it means it has already been deleted - continue - child.close() - child.deleteLater() + children = self.findChildren(BECWidget) + for child in children: + if not shiboken6.isValid(child): + # If the child is not valid, it means it has already been deleted + continue + child.close() + child.deleteLater() - # Tear down busy overlay explicitly to stop spinner and remove filters - overlay = getattr(self, "_busy_overlay", None) - if overlay is not None and shiboken6.isValid(overlay): - try: - overlay.hide() - filt = getattr(overlay, "_filter", None) - if filt is not None and shiboken6.isValid(filt): - try: - self.removeEventFilter(filt) - except Exception as exc: - logger.warning(f"Failed to remove event filter from busy overlay: {exc}") + # Tear down busy overlay explicitly to stop spinner and remove filters + overlay = getattr(self, "_busy_overlay", None) + if overlay is not None and shiboken6.isValid(overlay): + try: + overlay.hide() + filt = getattr(overlay, "_filter", None) + if filt is not None and shiboken6.isValid(filt): + try: + self.removeEventFilter(filt) + except Exception as exc: + logger.warning( + f"Failed to remove event filter from busy overlay: {exc}" + ) - # Cleanup the overlay widget. This will call cleanup on the custom widget if present. + # Cleanup the overlay widget. This will call cleanup on the custom widget if present. - overlay.cleanup() - overlay.deleteLater() - except Exception as exc: - logger.warning(f"Failed to delete busy overlay: {exc}") + overlay.cleanup() + overlay.deleteLater() + except Exception as exc: + logger.warning(f"Failed to delete busy overlay: {exc}") def closeEvent(self, event): """Wrap the close even to ensure the rpc_register is cleaned up.""" diff --git a/tests/unit_tests/test_bec_connector.py b/tests/unit_tests/test_bec_connector.py index 04be4659..803ef92b 100644 --- a/tests/unit_tests/test_bec_connector.py +++ b/tests/unit_tests/test_bec_connector.py @@ -6,6 +6,7 @@ from qtpy.QtCore import QObject from qtpy.QtWidgets import QApplication, QWidget from bec_widgets.utils.bec_connector import BECConnector +from bec_widgets.utils.bec_widget import BECWidget from bec_widgets.utils.error_popups import SafeProperty from bec_widgets.utils.error_popups import SafeSlot as Slot @@ -15,6 +16,9 @@ from .client_mocks import mocked_client class BECConnectorQObject(BECConnector, QObject): ... +class _CleanupBroadcastWidget(BECWidget, QWidget): ... + + @pytest.fixture def bec_connector(mocked_client): connector = BECConnectorQObject(client=mocked_client) @@ -146,6 +150,28 @@ def test_bec_connector_change_object_name(bec_connector): assert not any(obj.objectName() == previous_name for obj in all_objects) +def test_bec_widget_cleanup_broadcasts_after_children_are_unregistered(mocked_client, qtbot): + parent = _CleanupBroadcastWidget(client=mocked_client, object_name="cleanup_parent") + child = _CleanupBroadcastWidget( + parent=parent, client=mocked_client, object_name="cleanup_child" + ) + qtbot.addWidget(parent) + + observed_connections = [] + parent.rpc_register.callbacks.append( + lambda connections: observed_connections.append(set(connections)) + ) + + parent.close() + + assert parent._destroyed is True + assert child.gui_id not in parent.rpc_register.list_all_connections() + assert all( + parent.gui_id in snapshot or child.gui_id not in snapshot + for snapshot in observed_connections + ) + + def test_bec_connector_export_settings(): class MyWidget(BECConnector, QWidget): diff --git a/tests/unit_tests/test_launch_window.py b/tests/unit_tests/test_launch_window.py index 90b942f4..28fd0988 100644 --- a/tests/unit_tests/test_launch_window.py +++ b/tests/unit_tests/test_launch_window.py @@ -4,7 +4,9 @@ import os from unittest import mock import pytest +from qtpy.QtCore import QObject from qtpy.QtGui import QFontMetrics +from qtpy.QtWidgets import QWidget import bec_widgets from bec_widgets.applications.launch_window import START_EMPTY_PROFILE_OPTION, LaunchWindow @@ -16,6 +18,28 @@ from .client_mocks import mocked_client base_path = os.path.dirname(bec_widgets.__file__) +def _launcher_child_connection(launcher: LaunchWindow, name: str) -> QObject: + connection = QObject(parent=launcher) + connection.gui_id = f"{launcher.gui_id}:{name}" + connection.setObjectName(name) + return connection + + +def _top_level_connection(qtbot, name: str) -> QWidget: + connection = QWidget() + connection.gui_id = name + connection.setObjectName(name) + qtbot.addWidget(connection) + return connection + + +def _unparented_connection(name: str) -> QObject: + connection = QObject() + connection.gui_id = name + connection.setObjectName(name) + return connection + + @pytest.fixture def bec_launch_window(qtbot, mocked_client): widget = LaunchWindow(client=mocked_client) @@ -117,20 +141,20 @@ def test_open_dock_area_with_start_empty_option_calls_launch(bec_launch_window): (["launcher", "dock_area", "scan_progress_simple", "scan_progress_full"], False), ( ["launcher", "dock_area", "scan_progress_simple", "scan_progress_full", "hover_widget"], - True, + False, ), + (["launcher", "external_window"], True), ], ) -def test_gui_server_turns_off_the_lights(bec_launch_window, connection_names, hide): +def test_gui_server_turns_off_the_lights(bec_launch_window, qtbot, connection_names, hide): connections = {} for name in connection_names: - conn = mock.MagicMock() if name == "hover_widget": - conn.parent.return_value = None - conn.objectName.return_value = "HoverWidget" + conn = _unparented_connection("HoverWidget") + elif name == "external_window": + conn = _top_level_connection(qtbot, "external_window") else: - conn.parent.return_value = mock.MagicMock() - conn.objectName.return_value = bec_launch_window.objectName() + conn = _launcher_child_connection(bec_launch_window, name) connections[name] = conn with ( mock.patch.object(bec_launch_window, "show") as mock_show, @@ -153,15 +177,21 @@ def test_gui_server_turns_off_the_lights(bec_launch_window, connection_names, hi mock_set_quit_on_last_window_closed.assert_called_once_with(True) -def test_launcher_detects_external_main_window_without_info_log(bec_launch_window): - connection = mock.MagicMock() - connection.parent.return_value = None - connection.objectName.return_value = "BECMainWindowNoRPC" +def test_launcher_detects_external_main_window(bec_launch_window, qtbot): + connection = _top_level_connection(qtbot, "BECMainWindowNoRPC") - with mock.patch("bec_widgets.applications.launch_window.logger.info") as mock_info: - assert not bec_launch_window._launcher_is_last_widget({"window": connection}) + assert bec_launch_window._has_external_window({"window": connection}) - mock_info.assert_not_called() + +def test_launcher_logs_unparented_non_window_connection_once(bec_launch_window): + connection = _unparented_connection("HoverWidget") + + with mock.patch("bec_widgets.applications.launch_window.logger.warning") as mock_warning: + bec_launch_window._turn_off_the_lights({"window": connection}) + bec_launch_window._turn_off_the_lights({"window": connection}) + + mock_warning.assert_called_once() + assert "HoverWidget" in mock_warning.call_args.args[0] @pytest.mark.parametrize( @@ -174,11 +204,12 @@ def test_launcher_detects_external_main_window_without_info_log(bec_launch_windo (["launcher", "dock_area", "scan_progress_simple", "scan_progress_full"], True), ( ["launcher", "dock_area", "scan_progress_simple", "scan_progress_full", "hover_widget"], - False, + True, ), + (["launcher", "external_window"], False), ], ) -def test_launch_window_closes(bec_launch_window, connection_names, close_called): +def test_launch_window_closes(bec_launch_window, qtbot, connection_names, close_called): """ Test that the close event is handled correctly based on the connections. If there are no connections or only the launcher connection, the window should close. @@ -186,13 +217,12 @@ def test_launch_window_closes(bec_launch_window, connection_names, close_called) """ connections = {} for name in connection_names: - conn = mock.MagicMock() if name == "hover_widget": - conn.parent.return_value = None - conn.objectName.return_value = "HoverWidget" + conn = _unparented_connection("HoverWidget") + elif name == "external_window": + conn = _top_level_connection(qtbot, "external_window") else: - conn.parent.return_value = mock.MagicMock() - conn.objectName.return_value = bec_launch_window.objectName() + conn = _launcher_child_connection(bec_launch_window, name) connections[name] = conn close_event = mock.MagicMock() with mock.patch.object(