From 4d3734409d837a0360c1b5fff795deae0cdb2fd7 Mon Sep 17 00:00:00 2001 From: JeffreyChen Date: Fri, 21 Aug 2026 01:07:58 +0800 Subject: [PATCH] Give three skipped tests the isolation their skip reason asked for MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Their own reason said what they needed: "needs subprocess isolation (see test_actions_menu_gui) ... skip until then". They cover real wiring — a file received on a WebRTC worker thread reaching the GUI thread through a queued signal rather than a thread-affine singleShot, and the admin console's thumbnail poll deleting its QThread each tick instead of leaking one per interval. Skipping was right at the time. Building the WebRTC panel or the admin console and then tearing a worker QThread down aborts the shared pytest process under offscreen Qt, and since deleteLater is a no-op until an event loop runs, the abort lands in some later unrelated file with no traceback. Run all three in one child process that writes a JSON verdict per check and exits without teardown, the shape test_actions_menu_gui already uses. Verdicts are ok / failed / unavailable, so a machine without the webrtc extra reports a skip while one that has it checks the wiring. Confirmed to have teeth rather than assumed: dropping thread.finished.connect(thread.deleteLater) turns the third verdict into "failed: the QThread outlived finish" and leaves the other two green. --- Progress.md | 16 -- README/WHATS_NEW_zh-CN.md | 25 +++ README/WHATS_NEW_zh-TW.md | 25 +++ WHATS_NEW.md | 32 +++ architecture_explore.md | 2 +- .../headless/test_r3_gui_thread_marshal.py | 196 +++++++++++++----- 6 files changed, 231 insertions(+), 65 deletions(-) diff --git a/Progress.md b/Progress.md index fe0d2f81..5f844a12 100644 --- a/Progress.md +++ b/Progress.md @@ -219,22 +219,6 @@ capability enum 值與 variadic `ei_seat_bind_capabilities`、event-type enum --- -## 三個 Qt thread-marshal 測試永久跳過中 - -`TODO` — 需要子行程隔離，做法已知 - -`test/unit_test/headless/test_r3_gui_thread_marshal.py` 裡三個測試被無條件 skip： -`test_panel_signals_expose_file_received`、`test_webrtc_received_file_marshaled_to_gui`、 -`test_thumbnail_poll_thread_is_reaped`。skip 理由自己寫著「needs subprocess -isolation (see test_actions_menu_gui) … skip until then」——那句「until then」就是這條。 - -跟 `CLAUDE.md` §Testing 記的 0xC0000409 `__fastfail` 是同一個家族： -worker→GUI 的 teardown 在共用的 pytest 行程裡把整個監獸帶走。 -`test_actions_menu_gui.py` 已經示範過解法（把建 widget 的部分丟進子行程）， -這三個只是還沒改過去。沒改之前，這三條路徑沒有任何回歸保護。 - ---- - ## 兩個講好要爬、還沒爬的門檻 `TODO` — 兩者都寫在 `pyproject.toml` 的註解裡，但不在任何待辦清單上 diff --git a/README/WHATS_NEW_zh-CN.md b/README/WHATS_NEW_zh-CN.md index 5fba5d44..f2ef00e0 100644 --- a/README/WHATS_NEW_zh-CN.md +++ b/README/WHATS_NEW_zh-CN.md @@ -2,6 +2,31 @@ ## 本次更新 (2026-08-20) — 声称支援的平台，這回真的量過了 +### 三个从写出来就一直被跳过的测试 + +`test_r3_gui_thread_marshal.py` 里有三个 `@pytest.mark.skip`，而它们的 skip 理由 +自己就写着需要什么：“needs subprocess isolation (see test_actions_menu_gui) +… skip until then”。它们盖的是真的接线：WebRTC worker 线程收到的文件要通过 +queued signal 回到 GUI 线程（而不是线程绑定的 `QTimer.singleShot`），以及 +admin console 的缩图轮询每一 tick 都要把 `QThread` 删掉，而不是每个间隔漏一个。 + +当时跳过是对的：在**共用的** pytest 进程里建 WebRTC 面板或 admin console、 +再拆掉 worker `QThread`，在 offscreen Qt 下会直接 abort。而因为 `deleteLater` 在没有 +event loop 跑之前是 no-op，那个 abort 甚至不会落在胇事的那支测试上——它会在后面某支 +毫无关系的文件里引爆，而且没有 traceback。 + +**现在它们跑了，在自己的进程里。** 一支 probe 把三项检查一次做完，每项写一个 JSON +结论，然后 `os._exit(0)` 不做 teardown——跟 `test_actions_menu_gui` 对整组标签页用的是 +同一个形状。每项结论是 `ok`、`failed: …` 或 `unavailable: …`，所以没装 `[webrtc]` +extra 的机器（CI 的 `pytest-headless` 就是）得到的是 skip 而不是失败，装了的机器则真的在验接线。 + +这三项检查是有牙的，而且是验过不是假设的：把 `admin_console_tab.py` 里 +`thread.finished.connect(thread.deleteLater)` 那一行拿掉，第三项结论就变成 +`failed: the QThread outlived finish`，而另外两项依旧绿。 + +headless 整套现在不需要任何 `--ignore` 就能从头跑到尾——共 4,815 项通过， +剩下的 skip 全部是可选依赖与平台闸门。里面已经没有任何一句“skip until then”。 + ### Windows arm64 从来不是代码的问题 这一项原本标成 `BLOCKED`，而那只对一半。两个依赖确实没发 diff --git a/README/WHATS_NEW_zh-TW.md b/README/WHATS_NEW_zh-TW.md index 928718be..2f14c60f 100644 --- a/README/WHATS_NEW_zh-TW.md +++ b/README/WHATS_NEW_zh-TW.md @@ -2,6 +2,31 @@ ## 本次更新 (2026-08-20) — 嬣稱支援的平台，這回真的量過了 +### 三個從寫出來就一直被跳過的測試 + +`test_r3_gui_thread_marshal.py` 裡有三個 `@pytest.mark.skip`，而它們的 skip 理由 +自己就寫著需要什麼：「needs subprocess isolation (see test_actions_menu_gui) +… skip until then」。它們蓋的是真的接線：WebRTC worker 執行緒收到的檔案要透過 +queued signal 回到 GUI 執行緒（而不是執行緒綁定的 `QTimer.singleShot`），以及 +admin console 的縮圖輪詢每一 tick 都要把 `QThread` 刪掉，而不是每個間隔漏一個。 + +當時跳過是對的：在**共用的** pytest 行程裡建 WebRTC 面板或 admin console、 +再拆掉 worker `QThread`，在 offscreen Qt 下會直接 abort。而因為 `deleteLater` 在沒有 +event loop 跑之前是 no-op，那個 abort 甚至不會落在胇事的那支測試上——它會在後面某支 +毫無關係的檔案裡引爆，而且沒有 traceback。 + +**現在它們跑了，在自己的行程裡。** 一支 probe 把三項檢查一次做完，每項寫一個 JSON +結論，然後 `os._exit(0)` 不做 teardown——跟 `test_actions_menu_gui` 對整組分頁用的是 +同一個形狀。每項結論是 `ok`、`failed: …` 或 `unavailable: …`，所以沒裝 `[webrtc]` +extra 的機器（CI 的 `pytest-headless` 就是）得到的是 skip 而不是失敗，裝了的機器則真的在驗接線。 + +這三項檢查是有牙的，而且是驗過不是假設的：把 `admin_console_tab.py` 裡 +`thread.finished.connect(thread.deleteLater)` 那一行拿掉，第三項結論就變成 +`failed: the QThread outlived finish`，而另外兩項依舊綠。 + +headless 整套現在不需要任何 `--ignore` 就能從頭跑到尾——共 4,815 項通過， +剩下的 skip 全部是選用相依與平台閘門。裡面已經沒有任何一句「skip until then」。 + ### Windows arm64 從來不是程式的問題 這一項原本標成 `BLOCKED`，而那只對一半。兩個相依確實沒發 diff --git a/WHATS_NEW.md b/WHATS_NEW.md index b20a210c..f61dbddf 100644 --- a/WHATS_NEW.md +++ b/WHATS_NEW.md @@ -2,6 +2,38 @@ ## What's new (2026-08-20) +### Three Tests That Had Been Skipped Since They Were Written + +`test_r3_gui_thread_marshal.py` carried three `@pytest.mark.skip`s whose own +reason said what they needed: *"needs subprocess isolation (see +test_actions_menu_gui) … skip until then."* They covered real wiring — that a +file received on a WebRTC worker thread reaches the GUI thread through a +queued signal rather than a thread-affine `QTimer.singleShot`, and that the +admin console's thumbnail poll deletes its `QThread` each tick instead of +leaking one per interval. + +Skipping them was the right call at the time: building the WebRTC panel or the +admin console and then tearing a worker `QThread` down aborts the *shared* +pytest process under offscreen Qt. Because `deleteLater` is a no-op until an +event loop runs, the abort does not even land in the test that caused it — it +detonates inside some later, unrelated file, with no traceback. + +**They now run, in their own process.** One probe performs all three checks, +writes a JSON verdict per check, and `os._exit(0)`s without teardown — the same +shape `test_actions_menu_gui` has used for the full tab set. Each verdict is +`ok`, `failed: …` or `unavailable: …`, so a machine without the `[webrtc]` +extra (CI's `pytest-headless`, among others) reports a skip rather than a +failure, while a machine that has it actually checks the wiring. + +The checks have teeth, which was verified rather than assumed: deleting the +one line `thread.finished.connect(thread.deleteLater)` from +`admin_console_tab.py` turns the third verdict into `failed: the QThread +outlived finish`, and leaves the other two green. + +The headless suite now runs end to end with no `--ignore` flags — 4,815 +passing, and the only remaining skips are optional-dependency and +platform gates. No "skip until then" is left in it. + ### Windows arm64 Was Never a Code Problem The entry for this said `BLOCKED`, and that was half right. Two dependencies diff --git a/architecture_explore.md b/architecture_explore.md index 5e245314..79bf9d62 100644 --- a/architecture_explore.md +++ b/architecture_explore.md @@ -6,7 +6,7 @@ > 擷取每個模組的 docstring 與頂層公開名稱；統計數字取自實際檔案，非估算。 > 指令數與公開 API 數以 `executor.known_commands()` 與 `je_auto_control.__all__` 在工作樹上實測取得。 > -> **掃描時間**：2026-08-20　**版本**：`pyproject.toml` version `0.0.220`　**分支**：`feat/windows-arm64-install` +> **掃描時間**：2026-08-21　**版本**：`pyproject.toml` version `0.0.220`　**分支**：`feat/qt-thread-marshal-isolation` --- diff --git a/test/unit_test/headless/test_r3_gui_thread_marshal.py b/test/unit_test/headless/test_r3_gui_thread_marshal.py index a6c073fa..4c735ce9 100644 --- a/test/unit_test/headless/test_r3_gui_thread_marshal.py +++ b/test/unit_test/headless/test_r3_gui_thread_marshal.py @@ -9,8 +9,21 @@ Each test drives the real method from a background ``threading.Thread`` and pumps the GUI event loop, so a queued signal is required for the effect to appear (a thread-affine ``singleShot`` would not fire). + +Three of these run in a subprocess. Constructing the WebRTC panel or the +admin console, then tearing a worker QThread down, aborts the *shared* pytest +process under offscreen Qt (``0xC0000409`` / SIGABRT) — and because +``deleteLater`` is a no-op until an event loop runs, the abort lands inside +some later, unrelated test file. They were skipped outright for that reason; +quarantining the Qt lifetime in a child process is what +``test_actions_menu_gui`` already does, and it is what they needed. The child +writes a JSON verdict per check and ``os._exit(0)``s without teardown. """ +import json import os +import pathlib +import subprocess +import sys import threading import time @@ -19,22 +32,10 @@ os.environ.setdefault("QT_QPA_PLATFORM", "offscreen") pytest.importorskip("PySide6.QtWidgets", exc_type=ImportError) -import shiboken6 # noqa: E402 -from PySide6.QtCore import QEvent, QObject, QThread # noqa: E402 from PySide6.QtWidgets import QApplication # noqa: E402 -# Constructing the WebRTC panel / admin-console QThread teardown inside the -# SHARED pytest process natively aborts (SIGABRT/0xC0000409) under the offscreen -# Qt platform on CI — accumulated Qt state across GUI tests corrupts on teardown. -# The product paths these cover are exercised by the full-widget build in -# test_actions_menu_gui, which is deliberately run in an isolated subprocess for -# exactly this reason. These need the same subprocess isolation before they can -# run in-process; skip until then rather than crash the whole suite. -_OFFSCREEN_SHARED_PROC_ABORT = ( - "worker->GUI teardown aborts the shared pytest process under offscreen Qt; " - "needs subprocess isolation (see test_actions_menu_gui)" -) +REPO_ROOT = pathlib.Path(__file__).resolve().parents[3] @pytest.fixture(scope="module") @@ -93,16 +94,50 @@ def test_presence_registry_event_marshaled_to_gui(qapp): tab.deleteLater() -# --- Finding 8: WebRTC file-received callback ------------------------------ +# --- The three that need their own process -------------------------------- + +# Each check returns "ok", "failed: ...", or "unavailable: ..." so a missing +# optional extra reads as a skip rather than a failure: CI's pytest-headless +# does not install [webrtc], and importing the panel without it raises. +_PROBE = r""" +import json +import os +import pathlib +import sys +import tempfile +import threading +import time + +os.environ.setdefault("QT_QPA_PLATFORM", "offscreen") + +import shiboken6 +from PySide6.QtCore import QEvent, QObject, QThread +from PySide6.QtWidgets import QApplication + +app = QApplication.instance() or QApplication([]) +report = {} + + +def pump_until(predicate, timeout=3.0): + deadline = time.monotonic() + timeout + while not predicate() and time.monotonic() < deadline: + app.processEvents() + time.sleep(0.005) + return predicate() + + +def run_off_thread(target): + thread = threading.Thread(target=target) + thread.start() + thread.join(3.0) -@pytest.mark.skip(reason=_OFFSCREEN_SHARED_PROC_ABORT) -def test_panel_signals_expose_file_received(): + +def check_panel_signals(): from je_auto_control.gui.remote_desktop.webrtc_panel import _PanelSignals - assert hasattr(_PanelSignals(), "file_received") + assert hasattr(_PanelSignals(), "file_received"), "no file_received signal" -@pytest.mark.skip(reason=_OFFSCREEN_SHARED_PROC_ABORT) -def test_webrtc_received_file_marshaled_to_gui(qapp): +def check_webrtc_marshal(): import types from je_auto_control.gui.remote_desktop.webrtc_panel import ( _PanelSignals, _WebRTCViewerPanel, @@ -110,7 +145,7 @@ def test_webrtc_received_file_marshaled_to_gui(qapp): signals = _PanelSignals() - class _Receiver(QObject): + class Receiver(QObject): def __init__(self): super().__init__() self.got = None @@ -118,42 +153,107 @@ def __init__(self): def on_file(self, path): self.got = path - recv = _Receiver() - signals.file_received.connect(recv.on_file) + receiver = Receiver() + signals.file_received.connect(receiver.on_file) stub = types.SimpleNamespace(_signals=signals) - _run_off_thread(lambda: _WebRTCViewerPanel._on_received_file(stub, "file-123")) - assert _pump_until(qapp, lambda: recv.got == "file-123") - + run_off_thread( + lambda: _WebRTCViewerPanel._on_received_file(stub, "file-123")) + assert pump_until(lambda: receiver.got == "file-123"), ( + "the file-received callback never reached the GUI thread; a " + "thread-affine QTimer.singleShot would fail exactly like this") -# --- Finding 9: thumbnail poll thread is reaped on finish ------------------ -@pytest.mark.skip(reason=_OFFSCREEN_SHARED_PROC_ABORT) -def test_thumbnail_poll_thread_is_reaped(qapp, monkeypatch, tmp_path): +def check_thumbnail_reaped(): import je_auto_control.gui.admin_console_tab as admin_mod from je_auto_control.utils.admin.admin_client import AdminConsoleClient - client = AdminConsoleClient(persist_path=tmp_path / "hosts.json") - monkeypatch.setattr(admin_mod, "default_admin_console", lambda: client) + tmp = pathlib.Path(tempfile.mkdtemp()) + client = AdminConsoleClient(persist_path=tmp / "hosts.json") + admin_mod.default_admin_console = lambda: client # Don't run a real background thread: the reaping wiring is what matters, - # and this keeps the test deterministic (no timing, no dangling threads). - monkeypatch.setattr(admin_mod.QThread, "start", lambda self: None) + # and this keeps the check deterministic (no timing, no dangling threads). + admin_mod.QThread.start = lambda self: None tab = admin_mod.AdminConsoleTab() + tab._thumb_timer.stop() + tab._refresh_thumbnails() + + thread = tab._thumb_thread + assert thread is not None, "no thumbnail QThread was created" + assert thread in tab.findChildren(QThread), "thread is not a child of the tab" + + thread.finished.emit() # simulate the QThread finishing + assert tab._thumb_thread is None, "_on_thumb_thread_done did not run" + # Flush the deferred deletions the finished signal scheduled. + app.sendPostedEvents(None, QEvent.Type.DeferredDelete.value) + # Without the deleteLater wiring the QThread would linger as a child of + # the tab, accumulating one per poll tick. + assert not shiboken6.Shiboken.isValid(thread), "the QThread outlived finish" + assert tab.findChildren(QThread) == [], "a QThread lingers as a child" + + +for name, check in [ + ("panel_signals", check_panel_signals), + ("webrtc_marshal", check_webrtc_marshal), + ("thumbnail_reaped", check_thumbnail_reaped), +]: try: - tab._thumb_timer.stop() - tab._refresh_thumbnails() - thread = tab._thumb_thread - assert thread is not None - assert thread in tab.findChildren(QThread) - - thread.finished.emit() # simulate the QThread finishing - assert tab._thumb_thread is None # _on_thumb_thread_done ran - # Flush the deferred deletions the finished signal scheduled. - qapp.sendPostedEvents(None, QEvent.Type.DeferredDelete.value) - # Without the deleteLater wiring the QThread would linger as a child of - # the tab, accumulating one per poll tick. - assert not shiboken6.Shiboken.isValid(thread) - assert tab.findChildren(QThread) == [] - finally: - tab.deleteLater() + check() + except ImportError as error: + report[name] = "unavailable: %s" % (error,) + except AssertionError as error: + report[name] = "failed: %s" % (error or "assertion failed",) + except BaseException as error: + report[name] = "error: %s: %s" % (type(error).__name__, error) + else: + report[name] = "ok" + +sys.stdout.write(json.dumps(report)) +sys.stdout.flush() +# Skip Qt/native-thread teardown entirely -- that teardown is the whole +# reason this runs out of process. The report is already on stdout. +os._exit(0) +""" + + +@pytest.fixture(scope="module") +def marshal_report(): + """Run all three checks in one child process; return its verdicts.""" + env = dict(os.environ, PYTHONPATH=str(REPO_ROOT)) + env.setdefault("QT_QPA_PLATFORM", "offscreen") + # argv is this interpreter plus a module-level literal probe. No shell. + completed = subprocess.run( # nosec B603 # nosemgrep # reason: literal argv, no shell + [sys.executable, "-c", _PROBE], + capture_output=True, text=True, check=False, timeout=180, env=env, + ) + if completed.returncode != 0 or not completed.stdout: + pytest.fail( + "thread-marshal probe subprocess failed " + f"(exit {completed.returncode}):\n{completed.stdout}\n{completed.stderr}" + ) + return json.loads(completed.stdout) + + +def _verdict(report, key): + """Turn one probe verdict into a pass, a skip or a named failure.""" + status = report.get(key) + assert status is not None, f"{key} missing from the probe report: {report}" + if status.startswith("unavailable:"): + pytest.skip(status) + assert status == "ok", status + + +def test_panel_signals_expose_file_received(marshal_report): + """The panel declares the signal the worker hands the file back on.""" + _verdict(marshal_report, "panel_signals") + + +def test_webrtc_received_file_marshaled_to_gui(marshal_report): + """A file received off-thread reaches the GUI thread via a queued signal.""" + _verdict(marshal_report, "webrtc_marshal") + + +def test_thumbnail_poll_thread_is_reaped(marshal_report): + """The thumbnail poll deletes its QThread per tick instead of leaking one.""" + _verdict(marshal_report, "thumbnail_reaped")