From 7f76d282ddca6afe88e1f92c3f6ce5fd01cb3e1f Mon Sep 17 00:00:00 2001 From: Lukas Geiger Date: Fri, 2 Oct 2026 16:50:26 +0200 Subject: [PATCH 1/2] Fix atomic config saves and test setup --- .github/workflows/tests.yml | 3 +- CONTRIBUTING.md | 4 +- config.py | 24 +++++++++- requirements-dev.txt | 5 ++ tests/test_config.py | 94 +++++++++++++++++++++++++++++++++++++ tests/test_metadata.py | 5 +- 6 files changed, 129 insertions(+), 6 deletions(-) create mode 100644 requirements-dev.txt diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 07244d4..ea1a9d3 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -35,8 +35,7 @@ jobs: - name: Install dependencies run: | python -m pip install --upgrade pip - python -m pip install ruff pytest - python -m pip install -r requirements.txt + python -m pip install -r requirements-dev.txt - name: Lint with ruff run: python -m ruff check . diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a3736f3..2ffb161 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -13,7 +13,7 @@ Vielen Dank für Ihr Interesse, zu diesem Projekt beizutragen. ### Lokales Setup 1. Python 3.10+ installieren -2. `pip install -r requirements.txt` +2. `pip install -r requirements-dev.txt` 3. App mit `start.bat` oder `python main.py` starten 4. Tests mit `python -m pytest -q` ausführen @@ -47,7 +47,7 @@ Thank you for your interest in contributing to this project. ### Local Setup 1. Install Python 3.10+ -2. Run `pip install -r requirements.txt` +2. Run `pip install -r requirements-dev.txt` 3. Start the app with `start.bat` or `python main.py` 4. Run tests with `python -m pytest -q` diff --git a/config.py b/config.py index 249b110..cf2459c 100644 --- a/config.py +++ b/config.py @@ -2,6 +2,7 @@ import json import os +import tempfile from dataclasses import dataclass, field from pathlib import Path from typing import Optional @@ -87,4 +88,25 @@ def save(cfg: AppConfig) -> None: for tid, t in cfg.tools.items() }, } - CONFIG_FILE.write_text(json.dumps(data, indent=2, ensure_ascii=False), encoding="utf-8") + serialized = json.dumps(data, indent=2, ensure_ascii=False) + temporary_path: Optional[Path] = None + try: + with tempfile.NamedTemporaryFile( + mode="w", + encoding="utf-8", + dir=CONFIG_DIR, + prefix=".mp-", + suffix=".tmp", + delete=False, + ) as temp_file: + temporary_path = Path(temp_file.name) + temp_file.write(serialized) + os.replace(temporary_path, CONFIG_FILE) + temporary_path = None + finally: + if temporary_path is not None: + try: + temporary_path.unlink() + except OSError: + # Cleanup is best-effort so the write or replace error is preserved. + pass diff --git a/requirements-dev.txt b/requirements-dev.txt new file mode 100644 index 0000000..e820535 --- /dev/null +++ b/requirements-dev.txt @@ -0,0 +1,5 @@ +-r requirements.txt +Pillow>=11 +pytest +ruff +tomli>=2; python_version < "3.11" diff --git a/tests/test_config.py b/tests/test_config.py index 91bc7b8..9d0a436 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -48,6 +48,100 @@ def test_save_load_roundtrip(): assert t.installed_by == "scan" +def test_save_temp_write_failure_preserves_previous_config(monkeypatch): + config_file = cfg_module.CONFIG_FILE + previous = AppConfig(language="en", first_run=False, start_with_windows=True) + previous.tools["synthetic"] = ToolConfig( + enabled=True, + path="/synthetic/tool", + main_script="main.py", + installed_by="manual", + ) + cfg_module.save(previous) + previous_bytes = config_file.read_bytes() + + replacement = AppConfig(language="de", first_run=True) + real_named_temp_file = cfg_module.tempfile.NamedTemporaryFile + temporary_paths = [] + + class PartialWriter: + def __init__(self, wrapped): + self._wrapped = wrapped + self.name = wrapped.name + + def __enter__(self): + self._wrapped.__enter__() + return self + + def __exit__(self, *args): + return self._wrapped.__exit__(*args) + + def write(self, value): + self._wrapped.write(value[:16]) + self._wrapped.flush() + raise OSError("synthetic interrupted temp write") + + def fail_during_temp_write(*args, **kwargs): + wrapped = real_named_temp_file(*args, **kwargs) + temporary_paths.append(Path(wrapped.name)) + return PartialWriter(wrapped) + + monkeypatch.setattr(cfg_module.tempfile, "NamedTemporaryFile", fail_during_temp_write) + + with pytest.raises(OSError, match="synthetic interrupted temp write"): + cfg_module.save(replacement) + + assert config_file.read_bytes() == previous_bytes + loaded = cfg_module.load() + assert loaded.language == "en" + assert loaded.first_run is False + assert loaded.start_with_windows is True + assert loaded.tools["synthetic"].main_script == "main.py" + assert len(temporary_paths) == 1 + assert not temporary_paths[0].exists() + + +def test_save_replace_failure_preserves_original_error_and_config(monkeypatch): + config_file = cfg_module.CONFIG_FILE + previous = AppConfig(language="en", first_run=False, start_with_windows=True) + previous.tools["synthetic"] = ToolConfig( + enabled=True, + path="/synthetic/tool", + main_script="main.py", + installed_by="manual", + ) + cfg_module.save(previous) + previous_bytes = config_file.read_bytes() + + class FailingOS: + @staticmethod + def replace(source, destination): + raise OSError("synthetic atomic replace failure") + + real_unlink = Path.unlink + + def fail_temp_cleanup(path, *args, **kwargs): + if path.parent == config_file.parent and path.name.startswith(".mp-"): + raise OSError("synthetic temp cleanup failure") + return real_unlink(path, *args, **kwargs) + + monkeypatch.setattr(cfg_module, "os", FailingOS) + monkeypatch.setattr(Path, "unlink", fail_temp_cleanup) + + with pytest.raises(OSError, match="synthetic atomic replace failure"): + cfg_module.save(AppConfig(language="de", first_run=True)) + + assert config_file.read_bytes() == previous_bytes + loaded = cfg_module.load() + assert loaded.language == "en" + assert loaded.first_run is False + assert loaded.start_with_windows is True + assert loaded.tools["synthetic"].main_script == "main.py" + # The injected cleanup failure may leave only this test's unique temp file. + leftovers = list(config_file.parent.glob(".mp-*.tmp")) + assert len(leftovers) == 1 + + def test_load_corrupt_file(tmp_path): config_dir = Path(str(cfg_module.CONFIG_DIR)) config_dir.mkdir(parents=True, exist_ok=True) diff --git a/tests/test_metadata.py b/tests/test_metadata.py index 33c3f6b..80ccb44 100644 --- a/tests/test_metadata.py +++ b/tests/test_metadata.py @@ -2,7 +2,10 @@ from __future__ import annotations -import tomllib +try: + import tomllib +except ModuleNotFoundError: + import tomli as tomllib from pathlib import Path ROOT = Path(__file__).resolve().parents[1] From 49ceb7ed9aa7cbb46ad42eaff46ddc4c6e4511d6 Mon Sep 17 00:00:00 2001 From: Lukas Geiger Date: Sat, 3 Oct 2026 23:02:37 +0200 Subject: [PATCH 2/2] Add isolated frozen bundle self-test --- main.py | 124 +++++++++++++++++++++++++++++++++- tests/test_bundle_selftest.py | 105 ++++++++++++++++++++++++++++ 2 files changed, 228 insertions(+), 1 deletion(-) create mode 100644 tests/test_bundle_selftest.py diff --git a/main.py b/main.py index 193bcc8..2bdd27b 100644 --- a/main.py +++ b/main.py @@ -9,7 +9,127 @@ from i18n import set_language, tr +def _run_bundle_self_test() -> int: + """Exercise packaged assets and the primary Qt UI without user state.""" + from PySide6.QtCore import QEvent, QEventLoop, QObject, QTimer + from PySide6.QtGui import QIcon + from config import AppConfig + from settings_dialog import SettingsDialog + from tray import MailProcessorTray, _resource_path + + app = QApplication.instance() + owns_app = app is None + tray = None + dialog = None + menu = None + timer = None + result = 0 + + try: + if app is None: + QApplication.setHighDpiScaleFactorRoundingPolicy( + Qt.HighDpiScaleFactorRoundingPolicy.PassThrough + ) + app = QApplication([sys.argv[0]]) + app.setQuitOnLastWindowClosed(False) + app.setApplicationName("MailProcessor") + app.setOrganizationName("lukisch") + + # This flag is for a frozen test bundle only. It never loads or saves + # the real profile; all UI objects receive an empty synthetic config. + if not getattr(sys, "frozen", False) or not hasattr(sys, "_MEIPASS"): + result = 10 + else: + icon_path = _resource_path("resources/icon.ico") + if not icon_path.is_file(): + result = 11 + else: + icon = QIcon(str(icon_path)) + if icon.isNull(): + result = 12 + else: + app.setWindowIcon(icon) + cfg = AppConfig( + language="en", first_run=False, start_with_windows=False + ) + set_language(cfg.language) + tray = MailProcessorTray(cfg) + menu = tray.contextMenu() + if tray.icon().isNull() or menu is None or len(menu.actions()) < 4: + result = 13 + else: + dialog = SettingsDialog(cfg) + loop = QEventLoop() + timer = QTimer(dialog) + timer.setSingleShot(True) + painted = False + + class PaintProbe(QObject): + def eventFilter(self, watched, event): + nonlocal painted + if watched is dialog and event.type() == QEvent.Type.Paint: + painted = True + loop.quit() + return False + + paint_probe = PaintProbe(dialog) + dialog.installEventFilter(paint_probe) + dialog.show() + app.processEvents() + if not painted: + timer.timeout.connect(loop.quit) + timer.start(1500) + loop.exec() + timer.stop() + if not dialog.isVisible() or not painted: + result = 14 + except Exception: + result = 15 + finally: + if timer is not None: + try: + timer.stop() + except Exception: + if result == 0: + result = 16 + if dialog is not None: + try: + dialog.close() + except Exception: + if result == 0: + result = 16 + if menu is not None: + try: + menu.close() + except Exception: + if result == 0: + result = 16 + if tray is not None: + try: + tray.hide() + except Exception: + if result == 0: + result = 16 + if app is not None: + try: + app.processEvents() + except Exception: + if result == 0: + result = 16 + if owns_app: + try: + app.quit() + except Exception: + if result == 0: + result = 16 + + return result + + def main(): + if sys.argv[1:] == ["--self-test"]: + return _run_bundle_self_test() + # High-DPI QApplication.setHighDpiScaleFactorRoundingPolicy( Qt.HighDpiScaleFactorRoundingPolicy.PassThrough @@ -50,4 +170,6 @@ def main(): if __name__ == "__main__": - main() + result = main() + if result is not None: + sys.exit(result) diff --git a/tests/test_bundle_selftest.py b/tests/test_bundle_selftest.py new file mode 100644 index 0000000..13af4b6 --- /dev/null +++ b/tests/test_bundle_selftest.py @@ -0,0 +1,105 @@ +"""The frozen-bundle self-test must not touch a real user profile or host state.""" + +import builtins +import sys +from pathlib import Path + +import pytest +from PySide6.QtWidgets import QApplication + +import config as cfg_module +import main as main_module + + +@pytest.fixture +def isolated_qapp(monkeypatch, tmp_path): + profile = tmp_path / "profile" + monkeypatch.setenv("QT_QPA_PLATFORM", "offscreen") + monkeypatch.setenv("LOCALAPPDATA", str(profile / "localappdata")) + monkeypatch.setenv("APPDATA", str(profile / "appdata")) + monkeypatch.setenv("USERPROFILE", str(profile / "user")) + monkeypatch.setenv("TEMP", str(profile / "temp")) + monkeypatch.setenv("TMP", str(profile / "temp")) + + app = QApplication.instance() + if app is None: + app = QApplication([]) + return app, profile + + +def test_self_test_checks_frozen_ui_without_loading_or_saving_profile( + isolated_qapp, monkeypatch +): + _, profile = isolated_qapp + repo_root = Path(__file__).resolve().parents[1] + monkeypatch.setattr(sys, "frozen", True, raising=False) + monkeypatch.setattr(sys, "_MEIPASS", str(repo_root), raising=False) + + def unexpected_call(*_args, **_kwargs): + pytest.fail("self-test reached a forbidden profile or host-state path") + + monkeypatch.setattr(cfg_module, "load", unexpected_call) + monkeypatch.setattr(cfg_module, "save", unexpected_call) + + import settings_dialog + import tool_manager + import urllib.request + + monkeypatch.setattr(settings_dialog, "ensure_autostart_entry", unexpected_call) + monkeypatch.setattr(tool_manager.ToolManager, "scan", unexpected_call) + monkeypatch.setattr(tool_manager.ToolManager, "download_tool", unexpected_call) + monkeypatch.setattr(tool_manager.ToolManager, "launch", unexpected_call) + monkeypatch.setattr(urllib.request, "urlopen", unexpected_call) + + real_import = builtins.__import__ + + def block_installer(name, *args, **kwargs): + if name == "installer": + pytest.fail("self-test imported the first-run installer") + return real_import(name, *args, **kwargs) + + monkeypatch.setattr(builtins, "__import__", block_installer) + monkeypatch.setattr(sys, "argv", ["MailProcessor", "--self-test"]) + + assert main_module.main() == 0 + assert not (profile / "localappdata" / "MailProcessor").exists() + + +def test_self_test_reports_missing_asset(isolated_qapp, monkeypatch, tmp_path): + _, profile = isolated_qapp + monkeypatch.setattr(sys, "frozen", True, raising=False) + monkeypatch.setattr(sys, "_MEIPASS", str(tmp_path), raising=False) + monkeypatch.setattr(sys, "argv", ["MailProcessor", "--self-test"]) + + assert main_module.main() == 11 + assert not (profile / "localappdata" / "MailProcessor").exists() + + +def test_self_test_reports_unreadable_asset(isolated_qapp, monkeypatch, tmp_path): + _, profile = isolated_qapp + bad_resource = tmp_path / "resources" / "icon.ico" + bad_resource.parent.mkdir() + bad_resource.write_text("not an icon", encoding="utf-8") + monkeypatch.setattr(sys, "frozen", True, raising=False) + monkeypatch.setattr(sys, "_MEIPASS", str(tmp_path), raising=False) + monkeypatch.setattr(sys, "argv", ["MailProcessor", "--self-test"]) + + assert main_module.main() == 12 + assert not (profile / "localappdata" / "MailProcessor").exists() + + +def test_self_test_reports_ui_that_never_paints(isolated_qapp, monkeypatch): + _, profile = isolated_qapp + repo_root = Path(__file__).resolve().parents[1] + monkeypatch.setattr(sys, "frozen", True, raising=False) + monkeypatch.setattr(sys, "_MEIPASS", str(repo_root), raising=False) + monkeypatch.setattr(sys, "argv", ["MailProcessor", "--self-test"]) + + from PySide6.QtCore import QEventLoop + from settings_dialog import SettingsDialog + + monkeypatch.setattr(SettingsDialog, "show", lambda _self: None) + monkeypatch.setattr(QEventLoop, "exec", lambda _self: 0) + + assert main_module.main() == 14 + assert not (profile / "localappdata" / "MailProcessor").exists()