fix: logpanel error cycle

.github/workflows/end2end-conda.yml

@@ -12,6 +12,7 @@ jobs:
       CHILD_PIPELINE_BRANCH: main  # Set the branch you want for ophyd_devices
       BEC_CORE_BRANCH: main        # Set the branch you want for bec
       OPHYD_DEVICES_BRANCH: main   # Set the branch you want for ophyd_devices
+      PLUGIN_REPO_BRANCH: main     # Set the branch you want for the plugin repo
       PROJECT_PATH: ${{ github.repository }}
       QTWEBENGINE_DISABLE_SANDBOX: 1
       QT_QPA_PLATFORM: "offscreen"
@@ -39,10 +40,11 @@ jobs:
           echo -e "\033[35;1m Using branch $OPHYD_DEVICES_BRANCH of OPHYD_DEVICES \033[0;m";
           git clone --branch $OPHYD_DEVICES_BRANCH https://github.com/bec-project/ophyd_devices.git
           export OHPYD_DEVICES_PATH=$PWD/ophyd_devices
+          echo -e "\033[35;1m Using branch $PLUGIN_REPO_BRANCH of bec_testing_plugin \033[0;m";
+          git clone --branch $PLUGIN_REPO_BRANCH https://github.com/bec-project/bec_testing_plugin.git
           cd ./bec
           conda create -q -n test-environment python=3.11
           source ./bin/install_bec_dev.sh -t
           cd ../
-          pip install -e ./ophyd_devices
+          pip install -e ./ophyd_devices -e .[dev,pyside6] -e ./bec_testing_plugin
-          pip install -e .[dev,pyside6]
           pytest -v --files-path ./ --start-servers --random-order  ./tests/end-2-end

bec_widgets/widgets/utility/logpanel/logpanel.py

@@ -35,6 +35,7 @@ from qtpy.QtWidgets import (
     QWidget,
 )
 
+from bec_widgets.utils.bec_connector import BECConnector
 from bec_widgets.utils.colors import get_theme_palette, set_theme
 from bec_widgets.utils.error_popups import SafeSlot
 from bec_widgets.widgets.editors.text_box.text_box import TextBox
@@ -69,22 +70,22 @@ DEFAULT_LOG_COLORS = {
 }
 
 
-class BecLogsQueue(QObject):
+class BecLogsQueue(BECConnector, QObject):
     """Manages getting logs from BEC Redis and formatting them for display"""
 
+    RPC = False
     new_message = Signal()
 
     def __init__(
         self,
         parent: QObject | None,
-        conn: ConnectorBase,
         maxlen: int = 1000,
         line_formatter: LineFormatter = noop_format,
     ) -> None:
         super().__init__(parent=parent)
         self._timestamp_start: QDateTime | None = None
         self._timestamp_end: QDateTime | None = None
-        self._conn = conn
+        self._conn = self.client.connector
         self._max_length = maxlen
         self._data: deque[LogMessage] = deque([], self._max_length)
         self._display_queue: deque[str] = deque([], self._max_length)
@@ -92,20 +93,27 @@ class BecLogsQueue(QObject):
         self._search_query: Pattern | str | None = None
         self._selected_services: set[str] | None = None
         self._set_formatter_and_update_filter(line_formatter)
-        self._conn.register([MessageEndpoints.log()], None, self._process_incoming_log_msg)
+        # instance attribute still accessible after c++ object is deleted, so the callback can be unregistered
+        self._callback = lambda *args: self._process_incoming_log_msg(*args)
+        self._conn.register([MessageEndpoints.log()], None, self._callback)
 
-    def unsub_from_redis(self):
+    def unsub_from_redis(self, *_):
         """Stop listening to the Redis log stream"""
-        self._conn.unregister([MessageEndpoints.log()], None, self._process_incoming_log_msg)
+        self._conn.unregister([MessageEndpoints.log()], None, self._callback)
 
     def _process_incoming_log_msg(self, msg: dict):
         try:
-            _msg: LogMessage = msg["data"]
+            _msg: LogMessage | None = msg.get("data", None)
+            if _msg is None or not isinstance(_msg, LogMessage):
+                return
             self._data.append(_msg)
             if self.filter is None or self.filter(_msg):
                 self._display_queue.append(self._line_formatter(_msg))
                 self.new_message.emit()
         except Exception as e:
+            if "Internal C++ object (BecLogsQueue) already deleted." in e.args:
+                self._conn.unregister([MessageEndpoints.log()], None, self._callback)
+                return
             logger.warning(f"Error in LogPanel incoming message callback: {e}")
 
     def _set_formatter_and_update_filter(self, line_formatter: LineFormatter = noop_format):
@@ -407,17 +415,15 @@ class LogPanel(TextBox):
         **kwargs,
     ):
         """Initialize the LogPanel widget."""
-        super().__init__(parent=parent, client=client, **kwargs)
+        super().__init__(parent=parent, client=client, config={"text": ""}, **kwargs)
         self._update_colors()
         self._service_status = service_status or BECServiceStatusMixin(self, client=self.client)  # type: ignore
         self._log_manager = BecLogsQueue(
-            parent,
+            parent=self, line_formatter=partial(simple_color_format, colors=self._colors)
-            self.client.connector,  # type: ignore
-            line_formatter=partial(simple_color_format, colors=self._colors),
         )
         self._log_manager.new_message.connect(self._new_messages)
 
-        self.toolbar = LogPanelToolbar(parent=parent)
+        self.toolbar = LogPanelToolbar(parent=self)
         self.toolbar_area = QScrollArea()
         self.toolbar_area.setVerticalScrollBarPolicy(Qt.ScrollBarPolicy.ScrollBarAlwaysOff)
         self.toolbar_area.setSizeAdjustPolicy(QScrollArea.SizeAdjustPolicy.AdjustToContents)
@@ -529,9 +535,9 @@ class LogPanel(TextBox):
 
     def cleanup(self):
         self._service_status.cleanup()
+        self._log_manager.new_message.disconnect()
+        self._new_messages.disconnect()
         self._log_manager.unsub_from_redis()
-        self._log_manager.new_message.disconnect(self._new_messages)
-        self._new_messages.disconnect(self._on_append)
         super().cleanup()
 
 

tests/end-2-end/conftest.py

@@ -5,11 +5,20 @@ import random
 import pytest
 
 from bec_widgets.cli.client_utils import BECGuiClient
+from bec_widgets.widgets.control.scan_control import ScanControl
 
 # pylint: disable=unused-argument
 # pylint: disable=redefined-outer-name
 
 
+@pytest.fixture(scope="function")
+def scan_control(qtbot, bec_client_lib):  # , mock_dev):
+    widget = ScanControl(client=bec_client_lib)
+    qtbot.addWidget(widget)
+    qtbot.waitExposed(widget)
+    yield widget
+
+
 @pytest.fixture(autouse=True)
 def threads_check_fixture(threads_check):
     """

tests/end-2-end/test_scan_control_e2e.py

@@ -3,15 +3,6 @@ import time
 import pytest
 
 from bec_widgets.utils.widget_io import WidgetIO
-from bec_widgets.widgets.control.scan_control import ScanControl
-
-
-@pytest.fixture(scope="function")
-def scan_control(qtbot, bec_client_lib):  # , mock_dev):
-    widget = ScanControl(client=bec_client_lib)
-    qtbot.addWidget(widget)
-    qtbot.waitExposed(widget)
-    yield widget
 
 
 def test_scan_control_populate_scans_e2e(scan_control):
@@ -27,6 +18,7 @@ def test_scan_control_populate_scans_e2e(scan_control):
         "monitor_scan",
         "acquire",
         "line_scan",
+        "custom_testing_scan",
     ]
     items = [
         scan_control.comboBox_scan_selection.itemText(i)

tests/end-2-end/test_with_plugins_e2e.py

@@ -0,0 +1,89 @@
+import time
+
+import pytest
+from bec_testing_plugin.scans.metadata_schema.custom_test_scan_schema import CustomScanSchema
+from qtpy.QtWidgets import QGridLayout
+
+from bec_widgets.utils.widget_io import WidgetIO
+from bec_widgets.widgets.control.scan_control import ScanControl
+
+
+@pytest.fixture(scope="function")
+def scan_control(qtbot, bec_client_lib):  # , mock_dev):
+    widget = ScanControl(client=bec_client_lib)
+    qtbot.addWidget(widget)
+    qtbot.waitExposed(widget)
+    yield widget
+
+
+@pytest.mark.parametrize(
+    ["md", "valid"],
+    [
+        ({"treatment_description": "soaking", "treatment_temperature_k": 123}, True),
+        ({"treatment_description": "soaking", "treatment_temperature_k": "wrong type"}, False),
+        ({"treatment_description": "soaking", "wrong key": 123}, False),
+        (
+            {
+                "sample_name": "test sample",
+                "treatment_description": "soaking",
+                "treatment_temperature_k": 123,
+            },
+            True,
+        ),
+    ],
+)
+def test_scan_metadata_for_custom_scan(
+    scan_control: ScanControl, bec_client_lib, qtbot, md: dict, valid: bool
+):
+    client = bec_client_lib
+    queue = client.queue
+
+    scan_name = "custom_testing_scan"
+    kwargs = {"exp_time": 0.01, "steps": 10, "relative": True, "burst_at_each_point": 1}
+    args = {"device": "samx", "start": -5, "stop": 5}
+
+    scan_control.comboBox_scan_selection.setCurrentText(scan_name)
+
+    # Set kwargs in the UI
+    for kwarg_box in scan_control.kwarg_boxes:
+        for widget in kwarg_box.widgets:
+            for key, value in kwargs.items():
+                if widget.arg_name == key:
+                    WidgetIO.set_value(widget, value)
+                    break
+    # Set args in the UI
+    for widget in scan_control.arg_box.widgets:
+        for key, value in args.items():
+            if widget.arg_name == key:
+                WidgetIO.set_value(widget, value)
+                break
+
+    assert scan_control._metadata_form._md_schema == CustomScanSchema
+    assert not scan_control.button_run_scan.isEnabled()
+
+    def do_test():
+        # Set the metadata
+        grid: QGridLayout = scan_control._metadata_form._form_grid.layout()
+        for i in range(grid.rowCount()):  # type: ignore
+            field_name = grid.itemAtPosition(i, 0).widget().property("_model_field_name")
+            if (value_to_set := md.pop(field_name, None)) is not None:
+                grid.itemAtPosition(i, 1).widget().setValue(value_to_set)
+        # all values should be used
+        assert md == {}
+        assert scan_control.button_run_scan.isEnabled()
+
+        # Run the scan
+        scan_control.button_run_scan.click()
+        time.sleep(2)
+
+        last_scan = queue.scan_storage.storage[-1]
+        assert last_scan.status_message.info["scan_name"] == scan_name
+        assert last_scan.status_message.info["exp_time"] == kwargs["exp_time"]
+        assert last_scan.status_message.info["scan_motors"] == [args["device"]]
+        assert last_scan.status_message.info["num_points"] == kwargs["steps"]
+
+    if valid:
+        do_test()
+    else:
+        with pytest.raises(Exception):
+            do_test()

tests/unit_tests/test_logpanel.py

@@ -4,11 +4,12 @@
 # pylint: disable=protected-access
 
 from collections import deque
-from unittest.mock import MagicMock
+from unittest.mock import MagicMock, patch
 
 import pytest
 from bec_lib.messages import LogMessage
-from qtpy.QtCore import QDateTime, Qt, Signal  # type: ignore
+from bec_lib.redis_connector import StreamMessage
+from qtpy.QtCore import QDateTime
 
 from bec_widgets.widgets.utility.logpanel._util import (
     log_time,
@@ -136,3 +137,31 @@ def test_timestamp_filter(log_panel: LogPanel):
     assert not filter_(TEST_LOG_MESSAGES[0])
     assert filter_(TEST_LOG_MESSAGES[1])
     assert not filter_(TEST_LOG_MESSAGES[2])
+
+
+def test_error_handling_in_callback(log_panel: LogPanel):
+    log_panel._log_manager.new_message = MagicMock()
+
+    cbs = (lambda: log_panel._log_manager._process_incoming_log_msg, {})
+    with patch("bec_widgets.widgets.utility.logpanel.logpanel.logger") as logger:
+        # generally errors should be logged
+        log_panel._log_manager.new_message.emit = MagicMock(
+            side_effect=ValueError("Something went wrong")
+        )
+        log_panel.client.connector._handle_message(
+            msg=StreamMessage(
+                msg={"data": LogMessage(log_type="debug", log_msg="message")}, callbacks=[cbs]
+            )
+        )
+        logger.warning.assert_called_once()
+
+        # this specific error should be ignored and not relogged
+        log_panel._log_manager.new_message.emit = MagicMock(
+            side_effect=RuntimeError("Internal C++ object (BecLogsQueue) already deleted.")
+        )
+        log_panel.client.connector._handle_message(
+            msg=StreamMessage(
+                msg={"data": LogMessage(log_type="debug", log_msg="message")}, callbacks=[cbs]
+            )
+        )
+        logger.warning.assert_called_once()