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")