fix: install gate event filters after eventFilter's targets exist
CI / lint (pull_request) Successful in 1m13s
CI / test (3.12) (pull_request) Successful in 1m30s
CI / test (3.11) (pull_request) Successful in 1m38s
CI / test (3.13) (pull_request) Successful in 1m40s
CI / test-with-beamline-plugins (pxi_bec) (pull_request) Successful in 1m38s
CI / test-with-beamline-plugins (pxii_bec) (pull_request) Successful in 1m41s
CI / test-with-beamline-plugins (pxiii_bec) (pull_request) Successful in 1m51s
CI / test-with-coverage (pull_request) Successful in 2m6s
CI / coverage-analysis (pull_request) Successful in 3s
CI / lint (push) Successful in 31s
Docs build and publish / docker (push) Successful in 15s
CI / test (3.11) (push) Canceled after 37s
CI / test (3.12) (push) Canceled after 33s
CI / test (3.13) (push) Canceled after 32s
CI / test-with-beamline-plugins (pxi_bec) (push) Canceled after 28s
CI / test-with-beamline-plugins (pxii_bec) (push) Canceled after 27s
CI / test-with-beamline-plugins (pxiii_bec) (push) Canceled after 23s
CI / test-with-coverage (push) Canceled after 22s
CI / coverage-analysis (push) Canceled after 0s
Build and Publish / release (push) Successful in 21s

The locked-banner and Beamline-tab-bar filters were installed mid
__init__, but MainWindow.eventFilter reads sample_lists_tabs, which is
created later - every event delivered in between raised AttributeError
inside the filter ('Error calling Python override of eventFilter()'
spam) and broke widget teardown, cascading errors across CI tests and
wedging the pxiii_bec job. Filters now install late, next to
installEventFilter(self), and the filter guards its attribute reads so
construction/teardown-time events can never raise.

Also fold the staff-gate assertions into test_main_window_init: every
extra MainWindow construction raises the odds of the pre-existing
PySide SystemError flake, so don't build a window just for them.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit was merged in pull request #140.
This commit is contained in:
2026-08-14 10:54:20 +02:00
co-authored by Claude Fable 5
parent 727f19d52a
commit 190e728734
2 changed files with 73 additions and 71 deletions
+48 -34
View File
@@ -385,7 +385,8 @@ class MainWindow(QMainWindow):
banner = TitleLabel(title, holder)
banner.setCursor(Qt.CursorShape.PointingHandCursor)
banner.setToolTip("Only accessible for beamline scientists")
banner.installEventFilter(self)
# eventFilter install happens LATE in __init__ (next to
# installEventFilter(self)) — see the comment there.
holder_layout.addWidget(banner)
beamline_layout.addWidget(holder)
self._locked_beamline_banners.append(banner)
@@ -403,10 +404,6 @@ class MainWindow(QMainWindow):
self.left_column_tabs.addTab(beamline_page, "Beamline")
self.left_column_tabs.addTab(experiment_page, "Experiment")
# A click on the pgroup-gated (disabled) Beamline tab is otherwise
# eaten silently by the tab bar — the filter pops the explanation
# instead (same pattern as the Auxiliary-puck tab).
self.left_column_tabs.tabBar().installEventFilter(self)
# Only the visible page counts toward the height — same trick as the
# content stack below, else the taller page pads the other tab.
@@ -431,6 +428,7 @@ class MainWindow(QMainWindow):
# first widget now, not samcam.
else self._locked_beamline_banners[0].parentWidget()
)
assert beamline_first is not None # the locked banner always has its holder
for first in (beamline_first, self.data_collection, self.data_collection.file_path_panel):
first_layout = first.layout()
assert first_layout is not None # panels build their layouts in __init__
@@ -887,6 +885,15 @@ class MainWindow(QMainWindow):
self.daq = DAQWorker(base_url=self._base_url, token=self._token)
self.installEventFilter(self)
# These share MainWindow.eventFilter, which reads sample_lists_tabs
# and left_column_tabs on every event — so they must install only
# HERE, after every attribute the filter touches exists. Installing
# them at widget-construction time made each early event raise
# AttributeError inside the filter ("Error calling Python override
# of QObject::eventFilter()" spam that poisoned CI teardowns).
self.left_column_tabs.tabBar().installEventFilter(self)
for locked_banner in self._locked_beamline_banners:
locked_banner.installEventFilter(self)
self._idle_timer = QTimer(self)
self._idle_timer.setInterval(60_000)
@@ -3217,35 +3224,42 @@ class MainWindow(QMainWindow):
self._mark_user_interaction()
except Exception as e:
logger.debug(f"GUI interaction event filter error: {e}", exc_info=True)
# Non-staff click on the greyed-out Auxiliary-puck tab: only installed
# for non-staff, and tabAt() is geometric so it still sees the
# disabled tab — explain the lock instead of silently eating the click.
sample_tab_bar = self.sample_lists_tabs.tabBar()
if (
event.type() == QEvent.Type.MouseButtonPress
and sample_tab_bar is not None
and obj is sample_tab_bar
and sample_tab_bar.tabAt(event.position().toPoint()) == 1
):
self._show_reference_tools_staff_only_popup()
return True
# Non-staff click on a locked Beamline banner: same treatment as the
# Auxiliary-puck tab — explain the gate instead of eating the click.
if event.type() == QEvent.Type.MouseButtonPress and obj in self._locked_beamline_banners:
self._show_beamline_staff_only_popup()
return True
# Click on the pgroup-gated (disabled) Beamline tab: tabAt() is
# geometric, so it still sees the disabled tab under the cursor.
left_tab_bar = self.left_column_tabs.tabBar()
if (
event.type() == QEvent.Type.MouseButtonPress
and left_tab_bar is not None
and obj is left_tab_bar
and left_tab_bar.tabAt(event.position().toPoint()) == 0
and not self.left_column_tabs.isTabEnabled(0)
):
self._show_beamline_tab_gated_popup()
return True
# getattr defaults: this filter also runs for events delivered while
# __init__ is still building (or teardown is tearing down) the very
# widgets it inspects — a raise here spams every event and breaks
# widget cleanup, so missing attributes must mean "not my click".
if event.type() == QEvent.Type.MouseButtonPress:
# Non-staff click on the greyed-out Auxiliary-puck tab: only
# installed for non-staff, and tabAt() is geometric so it still
# sees the disabled tab — explain the lock instead of silently
# eating the click.
sample_tabs = getattr(self, "sample_lists_tabs", None)
sample_tab_bar = sample_tabs.tabBar() if sample_tabs is not None else None
if (
sample_tab_bar is not None
and obj is sample_tab_bar
and sample_tab_bar.tabAt(event.position().toPoint()) == 1
):
self._show_reference_tools_staff_only_popup()
return True
# Non-staff click on a locked Beamline banner: same treatment as
# the Auxiliary-puck tab — explain the gate, don't eat the click.
if obj in getattr(self, "_locked_beamline_banners", ()):
self._show_beamline_staff_only_popup()
return True
# Click on the pgroup-gated (disabled) Beamline tab: tabAt() is
# geometric, so it still sees the disabled tab under the cursor.
left_tabs = getattr(self, "left_column_tabs", None)
if left_tabs is not None:
left_tab_bar = left_tabs.tabBar()
if (
left_tab_bar is not None
and obj is left_tab_bar
and left_tab_bar.tabAt(event.position().toPoint()) == 0
and not left_tabs.isTabEnabled(0)
):
self._show_beamline_tab_gated_popup()
return True
return super().eventFilter(obj, event)
def _start_remote_close_countdown(
+25 -37
View File
@@ -50,6 +50,27 @@ def test_main_window_init(qtbot, mock_ui_state):
assert win.windowTitle() == "AareGUI"
assert win.isVisible()
# Staff-side gate behaviors piggyback on this window: every extra
# MainWindow construction raises the odds of the PySide SystemError
# flake, so don't build another one just for these.
from aarecommon.models.models import BeamlineStateEnum
with patch("aare.gui.main_window.QMessageBox") as popup:
win._show_beamline_tab_gated_popup()
assert popup.information.called, "staff popup must explain the visitor pgroup"
win._apply_default_sample_tab(BeamlineStateEnum.BeamLocation)
assert win.sample_lists_tabs.currentIndex() == 1 # Auxiliary puck
win._apply_default_sample_tab(BeamlineStateEnum.DataCollection)
assert win.sample_lists_tabs.currentIndex() == 0
# Energy row inside Exp. Config. emits eV (the GUI shows keV)
sent = []
win.data_collection.change_energy.connect(sent.append)
win.data_collection.energy_spin.setValue(12.4)
win.data_collection._emit_change_energy()
assert sent and abs(sent[0] - 12400.0) < 1e-6
def test_main_window_mount_view(qtbot, mock_ui_state):
with (
@@ -551,41 +572,8 @@ def test_nonstaff_beamline_gate_popups(qtbot, mock_ui_state):
qtbot.mousePress(bar, Qt.MouseButton.LeftButton, pos=bar.tabRect(0).center())
assert popup.information.called, "gated tab click must explain the gate"
def test_staff_gated_tab_and_aux_default_tab(qtbot, mock_ui_state):
"""Staff variants of the gates: the gated-tab popup explains the visitor
pgroup (not the staff gate), and alignment states raise the Auxiliary
puck as the default sample tab."""
from aarecommon.models.models import BeamlineStateEnum
with (
patch("requests.get"),
patch("aare.gui.main_window.DAQWorker"),
patch("aare.gui.main_window.PredictionSubscriber"),
patch("aare.gui.main_window.VideoThread"),
patch("aare.gui.main_window.JFJochDBusClient"),
patch("aare.gui.main_window.jwt.decode") as mock_jwt,
):
mock_jwt.return_value = {
"sub": "testuser",
"staff": True,
"pgroups": ["p123"],
"session": 15,
}
win = _make_window(qtbot)
# The greyed-out Auxiliary-puck tab explains itself the same way.
aux_bar = win.sample_lists_tabs.tabBar()
with patch("aare.gui.main_window.QMessageBox") as popup:
win._show_beamline_tab_gated_popup()
assert popup.information.called
win._apply_default_sample_tab(BeamlineStateEnum.BeamLocation)
assert win.sample_lists_tabs.currentIndex() == 1 # Auxiliary puck
win._apply_default_sample_tab(BeamlineStateEnum.DataCollection)
assert win.sample_lists_tabs.currentIndex() == 0
# Energy row inside Exp. Config. emits eV (the GUI shows keV)
sent = []
win.data_collection.change_energy.connect(sent.append)
win.data_collection.energy_spin.setValue(12.4)
win.data_collection._emit_change_energy()
assert sent and abs(sent[0] - 12400.0) < 1e-6
qtbot.mousePress(aux_bar, Qt.MouseButton.LeftButton, pos=aux_bar.tabRect(1).center())
assert popup.information.called, "aux-puck tab click must explain the lock"