diff --git a/app/update_controller.py b/app/update_controller.py new file mode 100644 index 0000000..1be40f3 --- /dev/null +++ b/app/update_controller.py @@ -0,0 +1,177 @@ +"""PDFApps – UpdateController: the MainWindow auto-update subsystem. + +Extracted verbatim from ``app/window.py`` (R4 refactor). Owns the +background update-check worker (a ``QObject`` moved onto a ``QThread``), +the notify/decision step and the update dialog. Behaviour, threading and +worker lifecycle are unchanged — only the code's home moved. + +Threading note (preserved deliberately): the controller is a ``QObject`` +whose parent is the main window, so it keeps main-thread affinity. The +worker's ``done`` signal therefore delivers ``_on_update_found`` back on +the main thread via Qt's automatic queued connection — exactly as it did +when these were ``MainWindow`` methods. No lambda crosses the thread +boundary (a bound QObject method is used), which is the Py3.14-safe +pattern this project relies on. +""" + +import contextlib +import os + +from PySide6.QtCore import QObject + +from app.i18n import t + + +class UpdateController(QObject): + """Encapsulates the MainWindow auto-update check, worker and dialog. + + Composed by ``MainWindow`` as ``self._update_controller``. The window + still owns the toolbar update button (part of its layout) and passes + it in so the controller can reveal it when an update is found and wire + its click to :meth:`show_update_dialog`. + """ + + def __init__(self, window, update_button): + # Parent to the window → main-thread affinity + lifetime tied to + # the window. This is what makes the worker's cross-thread ``done`` + # signal resume on the main thread. + super().__init__(window) + self._window = window + self._update_btn = update_button + self._update_release = None + self._update_thread = None + self._update_worker = None + # Holds the in-flight urlopen response of the background update + # check so release_worker can abort a blocked network read from + # closeEvent instead of waiting out the socket timeout. + self._update_cancel = None + + def check_async(self): + # Skip auto-update inside Flatpak/Snap — the host package + # manager handles updates. MSIX/Microsoft Store installs are + # short-circuited inside check_for_update() itself (the Store + # updates packaged apps automatically), so they never reach the + # NSIS download path. + if os.environ.get("FLATPAK_ID") or os.environ.get("SNAP"): + return + # Pre-import the updater module on the main thread BEFORE the + # worker thread starts. The worker would otherwise lazy-import + # `app.updater`, which transitively pulls in `urllib.request` + # → `http.client` → `ssl`. On Python 3.14, importing those + # heavy modules from a non-main thread while the main thread + # is still busy creating Qt widgets races against each other + # (CPython's import machinery + Qt's widget construction + + # garbage collection on the worker side) and produces a + # Windows access-violation crash. Doing the import here keeps + # all that import work on the main thread; the worker only + # calls the already-imported function. + from app.updater import check_for_update + from PySide6.QtCore import QThread, QObject, Signal as _Sig + + # Shared holder so the main thread can close the worker's urlopen + # response and unblock its network read on shutdown (M4). + self._update_cancel = {"resp": None} + + class _Worker(QObject): + done = _Sig() + def __init__(self, cancel_holder): + super().__init__() + self.release = None + self._cancel = cancel_holder + def run(self): + self.release = check_for_update(self._cancel) + if self.release: + self.done.emit() + self.thread().quit() + + self._update_thread = QThread() + self._update_worker = _Worker(self._update_cancel) + self._update_worker.moveToThread(self._update_thread) + self._update_thread.started.connect(self._update_worker.run) + self._update_worker.done.connect(self._on_update_found) + self._update_thread.finished.connect(self._update_thread.deleteLater) + self._update_thread.start() + + def _on_update_found(self): + if self._update_worker is not None: + self._update_release = self._update_worker.release + self._notify_update() + # R8-H2: the worker QObject lived for the lifetime of the + # application before this — only the QThread was scheduled for + # deleteLater. Drop the worker after the check completes so the + # closure (and its captured release dict) is freed. + self.release_worker() + + def release_worker(self): + """Tear down the update worker/thread defensively. + + Safe to call from both ``_on_update_found`` (the happy path) and + ``MainWindow.closeEvent`` (in case the worker never emitted + ``done`` — e.g. no update available, network failure). + + The check worker runs ``check_for_update()`` which blocks in + ``urllib.request.urlopen(..., timeout=10)``. ``quit()`` only signals + the worker's event loop AFTER ``run()`` returns, so it cannot + interrupt a network read that is still parked inside urlopen. If the + user closes the window during the ~10 s window right after launch + the old code's ``wait(1000)`` returned False and closeEvent went on + to destroy the MainWindow with the QThread still running ("QThread: + Destroyed while thread is still running" + possible abort()). + + Root-cause fix: (1) close the in-flight urlopen response so the + blocked read raises and ``run()`` returns immediately; (2) wait long + enough to cover the urlopen timeout; (3) ``terminate()`` as a last + resort so a running QThread is never left to be destroyed.""" + # (1) Abort the blocked network read, if any. + holder = getattr(self, "_update_cancel", None) + if holder is not None: + resp = holder.get("resp") + if resp is not None: + with contextlib.suppress(Exception): + resp.close() + worker = getattr(self, "_update_worker", None) + if worker is not None: + with contextlib.suppress(RuntimeError): + worker.deleteLater() + self._update_worker = None + thread = getattr(self, "_update_thread", None) + if thread is not None: + with contextlib.suppress(RuntimeError): + if thread.isRunning(): + thread.quit() + # (2) Cover the urlopen timeout; the aborted read above + # should return well before this elapses. + if not thread.wait(12000): + # (3) Last resort — never destroy a running QThread. + thread.terminate() + thread.wait(2000) + self._update_thread = None + self._update_cancel = None + + def _notify_update(self): + """Show update notification dialog automatically.""" + # Guard against a race with closeEvent -> release_worker, + # which nulls _update_worker (and thus leaves _update_release + # unset/None) between the worker's done.emit() on the worker thread + # and this queued slot running on the main thread. Without this + # guard the .get() below raises AttributeError on None. + if not self._update_release: + return + self._update_btn.setVisible(True) + tag = self._update_release.get("tag_name", "?") + from PySide6.QtWidgets import QMessageBox + msg = t("update.available").format(version=tag) + msg += "\n\n" + t("update.installer_info") + msg += "\n\n" + t("update.install") + "?" + reply = QMessageBox.question( + self._window, "PDFApps", msg, + QMessageBox.StandardButton.Yes | QMessageBox.StandardButton.No, + ) + if reply == QMessageBox.StandardButton.Yes: + self.show_update_dialog() + + def show_update_dialog(self): + if self._update_release: + from app.updater import UpdateDialog + dlg = UpdateDialog(self._update_release, parent=self._window) + dlg.exec() diff --git a/app/updater.py b/app/updater.py index b050590..26dd7eb 100644 --- a/app/updater.py +++ b/app/updater.py @@ -572,13 +572,13 @@ def run(self): self._signals.cancelled.connect(self._dl_thread.quit) # Explicit cleanup so retries within the same dialog (e.g. after # an error toast) don't leak QThread and _Worker objects. Mirrors - # the pattern in window.py:_update_thread.finished -> deleteLater. + # the pattern in update_controller.py:_update_thread.finished -> deleteLater. self._dl_thread.finished.connect(self._dl_thread.deleteLater) # Root-cause fix for the dangling-wrapper bug: once the thread is # done (success, error OR cancel) drop our Python reference so the # pending deleteLater can free the C++ object without leaving # reject()/closeEvent holding a stale wrapper. Mirrors the - # _release_update_worker pattern in window.py. + # release_worker pattern in update_controller.py. self._dl_thread.finished.connect(self._on_dl_thread_finished) self._signals.finished.connect(self._dl_worker.deleteLater) self._signals.error.connect(self._dl_worker.deleteLater) diff --git a/app/window.py b/app/window.py index 773f56c..28e1df9 100644 --- a/app/window.py +++ b/app/window.py @@ -22,6 +22,7 @@ from app.utils import resource_path, _make_palette from app.widgets import DropFileEdit from app.single_instance import SingleInstanceServer +from app.update_controller import UpdateController from app.viewer.panel import PdfViewerPanel from app.tools.split import TabDividir from app.tools.merge import TabJuntar @@ -265,22 +266,19 @@ def _a11y(btn, tip): f"QPushButton {{ border: 1.5px solid {ACCENT}; border-radius: 6px; }}" f"QPushButton:hover {{ background: rgba(20,184,166,0.15); }}" ) - self._update_btn.clicked.connect(self._show_update_dialog) + # Auto-update subsystem lives in a dedicated controller. The window + # keeps the toolbar button (part of this layout) and hands it over + # so the controller can reveal it and drive its click. + self._update_controller = UpdateController(self, self._update_btn) + self._update_btn.clicked.connect(self._update_controller.show_update_dialog) wb_h.addWidget(self._update_btn) - self._update_release = None - self._update_thread = None - self._update_worker = None - # Holds the in-flight urlopen response of the background update - # check so _release_update_worker can abort a blocked network - # read from closeEvent instead of waiting out the socket timeout. - self._update_cancel = None # R11-M7: defer the update check 2s past __init__ so the main # window can finish painting + showMaximized before any network # I/O races the UI. Previously fired before the window was even # visible, occasionally stalling first paint on slow networks. QTimer.singleShot( 2000, - lambda: self._check_for_updates_async() if isValid(self) else None, + lambda: self._update_controller.check_async() if isValid(self) else None, ) root_v.addWidget(self._workspace_bar) @@ -1319,7 +1317,7 @@ def closeEvent(self, event): # the QObject worker to release its closure / release dict # if the user closes the window before the check completes # (R8-H2 defensive path). - self._release_update_worker() + self._update_controller.release_worker() try: from app.i18n import _update_config sizes = self._splitter.sizes() @@ -1457,134 +1455,3 @@ def _apply_theme(self): if _is_valid(pres): pres.update_theme(self._dark_mode) - # ── Auto-update ─────────────────────────────────────────────────────── - - def _check_for_updates_async(self): - # Skip auto-update inside Flatpak/Snap — the host package - # manager handles updates. MSIX/Microsoft Store installs are - # short-circuited inside check_for_update() itself (the Store - # updates packaged apps automatically), so they never reach the - # NSIS download path. - if os.environ.get("FLATPAK_ID") or os.environ.get("SNAP"): - return - # Pre-import the updater module on the main thread BEFORE the - # worker thread starts. The worker would otherwise lazy-import - # `app.updater`, which transitively pulls in `urllib.request` - # → `http.client` → `ssl`. On Python 3.14, importing those - # heavy modules from a non-main thread while the main thread - # is still busy creating Qt widgets races against each other - # (CPython's import machinery + Qt's widget construction + - # garbage collection on the worker side) and produces a - # Windows access-violation crash. Doing the import here keeps - # all that import work on the main thread; the worker only - # calls the already-imported function. - from app.updater import check_for_update - from PySide6.QtCore import QThread, QObject, Signal as _Sig - - # Shared holder so the main thread can close the worker's urlopen - # response and unblock its network read on shutdown (M4). - self._update_cancel = {"resp": None} - - class _Worker(QObject): - done = _Sig() - def __init__(self, cancel_holder): - super().__init__() - self.release = None - self._cancel = cancel_holder - def run(self): - self.release = check_for_update(self._cancel) - if self.release: - self.done.emit() - self.thread().quit() - - self._update_thread = QThread() - self._update_worker = _Worker(self._update_cancel) - self._update_worker.moveToThread(self._update_thread) - self._update_thread.started.connect(self._update_worker.run) - self._update_worker.done.connect(self._on_update_found) - self._update_thread.finished.connect(self._update_thread.deleteLater) - self._update_thread.start() - - def _on_update_found(self): - if self._update_worker is not None: - self._update_release = self._update_worker.release - self._notify_update() - # R8-H2: the worker QObject lived for the lifetime of the - # application before this — only the QThread was scheduled for - # deleteLater. Drop the worker after the check completes so the - # closure (and its captured release dict) is freed. - self._release_update_worker() - - def _release_update_worker(self): - """Tear down the update worker/thread defensively. - - Safe to call from both ``_on_update_found`` (the happy path) and - ``closeEvent`` (in case the worker never emitted ``done`` — e.g. - no update available, network failure). - - The check worker runs ``check_for_update()`` which blocks in - ``urllib.request.urlopen(..., timeout=10)``. ``quit()`` only signals - the worker's event loop AFTER ``run()`` returns, so it cannot - interrupt a network read that is still parked inside urlopen. If the - user closes the window during the ~10 s window right after launch - the old code's ``wait(1000)`` returned False and closeEvent went on - to destroy the MainWindow with the QThread still running ("QThread: - Destroyed while thread is still running" + possible abort()). - - Root-cause fix: (1) close the in-flight urlopen response so the - blocked read raises and ``run()`` returns immediately; (2) wait long - enough to cover the urlopen timeout; (3) ``terminate()`` as a last - resort so a running QThread is never left to be destroyed.""" - # (1) Abort the blocked network read, if any. - holder = getattr(self, "_update_cancel", None) - if holder is not None: - resp = holder.get("resp") - if resp is not None: - with contextlib.suppress(Exception): - resp.close() - worker = getattr(self, "_update_worker", None) - if worker is not None: - with contextlib.suppress(RuntimeError): - worker.deleteLater() - self._update_worker = None - thread = getattr(self, "_update_thread", None) - if thread is not None: - with contextlib.suppress(RuntimeError): - if thread.isRunning(): - thread.quit() - # (2) Cover the urlopen timeout; the aborted read above - # should return well before this elapses. - if not thread.wait(12000): - # (3) Last resort — never destroy a running QThread. - thread.terminate() - thread.wait(2000) - self._update_thread = None - self._update_cancel = None - - def _notify_update(self): - """Show update notification dialog automatically.""" - # Guard against a race with closeEvent -> _release_update_worker, - # which nulls _update_worker (and thus leaves _update_release - # unset/None) between the worker's done.emit() on the worker thread - # and this queued slot running on the main thread. Without this - # guard the .get() below raises AttributeError on None. - if not self._update_release: - return - self._update_btn.setVisible(True) - tag = self._update_release.get("tag_name", "?") - from PySide6.QtWidgets import QMessageBox - msg = t("update.available").format(version=tag) - msg += "\n\n" + t("update.installer_info") - msg += "\n\n" + t("update.install") + "?" - reply = QMessageBox.question( - self, "PDFApps", msg, - QMessageBox.StandardButton.Yes | QMessageBox.StandardButton.No, - ) - if reply == QMessageBox.StandardButton.Yes: - self._show_update_dialog() - - def _show_update_dialog(self): - if self._update_release: - from app.updater import UpdateDialog - dlg = UpdateDialog(self._update_release, parent=self) - dlg.exec() diff --git a/tests/test_audit_mediums_lows.py b/tests/test_audit_mediums_lows.py index c84460e..6536571 100644 --- a/tests/test_audit_mediums_lows.py +++ b/tests/test_audit_mediums_lows.py @@ -141,12 +141,16 @@ def test_pipeline_save_logs_symlink_destination(): def test_update_check_deferred_post_init(): src = _read("app/window.py") - # The bare self._check_for_updates_async() call inside __init__ is - # replaced by a QTimer.singleShot. - init_block = src.split("self._update_release = None")[1].split("# ── Viewer property")[0] + # The bare update check inside __init__ is deferred via QTimer.singleShot. + # R4: the check moved behind self._update_controller.check_async(); the + # deferral wiring stays in MainWindow.__init__. + init_block = src.split("self._update_controller = UpdateController")[1].split("# ── Viewer property")[0] assert "QTimer.singleShot" in init_block, ( "Update check must be deferred via QTimer.singleShot from __init__." ) + assert "self._update_controller.check_async()" in init_block, ( + "The deferred call must go through the UpdateController." + ) # And guarded with isValid so a quick close won't crash. assert "isValid(self)" in init_block diff --git a/tests/test_round8_fixes.py b/tests/test_round8_fixes.py index a9f7681..06f945e 100644 --- a/tests/test_round8_fixes.py +++ b/tests/test_round8_fixes.py @@ -52,21 +52,34 @@ def test_encrypt_clears_password_fields(): def test_update_worker_release_helper_exists(): - src = _read("app/window.py") - assert "_release_update_worker" in src - # Both _on_update_found and closeEvent must invoke the helper. - assert src.count("_release_update_worker()") >= 2, ( - "expected _release_update_worker to be called from both " - "_on_update_found and closeEvent") - assert "worker.deleteLater()" in src + # R4: the update subsystem moved to app/update_controller.py. The + # teardown helper (renamed release_worker) must still be invoked from + # BOTH the happy path (_on_update_found, now in the controller) and the + # shutdown path (MainWindow.closeEvent), and must deleteLater the worker. + ctrl = _read("app/update_controller.py") + assert "def release_worker" in ctrl + # Happy path: _on_update_found tears the worker down. + on_found = ctrl[ctrl.find("def _on_update_found"): + ctrl.find("def release_worker")] + assert "self.release_worker()" in on_found, ( + "expected _on_update_found to call release_worker (happy path)") + assert "worker.deleteLater()" in ctrl + # Shutdown path: closeEvent tears the worker down via the controller. + win = _read("app/window.py") + assert "self._update_controller.release_worker()" in win, ( + "expected closeEvent to call the controller's release_worker") def test_update_ready_signal_removed(): - src = _read("app/window.py") - assert "_update_ready = Signal()" not in src, ( - "_update_ready was declared but never emitted — should be gone") - assert "self._update_ready.connect" not in src - assert "self._update_ready.emit" not in src + # The dead _update_ready signal must not resurface in either the window + # or the extracted update controller. + for rel in ("app/window.py", "app/update_controller.py"): + src = _read(rel) + assert "_update_ready = Signal()" not in src, ( + f"_update_ready was declared but never emitted in {rel} — " + "should be gone") + assert "self._update_ready.connect" not in src + assert "self._update_ready.emit" not in src # ── R8-M1 ──────────────────────────────────────────────────────────────── diff --git a/tests/test_update_controller.py b/tests/test_update_controller.py new file mode 100644 index 0000000..2b25219 --- /dev/null +++ b/tests/test_update_controller.py @@ -0,0 +1,187 @@ +"""Tests for app.update_controller.UpdateController (R4 refactor). + +The auto-update subsystem was extracted verbatim from MainWindow into a +dedicated controller. These tests pin the behaviour that matters and is +headless-testable: + + * Flatpak/Snap installs short-circuit the check (host handles updates). + * The controller is a main-thread QObject parented to the window — the + property that makes the worker's cross-thread ``done`` signal resume + ``_on_update_found`` on the MAIN thread (the Py3.14-safe pattern; no + lambda crosses the thread boundary). + * End-to-end worker wiring: ``check_async`` runs the worker, the queued + slot resumes on the main thread, ``_update_release`` is populated and + the worker/thread are torn down. + * The no-update path leaves ``_update_release`` None and ``release_worker`` + cleans up defensively. + * The notify decision only opens the update dialog when the user accepts. + +GUI limits: the modal QMessageBox in ``_notify_update`` and the real +UpdateDialog in ``show_update_dialog`` are not driven here (they block on +user input); the decision branch is exercised by stubbing the prompt. +""" +import os +import sys +import threading +from pathlib import Path + +import pytest + +os.environ.setdefault("QT_QPA_PLATFORM", "offscreen") +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from PySide6.QtCore import QObject, QElapsedTimer +from PySide6.QtWidgets import QApplication, QWidget, QPushButton, QMessageBox + +import app.updater as updater_mod +from app.update_controller import UpdateController + + +@pytest.fixture(scope="module") +def qt_app(): + app = QApplication.instance() or QApplication([]) + yield app + + +def _spin_until(app, predicate, timeout_ms=8000): + """Pump the event loop until ``predicate`` is true or we time out.""" + timer = QElapsedTimer() + timer.start() + while not predicate() and timer.elapsed() < timeout_ms: + app.processEvents() + return predicate() + + +def _make(qt_app): + window = QWidget() + btn = QPushButton(window) + # MainWindow creates the update button hidden (setVisible(False)); mirror + # that so isVisibleTo reflects whether _notify_update revealed it. + btn.setVisible(False) + ctrl = UpdateController(window, btn) + return window, btn, ctrl + + +# ── Flatpak / Snap short-circuit ─────────────────────────────────────── + + +@pytest.mark.parametrize("var", ["FLATPAK_ID", "SNAP"]) +def test_check_async_short_circuits_in_managed_packages(qt_app, monkeypatch, var): + """Inside Flatpak/Snap the host package manager owns updates, so + check_async must return before spinning up any worker/thread.""" + monkeypatch.setenv(var, "com.example.whatever") + _window, _btn, ctrl = _make(qt_app) + ctrl.check_async() + assert ctrl._update_thread is None + assert ctrl._update_worker is None + assert ctrl._update_cancel is None + + +# ── Threading contract: main-thread QObject ──────────────────────────── + + +def test_controller_is_main_thread_qobject(qt_app): + """The controller must be a QObject parented to the window so the + worker's cross-thread ``done`` signal resumes on the main thread via + Qt's automatic queued connection (Py3.14-safe; preserved from the + original MainWindow implementation).""" + window, _btn, ctrl = _make(qt_app) + assert isinstance(ctrl, QObject) + assert ctrl.parent() is window + assert ctrl.thread() is window.thread() + + +# ── End-to-end worker wiring ─────────────────────────────────────────── + + +def test_check_async_populates_release_on_main_thread(qt_app, monkeypatch): + """check_async runs the worker; when an update is found the queued + slot resumes on the MAIN thread, sets _update_release and tears the + worker/thread down.""" + release = {"tag_name": "v99.0.0"} + monkeypatch.setattr(updater_mod, "check_for_update", lambda *a, **k: dict(release)) + + main_ident = threading.get_ident() + recorded = {} + + # Replace the class-level _notify_update (called synchronously from + # inside the real _on_update_found) so no modal dialog appears and we + # can record the thread the queued slot resumed on. The Qt connection + # is to the *real* _on_update_found bound method — the property under + # test — so thread routing is genuine. + def fake_notify(self): + recorded["ident"] = threading.get_ident() + recorded["release"] = self._update_release + monkeypatch.setattr(UpdateController, "_notify_update", fake_notify) + + _window, _btn, ctrl = _make(qt_app) + ctrl.check_async() + + assert _spin_until(qt_app, lambda: "ident" in recorded), \ + "worker never resumed _on_update_found" + assert recorded["release"] == release + assert recorded["ident"] == main_ident, \ + "the queued slot must resume on the main thread (Py3.14-safe)" + assert ctrl._update_release == release + # _on_update_found tears the worker down after the check. + assert _spin_until(qt_app, lambda: ctrl._update_worker is None) + assert ctrl._update_thread is None + assert ctrl._update_cancel is None + + +def test_check_async_no_update_leaves_release_none(qt_app, monkeypatch): + """When check_for_update returns None the worker never emits ``done``; + _update_release stays None and release_worker cleans up without error.""" + monkeypatch.setattr(updater_mod, "check_for_update", lambda *a, **k: None) + # Guard: _notify_update must not run on this path. + monkeypatch.setattr( + UpdateController, "_notify_update", + lambda self: pytest.fail("_notify_update must not run when no update")) + + _window, _btn, ctrl = _make(qt_app) + ctrl.check_async() + # Give the worker time to finish (it quits its own thread). + _spin_until(qt_app, lambda: False, timeout_ms=500) + assert ctrl._update_release is None + # Simulate the closeEvent teardown path — must be safe even after the + # worker thread has already finished. + ctrl.release_worker() + assert ctrl._update_worker is None + assert ctrl._update_thread is None + assert ctrl._update_cancel is None + + +# ── Notify decision ──────────────────────────────────────────────────── + + +def test_notify_returns_without_prompt_when_release_none(qt_app, monkeypatch): + """The None-guard short-circuits before touching the button or prompt.""" + _window, btn, ctrl = _make(qt_app) + ctrl._update_release = None + monkeypatch.setattr( + QMessageBox, "question", + lambda *a, **k: pytest.fail("must not prompt when release is None")) + assert ctrl._notify_update() is None + assert not btn.isVisibleTo(_window) + + +def test_notify_opens_dialog_only_when_user_accepts(qt_app, monkeypatch): + """_notify_update reveals the button and opens the dialog only when the + user answers Yes to the install prompt.""" + _window, btn, ctrl = _make(qt_app) + ctrl._update_release = {"tag_name": "v1.2.3"} + calls = [] + monkeypatch.setattr(ctrl, "show_update_dialog", lambda: calls.append(1)) + + # User declines → button revealed, but no dialog. + monkeypatch.setattr( + QMessageBox, "question", lambda *a, **k: QMessageBox.StandardButton.No) + ctrl._notify_update() + assert btn.isVisibleTo(_window) + assert calls == [] + + # User accepts → dialog opened. + monkeypatch.setattr( + QMessageBox, "question", lambda *a, **k: QMessageBox.StandardButton.Yes) + ctrl._notify_update() + assert calls == [1] diff --git a/tests/test_window_update_lifecycle.py b/tests/test_window_update_lifecycle.py index 0f6de42..8276610 100644 --- a/tests/test_window_update_lifecycle.py +++ b/tests/test_window_update_lifecycle.py @@ -34,13 +34,16 @@ def _read(rel: str) -> str: # ── M5: _notify_update None-guard ────────────────────────────────────── -class _StubWindow: +class _StubController: """Minimal stand-in exposing only the attribute the guard reads. Deliberately lacks _update_btn: pre-fix, _notify_update touched self._update_btn.setVisible() and self._update_release.get() with _update_release=None, so calling the unbound method on this stub would - raise. Post-fix the guard returns before any of that runs.""" + raise. Post-fix the guard returns before any of that runs. + + The R4 refactor moved _notify_update from MainWindow to + UpdateController; the None-guard invariant is unchanged.""" def __init__(self): self._update_release = None @@ -49,16 +52,16 @@ def __init__(self): def test_notify_update_returns_when_release_none(): """M5: _notify_update must return without raising when _update_release is None (worker torn down mid-flight).""" - from app.window import MainWindow - stub = _StubWindow() + from app.update_controller import UpdateController + stub = _StubController() # Must not raise AttributeError — the guard short-circuits. - assert MainWindow._notify_update(stub) is None + assert UpdateController._notify_update(stub) is None def test_notify_update_has_none_guard_in_source(): - src = _read("app/window.py") + src = _read("app/update_controller.py") body = src[src.find("def _notify_update"): - src.find("def _show_update_dialog")] + src.find("def show_update_dialog")] assert "if not self._update_release:" in body, \ "_notify_update must guard against a None _update_release (M5)" @@ -77,8 +80,9 @@ def test_check_for_update_accepts_cancel_holder(): def test_async_check_threads_cancel_holder_into_worker(): - src = _read("app/window.py") - body = src[src.find("def _check_for_updates_async"): + # R4: the async check moved to UpdateController.check_async. + src = _read("app/update_controller.py") + body = src[src.find("def check_async"): src.find("def _on_update_found")] assert 'self._update_cancel = {"resp": None}' in body, \ "the async check must create a shared cancel holder" @@ -87,11 +91,12 @@ def test_async_check_threads_cancel_holder_into_worker(): def test_release_update_worker_aborts_and_terminates(): - """M4: _release_update_worker must (1) close the in-flight response, + """M4: release_worker must (1) close the in-flight response, (2) wait longer than the urlopen timeout, and (3) terminate() as a last resort so a running QThread is never destroyed.""" - src = _read("app/window.py") - body = src[src.find("def _release_update_worker"): + # R4: the teardown moved to UpdateController.release_worker. + src = _read("app/update_controller.py") + body = src[src.find("def release_worker"): src.find("def _notify_update")] # (1) abort the blocked read assert "resp.close()" in body, \