From 2e60770348727531c5fbd158ec9ebcd2899dbff5 Mon Sep 17 00:00:00 2001 From: Asher Norland Date: Mon, 9 Mar 2026 18:38:05 -0700 Subject: [PATCH 1/2] Lint Fixes --- synodic_client/application/bootstrap.py | 2 +- synodic_client/application/screen/settings.py | 6 +++- synodic_client/application/workers.py | 29 ++++++++++--------- synodic_client/subprocess_patch.py | 10 +++---- tests/unit/qt/test_settings.py | 2 +- 5 files changed, 27 insertions(+), 22 deletions(-) diff --git a/synodic_client/application/bootstrap.py b/synodic_client/application/bootstrap.py index 0eed0ff..fe2e532 100644 --- a/synodic_client/application/bootstrap.py +++ b/synodic_client/application/bootstrap.py @@ -17,8 +17,8 @@ from synodic_client.config import set_dev_mode from synodic_client.logging import configure_logging -from synodic_client.subprocess_patch import apply as _apply_subprocess_patch from synodic_client.protocol import extract_uri_from_args +from synodic_client.subprocess_patch import apply as _apply_subprocess_patch from synodic_client.updater import initialize_velopack # Parse flags early so logging uses the right filename and level. diff --git a/synodic_client/application/screen/settings.py b/synodic_client/application/screen/settings.py index eafbbb9..1caca8b 100644 --- a/synodic_client/application/screen/settings.py +++ b/synodic_client/application/screen/settings.py @@ -331,7 +331,11 @@ def show(self) -> None: # adjustSize() only reaches the minimum. Compute the ideal # height from the content widget directly. content_hint = self._scroll_content.sizeHint() - margins = self._scroll_content.layout().contentsMargins() + layout = self._scroll_content.layout() + if layout is None: + super().show() + return + margins = layout.contentsMargins() ideal_w = max(content_hint.width() + margins.left() + margins.right(), self.minimumWidth()) ideal_h = max(content_hint.height() + margins.top() + margins.bottom(), self.minimumHeight()) self.resize(ideal_w, ideal_h) diff --git a/synodic_client/application/workers.py b/synodic_client/application/workers.py index 6e2fe89..62d91ac 100644 --- a/synodic_client/application/workers.py +++ b/synodic_client/application/workers.py @@ -120,20 +120,21 @@ async def run_tool_updates( params, plugins=discovered_plugins, ): - if event.kind == ProgressEventKind.ACTION_COMPLETED and event.result is not None: - action_result = event.result - if action_result.skipped: - if action_result.skip_reason in { - SkipReason.ALREADY_LATEST, - SkipReason.ALREADY_INSTALLED, - }: - result.already_latest += 1 - elif action_result.success: - result.updated += 1 - if action_result.action.package: - result.updated_packages.add(str(action_result.action.package.name)) - else: - result.failed += 1 + if event.kind != ProgressEventKind.ACTION_COMPLETED or event.result is None: + continue + action_result = event.result + if action_result.skipped: + if action_result.skip_reason in { + SkipReason.ALREADY_LATEST, + SkipReason.ALREADY_INSTALLED, + }: + result.already_latest += 1 + elif action_result.success: + result.updated += 1 + if action_result.action.package: + result.updated_packages.add(str(action_result.action.package.name)) + else: + result.failed += 1 except asyncio.CancelledError: logger.debug('run_tool_updates cancelled during manifest processing') raise diff --git a/synodic_client/subprocess_patch.py b/synodic_client/subprocess_patch.py index 80f786c..53b2773 100644 --- a/synodic_client/subprocess_patch.py +++ b/synodic_client/subprocess_patch.py @@ -36,7 +36,7 @@ def apply() -> None: """Activate the subprocess-suppression patch (idempotent, Windows-only).""" global _applied # noqa: PLW0603 - if _applied or sys.platform != "win32": + if _applied or sys.platform != 'win32': return _applied = True @@ -51,7 +51,7 @@ def apply() -> None: _STARTF_USESHOWWINDOW: int = 0 _SW_HIDE: int = 0 -if sys.platform == "win32": +if sys.platform == 'win32': _CREATE_NO_WINDOW = subprocess.CREATE_NO_WINDOW # 0x0800_0000 _STARTF_USESHOWWINDOW = subprocess.STARTF_USESHOWWINDOW _SW_HIDE = 0 @@ -64,12 +64,12 @@ def _inject_hidden_flags(kwargs: dict[str, Any]) -> None: preserved. An existing ``startupinfo`` object is augmented rather than overwritten. """ - kwargs["creationflags"] = kwargs.get("creationflags", 0) | _CREATE_NO_WINDOW + kwargs['creationflags'] = kwargs.get('creationflags', 0) | _CREATE_NO_WINDOW - startupinfo = kwargs.get("startupinfo") or subprocess.STARTUPINFO() + startupinfo = kwargs.get('startupinfo') or subprocess.STARTUPINFO() startupinfo.dwFlags |= _STARTF_USESHOWWINDOW startupinfo.wShowWindow = _SW_HIDE - kwargs["startupinfo"] = startupinfo + kwargs['startupinfo'] = startupinfo def _patch_popen() -> None: diff --git a/tests/unit/qt/test_settings.py b/tests/unit/qt/test_settings.py index d2b2223..270bda7 100644 --- a/tests/unit/qt/test_settings.py +++ b/tests/unit/qt/test_settings.py @@ -440,7 +440,7 @@ def test_update_config_does_not_emit_settings_changed() -> None: def test_sync_after_update_config_uses_new_timestamp() -> None: """sync_from_config after update_config should display the refreshed timestamp.""" window = _make_window(_make_config(last_client_update=None)) - assert window._last_client_update_label.text() == '' + assert not window._last_client_update_label.text() new_config = _make_config(last_client_update='2026-03-09T12:00:00+00:00') window.update_config(new_config) From 50f9a919c8405673c472a0bd9117d7ec231c8653 Mon Sep 17 00:00:00 2001 From: Asher Norland Date: Mon, 9 Mar 2026 22:03:06 -0700 Subject: [PATCH 2/2] Single Instance Config --- synodic_client/application/config_store.py | 65 +++++++ synodic_client/application/qt.py | 8 +- synodic_client/application/schema.py | 21 +++ synodic_client/application/screen/projects.py | 15 +- synodic_client/application/screen/screen.py | 42 ++--- synodic_client/application/screen/settings.py | 34 +--- .../screen/tool_update_controller.py | 176 +++++++++--------- synodic_client/application/screen/tray.py | 42 ++--- .../application/screen/update_banner.py | 42 +++-- .../application/update_controller.py | 49 +++-- tests/unit/qt/test_gather_packages.py | 70 +++---- tests/unit/qt/test_settings.py | 107 ++++------- tests/unit/qt/test_tray_window_show.py | 32 +++- tests/unit/qt/test_update_controller.py | 65 +++---- 14 files changed, 414 insertions(+), 354 deletions(-) create mode 100644 synodic_client/application/config_store.py diff --git a/synodic_client/application/config_store.py b/synodic_client/application/config_store.py new file mode 100644 index 0000000..41ed3fc --- /dev/null +++ b/synodic_client/application/config_store.py @@ -0,0 +1,65 @@ +"""Centralized configuration store. + +Provides a single source of truth for :class:`ResolvedConfig` so that +every consumer (ToolsView, SettingsWindow, UpdateController, +ToolUpdateOrchestrator) always reads the same snapshot and receives +change notifications through a Qt signal. +""" + +from __future__ import annotations + +from PySide6.QtCore import QObject, Signal + +from synodic_client.resolution import ResolvedConfig, resolve_config, update_user_config + + +class ConfigStore(QObject): + """Observable wrapper around :class:`ResolvedConfig`. + + All config mutations go through :meth:`update` (which persists to + disk) or :meth:`set` (which replaces without persisting). Both + emit :attr:`changed` so every connected consumer stays in sync. + + Typical usage:: + + store = ConfigStore(initial_config) + store.changed.connect(some_consumer.on_config_changed) + store.update(auto_apply=False) # persists + emits + """ + + changed = Signal(object) + """Emitted with the new ``ResolvedConfig`` after every mutation.""" + + def __init__(self, config: ResolvedConfig | None = None, parent: QObject | None = None) -> None: + """Create a new store, optionally seeded with *config*.""" + super().__init__(parent) + self._config = config if config is not None else resolve_config() + + @property + def config(self) -> ResolvedConfig: + """The current configuration snapshot.""" + return self._config + + def update(self, **changes: object) -> ResolvedConfig: + """Persist *changes* to disk and broadcast the new config. + + Wraps :func:`~synodic_client.resolution.update_user_config`. + + Args: + **changes: Field-name / value pairs forwarded to + :func:`update_user_config`. + + Returns: + The fresh :class:`ResolvedConfig`. + """ + self._config = update_user_config(**changes) + self.changed.emit(self._config) + return self._config + + def set(self, config: ResolvedConfig) -> None: + """Replace the config without persisting and notify listeners. + + Use for externally resolved configs (e.g. passed at startup). + """ + self._config = config + self.changed.emit(self._config) diff --git a/synodic_client/application/qt.py b/synodic_client/application/qt.py index 54f9106..b789415 100644 --- a/synodic_client/application/qt.py +++ b/synodic_client/application/qt.py @@ -15,6 +15,7 @@ from PySide6.QtCore import QEvent, QObject, Qt, QTimer from PySide6.QtWidgets import QApplication, QWidget +from synodic_client.application.config_store import ConfigStore from synodic_client.application.icon import app_icon from synodic_client.application.init import run_startup_preamble from synodic_client.application.instance import SingleInstance @@ -216,8 +217,9 @@ def application(*, uri: str | None = None, dev_mode: bool = False, debug: bool = sys.exit(0) instance.start_server() - _screen = Screen(porringer, config) - _tray = TrayScreen(app, client, _screen.window, config=config) + _store = ConfigStore(config) + _screen = Screen(porringer, _store) + _tray = TrayScreen(app, client, _screen.window, store=_store) # Keep install preview windows alive until the app exits _install_windows: list[InstallPreviewWindow] = [] @@ -227,7 +229,7 @@ def _handle_install_uri(manifest_url: str) -> None: window = InstallPreviewWindow( porringer, manifest_url, - config=config, + config=_store.config, ) _install_windows.append(window) window.show() diff --git a/synodic_client/application/schema.py b/synodic_client/application/schema.py index e42ba1a..932d56d 100644 --- a/synodic_client/application/schema.py +++ b/synodic_client/application/schema.py @@ -51,3 +51,24 @@ class ToolUpdateResult: failed: int = 0 updated_packages: set[str] = field(default_factory=set) """Package names that were successfully upgraded.""" + + +@dataclass(frozen=True, slots=True) +class UpdateTarget: + """Identifies the scope of a manual tool update. + + Passed to the shared completion handler so it can clear the correct + updating state and derive timestamp keys. ``None`` (the default in + the handler) means the update was periodic / automatic. + + When *package* is empty the update targeted an entire plugin; + otherwise it targeted one specific package within the plugin. + *plugin* always carries the signal key (possibly composite + ``"plugin:tag"``). + """ + + plugin: str + """Signal key for the plugin (may be composite ``"name:tag"``).""" + + package: str = '' + """Package name, or empty when the whole plugin was updated.""" diff --git a/synodic_client/application/screen/projects.py b/synodic_client/application/screen/projects.py index ebf6e12..4475d61 100644 --- a/synodic_client/application/screen/projects.py +++ b/synodic_client/application/screen/projects.py @@ -5,6 +5,7 @@ import asyncio import logging from pathlib import Path +from typing import TYPE_CHECKING from porringer.api import API from porringer.backend.command.core.discovery import DiscoveredPlugins @@ -24,7 +25,9 @@ from synodic_client.application.screen.sidebar import ManifestSidebar from synodic_client.application.screen.spinner import LoadingIndicator from synodic_client.application.theme import COMPACT_MARGINS -from synodic_client.resolution import ResolvedConfig + +if TYPE_CHECKING: + from synodic_client.application.config_store import ConfigStore logger = logging.getLogger(__name__) @@ -41,7 +44,7 @@ class ProjectsView(QWidget): def __init__( self, porringer: API, - config: ResolvedConfig, + store: ConfigStore, parent: QWidget | None = None, *, coordinator: DataCoordinator | None = None, @@ -50,14 +53,14 @@ def __init__( Args: porringer: The porringer API instance. - config: Resolved configuration. + store: The centralised :class:`ConfigStore`. parent: Optional parent widget. coordinator: Shared data coordinator for validated directory data. """ super().__init__(parent) self._porringer = porringer - self._config = config + self._store = store self._coordinator = coordinator self._refresh_in_progress = False self._pending_select: Path | None = None @@ -163,7 +166,7 @@ async def _async_refresh(self) -> None: widget.load( str(path), project_directory=path if path.is_dir() else path.parent, - detect_updates=self._config.detect_updates, + detect_updates=self._store.config.detect_updates, ) except Exception: @@ -196,7 +199,7 @@ def _create_directory_widgets( self._porringer, self, show_close=False, - config=self._config, + config=self._store.config, ) widget._discovered_plugins = discovered widget.install_finished.connect(self._on_install_finished) diff --git a/synodic_client/application/screen/screen.py b/synodic_client/application/screen/screen.py index 943885d..76a3836 100644 --- a/synodic_client/application/screen/screen.py +++ b/synodic_client/application/screen/screen.py @@ -34,6 +34,7 @@ QWidget, ) +from synodic_client.application.config_store import ConfigStore from synodic_client.application.data import DataCoordinator from synodic_client.application.icon import app_icon from synodic_client.application.screen.plugin_row import ( @@ -65,7 +66,6 @@ SEARCH_INPUT_STYLE, SETTINGS_GEAR_STYLE, ) -from synodic_client.resolution import ResolvedConfig, update_user_config logger = logging.getLogger(__name__) @@ -111,7 +111,7 @@ class ToolsView(QWidget): def __init__( self, porringer: API, - config: ResolvedConfig, + store: ConfigStore, parent: QWidget | None = None, *, coordinator: DataCoordinator | None = None, @@ -120,7 +120,7 @@ def __init__( Args: porringer: The porringer API instance. - config: Resolved configuration (for auto-update toggles). + store: The centralised :class:`ConfigStore`. parent: Optional parent widget. coordinator: Shared data coordinator. When provided, the view delegates plugin/directory fetching to the @@ -128,7 +128,7 @@ def __init__( """ super().__init__(parent) self._porringer = porringer - self._config = config + self._store = store self._coordinator = coordinator self._section_widgets: list[QWidget] = [] self._filter_chips: dict[str, FilterChip] = {} @@ -379,7 +379,7 @@ def _build_widget_tree(self, data: RefreshData) -> None: """Clear existing widgets and rebuild the tool/package tree.""" self._clear_section_widgets() - auto_update_map = self._config.plugin_auto_update or {} + auto_update_map = self._store.config.plugin_auto_update or {} kind_buckets = self._bucket_by_kind( data.plugins, data.packages_map, @@ -446,7 +446,7 @@ def _build_runtime_sections( auto_val = auto_update_map.get(plugin.name, True) plugin_updates = self._updates_available.get(plugin.name, {}) - tool_timestamps = self._config.last_tool_updates or {} + tool_timestamps = self._store.config.last_tool_updates or {} default_exe = data.default_runtime_executable # Sort: default runtime first, then descending by tag @@ -534,7 +534,7 @@ def _build_plugin_section( plugin_manifest = data.manifest_packages.get(plugin.name, set()) raw_packages = data.packages_map.get(plugin.name, []) display_packages = self._build_display_packages(raw_packages, plugin_manifest) - tool_timestamps = self._config.last_tool_updates or {} + tool_timestamps = self._store.config.last_tool_updates or {} if display_packages: for pkg in display_packages: @@ -1069,7 +1069,7 @@ async def _gather_project_requirements( def _on_auto_update_toggled(self, plugin_name: str, enabled: bool) -> None: """Persist the plugin-level auto-update toggle change to config.""" - mapping = dict(self._config.plugin_auto_update or {}) + mapping = dict(self._store.config.plugin_auto_update or {}) if enabled: mapping.pop(plugin_name, None) @@ -1077,7 +1077,7 @@ def _on_auto_update_toggled(self, plugin_name: str, enabled: bool) -> None: mapping[plugin_name] = False new_value = mapping if mapping else None - self._config = update_user_config(plugin_auto_update=new_value) + self._store.update(plugin_auto_update=new_value) logger.info('Auto-update for %s set to %s', plugin_name, enabled) def _on_package_auto_update_toggled( @@ -1087,7 +1087,7 @@ def _on_package_auto_update_toggled( enabled: bool, ) -> None: """Persist a per-package auto-update override to the nested config dict.""" - mapping = dict(self._config.plugin_auto_update or {}) + mapping = dict(self._store.config.plugin_auto_update or {}) current = mapping.get(plugin_name) if isinstance(current, dict): @@ -1103,7 +1103,7 @@ def _on_package_auto_update_toggled( mapping.pop(plugin_name, None) new_value = mapping if mapping else None - self._config = update_user_config(plugin_auto_update=new_value) + self._store.update(plugin_auto_update=new_value) logger.info( 'Auto-update for %s/%s set to %s', plugin_name, @@ -1389,17 +1389,17 @@ class MainWindow(QMainWindow): def __init__( self, porringer: API | None = None, - config: ResolvedConfig | None = None, + store: ConfigStore | None = None, ) -> None: """Initialize the main window. Args: porringer: Optional porringer API instance for manifest display. - config: Resolved configuration for plugin auto-update state. + store: The centralised :class:`ConfigStore`. """ super().__init__() self._porringer = porringer - self._config = config + self._store = store self._coordinator: DataCoordinator | None = DataCoordinator(porringer) if porringer is not None else None self.setWindowTitle('Synodic Client') self.setMinimumSize(*MAIN_WINDOW_MIN_SIZE) @@ -1445,12 +1445,12 @@ def update_banner(self) -> UpdateBanner: def show(self) -> None: """Show the window, initializing UI lazily on first show.""" - if self._tabs is None and self._porringer is not None and self._config is not None: + if self._tabs is None and self._porringer is not None and self._store is not None: self._tabs = QTabWidget(self) self._projects_view = ProjectsView( self._porringer, - self._config, + self._store, self, coordinator=self._coordinator, ) @@ -1458,7 +1458,7 @@ def show(self) -> None: self._tools_view = ToolsView( self._porringer, - self._config, + self._store, self, coordinator=self._coordinator, ) @@ -1507,16 +1507,16 @@ class Screen: def __init__( self, porringer: API | None = None, - config: ResolvedConfig | None = None, + store: ConfigStore | None = None, ) -> None: """Initialize the screen. Args: porringer: Optional porringer API instance. - config: Resolved configuration. + store: The centralised :class:`ConfigStore`. """ self._porringer = porringer - self._config = config + self._store = store @property def window(self) -> MainWindow: @@ -1526,5 +1526,5 @@ def window(self) -> MainWindow: The MainWindow instance. """ if self._window is None: - self._window = MainWindow(self._porringer, self._config) + self._window = MainWindow(self._porringer, self._store) return self._window diff --git a/synodic_client/application/screen/settings.py b/synodic_client/application/screen/settings.py index 1caca8b..b058cc6 100644 --- a/synodic_client/application/screen/settings.py +++ b/synodic_client/application/screen/settings.py @@ -28,12 +28,12 @@ QWidget, ) +from synodic_client.application.config_store import ConfigStore from synodic_client.application.icon import app_icon from synodic_client.application.screen import _format_relative_time from synodic_client.application.screen.card import CardFrame from synodic_client.application.theme import SETTINGS_WINDOW_MIN_SIZE, UPDATE_STATUS_CHECKING_STYLE from synodic_client.logging import log_path, set_debug_level -from synodic_client.resolution import ResolvedConfig, update_user_config from synodic_client.schema import GITHUB_REPO_URL from synodic_client.startup import is_startup_registered, register_startup, remove_startup @@ -43,14 +43,11 @@ class SettingsWindow(QMainWindow): """Application settings window with grouped card sections. - All controls persist changes immediately via :func:`update_user_config` - and emit :attr:`settings_changed` so that the tray and updater can - react. The signal carries the new :class:`ResolvedConfig`. + All controls persist changes immediately via the shared + :class:`ConfigStore`, which broadcasts the new :class:`ResolvedConfig` + to every connected consumer. """ - settings_changed = Signal(object) - """Emitted with the new ``ResolvedConfig`` whenever a setting is changed and persisted.""" - check_updates_requested = Signal() """Emitted when the user clicks the *Check for Updates* button.""" @@ -74,19 +71,19 @@ def showEvent(self, event: QShowEvent) -> None: # noqa: N802 def __init__( self, - config: ResolvedConfig, + store: ConfigStore, version: str = '', parent: QWidget | None = None, ) -> None: """Initialise the settings window. Args: - config: The current resolved configuration snapshot. + store: The centralised configuration store. version: The application version string to display. parent: Optional parent widget. """ super().__init__(parent) - self._config = config + self._store = store self._version = version self.setWindowTitle('Synodic Settings') self.setMinimumSize(*SETTINGS_WINDOW_MIN_SIZE) @@ -254,7 +251,7 @@ def sync_from_config(self) -> None: Signals are blocked during the update to prevent feedback loops. """ - config = self._config + config = self._store.config with self._block_signals(): # Channel: index 0 = Stable, 1 = Development @@ -305,15 +302,6 @@ def reset_check_updates_button(self) -> None: """Re-enable the *Check for Updates* button after a check completes.""" self._check_updates_btn.setEnabled(True) - def update_config(self, config: ResolvedConfig) -> None: - """Replace the internal config snapshot without emitting signals. - - Called by controllers that persist timestamps so that the next - :meth:`sync_from_config` sees fresh data instead of the stale - snapshot captured at construction time. - """ - self._config = config - def set_last_checked(self, timestamp: str) -> None: """Update the *last updated* label from an ISO 8601 timestamp.""" relative = _format_relative_time(timestamp) @@ -353,8 +341,7 @@ def _persist(self, **changes: object) -> None: Args: **changes: Field-name / value pairs to persist. """ - self._config = update_user_config(**changes) - self.settings_changed.emit(self._config) + self._store.update(**changes) @contextmanager def _block_signals(self) -> Iterator[None]: @@ -410,13 +397,12 @@ def _on_auto_apply_changed(self, checked: bool) -> None: self._persist(auto_apply=checked) def _on_auto_start_changed(self, checked: bool) -> None: - self._config = update_user_config(auto_start=checked) + self._store.update(auto_start=checked) if getattr(sys, 'frozen', False): if checked: register_startup(sys.executable) else: remove_startup() - self.settings_changed.emit(self._config) def _on_debug_logging_changed(self, checked: bool) -> None: set_debug_level(enabled=checked) diff --git a/synodic_client/application/screen/tool_update_controller.py b/synodic_client/application/screen/tool_update_controller.py index 2f7c17e..ddcb13a 100644 --- a/synodic_client/application/screen/tool_update_controller.py +++ b/synodic_client/application/screen/tool_update_controller.py @@ -13,6 +13,7 @@ import logging from collections.abc import Callable from datetime import UTC, datetime +from typing import TYPE_CHECKING from porringer.api import API from porringer.core.schema import PackageRef @@ -20,23 +21,34 @@ from PySide6.QtCore import QTimer from PySide6.QtWidgets import QSystemTrayIcon -from synodic_client.application.schema import ToolUpdateResult +from synodic_client.application.schema import ToolUpdateResult, UpdateTarget from synodic_client.application.screen.screen import MainWindow, ToolsView from synodic_client.application.workers import ( run_runtime_package_updates, run_tool_updates, ) -from synodic_client.config import load_user_config from synodic_client.resolution import ( - ResolvedConfig, resolve_auto_update_scope, resolve_update_config, - update_user_config, ) +if TYPE_CHECKING: + from synodic_client.application.config_store import ConfigStore + logger = logging.getLogger(__name__) +def _parse_plugin_key(name: str) -> tuple[str, str | None]: + """Split a composite ``'plugin:tag'`` key into ``(bare_name, tag)``. + + Returns ``(name, None)`` when there is no tag component. + """ + if ':' in name: + bare, tag = name.split(':', 1) + return bare, tag + return name, None + + class ToolUpdateOrchestrator: """Background tool update lifecycle manager. @@ -46,14 +58,14 @@ class ToolUpdateOrchestrator: Args: window: The main application window (provides porringer / coordinator). - config_resolver: Callable returning the current resolved config. + store: The centralised :class:`ConfigStore`. tray: System tray icon for displaying notification messages. """ def __init__( self, window: MainWindow, - config_resolver: Callable[[], ResolvedConfig], + store: ConfigStore, tray: QSystemTrayIcon, is_user_active: Callable[[], bool] | None = None, ) -> None: @@ -61,14 +73,14 @@ def __init__( Args: window: The main application window. - config_resolver: Callable returning the current resolved config. + store: The centralised :class:`ConfigStore`. tray: System tray icon for notification messages. is_user_active: Predicate returning ``True`` when the user has a visible window. Periodic tool updates are deferred while active. """ self._window = window - self._resolve_config = config_resolver + self._store = store self._tray = tray self._is_user_active = is_user_active or (lambda: False) self._tool_task: asyncio.Task[None] | None = None @@ -120,7 +132,7 @@ def _restart_timer( def restart_tool_update_timer(self) -> None: """Start (or restart) the periodic tool update timer from config.""" - config = resolve_update_config(self._resolve_config()) + config = resolve_update_config(self._store.config) self._tool_update_timer = self._restart_timer( self._tool_update_timer, config.tool_update_interval_minutes, @@ -144,6 +156,32 @@ def connect_tools_view(self, tools_view: ToolsView) -> None: tools_view.package_update_requested.connect(self.on_single_package_update) tools_view.package_remove_requested.connect(self.on_single_package_remove) + # -- ToolsView error helpers -- + + def _fail_plugin_update(self, plugin_name: str, error: str) -> None: + """Reset plugin updating state and show an inline error.""" + tools_view = self._window.tools_view + if tools_view is not None: + tools_view.set_plugin_updating(plugin_name, False) + tools_view.set_plugin_error(plugin_name, error) + + def _fail_package_update( + self, + plugin_name: str, + package_name: str, + error: str, + *, + removing: bool = False, + ) -> None: + """Reset package updating/removing state and show an inline error.""" + tools_view = self._window.tools_view + if tools_view is not None: + if removing: + tools_view.set_package_removing(plugin_name, package_name, False) + else: + tools_view.set_package_updating(plugin_name, package_name, False) + tools_view.set_package_error(plugin_name, package_name, error) + # -- Full tool update -- def on_tool_update(self) -> None: @@ -158,7 +196,7 @@ def on_tool_update(self) -> None: async def _do_tool_update(self, porringer: API) -> None: """Resolve enabled plugins off-thread, then run the tool update.""" - config = self._resolve_config() + config = self._store.config coordinator = self._window.coordinator if coordinator is not None: @@ -210,8 +248,8 @@ def on_single_plugin_update(self, plugin_name: str) -> None: if tools_view is not None: tools_view.set_plugin_updating(plugin_name, True) - if ':' in plugin_name: - bare_plugin, runtime_tag = plugin_name.split(':', 1) + bare_plugin, runtime_tag = _parse_plugin_key(plugin_name) + if runtime_tag is not None: self._tool_task = asyncio.create_task( self._async_runtime_plugin_update(porringer, plugin_name, bare_plugin, runtime_tag), ) @@ -228,7 +266,7 @@ async def _async_runtime_plugin_update( runtime_tag: str, ) -> None: """Run a runtime-scoped plugin update and route results.""" - config = self._resolve_config() + config = self._store.config mapping = config.plugin_auto_update or {} pkg_entry = mapping.get(signal_key) or mapping.get(plugin_name) coordinator = self._window.coordinator @@ -250,20 +288,17 @@ async def _async_runtime_plugin_update( ) if coordinator is not None: coordinator.invalidate() - self._on_tool_update_finished(result, updating_plugin=signal_key, manual=True) + self._on_tool_update_finished(result, UpdateTarget(plugin=signal_key)) except asyncio.CancelledError: logger.debug('Runtime plugin update cancelled (shutdown)') raise except Exception as exc: logger.exception('Runtime tool update failed') - tools_view = self._window.tools_view - if tools_view is not None: - tools_view.set_plugin_updating(signal_key, False) - tools_view.set_plugin_error(signal_key, f'Update failed: {exc}') + self._fail_plugin_update(signal_key, f'Update failed: {exc}') async def _async_single_plugin_update(self, porringer: API, plugin_name: str) -> None: """Run a single-plugin tool update and route results.""" - config = self._resolve_config() + config = self._store.config mapping = config.plugin_auto_update or {} pkg_entry = mapping.get(plugin_name) coordinator = self._window.coordinator @@ -285,16 +320,13 @@ async def _async_single_plugin_update(self, porringer: API, plugin_name: str) -> ) if coordinator is not None: coordinator.invalidate() - self._on_tool_update_finished(result, updating_plugin=plugin_name, manual=True) + self._on_tool_update_finished(result, UpdateTarget(plugin=plugin_name)) except asyncio.CancelledError: logger.debug('Single plugin update cancelled (shutdown)') raise except Exception as exc: logger.exception('Tool update failed') - tools_view = self._window.tools_view - if tools_view is not None: - tools_view.set_plugin_updating(plugin_name, False) - tools_view.set_plugin_error(plugin_name, f'Update failed: {exc}') + self._fail_plugin_update(plugin_name, f'Update failed: {exc}') # -- Single package update -- @@ -328,8 +360,9 @@ async def _async_single_package_update( coordinator = self._window.coordinator discovered = coordinator.discovered_plugins if coordinator is not None else None - if ':' in plugin_name: - bare_plugin, runtime_tag = plugin_name.split(':', 1) + target = UpdateTarget(plugin=plugin_name, package=package_name) + bare_plugin, runtime_tag = _parse_plugin_key(plugin_name) + if runtime_tag is not None: try: result = await run_runtime_package_updates( porringer, @@ -340,20 +373,13 @@ async def _async_single_package_update( ) if coordinator is not None: coordinator.invalidate() - self._on_tool_update_finished( - result, - updating_package=(plugin_name, package_name), - manual=True, - ) + self._on_tool_update_finished(result, target) except asyncio.CancelledError: logger.debug('Runtime package update cancelled (shutdown)') raise except Exception as exc: logger.exception('Runtime package update failed') - tools_view = self._window.tools_view - if tools_view is not None: - tools_view.set_package_updating(plugin_name, package_name, False) - tools_view.set_package_error(plugin_name, package_name, f'Update failed: {exc}') + self._fail_package_update(plugin_name, package_name, f'Update failed: {exc}') return try: @@ -365,32 +391,28 @@ async def _async_single_package_update( ) if coordinator is not None: coordinator.invalidate() - self._on_tool_update_finished( - result, - updating_package=(plugin_name, package_name), - manual=True, - ) + self._on_tool_update_finished(result, target) except asyncio.CancelledError: logger.debug('Package update cancelled (shutdown)') raise except Exception as exc: logger.exception('Package update failed') - tools_view = self._window.tools_view - if tools_view is not None: - tools_view.set_package_updating(plugin_name, package_name, False) - tools_view.set_package_error(plugin_name, package_name, f'Update failed: {exc}') + self._fail_package_update(plugin_name, package_name, f'Update failed: {exc}') # -- Shared completion handler -- def _on_tool_update_finished( self, result: ToolUpdateResult, - *, - updating_plugin: str | None = None, - updating_package: tuple[str, str] | None = None, - manual: bool = False, + target: UpdateTarget | None = None, ) -> None: - """Handle tool update completion.""" + """Handle tool update completion. + + Args: + result: Summary of the update run. + target: Which plugin/package was updated. ``None`` for + periodic (automatic) updates. + """ logger.info( 'Tool update completed: %d manifest(s), %d updated, %d already latest, %d failed', result.manifests_processed, @@ -402,36 +424,25 @@ def _on_tool_update_finished( # Persist timestamps for updated packages if result.updated_packages: now = datetime.now(UTC).isoformat() - existing = dict(load_user_config().last_tool_updates or {}) - plugin_name = updating_plugin or (updating_package[0] if updating_package else '') + existing = dict(self._store.config.last_tool_updates or {}) + plugin_name = target.plugin if target else '' for pkg_name in result.updated_packages: key = f'{plugin_name}/{pkg_name}' if plugin_name else pkg_name existing[key] = now - resolved = update_user_config(last_tool_updates=existing) - # Refresh the config on the tools view so the next rebuild - # picks up the updated timestamps instead of stale data. - tools_view_ref = self._window.tools_view - if tools_view_ref is not None: - tools_view_ref._config = resolved - - # Clear updating state on widgets + self._store.update(last_tool_updates=existing) + + # Clear updating state and refresh the tools view. When the + # window is hidden we skip the refresh() cycle because + # MainWindow.show() already triggers a full refresh the next + # time the user opens the window. Manual updates (target is + # not None) call show() below which triggers the refresh. tools_view = self._window.tools_view - logger.info( - '[DIAG] _on_tool_update_finished: manual=%s, tools_view_exists=%s, window_visible=%s', - manual, - tools_view is not None, - self._window.isVisible(), - ) if tools_view is not None: - if updating_plugin is not None: - tools_view.set_plugin_updating(updating_plugin, False) - if updating_package is not None: - tools_view.set_package_updating(*updating_package, False) - # Refresh to pick up version changes and re-detect updates tools_view._updates_checked = False - tools_view.refresh() + if self._window.isVisible() and target is None: + tools_view.refresh() - if manual: + if target is not None: self._window.show() def _on_tool_update_error(self, error: str) -> None: @@ -470,10 +481,7 @@ async def _async_single_package_remove( coordinator = self._window.coordinator discovered = coordinator.discovered_plugins if coordinator is not None else None - bare_plugin = plugin_name - runtime_tag: str | None = None - if ':' in plugin_name: - bare_plugin, runtime_tag = plugin_name.split(':', 1) + bare_plugin, runtime_tag = _parse_plugin_key(plugin_name) try: package_ref = PackageRef(name=package_name) @@ -500,10 +508,9 @@ async def _async_single_package_remove( raise except Exception as exc: logger.exception('Package removal failed') - tools_view = self._window.tools_view - if tools_view is not None: - tools_view.set_package_removing(plugin_name, package_name, False) - tools_view.set_package_error(plugin_name, package_name, f'Failed to remove {package_name}: {exc}') + self._fail_package_update( + plugin_name, package_name, f'Failed to remove {package_name}: {exc}', removing=True, + ) def _on_package_remove_finished( self, @@ -512,18 +519,17 @@ def _on_package_remove_finished( package_name: str, ) -> None: """Handle package removal completion.""" - tools_view = self._window.tools_view - if not result.success or result.skipped: detail = result.message or 'Unknown error' logger.warning('Package removal failed for %s/%s: %s', plugin_name, package_name, detail) - if tools_view is not None: - tools_view.set_package_removing(plugin_name, package_name, False) - tools_view.set_package_error(plugin_name, package_name, f'Could not remove {package_name}: {detail}') + self._fail_package_update( + plugin_name, package_name, f'Could not remove {package_name}: {detail}', removing=True, + ) return logger.info('Package removal completed for %s/%s', plugin_name, package_name) + tools_view = self._window.tools_view if tools_view is not None: tools_view.set_package_removing(plugin_name, package_name, False) tools_view._updates_checked = False diff --git a/synodic_client/application/screen/tray.py b/synodic_client/application/screen/tray.py index e616758..3f1bda2 100644 --- a/synodic_client/application/screen/tray.py +++ b/synodic_client/application/screen/tray.py @@ -1,6 +1,7 @@ """Tray screen for the application.""" import logging +from typing import TYPE_CHECKING from PySide6.QtGui import QAction from PySide6.QtWidgets import ( @@ -16,10 +17,9 @@ from synodic_client.application.screen.tool_update_controller import ToolUpdateOrchestrator from synodic_client.application.update_controller import UpdateController from synodic_client.client import Client -from synodic_client.resolution import ( - ResolvedConfig, - resolve_config, -) + +if TYPE_CHECKING: + from synodic_client.application.config_store import ConfigStore logger = logging.getLogger(__name__) @@ -32,7 +32,8 @@ def __init__( app: QApplication, client: Client, window: MainWindow, - config: ResolvedConfig | None = None, + *, + store: ConfigStore, ) -> None: """Initialize the tray icon. @@ -40,13 +41,12 @@ def __init__( app: The running ``QApplication``. client: The Synodic Client service. window: The main application window. - config: Optional pre-resolved configuration. When ``None``, - the configuration is resolved from disk on demand. + store: The centralised :class:`ConfigStore`. """ self._app = app self._client = client self._window = window - self._config = config + self._store = store self.tray_icon = app_icon() @@ -59,10 +59,9 @@ def __init__( # Settings window (created once, shown/hidden on demand) self._settings_window = SettingsWindow( - self._resolve_config(), + self._store, version=str(self._client.version), ) - self._settings_window.settings_changed.connect(self._on_settings_changed) # MainWindow gear button -> open settings window.settings_requested.connect(self._show_settings) @@ -74,19 +73,22 @@ def __init__( client, [self._banner], settings_window=self._settings_window, - config=config, + store=self._store, ) self._update_controller.set_user_active_predicate(self._is_user_active) # Tool update orchestrator - owns tool/package update lifecycle self._tool_orchestrator = ToolUpdateOrchestrator( window, - self._resolve_config, + self._store, self.tray, is_user_active=self._is_user_active, ) self._tool_orchestrator.restart_tool_update_timer() + # Restart tool timer when config changes (e.g. interval edited) + self._store.changed.connect(lambda _: self._tool_orchestrator.restart_tool_update_timer()) + # Connect ToolsView signals - deferred because ToolsView is created lazily window.tools_view_created.connect(self._tool_orchestrator.connect_tools_view) @@ -112,14 +114,6 @@ def _build_menu(self, app: QApplication, window: MainWindow) -> None: self.tray.setContextMenu(self.menu) - # -- Config helpers -- - - def _resolve_config(self) -> ResolvedConfig: - """Return the injected config or resolve from disk.""" - if self._config is not None: - return self._config - return resolve_config() - def _on_tray_activated(self, reason: QSystemTrayIcon.ActivationReason) -> None: """Handle tray icon activation (e.g. double-click).""" if reason == QSystemTrayIcon.ActivationReason.DoubleClick: @@ -146,11 +140,3 @@ def shutdown(self) -> None: self._update_controller.shutdown() self._tool_orchestrator.shutdown() logger.info('TrayScreen shut down') - - def _on_settings_changed(self, config: ResolvedConfig) -> None: - """React to a change made in the settings window.""" - self._config = config - # Delegate updater reinit + immediate check to the controller - self._update_controller.on_settings_changed(config) - # Restart tool-update timer with new config - self._tool_orchestrator.restart_tool_update_timer() diff --git a/synodic_client/application/screen/update_banner.py b/synodic_client/application/screen/update_banner.py index f1bdcf8..5a6e62e 100644 --- a/synodic_client/application/screen/update_banner.py +++ b/synodic_client/application/screen/update_banner.py @@ -208,7 +208,7 @@ def hide_banner(self) -> None: if self._state == UpdateBannerState.HIDDEN: return self._state = UpdateBannerState.HIDDEN - self._slide_out() + self._animate_height(0) # --- Internal --- @@ -239,26 +239,34 @@ def _configure(self, config: _BannerConfig) -> None: self._progress.hide() target_height = _BANNER_HEIGHT_WITH_PROGRESS if config.show_progress else _BANNER_HEIGHT - self._slide_in(target_height) + self._animate_height(target_height) - def _slide_in(self, target_height: int) -> None: - """Animate the banner from collapsed to *target_height*.""" - self.setVisible(True) - self._anim.stop() - self._anim.setStartValue(self.maximumHeight()) - self._anim.setEndValue(target_height) - self._anim.start() + def _animate_height(self, target: int) -> None: + """Animate (or snap) the banner to *target* height. - def _slide_out(self) -> None: - """Animate the banner down to zero height, then hide.""" + When *target* > 0 the banner slides in; when 0 it slides out. + If the parent window is hidden the change is applied instantly + to avoid property-change cycles that can cause transient window + flashes on Windows. + """ self._anim.stop() + + parent_window = self.window() + if parent_window is not None and not parent_window.isVisible(): + self.setMaximumHeight(target) + self.setVisible(target > 0) + return + + if target > 0: + self.setVisible(True) + else: + self._anim.finished.connect( + self._on_slide_out_done, + type=Qt.ConnectionType.SingleShotConnection, + ) + self._anim.setStartValue(self.maximumHeight()) - self._anim.setEndValue(0) - # Use a one-shot connection to avoid accumulating slots. - self._anim.finished.connect( - self._on_slide_out_done, - type=Qt.ConnectionType.SingleShotConnection, - ) + self._anim.setEndValue(target) self._anim.start() def _on_slide_out_done(self) -> None: diff --git a/synodic_client/application/update_controller.py b/synodic_client/application/update_controller.py index b1c809e..e92b63b 100644 --- a/synodic_client/application/update_controller.py +++ b/synodic_client/application/update_controller.py @@ -27,13 +27,12 @@ from synodic_client.application.workers import check_for_update, download_update from synodic_client.resolution import ( ResolvedConfig, - resolve_config, resolve_update_config, - update_user_config, ) from synodic_client.schema import UpdateInfo if TYPE_CHECKING: + from synodic_client.application.config_store import ConfigStore from synodic_client.application.screen.settings import SettingsWindow from synodic_client.client import Client @@ -54,8 +53,8 @@ class UpdateController: state transitions to (typically ``UpdateBanner`` instances). settings_window: The ``SettingsWindow`` (check button + last-updated label). - config: - Optional pre-resolved configuration. ``None`` resolves from disk. + store: + The centralised :class:`ConfigStore`. """ def __init__( @@ -65,7 +64,7 @@ def __init__( views: list[UpdateView], *, settings_window: SettingsWindow, - config: ResolvedConfig | None = None, + store: ConfigStore, ) -> None: """Initialise the controller and start the periodic timer. @@ -74,20 +73,19 @@ def __init__( client: The Synodic Client service facade. views: One or more :class:`UpdateView` implementations. settings_window: The settings window (check button + timestamp). - config: Optional pre-resolved configuration. + store: The centralised :class:`ConfigStore`. """ self._app = app self._client = client self._views = views self._settings_window = settings_window - self._config = config + self._store = store self._is_user_active: Callable[[], bool] = lambda: False self._update_task: asyncio.Task[None] | None = None self._pending_version: str | None = None # Derive auto-apply preference from config - resolved = self._resolve_config() - self._auto_apply: bool = resolved.auto_apply + self._auto_apply: bool = store.config.auto_apply # Periodic auto-update timer self._auto_update_timer: QTimer | None = None @@ -103,6 +101,9 @@ def __init__( self._settings_window.check_updates_requested.connect(self._on_manual_check) self._settings_window.restart_requested.connect(self._apply_update) + # React to config changes from any source + self._store.changed.connect(self._on_config_changed) + def set_user_active_predicate(self, predicate: Callable[[], bool]) -> None: """Set the predicate used to defer auto-apply when the user is active. @@ -125,12 +126,6 @@ def shutdown(self) -> None: # Config helpers # ------------------------------------------------------------------ - def _resolve_config(self) -> ResolvedConfig: - """Return the injected config or resolve from disk.""" - if self._config is not None: - return self._config - return resolve_config() - def _can_auto_apply(self) -> bool: """Return whether a downloaded update should be applied automatically. @@ -142,8 +137,7 @@ def _can_auto_apply(self) -> bool: def _persist_check_timestamp(self) -> None: """Persist the current time as *last_client_update* and refresh the label.""" ts = datetime.now(UTC).isoformat() - resolved = update_user_config(last_client_update=ts) - self._settings_window.update_config(resolved) + self._store.update(last_client_update=ts) self._settings_window.set_last_checked(ts) def _report_error(self, message: str, *, silent: bool) -> None: @@ -165,7 +159,7 @@ def _report_error(self, message: str, *, silent: bool) -> None: def _restart_auto_update_timer(self) -> None: """Start (or restart) the periodic auto-update timer from config.""" - config = resolve_update_config(self._resolve_config()) + config = resolve_update_config(self._store.config) if self._auto_update_timer is not None: self._auto_update_timer.stop() @@ -196,13 +190,14 @@ def check_now(self, *, silent: bool = False) -> None: """ self._do_check(silent=silent) - def on_settings_changed(self, config: ResolvedConfig) -> None: - """React to a settings change — reinitialise the updater and timers. + def _on_config_changed(self, config: object) -> None: + """React to a config change — reinitialise the updater and timers. Also triggers an immediate (silent) check so the user gets feedback after switching channels. """ - self._config = config + if not isinstance(config, ResolvedConfig): + return self._auto_apply = config.auto_apply self._reinitialize_updater(config) self.check_now(silent=True) @@ -247,19 +242,20 @@ def _do_check(self, *, silent: bool) -> None: view.show_error('Updater is not initialized.') return - # Preserve the banner state when an update is already pending - if self._pending_version is None: + # Preserve the banner state when an update is already pending. + # Skip the visual "Checking…" transition when the settings + # window isn't visible to avoid widget operations on a hidden + # QMainWindow (which can cause transient flashes on Windows). + if self._pending_version is None and self._settings_window.isVisible(): self._settings_window.set_checking() self._update_task = asyncio.create_task(self._async_check(silent=silent)) async def _async_check(self, *, silent: bool) -> None: """Run the update check coroutine and route results.""" - logger.info('[DIAG] Self-update check starting (silent=%s)', silent) try: result = await check_for_update(self._client) self._on_check_finished(result, silent=silent) - logger.info('[DIAG] Self-update check completed (silent=%s)', silent) except asyncio.CancelledError: logger.debug('Update check cancelled (shutdown)') raise @@ -346,8 +342,7 @@ def _on_download_finished(self, success: bool, version: str) -> None: # Persist the client-update timestamp (actual update downloaded) ts = datetime.now(UTC).isoformat() - resolved = update_user_config(last_client_update=ts) - self._settings_window.update_config(resolved) + self._store.update(last_client_update=ts) self._settings_window.set_last_checked(ts) self._pending_version = version diff --git a/tests/unit/qt/test_gather_packages.py b/tests/unit/qt/test_gather_packages.py index 8e33075..b67730f 100644 --- a/tests/unit/qt/test_gather_packages.py +++ b/tests/unit/qt/test_gather_packages.py @@ -1,4 +1,4 @@ -"""Tests for ToolsView._gather_packages global + per-directory queries.""" +"""Tests for ToolsView._gather_packages global + per-directory queries.""" from __future__ import annotations @@ -13,6 +13,7 @@ from porringer.utility.exception import PluginError from PySide6.QtWidgets import QLabel, QPushButton +from synodic_client.application.config_store import ConfigStore from synodic_client.application.screen.plugin_row import ( FilterChip, PluginKindHeader, @@ -54,6 +55,11 @@ def _make_config() -> ResolvedConfig: ) +def _make_store(config: ResolvedConfig | None = None) -> ConfigStore: + """Build a ConfigStore seeded with *config*.""" + return ConfigStore(config or _make_config()) + + def _make_porringer() -> MagicMock: """Build a MagicMock standing in for the porringer API.""" mock = MagicMock() @@ -82,7 +88,7 @@ def test_global_query_returns_packages_with_no_directories() -> None: ], ) - view = ToolsView(porringer, _make_config()) + view = ToolsView(porringer, _make_store()) result = asyncio.run(view._gather_packages('pipx', [])) assert {e.name for e in result} == {'pdm', 'cppython'} @@ -97,7 +103,7 @@ def test_global_query_returns_packages_with_empty_project_path() -> None: ], ) - view = ToolsView(porringer, _make_config()) + view = ToolsView(porringer, _make_store()) result = asyncio.run(view._gather_packages('pipx', [])) matching = [e for e in result if e.name == 'pdm'] @@ -110,7 +116,7 @@ def test_global_query_called_without_project_path() -> None: porringer = _make_porringer() porringer.package.list = AsyncMock(return_value=[]) - view = ToolsView(porringer, _make_config()) + view = ToolsView(porringer, _make_store()) asyncio.run(view._gather_packages('pipx', [])) # At least one call should have been made with only plugin_name (no path) @@ -137,7 +143,7 @@ async def _mock_list(plugin_name: str, project_path: Path | None = None, **kwarg porringer.package.list = AsyncMock(side_effect=_mock_list) directory = ManifestDirectory(path=Path('/fake/project')) - view = ToolsView(porringer, _make_config()) + view = ToolsView(porringer, _make_store()) result = asyncio.run(view._gather_packages('pipx', [directory])) names = {entry.name for entry in result} @@ -157,7 +163,7 @@ async def _mock_list(plugin_name: str, project_path: Path | None = None, **kwarg porringer.package.list = AsyncMock(side_effect=_mock_list) directory = ManifestDirectory(path=Path('/fake/project')) - view = ToolsView(porringer, _make_config()) + view = ToolsView(porringer, _make_store()) result = asyncio.run(view._gather_packages('pipx', [directory])) matching = [e for e in result if e.name == 'mylib'] @@ -172,7 +178,7 @@ def test_global_packages_have_empty_project_label() -> None: return_value=[Package(name='cppython', version='0.5.0')], ) - view = ToolsView(porringer, _make_config()) + view = ToolsView(porringer, _make_store()) result = asyncio.run(view._gather_packages('pipx', [])) matching = [e for e in result if e.name == 'cppython'] @@ -196,7 +202,7 @@ async def _mock_list(plugin_name: str, project_path: Path | None = None, **kwarg porringer.package.list = AsyncMock(side_effect=_mock_list) directory = ManifestDirectory(path=Path('/fake/project')) - view = ToolsView(porringer, _make_config()) + view = ToolsView(porringer, _make_store()) result = asyncio.run(view._gather_packages('pipx', [directory])) names = {entry.name for entry in result} @@ -222,7 +228,7 @@ def test_relation_host_extracted_into_host_tool() -> None: ], ) - view = ToolsView(porringer, _make_config()) + view = ToolsView(porringer, _make_store()) result = asyncio.run(view._gather_packages('pipx', [])) by_name = {entry.name: entry.host_tool for entry in result} @@ -242,7 +248,7 @@ class TestGatherToolPlugins: def test_returns_plugins_keyed_by_host_tool(monkeypatch) -> None: """installed_plugins() results are keyed by tool name and returned as PackageEntry.""" porringer = _make_porringer() - view = ToolsView(porringer, _make_config()) + view = ToolsView(porringer, _make_store()) # Mock _discover_plugin_managers to return a fake manager mock_manager = MagicMock() @@ -275,7 +281,7 @@ def test_returns_plugins_keyed_by_host_tool(monkeypatch) -> None: def test_empty_when_no_managers(monkeypatch) -> None: """Returns empty dict when no PluginManager instances are discovered.""" porringer = _make_porringer() - view = ToolsView(porringer, _make_config()) + view = ToolsView(porringer, _make_store()) monkeypatch.setattr( ToolsView, '_discover_plugin_managers', @@ -289,7 +295,7 @@ def test_empty_when_no_managers(monkeypatch) -> None: def test_manager_failure_does_not_propagate(monkeypatch) -> None: """A failing installed_plugins() call produces an empty entry, not an exception.""" porringer = _make_porringer() - view = ToolsView(porringer, _make_config()) + view = ToolsView(porringer, _make_store()) mock_manager = MagicMock() mock_manager.installed_plugins = AsyncMock(side_effect=RuntimeError('boom')) @@ -307,7 +313,7 @@ def test_manager_failure_does_not_propagate(monkeypatch) -> None: def test_multiple_managers(monkeypatch) -> None: """Multiple PluginManager instances are queried in parallel.""" porringer = _make_porringer() - view = ToolsView(porringer, _make_config()) + view = ToolsView(porringer, _make_store()) mgr_pdm = MagicMock() mgr_pdm.installed_plugins = AsyncMock( @@ -417,7 +423,7 @@ def test_transitive_dependency_marked() -> None: project_path='/projects/periapsis', ), ] - # 'cppython' is NOT in the manifest set → transitive + # 'cppython' is NOT in the manifest set → transitive result = ToolsView._build_display_packages(entries, set()) assert result[0].project_instances[0].is_transitive is True @@ -522,7 +528,7 @@ def test_navigate_signal_emitted(self) -> None: spy = MagicMock() row.navigate_to_project.connect(spy) - # Find the navigate button (→) + # Find the navigate button (→) nav_btns = [w for w in row.findChildren(QPushButton) if w.text() == '\u2192'] assert len(nav_btns) == 1 nav_btns[0].click() @@ -590,7 +596,7 @@ def _make_view() -> ToolsView: """Build a ToolsView with a mock porringer.""" porringer = _make_porringer() config = _make_config() - return ToolsView(porringer, config) + return ToolsView(porringer, _make_store(config)) @staticmethod def _populate_section_widgets(view: ToolsView) -> None: @@ -682,7 +688,7 @@ def test_chip_reselection_restores(self) -> None: assert len(visible_rows) == _EXPECTED_VISIBLE_ROWS_ALL def test_search_plus_chip_filter(self) -> None: - """Search and chip filtering compose — only matching rows in active plugins survive.""" + """Search and chip filtering compose — only matching rows in active plugins survive.""" view = self._make_view() self._populate_section_widgets(view) @@ -757,7 +763,7 @@ def _make_view() -> ToolsView: """Build a ToolsView with a mock porringer.""" porringer = _make_porringer() config = _make_config() - return ToolsView(porringer, config) + return ToolsView(porringer, _make_store(config)) def test_panel_starts_collapsed(self) -> None: """The filter panel is hidden and has zero max-height on init.""" @@ -771,7 +777,7 @@ def test_toggle_opens_panel(self) -> None: view = self._make_view() view._toggle_filter_panel() assert view._filter_panel_open - assert view._filter_panel.isVisible() + assert not view._filter_panel.isHidden() def test_toggle_closes_open_panel(self) -> None: """Calling _toggle_filter_panel on an open panel closes it.""" @@ -892,7 +898,7 @@ async def _mock_list(plugin_name: str, project_path: Path | None = None, **kwarg MagicMock(directory=ManifestDirectory(path=Path('/fake/project'))), ] - view = ToolsView(porringer, _make_config()) + view = ToolsView(porringer, _make_store()) # Use _gather_packages directly with an empty directory list # (as _gather_refresh_data would do for RUNTIME plugins) result = asyncio.run(view._gather_packages('pim', [])) @@ -964,7 +970,7 @@ def _make_runtime_results(default_exe: Path) -> list[RuntimePackageResult]: def test_runtime_sections_create_separate_provider_headers(self) -> None: """Each RuntimePackageResult produces its own PluginProviderHeader.""" - view = ToolsView(_make_porringer(), _make_config()) + view = ToolsView(_make_porringer(), _make_store()) default_exe = Path('C:/Python314/python.exe') plugin = self._pip_plugin() rt_results = self._make_runtime_results(default_exe) @@ -984,7 +990,7 @@ def test_runtime_sections_create_separate_provider_headers(self) -> None: def test_default_runtime_comes_first(self) -> None: """The default runtime's provider header is the first one.""" - view = ToolsView(_make_porringer(), _make_config()) + view = ToolsView(_make_porringer(), _make_store()) default_exe = Path('C:/Python314/python.exe') plugin = self._pip_plugin() rt_results = self._make_runtime_results(default_exe) @@ -1006,7 +1012,7 @@ def test_default_runtime_comes_first(self) -> None: def test_default_runtime_packages_displayed(self) -> None: """Package rows for the default runtime appear after its header.""" - view = ToolsView(_make_porringer(), _make_config()) + view = ToolsView(_make_porringer(), _make_store()) default_exe = Path('C:/Python314/python.exe') plugin = self._pip_plugin() rt_results = self._make_runtime_results(default_exe) @@ -1029,7 +1035,7 @@ def test_default_runtime_packages_displayed(self) -> None: def test_non_default_runtime_packages_displayed(self) -> None: """Package rows for the non-default runtime appear after its header.""" - view = ToolsView(_make_porringer(), _make_config()) + view = ToolsView(_make_porringer(), _make_store()) default_exe = Path('C:/Python314/python.exe') plugin = self._pip_plugin() rt_results = self._make_runtime_results(default_exe) @@ -1051,7 +1057,7 @@ def test_non_default_runtime_packages_displayed(self) -> None: def test_venv_packages_excluded_for_runtime_probed_plugin(self) -> None: """Venv packages must not appear in ToolsView for runtime-probed plugins.""" - view = ToolsView(_make_porringer(), _make_config()) + view = ToolsView(_make_porringer(), _make_store()) default_exe = Path('C:/Python314/python.exe') plugin = self._pip_plugin() rt_results = self._make_runtime_results(default_exe) @@ -1077,12 +1083,12 @@ def test_venv_packages_excluded_for_runtime_probed_plugin(self) -> None: view._build_widget_tree(data) providers = [w for w in view._section_widgets if isinstance(w, PluginProviderHeader)] - # Only the 2 runtime providers — no extra venv provider + # Only the 2 runtime providers — no extra venv provider assert len(providers) == _EXPECTED_RUNTIME_PROVIDERS def test_runtime_tag_uses_default_style(self) -> None: """The default runtime tag uses the green highlight style.""" - view = ToolsView(_make_porringer(), _make_config()) + view = ToolsView(_make_porringer(), _make_store()) default_exe = Path('C:/Python314/python.exe') plugin = self._pip_plugin() rt_results = self._make_runtime_results(default_exe) @@ -1105,7 +1111,7 @@ def test_runtime_tag_uses_default_style(self) -> None: def test_runtime_tag_uses_normal_style_for_non_default(self) -> None: """Non-default runtime tags use the blue style.""" - view = ToolsView(_make_porringer(), _make_config()) + view = ToolsView(_make_porringer(), _make_store()) default_exe = Path('C:/Python314/python.exe') plugin = self._pip_plugin() rt_results = self._make_runtime_results(default_exe) @@ -1129,7 +1135,7 @@ def test_runtime_tag_uses_normal_style_for_non_default(self) -> None: def test_filter_chips_work_with_runtime_providers(self) -> None: """Filter chips are built from runtime provider headers and filter works.""" - view = ToolsView(_make_porringer(), _make_config()) + view = ToolsView(_make_porringer(), _make_store()) default_exe = Path('C:/Python314/python.exe') plugin = self._pip_plugin() rt_results = self._make_runtime_results(default_exe) @@ -1160,7 +1166,7 @@ def test_gather_runtime_packages_returns_none_for_non_consumer() -> None: side_effect=PluginError('not a RuntimeConsumer'), ) - view = ToolsView(porringer, _make_config()) + view = ToolsView(porringer, _make_store()) result = asyncio.run(view._gather_runtime_packages('pipx', MagicMock())) assert result is None @@ -1178,7 +1184,7 @@ def test_gather_runtime_packages_returns_results() -> None: ] porringer.package.list_by_runtime = AsyncMock(return_value=expected) - view = ToolsView(porringer, _make_config()) + view = ToolsView(porringer, _make_store()) result = asyncio.run(view._gather_runtime_packages('pip', MagicMock())) assert result == expected @@ -1197,7 +1203,7 @@ async def _mock_list(plugin_name: str, project_path: Path | None = None, **kwarg porringer.package.list = AsyncMock(side_effect=_mock_list) directory = ManifestDirectory(path=Path('/fake/project')) - view = ToolsView(porringer, _make_config()) + view = ToolsView(porringer, _make_store()) result = asyncio.run(view._gather_packages('pip', [directory], skip_global=True)) assert all(p is not None for p in call_paths), 'global query should not be issued' diff --git a/tests/unit/qt/test_settings.py b/tests/unit/qt/test_settings.py index 270bda7..7eec9d0 100644 --- a/tests/unit/qt/test_settings.py +++ b/tests/unit/qt/test_settings.py @@ -5,6 +5,7 @@ from typing import Any from unittest.mock import MagicMock, patch +from synodic_client.application.config_store import ConfigStore from synodic_client.application.screen.settings import SettingsWindow from synodic_client.application.theme import SETTINGS_WINDOW_MIN_SIZE from synodic_client.resolution import ResolvedConfig @@ -35,10 +36,15 @@ def _make_config(**overrides: Any) -> ResolvedConfig: return ResolvedConfig(**defaults) +def _make_store(config: ResolvedConfig | None = None) -> ConfigStore: + """Create a ``ConfigStore`` seeded with *config*.""" + return ConfigStore(config or _make_config()) + + def _make_window(config: ResolvedConfig | None = None, version: str = '0.0.0.test') -> SettingsWindow: """Create a ``SettingsWindow`` without showing it.""" - cfg = config or _make_config() - window = SettingsWindow(cfg, version=version) + store = _make_store(config) + window = SettingsWindow(store, version=version) return window @@ -167,21 +173,19 @@ def test_auto_start_reflects_registry() -> None: class TestSettingsCallbacks: - """Verify that control changes persist via update_user_config and emit the signal.""" + """Verify that control changes persist via the ConfigStore.""" @staticmethod def test_channel_change_to_dev() -> None: - """Switching to dev calls update_user_config and emits settings_changed.""" + """Switching to dev calls store.update with update_channel='dev'.""" config = _make_config() window = _make_window(config) - signal_spy = MagicMock() - window.settings_changed.connect(signal_spy) new_config = _make_config(update_channel='dev') - with patch('synodic_client.application.screen.settings.update_user_config', return_value=new_config): + with patch.object(window._store, 'update', return_value=new_config) as mock_update: window._channel_combo.setCurrentIndex(1) - signal_spy.assert_called_once_with(new_config) + mock_update.assert_called_with(update_channel='dev') @staticmethod def test_channel_change_to_stable() -> None: @@ -191,26 +195,23 @@ def test_channel_change_to_stable() -> None: window.sync_from_config() new_config = _make_config(update_channel='stable') - target = 'synodic_client.application.screen.settings.update_user_config' - with patch(target, return_value=new_config) as mock_update: + with patch.object(window._store, 'update', return_value=new_config) as mock_update: window._channel_combo.setCurrentIndex(0) mock_update.assert_called_with(update_channel='stable') @staticmethod def test_source_change() -> None: - """Editing the source saves and emits.""" + """Editing the source saves via the store.""" config = _make_config() window = _make_window(config) - signal_spy = MagicMock() - window.settings_changed.connect(signal_spy) new_config = _make_config(update_source='https://custom.example.com') - with patch('synodic_client.application.screen.settings.update_user_config', return_value=new_config): + with patch.object(window._store, 'update', return_value=new_config) as mock_update: window._source_edit.setText('https://custom.example.com') window._on_source_changed() - signal_spy.assert_called_once_with(new_config) + mock_update.assert_called_with(update_source='https://custom.example.com') @staticmethod def test_source_blank_sets_none() -> None: @@ -219,8 +220,7 @@ def test_source_blank_sets_none() -> None: window = _make_window(config) new_config = _make_config(update_source=None) - target = 'synodic_client.application.screen.settings.update_user_config' - with patch(target, return_value=new_config) as mock_update: + with patch.object(window._store, 'update', return_value=new_config) as mock_update: window._source_edit.setText('') window._on_source_changed() @@ -228,48 +228,39 @@ def test_source_blank_sets_none() -> None: @staticmethod def test_auto_update_interval_change() -> None: - """Changing auto-update interval saves and emits.""" + """Changing auto-update interval saves via the store.""" new_interval = 90 config = _make_config() window = _make_window(config) - signal_spy = MagicMock() - window.settings_changed.connect(signal_spy) - new_config = _make_config(auto_update_interval_minutes=new_interval) - with patch('synodic_client.application.screen.settings.update_user_config', return_value=new_config): + with patch.object(window._store, 'update') as mock_update: window._auto_update_spin.setValue(new_interval) - signal_spy.assert_called_once_with(new_config) + mock_update.assert_called_with(auto_update_interval_minutes=new_interval) @staticmethod def test_tool_update_interval_change() -> None: - """Changing tool-update interval saves and emits.""" + """Changing tool-update interval saves via the store.""" new_interval = 120 config = _make_config() window = _make_window(config) - signal_spy = MagicMock() - window.settings_changed.connect(signal_spy) - new_config = _make_config(tool_update_interval_minutes=new_interval) - with patch('synodic_client.application.screen.settings.update_user_config', return_value=new_config): + with patch.object(window._store, 'update') as mock_update: window._tool_update_spin.setValue(new_interval) - signal_spy.assert_called_once_with(new_config) + mock_update.assert_called_with(tool_update_interval_minutes=new_interval) @staticmethod def test_detect_updates_change() -> None: - """Toggling detect_updates saves and emits.""" + """Toggling detect_updates saves via the store.""" config = _make_config() window = _make_window(config) window.sync_from_config() - signal_spy = MagicMock() - window.settings_changed.connect(signal_spy) - new_config = _make_config(detect_updates=False) - with patch('synodic_client.application.screen.settings.update_user_config', return_value=new_config): + with patch.object(window._store, 'update') as mock_update: window._detect_updates_check.setChecked(False) - signal_spy.assert_called_once_with(new_config) + mock_update.assert_called_with(detect_updates=False) @staticmethod def test_auto_start_registers_startup_when_frozen() -> None: @@ -279,7 +270,7 @@ def test_auto_start_registers_startup_when_frozen() -> None: new_config = _make_config(auto_start=True) with ( - patch('synodic_client.application.screen.settings.update_user_config', return_value=new_config), + patch.object(window._store, 'update', return_value=new_config), patch('synodic_client.application.screen.settings.register_startup') as mock_register, patch('synodic_client.application.screen.settings.is_startup_registered', return_value=False), patch('synodic_client.application.screen.settings.getattr', return_value=True), @@ -300,7 +291,7 @@ def test_auto_start_removes_startup_when_frozen() -> None: new_config = _make_config(auto_start=False) with ( - patch('synodic_client.application.screen.settings.update_user_config', return_value=new_config), + patch.object(window._store, 'update', return_value=new_config), patch('synodic_client.application.screen.settings.remove_startup') as mock_remove, patch('synodic_client.application.screen.settings.getattr', return_value=True), ): @@ -316,7 +307,7 @@ def test_auto_start_skips_registry_when_not_frozen() -> None: new_config = _make_config(auto_start=True) with ( - patch('synodic_client.application.screen.settings.update_user_config', return_value=new_config), + patch.object(window._store, 'update', return_value=new_config), patch('synodic_client.application.screen.settings.register_startup') as mock_register, patch('synodic_client.application.screen.settings.remove_startup') as mock_remove, patch('synodic_client.application.screen.settings.is_startup_registered', return_value=False), @@ -334,15 +325,15 @@ def test_auto_start_skips_registry_when_not_frozen() -> None: class TestSyncDoesNotEmit: - """Verify that sync_from_config does not trigger settings_changed.""" + """Verify that sync_from_config does not trigger store.changed.""" @staticmethod def test_sync_no_signal() -> None: - """Syncing controls from config must not emit settings_changed.""" + """Syncing controls from config must not emit store.changed.""" config = _make_config(update_channel='dev', auto_update_interval_minutes=60) window = _make_window(config) signal_spy = MagicMock() - window.settings_changed.connect(signal_spy) + window._store.changed.connect(signal_spy) window.sync_from_config() @@ -407,43 +398,21 @@ def test_set_checking() -> None: # --------------------------------------------------------------------------- -# update_config — silent config refresh +# Store config is always fresh # --------------------------------------------------------------------------- -class TestUpdateConfig: - """Verify that update_config refreshes _config without emitting signals.""" - - @staticmethod - def test_update_config_replaces_internal_config() -> None: - """update_config should replace _config with the new snapshot.""" - window = _make_window(_make_config(last_client_update=None)) - new_config = _make_config(last_client_update='2026-03-09T12:00:00+00:00') - - window.update_config(new_config) - - assert window._config is new_config - assert window._config.last_client_update == '2026-03-09T12:00:00+00:00' - - @staticmethod - def test_update_config_does_not_emit_settings_changed() -> None: - """update_config must NOT emit settings_changed to avoid circular reinit.""" - window = _make_window() - signal_spy = MagicMock() - window.settings_changed.connect(signal_spy) - - window.update_config(_make_config(update_channel='dev')) - - signal_spy.assert_not_called() +class TestStoreConfig: + """Verify that store.config reflects updates without a manual refresh.""" @staticmethod - def test_sync_after_update_config_uses_new_timestamp() -> None: - """sync_from_config after update_config should display the refreshed timestamp.""" + def test_sync_after_store_update_uses_new_timestamp() -> None: + """sync_from_config after a store update should display the refreshed timestamp.""" window = _make_window(_make_config(last_client_update=None)) assert not window._last_client_update_label.text() new_config = _make_config(last_client_update='2026-03-09T12:00:00+00:00') - window.update_config(new_config) + window._store.set(new_config) window.sync_from_config() assert 'Last updated:' in window._last_client_update_label.text() diff --git a/tests/unit/qt/test_tray_window_show.py b/tests/unit/qt/test_tray_window_show.py index 3c88162..e6b1307 100644 --- a/tests/unit/qt/test_tray_window_show.py +++ b/tests/unit/qt/test_tray_window_show.py @@ -6,15 +6,34 @@ import pytest -from synodic_client.application.schema import ToolUpdateResult +from synodic_client.application.config_store import ConfigStore +from synodic_client.application.schema import ToolUpdateResult, UpdateTarget from synodic_client.application.screen.tray import TrayScreen +from synodic_client.resolution import ResolvedConfig +from synodic_client.schema import DEFAULT_AUTO_UPDATE_INTERVAL_MINUTES, DEFAULT_TOOL_UPDATE_INTERVAL_MINUTES + + +def _make_config() -> ResolvedConfig: + return ResolvedConfig( + update_source=None, + update_channel='stable', + auto_update_interval_minutes=DEFAULT_AUTO_UPDATE_INTERVAL_MINUTES, + tool_update_interval_minutes=DEFAULT_TOOL_UPDATE_INTERVAL_MINUTES, + plugin_auto_update=None, + detect_updates=True, + prerelease_packages=None, + auto_apply=True, + auto_start=True, + debug_logging=False, + last_client_update=None, + last_tool_updates=None, + ) @pytest.fixture def tray_screen(): """Build a minimal ``TrayScreen`` with mocked collaborators.""" with ( - patch('synodic_client.application.screen.tray.resolve_config'), patch('synodic_client.application.screen.tool_update_controller.resolve_update_config') as mock_ucfg, patch('synodic_client.application.screen.tray.UpdateController'), ): @@ -27,9 +46,9 @@ def tray_screen(): app = MagicMock() client = MagicMock() window = MagicMock() - # SettingsWindow expects a ResolvedConfig – pass a mock + store = ConfigStore(_make_config()) with patch('synodic_client.application.screen.tray.SettingsWindow'): - ts = TrayScreen(app, client, window) + ts = TrayScreen(app, client, window, store=store) return ts @@ -48,7 +67,7 @@ def test_auto_update_does_not_show_window(tray_screen) -> None: def test_manual_plugin_update_shows_window(tray_screen) -> None: """A user-initiated single-plugin update should show the window.""" result = ToolUpdateResult(manifests_processed=1, updated=1) - tray_screen._tool_orchestrator._on_tool_update_finished(result, updating_plugin='pipx', manual=True) + tray_screen._tool_orchestrator._on_tool_update_finished(result, UpdateTarget(plugin='pipx')) tray_screen._window.show.assert_called_once() @staticmethod @@ -57,8 +76,7 @@ def test_manual_package_update_shows_window(tray_screen) -> None: result = ToolUpdateResult(manifests_processed=1, updated=1) tray_screen._tool_orchestrator._on_tool_update_finished( result, - updating_package=('pipx', 'ruff'), - manual=True, + UpdateTarget(plugin='pipx', package='ruff'), ) tray_screen._window.show.assert_called_once() diff --git a/tests/unit/qt/test_update_controller.py b/tests/unit/qt/test_update_controller.py index 1904b77..51a3ffa 100644 --- a/tests/unit/qt/test_update_controller.py +++ b/tests/unit/qt/test_update_controller.py @@ -7,6 +7,7 @@ from packaging.version import Version +from synodic_client.application.config_store import ConfigStore from synodic_client.application.screen.update_banner import UpdateBanner from synodic_client.application.theme import ( UPDATE_STATUS_AVAILABLE_STYLE, @@ -66,6 +67,7 @@ def _make_controller( client.updater = MagicMock() banner = UpdateBanner() settings = MagicMock() + store = ConfigStore(config) with patch('synodic_client.application.update_controller.resolve_update_config') as mock_ucfg: mock_ucfg.return_value = MagicMock( @@ -76,7 +78,7 @@ def _make_controller( client, [banner], settings_window=settings, - config=config, + store=store, ) controller.set_user_active_predicate(lambda: is_user_active) @@ -183,7 +185,8 @@ def test_auto_apply_does_not_show_ready_banner() -> None: def test_no_auto_apply_shows_ready_banner() -> None: """When auto_apply=False, a successful download should show the ready banner.""" ctrl, app, client, banner, settings = _make_controller(auto_apply=False) - ctrl._on_download_finished(True, '2.0.0') + with patch.object(ctrl._store, 'update'): + ctrl._on_download_finished(True, '2.0.0') assert banner.state.name == 'READY' @@ -191,7 +194,8 @@ def test_no_auto_apply_shows_ready_banner() -> None: def test_no_auto_apply_sets_ready_status() -> None: """When auto_apply=False, status should show 'v2.0.0 ready' in green.""" ctrl, app, client, banner, settings = _make_controller(auto_apply=False) - ctrl._on_download_finished(True, '2.0.0') + with patch.object(ctrl._store, 'update'): + ctrl._on_download_finished(True, '2.0.0') settings.set_update_status.assert_called_with( 'v2.0.0 ready', @@ -202,7 +206,8 @@ def test_no_auto_apply_sets_ready_status() -> None: def test_no_auto_apply_shows_restart_button() -> None: """When auto_apply=False, the restart button should be shown in settings.""" ctrl, app, client, banner, settings = _make_controller(auto_apply=False) - ctrl._on_download_finished(True, '2.0.0') + with patch.object(ctrl._store, 'update'): + ctrl._on_download_finished(True, '2.0.0') settings.show_restart_button.assert_called_once() @@ -348,12 +353,12 @@ def test_restart_requested_signal_triggers_apply() -> None: # --------------------------------------------------------------------------- -class TestSettingsChanged: - """Verify on_settings_changed triggers reinit and immediate check.""" +class TestConfigChanged: + """Verify _on_config_changed triggers reinit and immediate check.""" @staticmethod - def test_settings_changed_triggers_reinit_and_check() -> None: - """Changing settings should reinitialise the updater and check.""" + def test_config_changed_triggers_reinit_and_check() -> None: + """Changing config should reinitialise the updater and check.""" ctrl, app, client, banner, settings = _make_controller() new_config = _make_config(update_channel='dev') @@ -362,14 +367,14 @@ def test_settings_changed_triggers_reinit_and_check() -> None: patch.object(ctrl, '_reinitialize_updater') as mock_reinit, patch.object(ctrl, 'check_now') as mock_check, ): - ctrl.on_settings_changed(new_config) + ctrl._on_config_changed(new_config) mock_reinit.assert_called_once_with(new_config) mock_check.assert_called_once_with(silent=True) @staticmethod - def test_settings_changed_updates_auto_apply() -> None: - """Changing settings should update the auto_apply flag.""" + def test_config_changed_updates_auto_apply() -> None: + """Changing config should update the auto_apply flag.""" ctrl, app, client, banner, settings = _make_controller(auto_apply=True) new_config = _make_config(auto_apply=False) @@ -378,7 +383,7 @@ def test_settings_changed_updates_auto_apply() -> None: patch.object(ctrl, '_reinitialize_updater'), patch.object(ctrl, 'check_now'), ): - ctrl.on_settings_changed(new_config) + ctrl._on_config_changed(new_config) assert ctrl._auto_apply is False @@ -422,49 +427,39 @@ def test_check_error_no_banner_when_silent() -> None: class TestPersistCheckTimestamp: - """Verify _persist_check_timestamp syncs the settings config.""" + """Verify _persist_check_timestamp syncs the settings config via the store.""" @staticmethod - def test_persist_updates_settings_config() -> None: - """_persist_check_timestamp should call update_config on the settings window.""" + def test_persist_updates_store() -> None: + """_persist_check_timestamp should call store.update with a timestamp.""" ctrl, _app, _client, _banner, settings = _make_controller() fake_resolved = _make_config(last_client_update='2026-03-09T00:00:00+00:00') - with patch( - 'synodic_client.application.update_controller.update_user_config', - return_value=fake_resolved, - ): + with patch.object(ctrl._store, 'update', return_value=fake_resolved) as mock_update: ctrl._persist_check_timestamp() + mock_update.assert_called_once() - settings.update_config.assert_called_once_with(fake_resolved) settings.set_last_checked.assert_called_once() @staticmethod - def test_on_check_finished_success_syncs_config() -> None: - """A successful check should persist timestamp AND sync settings config.""" + def test_on_check_finished_success_syncs_via_store() -> None: + """A successful check should persist timestamp via the store.""" ctrl, _app, _client, _banner, settings = _make_controller() result = UpdateInfo(available=False, current_version=Version('1.0.0')) fake_resolved = _make_config(last_client_update='2026-03-09T00:00:00+00:00') - with patch( - 'synodic_client.application.update_controller.update_user_config', - return_value=fake_resolved, - ): + with patch.object(ctrl._store, 'update', return_value=fake_resolved) as mock_update: ctrl._on_check_finished(result, silent=True) - - settings.update_config.assert_called_once_with(fake_resolved) + mock_update.assert_called_once() @staticmethod - def test_download_finished_syncs_config_and_label() -> None: - """_on_download_finished should sync config and update the label.""" + def test_download_finished_syncs_via_store() -> None: + """_on_download_finished should sync via the store and update the label.""" ctrl, _app, _client, _banner, settings = _make_controller(auto_apply=False) fake_resolved = _make_config(last_client_update='2026-03-09T00:00:00+00:00') - with patch( - 'synodic_client.application.update_controller.update_user_config', - return_value=fake_resolved, - ): + with patch.object(ctrl._store, 'update', return_value=fake_resolved) as mock_update: ctrl._on_download_finished(True, '2.0.0') + mock_update.assert_called_once() - settings.update_config.assert_called_once_with(fake_resolved) settings.set_last_checked.assert_called_once()