Skip to content

fix(ci): run GUI test under pyqt5 instead of skipping#595

Open
hongquanli wants to merge 6 commits into
masterfrom
fix/ci-gui-test
Open

fix(ci): run GUI test under pyqt5 instead of skipping#595
hongquanli wants to merge 6 commits into
masterfrom
fix/ci-gui-test

Conversation

@hongquanli

Copy link
Copy Markdown
Contributor

Summary

tests/control/test_HighContentScreeningGui.py has been --ignore'd in CI since #391 with the note "PySide6 QObject double-init bug". This PR stops skipping it:

  • Set QT_API: pyqt5 and PYTEST_QT_API: pyqt5 as job-level env in main.yml, forcing qtpy/pytest-qt onto PyQt5 (the known-good local recipe).
  • Drop the --ignore from the pytest invocation.

setup_22.04.sh already installs PyQt5 via apt (python3-pyqt5), so no dependency changes are needed.

Verification

  • Locally headless under the pin (pristine configurations/configuration_Squid+.ini, same file CI copies): 5 passed in 17.9s.
  • CI on this PR is the authoritative check — it runs the full suite including the previously-skipped file under Xvfb.

Open question (documented, not fixed here)

The PySide6 QObject double-init failure itself is unchanged: under PySide6, GUI construction in the test fails (qtpy picks PySide6 when it's importable). Pinning CI to PyQt5 matches the production launch path (main_hcs.py and run_acquisition.py both set QT_API=pyqt5), so this is an acceptable steady state; root-causing the PySide6 path (likely the QtMultiPointController(MultiPointController, QObject) multiple-inheritance init pattern) remains a separate issue if PySide6 support is ever wanted.

🤖 Generated with Claude Code

hongquanli and others added 2 commits July 12, 2026 16:25
test_HighContentScreeningGui.py has been --ignore'd in CI since #391 due to
a PySide6 QObject double-init failure. The known-good local recipe (force
qtpy/pytest-qt onto PyQt5) works headless, and setup_22.04.sh already
installs PyQt5 via apt (python3-pyqt5), so pin QT_API/PYTEST_QT_API=pyqt5
at the job level and drop the --ignore.

Verified locally headless under the pin: 5 passed in 17.9s.

The PySide6 double-init itself remains unfixed (tracked separately).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
First CI run showed the pyqt5 pin works (the GUI test passed), but running
it in the shared pytest process deterministically segfaults a later Qt test
(test_channel_sequence.py:116 qtbot.waitUntil) — reproduced locally on
macOS with the same two-file ordering (exit 139). The GUI test leaves
Qt/thread state behind (faulthandler dump shows the simulated-camera
stream thread and slack-notifier worker still alive). Keep the main run
ignoring it and add a second pytest invocation for the GUI test alone.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@hongquanli

Copy link
Copy Markdown
Contributor Author

CI finding + fix (second commit): the first run proved the pyqt5 pin works — test_HighContentScreeningGui.py passed (5/5) in CI — but the job then segfaulted at test_channel_sequence.py:116 (qtbot.waitUntil, exit 139). Reproduced deterministically on macOS with just the two files in one process: the full-application GUI test leaves Qt/thread state behind (faulthandler dump shows the simulated-camera stream thread and slack-notifier worker still alive) that crashes a later Qt event loop.

Fix: keep the main pytest run ignoring the GUI test and run it as its own pytest invocation in a second step — it is now enforced in CI, just process-isolated.

This also resolves the original mystery: with no env pin, pytest-qt picked PySide6 for qtbot while the app code (qtpy) preferred PyQt5 — the "QObject double-init" was a mixed-bindings artifact, not a PySide6 port bug per se. Follow-up candidates (separate issues): teardown of the GUI test's background threads, and a real PySide6 compatibility pass if ever desired.

With the job-level pin, the main pytest run passed all 1413 tests and then
segfaulted at interpreter shutdown — the pin flips pytest-qt/qtpy binding
selection for the whole suite, hitting PyQt5's destructor-order-vs-GC
conflict at exit (the reason main_hcs.py uses os._exit()). Master runs the
main suite unpinned and green, so keep it identical and pin only the
isolated GUI-test step, which is the invocation that needs it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@hongquanli

Copy link
Copy Markdown
Contributor Author

Second CI finding: the job-level pin had a wider blast radius than intended — the main run (still --ignoreing the GUI test, i.e. exactly master's suite) passed all 1413 tests and then segfaulted at interpreter shutdown. The pin flips binding selection for every Qt test's teardown, hitting PyQt5's destructor-order-vs-GC conflict at exit (the documented reason main_hcs.py exits via os._exit()). Master runs this suite unpinned and green.

Pushed 0d3be569: the main run is now byte-identical to master's invocation, and the QT_API/PYTEST_QT_API=pyqt5 pin is scoped to the isolated GUI-test step only — the one invocation that needs it (and which passed 5/5 in CI under the pin in the first run).

Third CI iteration: the isolated GUI-test step passed (5/5) but the
process segfaulted during interpreter shutdown — under PyQt5, a process
that constructed the full HCS GUI crashes in Qt C++ destructors at exit
even when every test passed. This is the same crash main_hcs.py avoids
with os._exit(). Add an env-gated (SQUID_PYTEST_HARD_EXIT=1)
pytest_unconfigure hook that flushes output and os._exit()s with
pytest's real exit status, and set the variable only on the isolated CI
step. Verified locally: summary still prints, pass exits 0, failure
exits 1.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@hongquanli

Copy link
Copy Markdown
Contributor Author

Third CI finding + fix (9de2f061): with the step-scoped pin, the main suite passed and the isolated GUI-test step passed 5/5 — but that process still segfaulted during interpreter shutdown (after the green summary). Under PyQt5, a process that constructed the full HCS GUI crashes in Qt C++ destructors at exit even when every test passed — the exact crash main_hcs.py avoids with os._exit().

Fix mirrors the product's own solution: an env-gated (SQUID_PYTEST_HARD_EXIT=1) pytest_unconfigure hook that flushes output and os._exit()s with pytest's real exit status, set only on the isolated CI step. Verified locally: summary still prints, pass → exit 0, failure → exit 1.

hongquanli and others added 2 commits July 12, 2026 17:52
Review finding (Opus): if pytest_sessionfinish never runs (a
pytest_sessionstart failure leaves initstate==1, where unconfigure still
fires), the previous default of 0 would turn a broken session into a
green CI step. Default to 1 so an unrecorded status fails the gate.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… suite has the same flaky exit segfault

The round-4 CI failure on this PR was the main run (byte-identical
invocation to master's) segfaulting at interpreter shutdown after 1413
tests passed. Master itself fails the same way — 3 of its last 15 runs:
'N passed' followed by 'Segmentation fault (core dumped)' exit 139
(runs 28883871972, 28762269294, 28559414408). This also retroactively
explains this PR's round-2 failure, which was misattributed to the
job-level Qt pin.

Set SQUID_PYTEST_HARD_EXIT=1 on the main step as well: after the session
completes and all reporting is written, the conftest hook os._exit()s
with pytest's real status — genuine failures still fail the step, and
the ~20%-flaky Qt-teardown crash stops failing green runs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@hongquanli

Copy link
Copy Markdown
Contributor Author

Round-4 CI failure root-caused — it's a pre-existing master flake, and this PR now fixes it (649c0b8a).

The failing run was the main pytest step — byte-identical invocation to master's (--ignore on the GUI test, no Qt pin) — passing all 1413 tests and then segfaulting at interpreter shutdown. Master itself fails exactly this way in 3 of its last 15 runs (28883871972 on 24276973, 28762269294 on 780d80e3, 28559414408 on 5faaad47): N passedSegmentation fault (core dumped) → exit 139. This also retroactively explains round 2 here, which I misattributed to the job-level Qt pin.

Fix: set SQUID_PYTEST_HARD_EXIT=1 on the main step too. The hook runs after the session completes and all reporting (junit/terminal/coverage) is written, and exits with pytest's real status — real failures still fail the step; only the post-green Qt-teardown crash is neutralized. Net effect for the repo: a ~20% CI flake on master goes away.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant