From f6d2bdb9ba6f1e2703890ac18c86308526f80be9 Mon Sep 17 00:00:00 2001 From: sligara7 Date: Tue, 6 Oct 2026 11:08:47 -0400 Subject: [PATCH 1/3] GUI: Resume runs RE.resume() on the main thread In local mode the Resume button ran RE.resume() in a FunctionWorker thread. resume() enters bluesky's SigintHandler, and Python lets only the main thread install signal handlers, so it raised ValueError('signal only works in main thread of the main interpreter'). The button listened only for 'finished', never 'errored', so the click did nothing visible and the plan stayed paused. Resume now goes through run_in_ipython like RE(plan(...)) does. Verified with mock tests: test_resume_button_resumes_a_paused_plan pauses a running scan with Pause: Immediate and presses Resume; it failed (RE stayed paused) before this change. Assisted-by: copilot:claude-opus-4-8 --- src/hextools/gui/re_execution_controls.py | 17 +++---- tests/test_re_execution_controls.py | 57 +++++++++++++++++++++++ 2 files changed, 63 insertions(+), 11 deletions(-) create mode 100644 tests/test_re_execution_controls.py diff --git a/src/hextools/gui/re_execution_controls.py b/src/hextools/gui/re_execution_controls.py index a7a4101..4856251 100644 --- a/src/hextools/gui/re_execution_controls.py +++ b/src/hextools/gui/re_execution_controls.py @@ -16,9 +16,10 @@ from bluesky_widgets.qt.run_engine_client import ( QtReExecutionControls as _QtReExecutionControls, ) -from bluesky_widgets.qt.threading import FunctionWorker from qtpy.QtCore import QTimer +from hextools.gui._ipython import run_in_ipython + class _NullSignal: def connect(self, *args, **kwargs): @@ -74,7 +75,6 @@ def __init__( self._re_name = re_name self._namespace = namespace self._timer = None - self._resume_worker = None super().__init__(_NullModel() if local else model, parent) @@ -132,16 +132,11 @@ def _pb_plan_pause_immediate_clicked(self): def _pb_plan_resume_clicked(self): if not self._local: return super()._pb_plan_resume_clicked() - # resume() blocks until the plan pauses or completes; run it off-thread. - run_engine = self._resolve_re() - if run_engine is None or self._resume_worker is not None: + if self._resolve_re() is None: return - self._resume_worker = FunctionWorker(run_engine.resume) - self._resume_worker.finished.connect(self._on_resume_finished) - self._resume_worker.start() - - def _on_resume_finished(self): - self._resume_worker = None + # resume() installs a SIGINT handler, which only the main thread may do, + # so run it like RE(plan): an IPython cell, after this click returns. + QTimer.singleShot(0, lambda: run_in_ipython(f"{self._re_name}.resume()")) def _pb_plan_stop_clicked(self): if not self._local: diff --git a/tests/test_re_execution_controls.py b/tests/test_re_execution_controls.py new file mode 100644 index 0000000..af08c39 --- /dev/null +++ b/tests/test_re_execution_controls.py @@ -0,0 +1,57 @@ +import os + +import pytest + +os.environ.setdefault("QT_QPA_PLATFORM", "offscreen") +pytest.importorskip("qtpy") +pytest.importorskip("bluesky_widgets") + +import bluesky.plans as bp # noqa: E402 +import matplotlib # noqa: E402 +from bluesky import RunEngine # noqa: E402 +from bluesky import plan_stubs as bps # noqa: E402 +from IPython.core.interactiveshell import InteractiveShell # noqa: E402 +from ophyd.sim import det, motor # noqa: E402 +from qtpy.QtCore import QTimer # noqa: E402 +from qtpy.QtWidgets import QApplication # noqa: E402 + +from hextools.gui._ipython import run_in_ipython # noqa: E402, PLC2701 +from hextools.gui.re_execution_controls import QtReExecutionControls # noqa: E402 + + +def _slow_step(detectors, step, pos_cache): + yield from bps.one_nd_step(detectors, step, pos_cache) + yield from bps.sleep(0.1) + + +# RE.resume() installs a SIGINT handler, which Python allows only on the main +# thread; resuming from a worker thread raised and the plan stayed paused. +def test_resume_button_resumes_a_paused_plan(): + # As in the GUI: with a Qt matplotlib backend, RE() keeps Qt events flowing. + matplotlib.use("qtagg") + app = QApplication.instance() or QApplication([]) + shell = InteractiveShell.instance() + RE = RunEngine({}) + shell.user_ns.update(RE=RE, bp=bp, det=det, motor=motor, _slow_step=_slow_step) + controls = QtReExecutionControls(local=True, namespace=shell.user_ns) + seen = [] + + def resume(): + seen.append(RE.state) + controls._pb_plan_resume_clicked() + + QTimer.singleShot( + 0, + lambda: run_in_ipython( + "RE(bp.scan([det], motor, 0, 1, 10, per_step=_slow_step))" + ), + ) + QTimer.singleShot(300, controls._pb_plan_pause_immediate_clicked) + QTimer.singleShot(800, resume) + QTimer.singleShot(5000, app.quit) + app.exec() + controls._timer.stop() + + assert seen == ["paused"] + assert RE.state == "idle", f"plan did not resume: RE.state={RE.state}" + assert motor.position == pytest.approx(1) From 7c94b060d4b444bff54b38177ae3cf89c8fc1f9d Mon Sep 17 00:00:00 2001 From: sligara7 Date: Tue, 6 Oct 2026 11:42:42 -0400 Subject: [PATCH 2/3] GUI Resume acts on the controls' own RunEngine; GUI tests run in CI Review on PR 94: resuming by the name in the IPython shell could resume a different engine (or raise) when the controls were given re= or their own namespace. Resume now targets the resolved engine, still on the main thread: through IPython when that engine is the shell's RE (so the terminal echoes it), directly after the click otherwise. Pinned by test_resume_button_resumes_a_paused_plan[re|namespace], observed failing before this change. GUI tests move to tests/gui/ and a new CI job runs them in the gui-dev environment (offscreen Qt); the py3.11-3.13 jobs lack Qt and skip them. Assisted-by: copilot:claude-opus-4-8 --- .github/workflows/ci.yml | 25 ++++++++++++++++ src/hextools/gui/re_execution_controls.py | 13 +++++++-- tests/{ => gui}/test_re_execution_controls.py | 29 ++++++++++++++----- 3 files changed, 57 insertions(+), 10 deletions(-) rename tests/{ => gui}/test_re_execution_controls.py (63%) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f00b6cd..7fbbc7a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -81,6 +81,31 @@ jobs: - name: Run tests run: pixi run --as-is -e ${{ steps.pixi.outputs.env }} pytest --cov=src/hextools --cov-report=xml --cov-report=term + gui-test: + name: GUI Tests + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@v7 + with: + persist-credentials: false + + - name: Install Qt system libraries + run: sudo apt-get update && sudo apt-get install -y libegl1 libgl1 libxkbcommon0 libfontconfig1 libdbus-1-3 + + - name: Setup Pixi + uses: prefix-dev/setup-pixi@2253d9c9310442c09975d64615591b5b381da6e7 # v0.10.0 + with: + pixi-version: v0.79.0 + environments: gui-dev + cache: false + frozen: true + + - name: Run GUI tests + env: + QT_QPA_PLATFORM: offscreen + run: pixi run --as-is -e gui-dev pytest tests/gui + # Uncomment once codecov is enabled. # - name: Upload coverage to Codecov # uses: codecov/codecov-action@v5 diff --git a/src/hextools/gui/re_execution_controls.py b/src/hextools/gui/re_execution_controls.py index 4856251..414b8d9 100644 --- a/src/hextools/gui/re_execution_controls.py +++ b/src/hextools/gui/re_execution_controls.py @@ -132,11 +132,18 @@ def _pb_plan_pause_immediate_clicked(self): def _pb_plan_resume_clicked(self): if not self._local: return super()._pb_plan_resume_clicked() - if self._resolve_re() is None: + run_engine = self._resolve_re() + if run_engine is None: return # resume() installs a SIGINT handler, which only the main thread may do, - # so run it like RE(plan): an IPython cell, after this click returns. - QTimer.singleShot(0, lambda: run_in_ipython(f"{self._re_name}.resume()")) + # so run it after this click returns, never on a worker thread. + QTimer.singleShot(0, lambda: self._resume_on_main_thread(run_engine)) + + def _resume_on_main_thread(self, run_engine): + if _ipython_namespace().get(self._re_name) is run_engine: + run_in_ipython(f"{self._re_name}.resume()") + else: + self._call_re("resume") def _pb_plan_stop_clicked(self): if not self._local: diff --git a/tests/test_re_execution_controls.py b/tests/gui/test_re_execution_controls.py similarity index 63% rename from tests/test_re_execution_controls.py rename to tests/gui/test_re_execution_controls.py index af08c39..fb2b1aa 100644 --- a/tests/test_re_execution_controls.py +++ b/tests/gui/test_re_execution_controls.py @@ -26,24 +26,39 @@ def _slow_step(detectors, step, pos_cache): # RE.resume() installs a SIGINT handler, which Python allows only on the main # thread; resuming from a worker thread raised and the plan stayed paused. -def test_resume_button_resumes_a_paused_plan(): +# PR 94 review: Resume must act on the engine the controls hold, even when the +# shell's RE is a different one. +@pytest.mark.parametrize("given", ["shell", "re", "namespace"]) +def test_resume_button_resumes_a_paused_plan(given): # As in the GUI: with a Qt matplotlib backend, RE() keeps Qt events flowing. matplotlib.use("qtagg") app = QApplication.instance() or QApplication([]) shell = InteractiveShell.instance() - RE = RunEngine({}) - shell.user_ns.update(RE=RE, bp=bp, det=det, motor=motor, _slow_step=_slow_step) - controls = QtReExecutionControls(local=True, namespace=shell.user_ns) + engine = RunEngine({}) + shell.user_ns.update( + RE=engine if given == "shell" else RunEngine({}), + engine=engine, + bp=bp, + det=det, + motor=motor, + _slow_step=_slow_step, + ) + if given == "shell": + controls = QtReExecutionControls(local=True, namespace=shell.user_ns) + elif given == "re": + controls = QtReExecutionControls(local=True, re=engine) + else: + controls = QtReExecutionControls(local=True, namespace={"RE": engine}) seen = [] def resume(): - seen.append(RE.state) + seen.append(engine.state) controls._pb_plan_resume_clicked() QTimer.singleShot( 0, lambda: run_in_ipython( - "RE(bp.scan([det], motor, 0, 1, 10, per_step=_slow_step))" + "engine(bp.scan([det], motor, 0, 1, 10, per_step=_slow_step))" ), ) QTimer.singleShot(300, controls._pb_plan_pause_immediate_clicked) @@ -53,5 +68,5 @@ def resume(): controls._timer.stop() assert seen == ["paused"] - assert RE.state == "idle", f"plan did not resume: RE.state={RE.state}" + assert engine.state == "idle", f"plan did not resume: state={engine.state}" assert motor.position == pytest.approx(1) From a8be7d306a7ced0681423f45b17e762054f120b2 Mon Sep 17 00:00:00 2001 From: sligara7 Date: Tue, 6 Oct 2026 13:34:09 -0400 Subject: [PATCH 3/3] GUI Resume: ignore a second click until the first resume returns; test uses RE Review on PR 94 (Jakub, agreeing with Copilot): scheduling resume() for after the click left the button enabled until the 0.5 s poll, so a double-click queued two resume() calls; the second ran while the plan was already running. Resume now disables itself on click and a pending flag keeps it disabled (the poller included) until resume() returns, at completion or at the next pause. The old worker-thread version had this guard; moving to the main thread dropped it. Verified with mock tests: test_double_click_on_resume_resumes_once observed failing (resume() called twice: paused, running), passes now; all 4 GUI tests pass. Tests name the engine RE (Jakub). Assisted-by: copilot:claude-opus-4-8 --- src/hextools/gui/re_execution_controls.py | 19 +++++--- tests/gui/test_re_execution_controls.py | 58 +++++++++++++++++++---- 2 files changed, 63 insertions(+), 14 deletions(-) diff --git a/src/hextools/gui/re_execution_controls.py b/src/hextools/gui/re_execution_controls.py index 414b8d9..571c3ad 100644 --- a/src/hextools/gui/re_execution_controls.py +++ b/src/hextools/gui/re_execution_controls.py @@ -75,6 +75,7 @@ def __init__( self._re_name = re_name self._namespace = namespace self._timer = None + self._resume_pending = False super().__init__(_NullModel() if local else model, parent) @@ -105,7 +106,7 @@ def _poll_local_state(self): paused = state == "paused" self._pb_plan_pause_deferred.setEnabled(running) self._pb_plan_pause_immediate.setEnabled(running) - self._pb_plan_resume.setEnabled(paused) + self._pb_plan_resume.setEnabled(paused and not self._resume_pending) self._pb_plan_stop.setEnabled(paused) self._pb_plan_abort.setEnabled(paused) self._pb_plan_halt.setEnabled(paused) @@ -133,17 +134,23 @@ def _pb_plan_resume_clicked(self): if not self._local: return super()._pb_plan_resume_clicked() run_engine = self._resolve_re() - if run_engine is None: + if run_engine is None or self._resume_pending: return + # A second click before resume() returns would queue another resume(). + self._resume_pending = True + self._pb_plan_resume.setEnabled(False) # resume() installs a SIGINT handler, which only the main thread may do, # so run it after this click returns, never on a worker thread. QTimer.singleShot(0, lambda: self._resume_on_main_thread(run_engine)) def _resume_on_main_thread(self, run_engine): - if _ipython_namespace().get(self._re_name) is run_engine: - run_in_ipython(f"{self._re_name}.resume()") - else: - self._call_re("resume") + try: + if _ipython_namespace().get(self._re_name) is run_engine: + run_in_ipython(f"{self._re_name}.resume()") + else: + self._call_re("resume") + finally: + self._resume_pending = False def _pb_plan_stop_clicked(self): if not self._local: diff --git a/tests/gui/test_re_execution_controls.py b/tests/gui/test_re_execution_controls.py index fb2b1aa..faf8ac9 100644 --- a/tests/gui/test_re_execution_controls.py +++ b/tests/gui/test_re_execution_controls.py @@ -34,10 +34,10 @@ def test_resume_button_resumes_a_paused_plan(given): matplotlib.use("qtagg") app = QApplication.instance() or QApplication([]) shell = InteractiveShell.instance() - engine = RunEngine({}) + RE = RunEngine({}) shell.user_ns.update( - RE=engine if given == "shell" else RunEngine({}), - engine=engine, + RE=RE if given == "shell" else RunEngine({}), + test_RE=RE, bp=bp, det=det, motor=motor, @@ -46,19 +46,19 @@ def test_resume_button_resumes_a_paused_plan(given): if given == "shell": controls = QtReExecutionControls(local=True, namespace=shell.user_ns) elif given == "re": - controls = QtReExecutionControls(local=True, re=engine) + controls = QtReExecutionControls(local=True, re=RE) else: - controls = QtReExecutionControls(local=True, namespace={"RE": engine}) + controls = QtReExecutionControls(local=True, namespace={"RE": RE}) seen = [] def resume(): - seen.append(engine.state) + seen.append(RE.state) controls._pb_plan_resume_clicked() QTimer.singleShot( 0, lambda: run_in_ipython( - "engine(bp.scan([det], motor, 0, 1, 10, per_step=_slow_step))" + "test_RE(bp.scan([det], motor, 0, 1, 10, per_step=_slow_step))" ), ) QTimer.singleShot(300, controls._pb_plan_pause_immediate_clicked) @@ -68,5 +68,47 @@ def resume(): controls._timer.stop() assert seen == ["paused"] - assert engine.state == "idle", f"plan did not resume: state={engine.state}" + assert RE.state == "idle", f"plan did not resume: state={RE.state}" assert motor.position == pytest.approx(1) + + +# PR 94 review: a double-click on Resume must resume once, not queue a second +# resume() that could run through a later pause or hit an idle engine. +def test_double_click_on_resume_resumes_once(): + matplotlib.use("qtagg") + app = QApplication.instance() or QApplication([]) + shell = InteractiveShell.instance() + RE = RunEngine({}) + shell.user_ns.update(RE=RE, bp=bp, det=det, motor=motor, _slow_step=_slow_step) + controls = QtReExecutionControls(local=True, namespace=shell.user_ns) + resumes = [] + real_resume = RE.resume + + def counting_resume(): + resumes.append(RE.state) + return real_resume() + + RE.resume = counting_resume + enabled_after_click = [] + + def double_click(): + controls._poll_local_state() + controls._pb_plan_resume_clicked() + enabled_after_click.append(controls._pb_plan_resume.isEnabled()) + controls._pb_plan_resume_clicked() + + QTimer.singleShot( + 0, + lambda: run_in_ipython( + "RE(bp.scan([det], motor, 0, 1, 10, per_step=_slow_step))" + ), + ) + QTimer.singleShot(300, controls._pb_plan_pause_immediate_clicked) + QTimer.singleShot(800, double_click) + QTimer.singleShot(5000, app.quit) + app.exec() + controls._timer.stop() + + assert resumes == ["paused"], f"resume() called {len(resumes)} times: {resumes}" + assert enabled_after_click == [False] + assert RE.state == "idle"