From b22c4b01c978999727554d0f0788a09a5c525467 Mon Sep 17 00:00:00 2001 From: Asher Norland Date: Wed, 18 Mar 2026 13:22:56 -0700 Subject: [PATCH 1/5] format --- synodic_client/application/debug.py | 28 ++++++++++++++------------- tests/unit/operations/test_install.py | 24 +++++++++++++---------- 2 files changed, 29 insertions(+), 23 deletions(-) diff --git a/synodic_client/application/debug.py b/synodic_client/application/debug.py index 83f0f55..946d39d 100644 --- a/synodic_client/application/debug.py +++ b/synodic_client/application/debug.py @@ -229,19 +229,21 @@ def _handle_project_status(self, arg: str | None) -> str: pending = sum(1 for s in model.action_states if classify_status(s.status) == 'pending') upgradable = sum(1 for s in model.action_states if s.status == 'Update available') - return json.dumps({ - 'path': str(target), - 'phase': model.phase.name, - 'action_count': len(model.action_states), - 'checked_count': model.checked_count, - 'actions': actions, - 'summary': { - 'needed': needed, - 'satisfied': satisfied, - 'pending': pending, - 'upgradable': upgradable, - }, - }) + return json.dumps( + { + 'path': str(target), + 'phase': model.phase.name, + 'action_count': len(model.action_states), + 'checked_count': model.checked_count, + 'actions': actions, + 'summary': { + 'needed': needed, + 'satisfied': satisfied, + 'pending': pending, + 'upgradable': upgradable, + }, + } + ) def _handle_select_project(self, arg: str | None) -> str: """Select a project in the sidebar.""" diff --git a/tests/unit/operations/test_install.py b/tests/unit/operations/test_install.py index 0cb771b..342739a 100644 --- a/tests/unit/operations/test_install.py +++ b/tests/unit/operations/test_install.py @@ -231,11 +231,13 @@ def test_exclude_post_sync(tmp_path: Path) -> None: """exclude_post_sync=True strips post_sync from manifest before execution.""" manifest = tmp_path / 'porringer.json' manifest.write_text( - json.dumps({ - 'version': '1', - 'actions': [{'description': 'install something'}], - 'post_sync': [{'command': 'echo hello'}], - }), + json.dumps( + { + 'version': '1', + 'actions': [{'description': 'install something'}], + 'post_sync': [{'command': 'echo hello'}], + } + ), encoding='utf-8', ) @@ -315,11 +317,13 @@ def test_yields_events_from_post_sync_manifest(tmp_path: Path) -> None: manifest = tmp_path / 'porringer.json' manifest.write_text( - json.dumps({ - 'version': '1', - 'actions': [{'description': 'install something'}], - 'post_sync': [{'command': 'echo hello'}], - }), + json.dumps( + { + 'version': '1', + 'actions': [{'description': 'install something'}], + 'post_sync': [{'command': 'echo hello'}], + } + ), encoding='utf-8', ) From 78319c8c11025dbd8e5dfffccd089de5d1ac3bcc Mon Sep 17 00:00:00 2001 From: Asher Norland Date: Wed, 18 Mar 2026 14:51:01 -0700 Subject: [PATCH 2/5] ye --- synodic_client/application/debug.py | 28 +++--- .../application/screen/plugin_row.py | 74 ++++++++++----- synodic_client/application/screen/screen.py | 35 ++++++++ .../screen/tool_update_controller.py | 37 ++++++++ synodic_client/application/theme.py | 3 + synodic_client/operations/tool.py | 87 +++++++++++++++--- tests/unit/operations/test_install.py | 24 +++-- tests/unit/qt/test_update_feedback.py | 89 ++++++++++++++++--- tests/unit/test_workers.py | 70 +++++++++++++++ 9 files changed, 375 insertions(+), 72 deletions(-) diff --git a/synodic_client/application/debug.py b/synodic_client/application/debug.py index 946d39d..83f0f55 100644 --- a/synodic_client/application/debug.py +++ b/synodic_client/application/debug.py @@ -229,21 +229,19 @@ def _handle_project_status(self, arg: str | None) -> str: pending = sum(1 for s in model.action_states if classify_status(s.status) == 'pending') upgradable = sum(1 for s in model.action_states if s.status == 'Update available') - return json.dumps( - { - 'path': str(target), - 'phase': model.phase.name, - 'action_count': len(model.action_states), - 'checked_count': model.checked_count, - 'actions': actions, - 'summary': { - 'needed': needed, - 'satisfied': satisfied, - 'pending': pending, - 'upgradable': upgradable, - }, - } - ) + return json.dumps({ + 'path': str(target), + 'phase': model.phase.name, + 'action_count': len(model.action_states), + 'checked_count': model.checked_count, + 'actions': actions, + 'summary': { + 'needed': needed, + 'satisfied': satisfied, + 'pending': pending, + 'upgradable': upgradable, + }, + }) def _handle_select_project(self, arg: str | None) -> str: """Select a project in the sidebar.""" diff --git a/synodic_client/application/screen/plugin_row.py b/synodic_client/application/screen/plugin_row.py index 3027288..1eedfb6 100644 --- a/synodic_client/application/screen/plugin_row.py +++ b/synodic_client/application/screen/plugin_row.py @@ -40,6 +40,7 @@ PLUGIN_ROW_PROJECT_TAG_TRANSITIVE_STYLE, PLUGIN_ROW_REMOVE_STYLE, PLUGIN_ROW_STATUS_MIN_WIDTH, + PLUGIN_ROW_STATUS_PENDING_STYLE, PLUGIN_ROW_STATUS_STYLE, PLUGIN_ROW_STYLE, PLUGIN_ROW_TIMESTAMP_MIN_WIDTH, @@ -249,15 +250,18 @@ def _build_controls( update_btn.setToolTip('Not installed \u2014 cannot update') def set_updating(self, updating: bool) -> None: - """Toggle the button between *Updating…* and *Update* states.""" - if self._update_btn is None: - return + """Toggle between *Updating…* (with spinner) and *Update* states.""" if updating: - self._update_btn.setText('Updating\u2026') - self._update_btn.setEnabled(False) + if self._checking_spinner is not None: + self._checking_spinner.start() + if self._update_btn is not None: + self._update_btn.hide() else: - self._update_btn.setText('Update') - self._update_btn.setEnabled(True) + if self._checking_spinner is not None: + self._checking_spinner.stop() + if self._update_btn is not None: + self._update_btn.setText('Update') + self._update_btn.setEnabled(True) def set_checking(self, checking: bool) -> None: """Show or hide the inline checking spinner.""" @@ -331,7 +335,7 @@ def __init__( self._signal_key = f'{data.plugin_name}:{data.runtime_tag}' if data.runtime_tag else data.plugin_name self._update_btn: QPushButton | None = None self._remove_btn: QPushButton | None = None - self._checking_spinner: _RowSpinner | None = None + self._row_spinner: _RowSpinner | None = None self._host_label: QLabel | None = None self._project_paths: list[str] = list(data.project_paths) self._project_labels: list[str] = [p.project_label for p in data.project_instances] @@ -386,6 +390,12 @@ def _build_controls(self, layout: QHBoxLayout, data: PluginRowData) -> None: if data.show_toggle: self._build_toggle(layout, data) + # Inline spinner — always created so both checking and updating + # flows can use it regardless of whether the toggle is shown. + if self._row_spinner is None: + self._row_spinner = _RowSpinner(self) + layout.addWidget(self._row_spinner) + # Update button — always created for alignment, hidden when no update self._build_update_button(layout, data) @@ -421,7 +431,7 @@ def _build_controls(self, layout: QHBoxLayout, data: PluginRowData) -> None: self._build_remove_button(layout, data) def _build_toggle(self, layout: QHBoxLayout, data: PluginRowData) -> None: - """Add the auto-update toggle and inline checking spinner.""" + """Add the auto-update toggle button.""" toggle_btn = QPushButton('Auto') toggle_btn.setCheckable(True) toggle_btn.setChecked(data.auto_update) @@ -436,8 +446,8 @@ def _build_toggle(self, layout: QHBoxLayout, data: PluginRowData) -> None: ) layout.addWidget(toggle_btn) - self._checking_spinner = _RowSpinner(self) - layout.addWidget(self._checking_spinner) + self._row_spinner = _RowSpinner(self) + layout.addWidget(self._row_spinner) def _build_update_button(self, layout: QHBoxLayout, data: PluginRowData) -> None: """Add the per-package update button (always created, visibility toggled).""" @@ -484,26 +494,48 @@ def _build_remove_button(self, layout: QHBoxLayout, data: PluginRowData) -> None layout.addWidget(remove_btn) def set_updating(self, updating: bool) -> None: - """Toggle the button between *Updating…* and *Update* states.""" - if self._update_btn is None: - return + """Toggle the inline spinner and button between updating/idle states.""" if updating: - self._update_btn.setText('Updating\u2026') - self._update_btn.setEnabled(False) + if self._row_spinner is not None: + self._row_spinner.start() + if self._update_btn is not None: + self._update_btn.hide() + if self._update_status_label is not None: + self._update_status_label.hide() else: - self._update_btn.setText('Update') - self._update_btn.setEnabled(True) + if self._row_spinner is not None: + self._row_spinner.stop() + if self._update_btn is not None: + self._update_btn.setText('Update') + self._update_btn.setEnabled(True) def set_checking(self, checking: bool) -> None: """Show or hide the inline checking spinner.""" - if self._checking_spinner is None: + if self._row_spinner is None: return if checking: - self._checking_spinner.start() + self._row_spinner.start() if self._update_btn is not None: self._update_btn.hide() else: - self._checking_spinner.stop() + self._row_spinner.stop() + + def set_pending(self, pending: bool) -> None: + """Show or clear the *Pending* status on this row. + + When *pending* is ``True`` the update button is hidden and + ``Pending`` text is shown. When ``False`` the text is cleared + (button visibility is restored by the next badge refresh). + """ + if self._update_status_label is not None: + if pending: + self._update_status_label.setText('Pending') + self._update_status_label.setStyleSheet(PLUGIN_ROW_STATUS_PENDING_STYLE) + self._update_status_label.show() + else: + self._update_status_label.hide() + if pending and self._update_btn is not None: + self._update_btn.hide() def set_removing(self, removing: bool) -> None: """Toggle the remove button between *Removing…* and *×* states.""" diff --git a/synodic_client/application/screen/screen.py b/synodic_client/application/screen/screen.py index e325c8d..7384787 100644 --- a/synodic_client/application/screen/screen.py +++ b/synodic_client/application/screen/screen.py @@ -1382,6 +1382,41 @@ def set_plugin_error(self, plugin_name: str, message: str) -> None: widget.set_error(message) break + # -- Per-package progress helpers for plugin-level updates -- + + def get_plugin_update_packages(self, signal_key: str) -> set[str]: + """Return the set of package names with known updates for *signal_key*.""" + return set(self._get_plugin_updates(signal_key).keys()) + + def set_packages_pending(self, signal_key: str, package_names: set[str]) -> None: + """Mark matching package rows as *Pending* for a plugin-level update.""" + for widget in self._section_widgets: + if ( + isinstance(widget, PluginRow) + and widget._signal_key == signal_key + and widget._package_name in package_names + ): + widget.set_pending(True) + + def set_package_active(self, signal_key: str, package_name: str) -> None: + """Transition a package row from *Pending* to *Updating* (spinner).""" + for widget in self._section_widgets: + if ( + isinstance(widget, PluginRow) + and widget._signal_key == signal_key + and widget._package_name == package_name + ): + widget.set_pending(False) + widget.set_updating(True) + break + + def clear_plugin_row_states(self, signal_key: str) -> None: + """Reset pending / updating state on all package rows for *signal_key*.""" + for widget in self._section_widgets: + if isinstance(widget, PluginRow) and widget._signal_key == signal_key: + widget.set_pending(False) + widget.set_updating(False) + class MainWindow(QMainWindow): """Main window for the application.""" diff --git a/synodic_client/application/screen/tool_update_controller.py b/synodic_client/application/screen/tool_update_controller.py index 32e0bf4..d17cdb7 100644 --- a/synodic_client/application/screen/tool_update_controller.py +++ b/synodic_client/application/screen/tool_update_controller.py @@ -152,6 +152,31 @@ 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) + # -- Per-package progress callbacks -- + + def _make_package_starting_cb(self, signal_key: str) -> Callable[[str], None]: + """Return a callback that transitions a package row to *Updating*.""" + + def _on_starting(package_name: str) -> None: + tools_view = self._window.tools_view + if tools_view is not None: + tools_view.set_package_active(signal_key, package_name) + + return _on_starting + + def _make_package_completed_cb( + self, + signal_key: str, + ) -> Callable[[str, bool, bool], None]: + """Return a callback that clears a package row's updating state.""" + + def _on_completed(package_name: str, _success: bool, _skipped: bool) -> None: + tools_view = self._window.tools_view + if tools_view is not None: + tools_view.set_package_updating(signal_key, package_name, False) + + return _on_completed + # -- ToolsView error helpers -- def _fail_plugin_update(self, plugin_name: str, error: str) -> None: @@ -159,6 +184,7 @@ def _fail_plugin_update(self, plugin_name: str, error: str) -> None: tools_view = self._window.tools_view if tools_view is not None: tools_view.set_plugin_updating(plugin_name, False) + tools_view.clear_plugin_row_states(plugin_name) tools_view.set_plugin_error(plugin_name, error) def _fail_package_update( @@ -243,6 +269,9 @@ def on_single_plugin_update(self, plugin_name: str) -> None: tools_view = self._window.tools_view if tools_view is not None: tools_view.set_plugin_updating(plugin_name, True) + pending = tools_view.get_plugin_update_packages(plugin_name) + if pending: + tools_view.set_packages_pending(plugin_name, pending) bare_plugin, runtime_tag = parse_plugin_key(plugin_name) if runtime_tag is not None: @@ -281,6 +310,8 @@ async def _async_runtime_plugin_update( runtime_tag, include_packages=include_packages, discovered=discovered, + on_package_starting=self._make_package_starting_cb(signal_key), + on_package_completed=self._make_package_completed_cb(signal_key), ) if coordinator is not None: coordinator.invalidate() @@ -302,6 +333,8 @@ async def _async_single_plugin_update(self, porringer: API, plugin_name: str) -> porringer, plugins={plugin_name}, discovered=discovered, + on_package_starting=self._make_package_starting_cb(plugin_name), + on_package_completed=self._make_package_completed_cb(plugin_name), ) if coordinator is not None: coordinator.invalidate() @@ -406,6 +439,10 @@ def _on_tool_update_finished( if result.version_map: signal_key = target.plugin if target else result.plugin tools_view.record_updates_completed(signal_key, result.version_map) + # Clear pending / updating spinners left on child rows + if target is not None and not target.package: + tools_view.set_plugin_updating(target.plugin, False) + tools_view.clear_plugin_row_states(target.plugin) tools_view.invalidate_update_data() if self._window.isVisible() and target is None: tools_view.refresh() diff --git a/synodic_client/application/theme.py b/synodic_client/application/theme.py index fe9aad5..24cac22 100644 --- a/synodic_client/application/theme.py +++ b/synodic_client/application/theme.py @@ -217,6 +217,9 @@ PLUGIN_ROW_STATUS_STYLE = 'font-size: 10px; color: #808080;' """Muted inline status text shown after an auto-update check (e.g. 'Up to date').""" +PLUGIN_ROW_STATUS_PENDING_STYLE = 'font-size: 10px; color: #569cd6;' +"""Blue status text for 'Pending' during a plugin-level update.""" + PLUGIN_ROW_STATUS_UP_TO_DATE_STYLE = 'font-size: 10px; color: #89d185;' """Green status text for 'Up to date'.""" diff --git a/synodic_client/operations/tool.py b/synodic_client/operations/tool.py index 60db79d..3179a84 100644 --- a/synodic_client/operations/tool.py +++ b/synodic_client/operations/tool.py @@ -9,12 +9,14 @@ import asyncio import logging +from collections.abc import Callable from pathlib import Path from typing import TYPE_CHECKING from porringer.core.schema import PackageRef from porringer.schema import ( ActionCompletedEvent, + ActionStartedEvent, SetupParameters, SkipReason, SyncStrategy, @@ -151,6 +153,8 @@ async def update_tool( *, runtime_tag: str | None = None, discovered: DiscoveredPlugins | None = None, + on_package_starting: Callable[[str], None] | None = None, + on_package_completed: Callable[[str, bool, bool], None] | None = None, ) -> UpdateResult: """Upgrade a single plugin or a specific package within it. @@ -164,6 +168,10 @@ async def update_tool( package_name: Optional specific package to upgrade. runtime_tag: Optional runtime tag for per-runtime updates. discovered: Pre-discovered plugins. + on_package_starting: Called with ``(package_name)`` before each + package upgrade begins. + on_package_completed: Called with ``(package_name, success, skipped)`` + after each package upgrade finishes. Returns: An :class:`UpdateResult` summarising the operation. @@ -196,12 +204,16 @@ async def update_tool( plugin_name, runtime_tag, discovered=discovered, + on_package_starting=on_package_starting, + on_package_completed=on_package_completed, ) return await _update_plugin_via_manifests( porringer, plugin_name, discovered=discovered, + on_package_starting=on_package_starting, + on_package_completed=on_package_completed, ) @@ -210,12 +222,16 @@ async def _update_plugin_via_manifests( plugin_name: str, *, discovered: DiscoveredPlugins | None = None, + on_package_starting: Callable[[str], None] | None = None, + on_package_completed: Callable[[str, bool, bool], None] | None = None, ) -> UpdateResult: """Re-sync cached manifests scoped to a single plugin.""" result = await update_all_tools( porringer, plugins={plugin_name}, discovered=discovered, + on_package_starting=on_package_starting, + on_package_completed=on_package_completed, ) result.plugin = plugin_name return result @@ -228,8 +244,22 @@ async def update_runtime_plugin( *, include_packages: set[str] | None = None, discovered: DiscoveredPlugins | None = None, + on_package_starting: Callable[[str], None] | None = None, + on_package_completed: Callable[[str, bool, bool], None] | None = None, ) -> UpdateResult: - """Upgrade packages for a plugin scoped to a specific runtime.""" + """Upgrade packages for a plugin scoped to a specific runtime. + + Args: + porringer: The porringer API instance. + plugin_name: The installer plugin name. + runtime_tag: Runtime tag to scope the upgrade. + include_packages: Optional include-set of package names. + discovered: Pre-discovered plugins. + on_package_starting: Called with ``(package_name)`` before each + package upgrade begins. + on_package_completed: Called with ``(package_name, success, skipped)`` + after each package upgrade finishes. + """ result = UpdateResult(plugin=plugin_name) packages = await porringer.package.list_by_runtime(plugin_name, plugins=discovered) if packages is None: @@ -241,6 +271,8 @@ async def update_runtime_plugin( pkg_name = str(pkg.name) if include_packages is not None and pkg_name not in include_packages: continue + if on_package_starting is not None: + on_package_starting(pkg_name) ref = PackageRef(name=pkg_name) ar = await porringer.package.upgrade( plugin_name, @@ -250,12 +282,18 @@ async def update_runtime_plugin( ) if ar.skipped: result.already_latest.append(pkg_name) + if on_package_completed is not None: + on_package_completed(pkg_name, False, True) elif ar.success: result.packages_updated.append(pkg_name) result.updated_packages.add(pkg_name) _capture_versions(result, pkg_name, ar) + if on_package_completed is not None: + on_package_completed(pkg_name, True, False) else: result.packages_failed.append(pkg_name) + if on_package_completed is not None: + on_package_completed(pkg_name, False, False) break return result @@ -290,12 +328,39 @@ async def remove_package( return action_result.success +def _record_completed_event( + result: UpdateResult, + ar: object, + pkg_name: str, + on_package_completed: Callable[[str, bool, bool], None] | None, +) -> None: + """Record a single completed-event into *result* and fire the callback.""" + if ar.skipped: # type: ignore[union-attr] + if ar.skip_reason in {SkipReason.ALREADY_LATEST, SkipReason.ALREADY_INSTALLED}: # type: ignore[union-attr] + result.already_latest.append(pkg_name) + if on_package_completed is not None and pkg_name: + on_package_completed(pkg_name, False, True) + elif ar.success: # type: ignore[union-attr] + result.packages_updated.append(pkg_name) + if pkg_name: + result.updated_packages.add(pkg_name) + _capture_versions(result, pkg_name, ar) + if on_package_completed is not None and pkg_name: + on_package_completed(pkg_name, True, False) + else: + result.packages_failed.append(pkg_name) + if on_package_completed is not None and pkg_name: + on_package_completed(pkg_name, False, False) + + async def update_all_tools( porringer: API, plugins: set[str] | None = None, include_packages: set[str] | None = None, *, discovered: DiscoveredPlugins | None = None, + on_package_starting: Callable[[str], None] | None = None, + on_package_completed: Callable[[str, bool, bool], None] | None = None, ) -> UpdateResult: """Re-sync all cached project manifests (bulk update). @@ -304,6 +369,10 @@ async def update_all_tools( plugins: Optional include-set of plugin names. include_packages: Optional include-set of package names. discovered: Pre-discovered plugins. + on_package_starting: Called with ``(package_name)`` before each + package action begins. + on_package_completed: Called with ``(package_name, success, skipped)`` + after each package action completes. Returns: An :class:`UpdateResult` summarising the full run. @@ -328,20 +397,16 @@ async def update_all_tools( ) try: async for event in porringer.sync.execute_stream(params, plugins=discovered): + if isinstance(event, ActionStartedEvent): + pkg_name = str(event.action.package.name) if event.action.package else '' + if pkg_name and on_package_starting is not None: + on_package_starting(pkg_name) + continue if not isinstance(event, ActionCompletedEvent): continue ar = event.result pkg_name = str(ar.action.package.name) if ar.action.package else '' - if ar.skipped: - if ar.skip_reason in {SkipReason.ALREADY_LATEST, SkipReason.ALREADY_INSTALLED}: - result.already_latest.append(pkg_name) - elif ar.success: - result.packages_updated.append(pkg_name) - if pkg_name: - result.updated_packages.add(pkg_name) - _capture_versions(result, pkg_name, ar) - else: - result.packages_failed.append(pkg_name) + _record_completed_event(result, ar, pkg_name, on_package_completed) except asyncio.CancelledError: raise result.manifests_processed += 1 diff --git a/tests/unit/operations/test_install.py b/tests/unit/operations/test_install.py index 342739a..0cb771b 100644 --- a/tests/unit/operations/test_install.py +++ b/tests/unit/operations/test_install.py @@ -231,13 +231,11 @@ def test_exclude_post_sync(tmp_path: Path) -> None: """exclude_post_sync=True strips post_sync from manifest before execution.""" manifest = tmp_path / 'porringer.json' manifest.write_text( - json.dumps( - { - 'version': '1', - 'actions': [{'description': 'install something'}], - 'post_sync': [{'command': 'echo hello'}], - } - ), + json.dumps({ + 'version': '1', + 'actions': [{'description': 'install something'}], + 'post_sync': [{'command': 'echo hello'}], + }), encoding='utf-8', ) @@ -317,13 +315,11 @@ def test_yields_events_from_post_sync_manifest(tmp_path: Path) -> None: manifest = tmp_path / 'porringer.json' manifest.write_text( - json.dumps( - { - 'version': '1', - 'actions': [{'description': 'install something'}], - 'post_sync': [{'command': 'echo hello'}], - } - ), + json.dumps({ + 'version': '1', + 'actions': [{'description': 'install something'}], + 'post_sync': [{'command': 'echo hello'}], + }), encoding='utf-8', ) diff --git a/tests/unit/qt/test_update_feedback.py b/tests/unit/qt/test_update_feedback.py index 44cee0e..700253c 100644 --- a/tests/unit/qt/test_update_feedback.py +++ b/tests/unit/qt/test_update_feedback.py @@ -72,8 +72,9 @@ def test_set_updating_true_disables_button() -> None: ) header.set_updating(True) assert header._update_btn is not None - assert header._update_btn.text() == 'Updating\u2026' - assert not header._update_btn.isEnabled() + assert header._update_btn.isHidden() + assert header._checking_spinner is not None + assert not header._checking_spinner.isHidden() @staticmethod def test_set_updating_false_restores_button() -> None: @@ -89,6 +90,8 @@ def test_set_updating_false_restores_button() -> None: assert header._update_btn is not None assert header._update_btn.text() == 'Update' assert header._update_btn.isEnabled() + assert header._checking_spinner is not None + assert header._checking_spinner.isHidden() @staticmethod def test_set_updating_noop_without_controls() -> None: @@ -200,8 +203,9 @@ def test_set_updating_true_disables() -> None: ) row.set_updating(True) assert row._update_btn is not None - assert row._update_btn.text() == 'Updating\u2026' - assert not row._update_btn.isEnabled() + assert row._update_btn.isHidden() + assert row._row_spinner is not None + assert not row._row_spinner.isHidden() @staticmethod def test_set_updating_false_restores() -> None: @@ -219,6 +223,8 @@ def test_set_updating_false_restores() -> None: assert row._update_btn is not None assert row._update_btn.text() == 'Update' assert row._update_btn.isEnabled() + assert row._row_spinner is not None + assert row._row_spinner.isHidden() @staticmethod def test_update_requested_signal() -> None: @@ -257,8 +263,8 @@ def test_set_checking_shows_spinner() -> None: ) ) row.set_checking(True) - assert row._checking_spinner is not None - assert not row._checking_spinner.isHidden() + assert row._row_spinner is not None + assert not row._row_spinner.isHidden() assert row._update_btn is not None assert row._update_btn.isHidden() @@ -275,15 +281,76 @@ def test_set_checking_false_hides_spinner() -> None: ) row.set_checking(True) row.set_checking(False) - assert row._checking_spinner is not None - assert row._checking_spinner.isHidden() + assert row._row_spinner is not None + assert row._row_spinner.isHidden() @staticmethod - def test_set_checking_noop_without_toggle() -> None: - """set_checking is a no-op when show_toggle is False (no spinner created).""" + def test_spinner_exists_without_toggle() -> None: + """The row spinner is always created even without show_toggle.""" + row = PluginRow(PluginRowData(name='pdm', plugin_name='pipx')) + assert row._row_spinner is not None + + @staticmethod + def test_set_checking_works_without_toggle() -> None: + """set_checking works for rows without a toggle (spinner still created).""" row = PluginRow(PluginRowData(name='pdm', plugin_name='pipx')) row.set_checking(True) - assert row._checking_spinner is None + assert row._row_spinner is not None + assert not row._row_spinner.isHidden() + row.set_checking(False) + assert row._row_spinner.isHidden() + + @staticmethod + def test_set_pending_true_shows_status() -> None: + """set_pending(True) shows 'Pending' text and hides update button.""" + row = PluginRow( + PluginRowData( + name='pdm', + plugin_name='pipx', + show_toggle=True, + has_update=True, + ) + ) + row.set_pending(True) + assert row._update_status_label is not None + assert row._update_status_label.text() == 'Pending' + assert not row._update_status_label.isHidden() + assert row._update_btn is not None + assert row._update_btn.isHidden() + + @staticmethod + def test_set_pending_false_clears_status() -> None: + """set_pending(False) hides the status label.""" + row = PluginRow( + PluginRowData( + name='pdm', + plugin_name='pipx', + show_toggle=True, + has_update=True, + ) + ) + row.set_pending(True) + row.set_pending(False) + assert row._update_status_label is not None + assert row._update_status_label.isHidden() + + @staticmethod + def test_set_updating_clears_pending() -> None: + """set_updating(True) hides the pending status label.""" + row = PluginRow( + PluginRowData( + name='pdm', + plugin_name='pipx', + show_toggle=True, + has_update=True, + ) + ) + row.set_pending(True) + row.set_updating(True) + assert row._update_status_label is not None + assert row._update_status_label.isHidden() + assert row._row_spinner is not None + assert not row._row_spinner.isHidden() @staticmethod def test_host_tool_label_shown_when_set() -> None: diff --git a/tests/unit/test_workers.py b/tests/unit/test_workers.py index 74c43d4..54a79aa 100644 --- a/tests/unit/test_workers.py +++ b/tests/unit/test_workers.py @@ -159,3 +159,73 @@ def test_include_packages_filters() -> None: assert result.updated == 1 assert result.updated_packages == {'ruff'} porringer.package.upgrade.assert_called_once() + + @staticmethod + def test_callbacks_fire_in_order() -> None: + """on_package_starting and on_package_completed fire per package.""" + porringer = MagicMock() + porringer.package.list_by_runtime = AsyncMock( + return_value=[ + RuntimePackageResult( + provider='pim', + tag='3.12', + executable=Path('/usr/bin/python3.12'), + packages=[ + Package(name='pdm', version='2.22.0'), + Package(name='ruff', version='0.1.0'), + ], + ), + ], + ) + porringer.package.upgrade = AsyncMock( + return_value=SetupActionResult( + action=SetupAction(description='upgrade'), + success=True, + ), + ) + started: list[str] = [] + completed: list[tuple[str, bool, bool]] = [] + result = asyncio.run( + update_runtime_plugin( + porringer, + 'pipx', + '3.12', + on_package_starting=started.append, + on_package_completed=lambda name, ok, skip: completed.append((name, ok, skip)), + ), + ) + assert result.updated == _EXPECTED_RUNTIME_UPGRADES + assert started == ['pdm', 'ruff'] + assert completed == [('pdm', True, False), ('ruff', True, False)] + + @staticmethod + def test_callbacks_skipped_packages() -> None: + """on_package_completed reports skipped=True for already-latest packages.""" + porringer = MagicMock() + porringer.package.list_by_runtime = AsyncMock( + return_value=[ + RuntimePackageResult( + provider='pim', + tag='3.12', + executable=Path('/usr/bin/python3.12'), + packages=[Package(name='pdm', version='2.22.0')], + ), + ], + ) + porringer.package.upgrade = AsyncMock( + return_value=SetupActionResult( + action=SetupAction(description='upgrade'), + success=False, + skipped=True, + ), + ) + completed: list[tuple[str, bool, bool]] = [] + asyncio.run( + update_runtime_plugin( + porringer, + 'pipx', + '3.12', + on_package_completed=lambda name, ok, skip: completed.append((name, ok, skip)), + ), + ) + assert completed == [('pdm', False, True)] From ff1b745b303d3148c58978689c06985109da3ddb Mon Sep 17 00:00:00 2001 From: Asher Norland Date: Wed, 18 Mar 2026 15:57:27 -0700 Subject: [PATCH 3/5] cleanup --- .../application/screen/plugin_row.py | 112 +++++++++++------- synodic_client/application/screen/screen.py | 9 +- .../screen/tool_update_controller.py | 84 ++++++------- synodic_client/operations/tool.py | 30 ++--- tests/unit/operations/test_tool.py | 3 + tests/unit/test_workers.py | 3 +- 6 files changed, 119 insertions(+), 122 deletions(-) diff --git a/synodic_client/application/screen/plugin_row.py b/synodic_client/application/screen/plugin_row.py index 1eedfb6..f743295 100644 --- a/synodic_client/application/screen/plugin_row.py +++ b/synodic_client/application/screen/plugin_row.py @@ -7,6 +7,8 @@ from __future__ import annotations +from enum import Enum + from porringer.schema import PluginInfo from porringer.schema.plugin import PluginCapability, PluginKind from PySide6.QtCore import Qt, QTimer, Signal @@ -60,6 +62,16 @@ PROJECT_CHILD_VERSION_STYLE, ) + +class RowPhase(Enum): + """Mutually exclusive visual state for a :class:`PluginRow`.""" + + IDLE = 'idle' + CHECKING = 'checking' + PENDING = 'pending' + UPDATING = 'updating' + + # Row-spinner dimensions _ROW_SPINNER_SIZE = 12 _ROW_SPINNER_PEN = 2 @@ -390,11 +402,10 @@ def _build_controls(self, layout: QHBoxLayout, data: PluginRowData) -> None: if data.show_toggle: self._build_toggle(layout, data) - # Inline spinner — always created so both checking and updating - # flows can use it regardless of whether the toggle is shown. - if self._row_spinner is None: - self._row_spinner = _RowSpinner(self) - layout.addWidget(self._row_spinner) + # Inline spinner — always created so checking, pending, and + # updating flows can use it regardless of whether the toggle is shown. + self._row_spinner = _RowSpinner(self) + layout.addWidget(self._row_spinner) # Update button — always created for alignment, hidden when no update self._build_update_button(layout, data) @@ -446,9 +457,6 @@ def _build_toggle(self, layout: QHBoxLayout, data: PluginRowData) -> None: ) layout.addWidget(toggle_btn) - self._row_spinner = _RowSpinner(self) - layout.addWidget(self._row_spinner) - def _build_update_button(self, layout: QHBoxLayout, data: PluginRowData) -> None: """Add the per-package update button (always created, visibility toggled).""" update_btn = QPushButton('Update') @@ -493,50 +501,66 @@ def _build_remove_button(self, layout: QHBoxLayout, data: PluginRowData) -> None self._remove_btn = remove_btn layout.addWidget(remove_btn) - def set_updating(self, updating: bool) -> None: - """Toggle the inline spinner and button between updating/idle states.""" - if updating: - if self._row_spinner is not None: - self._row_spinner.start() - if self._update_btn is not None: - self._update_btn.hide() - if self._update_status_label is not None: - self._update_status_label.hide() - else: - if self._row_spinner is not None: - self._row_spinner.stop() - if self._update_btn is not None: - self._update_btn.setText('Update') - self._update_btn.setEnabled(True) + def set_phase(self, phase: RowPhase) -> None: + """Transition the row to a mutually exclusive visual *phase*. - def set_checking(self, checking: bool) -> None: - """Show or hide the inline checking spinner.""" - if self._row_spinner is None: - return - if checking: - self._row_spinner.start() - if self._update_btn is not None: - self._update_btn.hide() + * ``IDLE`` — spinner stopped, status hidden, button restored. + * ``CHECKING`` — spinner running, button hidden. + * ``PENDING`` — spinner stopped, *Pending* text shown, button hidden. + * ``UPDATING`` — spinner running, status hidden, button hidden. + """ + self._apply_phase_reset() + + if phase == RowPhase.IDLE: + self._apply_phase_idle() + elif phase == RowPhase.PENDING: + self._apply_phase_pending() else: + # CHECKING and UPDATING both start spinner and hide update btn + self._apply_phase_spinner() + + def _apply_phase_reset(self) -> None: + """Stop spinner and hide status — common entry for every transition.""" + if self._row_spinner is not None: self._row_spinner.stop() + if self._update_status_label is not None: + self._update_status_label.hide() - def set_pending(self, pending: bool) -> None: - """Show or clear the *Pending* status on this row. + def _apply_phase_idle(self) -> None: + """Restore the update button to its default label and state.""" + if self._update_btn is not None: + self._update_btn.setText('Update') + self._update_btn.setEnabled(True) - When *pending* is ``True`` the update button is hidden and - ``Pending`` text is shown. When ``False`` the text is cleared - (button visibility is restored by the next badge refresh). - """ + def _apply_phase_pending(self) -> None: + """Show *Pending* text and hide the update button.""" if self._update_status_label is not None: - if pending: - self._update_status_label.setText('Pending') - self._update_status_label.setStyleSheet(PLUGIN_ROW_STATUS_PENDING_STYLE) - self._update_status_label.show() - else: - self._update_status_label.hide() - if pending and self._update_btn is not None: + self._update_status_label.setText('Pending') + self._update_status_label.setStyleSheet(PLUGIN_ROW_STATUS_PENDING_STYLE) + self._update_status_label.show() + if self._update_btn is not None: self._update_btn.hide() + def _apply_phase_spinner(self) -> None: + """Start the spinner and hide the update button.""" + if self._row_spinner is not None: + self._row_spinner.start() + if self._update_btn is not None: + self._update_btn.hide() + + # Convenience aliases for backward-compatible call sites + def set_updating(self, updating: bool) -> None: + """Toggle between updating and idle states.""" + self.set_phase(RowPhase.UPDATING if updating else RowPhase.IDLE) + + def set_checking(self, checking: bool) -> None: + """Toggle between checking and idle states.""" + self.set_phase(RowPhase.CHECKING if checking else RowPhase.IDLE) + + def set_pending(self, pending: bool) -> None: + """Toggle between pending and idle states.""" + self.set_phase(RowPhase.PENDING if pending else RowPhase.IDLE) + def set_removing(self, removing: bool) -> None: """Toggle the remove button between *Removing…* and *×* states.""" if self._remove_btn is None: diff --git a/synodic_client/application/screen/screen.py b/synodic_client/application/screen/screen.py index 7384787..62537a6 100644 --- a/synodic_client/application/screen/screen.py +++ b/synodic_client/application/screen/screen.py @@ -38,6 +38,7 @@ PluginKindHeader, PluginProviderHeader, PluginRow, + RowPhase, ) from synodic_client.application.screen.projects import ProjectsView from synodic_client.application.screen.schema import ( @@ -1396,7 +1397,7 @@ def set_packages_pending(self, signal_key: str, package_names: set[str]) -> None and widget._signal_key == signal_key and widget._package_name in package_names ): - widget.set_pending(True) + widget.set_phase(RowPhase.PENDING) def set_package_active(self, signal_key: str, package_name: str) -> None: """Transition a package row from *Pending* to *Updating* (spinner).""" @@ -1406,16 +1407,14 @@ def set_package_active(self, signal_key: str, package_name: str) -> None: and widget._signal_key == signal_key and widget._package_name == package_name ): - widget.set_pending(False) - widget.set_updating(True) + widget.set_phase(RowPhase.UPDATING) break def clear_plugin_row_states(self, signal_key: str) -> None: """Reset pending / updating state on all package rows for *signal_key*.""" for widget in self._section_widgets: if isinstance(widget, PluginRow) and widget._signal_key == signal_key: - widget.set_pending(False) - widget.set_updating(False) + widget.set_phase(RowPhase.IDLE) class MainWindow(QMainWindow): diff --git a/synodic_client/application/screen/tool_update_controller.py b/synodic_client/application/screen/tool_update_controller.py index d17cdb7..2a43271 100644 --- a/synodic_client/application/screen/tool_update_controller.py +++ b/synodic_client/application/screen/tool_update_controller.py @@ -22,11 +22,11 @@ from synodic_client.application.screen.screen import MainWindow, ToolsView from synodic_client.operations.schema import UpdateResult from synodic_client.operations.tool import ( + log_update_result, parse_plugin_key, remove_package, resolve_auto_update_scope, update_all_tools, - update_runtime_plugin, update_tool, ) from synodic_client.resolution import ( @@ -274,40 +274,49 @@ def on_single_plugin_update(self, plugin_name: str) -> None: tools_view.set_packages_pending(plugin_name, pending) bare_plugin, runtime_tag = parse_plugin_key(plugin_name) - if runtime_tag is not None: - self._set_task( - self._async_runtime_plugin_update(porringer, plugin_name, bare_plugin, runtime_tag), - ) - else: - self._set_task( - self._async_single_plugin_update(porringer, plugin_name), - ) + include_packages = self._resolve_include_packages(plugin_name, bare_plugin, runtime_tag) + self._set_task( + self._async_plugin_update(porringer, plugin_name, bare_plugin, runtime_tag, include_packages), + ) - async def _async_runtime_plugin_update( + def _resolve_include_packages( + self, + signal_key: str, + bare_plugin: str, + runtime_tag: str | None, + ) -> set[str] | None: + """Derive the include-set of package names for a plugin update. + + Only runtime-scoped updates consult the per-package auto-update + config; manifest-scoped updates include everything. + """ + if runtime_tag is None: + return None + mapping = self._store.config.plugin_auto_update or {} + pkg_entry = mapping.get(signal_key) or mapping.get(bare_plugin) + if isinstance(pkg_entry, dict): + enabled_pkgs = {name for name, enabled in pkg_entry.items() if enabled} + if enabled_pkgs: + return enabled_pkgs + return None + + async def _async_plugin_update( self, porringer: API, signal_key: str, plugin_name: str, - runtime_tag: str, + runtime_tag: str | None, + include_packages: set[str] | None, ) -> None: - """Run a runtime-scoped plugin update and route results.""" - config = self._store.config - mapping = config.plugin_auto_update or {} - pkg_entry = mapping.get(signal_key) or mapping.get(plugin_name) + """Run a plugin update through the shared operations layer.""" coordinator = self._window.coordinator discovered = coordinator.discovered_plugins if coordinator is not None else None - include_packages: set[str] | None = None - if isinstance(pkg_entry, dict): - enabled_pkgs = {name for name, enabled in pkg_entry.items() if enabled} - if enabled_pkgs: - include_packages = enabled_pkgs - try: - result = await update_runtime_plugin( + result = await update_tool( porringer, plugin_name, - runtime_tag, + runtime_tag=runtime_tag, include_packages=include_packages, discovered=discovered, on_package_starting=self._make_package_starting_cb(signal_key), @@ -317,34 +326,11 @@ async def _async_runtime_plugin_update( coordinator.invalidate() 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') - 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.""" - coordinator = self._window.coordinator - discovered = coordinator.discovered_plugins if coordinator is not None else None - - try: - result = await update_all_tools( - porringer, - plugins={plugin_name}, - discovered=discovered, - on_package_starting=self._make_package_starting_cb(plugin_name), - on_package_completed=self._make_package_completed_cb(plugin_name), - ) - if coordinator is not None: - coordinator.invalidate() - self._on_tool_update_finished(result, UpdateTarget(plugin=plugin_name)) - except asyncio.CancelledError: - logger.debug('Single plugin update cancelled (shutdown)') + logger.debug('Plugin update cancelled (shutdown)') raise except Exception as exc: logger.exception('Tool update failed') - self._fail_plugin_update(plugin_name, f'Update failed: {exc}') + self._fail_plugin_update(signal_key, f'Update failed: {exc}') # -- Single package update -- @@ -414,8 +400,6 @@ def _on_tool_update_finished( periodic (automatic) updates. """ # Log summary + per-package version transitions (shared with CLI) - from synodic_client.operations.tool import log_update_result - log_update_result(result) # Persist timestamps for updated packages diff --git a/synodic_client/operations/tool.py b/synodic_client/operations/tool.py index 3179a84..0cfd921 100644 --- a/synodic_client/operations/tool.py +++ b/synodic_client/operations/tool.py @@ -152,6 +152,7 @@ async def update_tool( package_name: str | None = None, *, runtime_tag: str | None = None, + include_packages: set[str] | None = None, discovered: DiscoveredPlugins | None = None, on_package_starting: Callable[[str], None] | None = None, on_package_completed: Callable[[str, bool, bool], None] | None = None, @@ -167,6 +168,8 @@ async def update_tool( plugin_name: The installer plugin name. package_name: Optional specific package to upgrade. runtime_tag: Optional runtime tag for per-runtime updates. + include_packages: Optional include-set of package names + (forwarded to runtime updates). discovered: Pre-discovered plugins. on_package_starting: Called with ``(package_name)`` before each package upgrade begins. @@ -181,20 +184,15 @@ async def update_tool( if package_name is not None: # Single-package upgrade ref = PackageRef(name=package_name) + if on_package_starting is not None: + on_package_starting(package_name) action_result = await porringer.package.upgrade( plugin_name, ref, runtime_tag=runtime_tag, plugins=discovered, ) - if action_result.skipped: - result.already_latest.append(package_name) - elif action_result.success: - result.packages_updated.append(package_name) - result.updated_packages.add(package_name) - _capture_versions(result, package_name, action_result) - else: - result.packages_failed.append(package_name) + _record_completed_event(result, action_result, package_name, on_package_completed) return result # Full-plugin update: re-sync all cached manifests for this plugin. @@ -203,6 +201,7 @@ async def update_tool( porringer, plugin_name, runtime_tag, + include_packages=include_packages, discovered=discovered, on_package_starting=on_package_starting, on_package_completed=on_package_completed, @@ -280,20 +279,7 @@ async def update_runtime_plugin( runtime_tag=runtime_tag, plugins=discovered, ) - if ar.skipped: - result.already_latest.append(pkg_name) - if on_package_completed is not None: - on_package_completed(pkg_name, False, True) - elif ar.success: - result.packages_updated.append(pkg_name) - result.updated_packages.add(pkg_name) - _capture_versions(result, pkg_name, ar) - if on_package_completed is not None: - on_package_completed(pkg_name, True, False) - else: - result.packages_failed.append(pkg_name) - if on_package_completed is not None: - on_package_completed(pkg_name, False, False) + _record_completed_event(result, ar, pkg_name, on_package_completed) break return result diff --git a/tests/unit/operations/test_tool.py b/tests/unit/operations/test_tool.py index 7e8da49..5e5cd45 100644 --- a/tests/unit/operations/test_tool.py +++ b/tests/unit/operations/test_tool.py @@ -6,6 +6,8 @@ from pathlib import Path from unittest.mock import AsyncMock, MagicMock +from porringer.schema import SkipReason + from synodic_client.operations.schema import UpdateResult from synodic_client.operations.tool import ( check_tool_updates, @@ -134,6 +136,7 @@ def test_single_package_already_latest() -> None: api = MagicMock() action_result = MagicMock() action_result.skipped = True + action_result.skip_reason = SkipReason.ALREADY_LATEST api.package.upgrade = AsyncMock(return_value=action_result) result = asyncio.run(update_tool(api, 'pip', 'requests')) diff --git a/tests/unit/test_workers.py b/tests/unit/test_workers.py index 54a79aa..04bfd33 100644 --- a/tests/unit/test_workers.py +++ b/tests/unit/test_workers.py @@ -7,7 +7,7 @@ from unittest.mock import AsyncMock, MagicMock from porringer.core.schema import Package -from porringer.schema.execution import SetupAction, SetupActionResult +from porringer.schema.execution import SetupAction, SetupActionResult, SkipReason from porringer.schema.plugin import RuntimePackageResult from synodic_client.operations.schema import UpdateResult @@ -217,6 +217,7 @@ def test_callbacks_skipped_packages() -> None: action=SetupAction(description='upgrade'), success=False, skipped=True, + skip_reason=SkipReason.ALREADY_LATEST, ), ) completed: list[tuple[str, bool, bool]] = [] From 5d8aa7d14f5e584116139faaeb03febd298cd4f9 Mon Sep 17 00:00:00 2001 From: Asher Norland Date: Wed, 18 Mar 2026 17:07:35 -0700 Subject: [PATCH 4/5] More CLI/GUI Convergence --- synodic_client/application/schema.py | 33 -- .../application/screen/action_card.py | 18 +- synodic_client/application/screen/install.py | 47 ++- .../application/screen/install_workers.py | 108 ++--- synodic_client/application/screen/projects.py | 17 +- synodic_client/application/screen/schema.py | 38 +- .../screen/tool_update_controller.py | 2 +- .../application/update_controller.py | 5 +- synodic_client/application/update_model.py | 23 +- synodic_client/application/workers.py | 20 - synodic_client/cli/install.py | 69 ++- synodic_client/operations/config.py | 24 +- synodic_client/operations/install.py | 96 ++++- synodic_client/operations/schema.py | 3 - synodic_client/operations/tool.py | 15 +- synodic_client/resolution.py | 395 ++++++++---------- tests/unit/operations/test_tool.py | 5 +- tests/unit/qt/test_install_preview.py | 57 +-- tests/unit/qt/test_tray_window_show.py | 2 +- tests/unit/test_resolution.py | 14 +- 20 files changed, 488 insertions(+), 503 deletions(-) delete mode 100644 synodic_client/application/workers.py diff --git a/synodic_client/application/schema.py b/synodic_client/application/schema.py index 9c27450..8d78a38 100644 --- a/synodic_client/application/schema.py +++ b/synodic_client/application/schema.py @@ -43,36 +43,3 @@ class Snapshot: plugin_capabilities: dict[str, frozenset[PluginCapability]] = field(default_factory=dict) """Protocol capabilities reported for each discovered plugin.""" - - -@dataclass(slots=True) -class ToolUpdateResult: - """Summary of a tool-update run across cached manifests.""" - - manifests_processed: int = 0 - updated: int = 0 - already_latest: int = 0 - 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/action_card.py b/synodic_client/application/screen/action_card.py index c445101..9b6f3d0 100644 --- a/synodic_client/application/screen/action_card.py +++ b/synodic_client/application/screen/action_card.py @@ -79,6 +79,15 @@ #: display order always matches the order actions actually execute. _KIND_ORDER: dict[PluginKind | None, int] = {kind: i for i, kind in enumerate(PHASE_ORDER)} +#: Mapping of resolved status label → stylesheet for dry-run badge styling. +_STATUS_STYLES: dict[str, str] = { + 'Update available': ACTION_CARD_STATUS_UPDATE, + 'Failed': ACTION_CARD_STATUS_FAILED, + 'Pending': ACTION_CARD_STATUS_PENDING, + 'Ready': ACTION_CARD_STATUS_SATISFIED, + 'Needed': ACTION_CARD_STATUS_NEEDED, +} + def action_sort_key(action: SetupAction) -> int: """Return a sort key that groups cards by execution phase. @@ -465,15 +474,6 @@ def set_check_result(self, result: SetupActionResult, status: str) -> None: self._stop_spinner() - # Status-to-style mapping - _STATUS_STYLES: dict[str, str] = { - 'Update available': ACTION_CARD_STATUS_UPDATE, - 'Failed': ACTION_CARD_STATUS_FAILED, - 'Pending': ACTION_CARD_STATUS_PENDING, - 'Ready': ACTION_CARD_STATUS_SATISFIED, - 'Needed': ACTION_CARD_STATUS_NEEDED, - } - style = _STATUS_STYLES.get(status, ACTION_CARD_STATUS_SATISFIED) display = status diff --git a/synodic_client/application/screen/install.py b/synodic_client/application/screen/install.py index 2281678..e27ccf8 100644 --- a/synodic_client/application/screen/install.py +++ b/synodic_client/application/screen/install.py @@ -50,7 +50,6 @@ ActionState, InstallCallbacks, InstallConfig, - PreviewCallbacks, PreviewConfig, PreviewModel, PreviewPhase, @@ -499,12 +498,7 @@ async def _run_preview_task( project_directory=project_directory, prerelease_packages=prerelease_packages, ), - callbacks=PreviewCallbacks( - on_manifest_parsed=self._on_manifest_parsed, - on_plugins_queried=self._on_plugins_queried, - on_preview_ready=self._on_preview_resolved, - on_action_checked=self._on_action_checked, - ), + on_event=self._on_preview_event, plugins=self._discovered_plugins, ) self._on_preview_finished() @@ -540,6 +534,26 @@ async def _run_install_task( logger.exception('Install execution failed') self._on_install_error(str(exc)) + # --- Preview event dispatcher --- + + def _on_preview_event(self, event: object) -> None: + """Route a :data:`PreviewEvent` to the appropriate handler.""" + from synodic_client.operations.schema import ( + PreviewActionChecked, + PreviewManifestParsed, + PreviewPluginsQueried, + PreviewReady, + ) + + if isinstance(event, PreviewManifestParsed): + self._on_manifest_parsed(event.manifest, event.manifest_path, event.temp_dir) + elif isinstance(event, PreviewPluginsQueried): + self._on_plugins_queried(event.availability, event.capabilities) + elif isinstance(event, PreviewReady): + self._on_preview_resolved(event.manifest, event.manifest_path, event.temp_dir) + elif isinstance(event, PreviewActionChecked): + self._on_action_checked(event.index, event.result, event.status) + # --- Preview callbacks (wired by load()) --- def _on_manifest_parsed(self, preview: SetupResults, manifest_path: str, temp_dir_path: str) -> None: @@ -577,9 +591,6 @@ def _on_preview_ready(self, preview: SetupResults, manifest_path: str, temp_dir_ self._show_metadata(preview) - if preview.metadata: - self.metadata_ready.emit(preview) - if not preview.actions: self._card_list.clear() self._status_label.setText('No actions to perform — the manifest is empty.') @@ -618,16 +629,13 @@ def _on_preview_resolved(self, preview: SetupResults, manifest_path: str, temp_d Called after ``MANIFEST_LOADED`` — cards are already visible from the earlier ``_on_manifest_parsed`` handler. This updates - the temp-dir reference and emits metadata. + the temp-dir reference. """ if self._model.preview is None: return self._model.temp_dir = temp_dir_path - if preview.metadata: - self.metadata_ready.emit(preview) - def _on_action_checked(self, row: int, result: SetupActionResult, status: str) -> None: """Update the model and action card with a dry-run result. @@ -976,9 +984,7 @@ def _on_post_sync_finished(self, results: SetupResults) -> None: pre_skipped = len(m.install_plan.satisfied_indices) if m.install_plan else 0 # If we have stashed install results (auto-run path), use combined summary - install_results = ( - list(self._install_results.results) if hasattr(self, '_install_results') and self._install_results else None - ) + install_results = list(self._install_results.results) if self._install_results else None summary = format_install_summary( install_results=install_results, post_sync_results=m.post_sync_results, @@ -988,7 +994,12 @@ def _on_post_sync_finished(self, results: SetupResults) -> None: self._install_btn.setEnabled(False) self._run_commands_btn.setEnabled(False) self._close_btn.setEnabled(True) - self.install_finished.emit(results) + + # Only emit when coming from the auto-run path (install_finished + # was not yet emitted). On the manual "Run Commands" path the + # signal was already emitted by _on_install_finished. + if self._install_results is not None: + self.install_finished.emit(results) # --------------------------------------------------------------------------- diff --git a/synodic_client/application/screen/install_workers.py b/synodic_client/application/screen/install_workers.py index c51cd71..23e58f5 100644 --- a/synodic_client/application/screen/install_workers.py +++ b/synodic_client/application/screen/install_workers.py @@ -10,6 +10,7 @@ import asyncio import logging +from collections.abc import Callable from pathlib import Path from porringer.api import API @@ -17,9 +18,6 @@ from porringer.schema import ( ActionCompletedEvent, ActionStartedEvent, - ManifestLoadedEvent, - SetupAction, - SetupActionResult, SetupResults, SubActionProgressEvent, ) @@ -27,16 +25,13 @@ from synodic_client.application.screen.schema import ( InstallCallbacks, InstallConfig, - PreviewCallbacks, PreviewConfig, ) from synodic_client.application.uri import safe_rmtree -from synodic_client.operations.install import execute_install, execute_post_sync, preview_manifest_stream +from synodic_client.operations.install import collect_install, collect_post_sync, preview_manifest_stream from synodic_client.operations.schema import ( - PreviewActionChecked, + PreviewEvent, PreviewManifestParsed, - PreviewPluginsQueried, - PreviewReady, ) logger = logging.getLogger(__name__) @@ -58,16 +53,21 @@ async def run_install( ) -> SetupResults: """Execute setup actions via the operations layer and stream progress. - Delegates to :func:`~synodic_client.operations.install.execute_install` - and routes the tagged ``(stage, event)`` stream to GUI callbacks. + Delegates to :func:`~synodic_client.operations.install.collect_install` + and routes progress events to GUI callbacks. """ cfg = config or InstallConfig() cb = callbacks or InstallCallbacks() - actions: list[SetupAction] = [] - collected: list[SetupActionResult] = [] - manifest_result: SetupResults | None = None - async for stage, event in execute_install( + def _on_progress(stage: str, event: object) -> None: + if stage == 'action_started' and isinstance(event, ActionStartedEvent) and cb.on_action_started is not None: + cb.on_action_started(event.action) + elif stage == 'sub_progress' and isinstance(event, SubActionProgressEvent) and cb.on_sub_progress is not None: + cb.on_sub_progress(event.action, event.sub_action) + elif stage == 'action_completed' and isinstance(event, ActionCompletedEvent) and cb.on_progress is not None: + cb.on_progress(event.action, event.result) + + return await collect_install( porringer, manifest_path, project_directory=cfg.project_directory, @@ -75,27 +75,7 @@ async def run_install( prerelease_packages=cfg.prerelease_packages, discovered=plugins, exclude_post_sync=exclude_post_sync, - ): - if stage == 'manifest_loaded' and isinstance(event, ManifestLoadedEvent): - manifest_result = event.manifest - actions = list(event.manifest.actions) - - elif stage == 'action_started' and isinstance(event, ActionStartedEvent) and cb.on_action_started is not None: - cb.on_action_started(event.action) - - elif stage == 'sub_progress' and isinstance(event, SubActionProgressEvent) and cb.on_sub_progress is not None: - cb.on_sub_progress(event.action, event.sub_action) - - elif stage == 'action_completed' and isinstance(event, ActionCompletedEvent): - collected.append(event.result) - if cb.on_progress is not None: - cb.on_progress(event.action, event.result) - - return SetupResults( - actions=actions, - results=collected, - manifest_path=manifest_result.manifest_path if manifest_result else None, - metadata=manifest_result.metadata if manifest_result else None, + on_progress=_on_progress, ) @@ -114,40 +94,25 @@ async def run_post_sync( ) -> SetupResults: """Execute only the post-sync commands from a manifest. - Delegates to :func:`~synodic_client.operations.install.execute_post_sync` - and routes events to GUI callbacks. + Delegates to :func:`~synodic_client.operations.install.collect_post_sync` + and routes progress events to GUI callbacks. """ cb = callbacks or InstallCallbacks() - actions: list[SetupAction] = [] - collected: list[SetupActionResult] = [] - manifest_result: SetupResults | None = None - - async for stage, event in execute_post_sync( - porringer, - manifest_path, - project_directory=project_directory, - discovered=plugins, - ): - if stage == 'manifest_loaded' and isinstance(event, ManifestLoadedEvent): - manifest_result = event.manifest - actions = list(event.manifest.actions) - elif stage == 'action_started' and isinstance(event, ActionStartedEvent) and cb.on_action_started is not None: + def _on_progress(stage: str, event: object) -> None: + if stage == 'action_started' and isinstance(event, ActionStartedEvent) and cb.on_action_started is not None: cb.on_action_started(event.action) - elif stage == 'sub_progress' and isinstance(event, SubActionProgressEvent) and cb.on_sub_progress is not None: cb.on_sub_progress(event.action, event.sub_action) + elif stage == 'action_completed' and isinstance(event, ActionCompletedEvent) and cb.on_progress is not None: + cb.on_progress(event.action, event.result) - elif stage == 'action_completed' and isinstance(event, ActionCompletedEvent): - collected.append(event.result) - if cb.on_progress is not None: - cb.on_progress(event.action, event.result) - - return SetupResults( - actions=actions, - results=collected, - manifest_path=manifest_result.manifest_path if manifest_result else None, - metadata=manifest_result.metadata if manifest_result else None, + return await collect_post_sync( + porringer, + manifest_path, + project_directory=project_directory, + discovered=plugins, + on_progress=_on_progress, ) @@ -161,17 +126,15 @@ async def run_preview( url: str, *, config: PreviewConfig | None = None, - callbacks: PreviewCallbacks | None = None, + on_event: Callable[[PreviewEvent], object] | None = None, plugins: DiscoveredPlugins | None = None, ) -> None: """Download a manifest and perform a dry-run preview. Delegates to :func:`preview_manifest_stream` in the operations - layer, then routes each :data:`PreviewEvent` to the appropriate - callback. + layer, then yields each :data:`PreviewEvent` to *on_event*. """ logger.info('run_preview starting for: %s', url) - cb = callbacks or PreviewCallbacks() cfg = config or PreviewConfig() temp_dir: str | None = None try: @@ -184,18 +147,9 @@ async def run_preview( ): if isinstance(event, PreviewManifestParsed): temp_dir = event.temp_dir or None - if cb.on_manifest_parsed is not None: - cb.on_manifest_parsed(event.manifest, event.manifest_path, event.temp_dir) - - elif isinstance(event, PreviewPluginsQueried) and cb.on_plugins_queried is not None: - cb.on_plugins_queried(event.availability, event.capabilities) - - elif isinstance(event, PreviewReady): - if cb.on_preview_ready is not None: - cb.on_preview_ready(event.manifest, event.manifest_path, event.temp_dir) - elif isinstance(event, PreviewActionChecked) and cb.on_action_checked is not None: - cb.on_action_checked(event.index, event.result, event.status) + if on_event is not None: + on_event(event) except asyncio.CancelledError: if temp_dir: diff --git a/synodic_client/application/screen/projects.py b/synodic_client/application/screen/projects.py index fad6ef5..9700717 100644 --- a/synodic_client/application/screen/projects.py +++ b/synodic_client/application/screen/projects.py @@ -9,6 +9,7 @@ from porringer.api import API from porringer.backend.command.core.discovery import DiscoveredPlugins +from porringer.schema import DirectoryValidationResult, ManifestDirectory from PySide6.QtCore import Qt, Signal from PySide6.QtWidgets import ( QFileDialog, @@ -138,15 +139,11 @@ async def _async_refresh(self) -> None: # Convert ProjectInfo list to the same shape as validated_directories results = [] for p in projects: - result = type( - '_Result', - (), - { - 'directory': type('_Dir', (), {'path': p.path, 'name': p.name})(), - 'exists': p.exists, - 'has_manifest': p.has_manifest, - }, - )() + result = DirectoryValidationResult( + directory=ManifestDirectory(path=Path(p.path), name=p.name), + exists=p.exists, + has_manifest=p.has_manifest, + ) results.append(result) discovered = None @@ -154,7 +151,7 @@ async def _async_refresh(self) -> None: current_paths: set[Path] = set() for result in results: d = result.directory - valid = bool(result.exists and result.has_manifest is not False) + valid = bool(result.exists and result.has_manifest) path = Path(d.path) directories.append((path, d.name or '', valid)) current_paths.add(path) diff --git a/synodic_client/application/screen/schema.py b/synodic_client/application/screen/schema.py index 9a9db17..ea4de43 100644 --- a/synodic_client/application/screen/schema.py +++ b/synodic_client/application/screen/schema.py @@ -238,8 +238,6 @@ class PreviewModel: def __init__(self) -> None: """Initialise a blank preview model.""" - self._normalize = normalize_manifest_key - self.phase: PreviewPhase = PreviewPhase.IDLE self.preview: SetupResults | None = None self.manifest_path: Path | None = None @@ -295,7 +293,7 @@ def action_state_for(self, act: SetupAction) -> ActionState | None: def has_same_manifest(self, key: str) -> bool: """Return ``True`` if *key* matches the current manifest key.""" - return self.manifest_key is not None and self.manifest_key == self._normalize(key) + return self.manifest_key is not None and self.manifest_key == normalize_manifest_key(key) @dataclass(frozen=True, slots=True) @@ -322,25 +320,29 @@ class InstallCallbacks: @dataclass(frozen=True, slots=True) -class PreviewCallbacks: - """Callbacks for :func:`run_preview` progress reporting.""" +class PreviewConfig: + """Optional execution parameters for :func:`run_preview`.""" - on_manifest_parsed: Callable[[SetupResults, str, str], None] | None = None - """``(SetupResults, manifest_path, temp_dir)`` — after JSON load.""" + project_directory: Path | None = None + prerelease_packages: set[str] | None = None - on_plugins_queried: Callable[[dict[str, bool], dict[str, frozenset[PluginCapability]]], None] | None = None - """``(dict[str, bool], dict[str, frozenset[PluginCapability]])`` — plugin → installed + capabilities mappings.""" - on_preview_ready: Callable[[SetupResults, str, str], None] | None = None - """``(SetupResults, manifest_path, temp_dir)`` — CLI commands resolved.""" +@dataclass(frozen=True, slots=True) +class UpdateTarget: + """Identifies the scope of a manual tool update. - on_action_checked: Callable[[int, SetupActionResult, str], None] | None = None - """``(row_index, SetupActionResult, status)`` — per-action dry-run result with resolved status.""" + 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"``). + """ -@dataclass(frozen=True, slots=True) -class PreviewConfig: - """Optional execution parameters for :func:`run_preview`.""" + plugin: str + """Signal key for the plugin (may be composite ``"name:tag"``).""" - project_directory: Path | None = None - prerelease_packages: set[str] | None = None + package: str = '' + """Package name, or empty when the whole plugin was updated.""" diff --git a/synodic_client/application/screen/tool_update_controller.py b/synodic_client/application/screen/tool_update_controller.py index 2a43271..8b10f0b 100644 --- a/synodic_client/application/screen/tool_update_controller.py +++ b/synodic_client/application/screen/tool_update_controller.py @@ -18,7 +18,7 @@ from PySide6.QtCore import QTimer from PySide6.QtWidgets import QSystemTrayIcon -from synodic_client.application.schema import UpdateTarget +from synodic_client.application.screen.schema import UpdateTarget from synodic_client.application.screen.screen import MainWindow, ToolsView from synodic_client.operations.schema import UpdateResult from synodic_client.operations.tool import ( diff --git a/synodic_client/application/update_controller.py b/synodic_client/application/update_controller.py index b425204..cd259fd 100644 --- a/synodic_client/application/update_controller.py +++ b/synodic_client/application/update_controller.py @@ -30,7 +30,7 @@ ) from synodic_client.application.update_model import UpdateModel from synodic_client.operations.schema import UpdateCheckResult -from synodic_client.operations.update import check_self_update, download_self_update +from synodic_client.operations.update import apply_self_update, check_self_update, download_self_update from synodic_client.resolution import ( ResolvedConfig, resolve_update_config, @@ -288,6 +288,7 @@ def _do_check(self, *, silent: bool) -> None: self._model.set_check_button_enabled(False) if self._pending_version is None: self._model.set_restart_visible(False) + self._model.set_checking() self._model.set_status('Checking\u2026', UPDATE_STATUS_CHECKING_STYLE) self._set_task(self._async_check(silent=silent)) @@ -436,7 +437,7 @@ def _apply_update(self, *, silent: bool = False) -> None: # the next launch. sync_startup(sys.executable, auto_start=self._store.config.auto_start) - self._client.apply_update_on_exit(restart=True, silent=silent) + apply_self_update(self._client, restart=True, silent=silent) self._pending_version = None logger.info('Update scheduled — restarting application') self._app.quit() diff --git a/synodic_client/application/update_model.py b/synodic_client/application/update_model.py index 386901a..ed87287 100644 --- a/synodic_client/application/update_model.py +++ b/synodic_client/application/update_model.py @@ -75,28 +75,35 @@ def error_message(self) -> str: # --- Lifecycle transitions (controller writes) --- + def _transition(self, phase: UpdatePhase) -> None: + """Common transition logic — clears stale error state and emits.""" + if phase != UpdatePhase.ERROR: + self._error_message = '' + self._phase = phase + self.phase_changed.emit(self._phase) + + def set_checking(self) -> None: + """Enter the *CHECKING* phase.""" + self._transition(UpdatePhase.CHECKING) + def set_downloading(self, version: str) -> None: """Enter the *DOWNLOADING* phase for *version*.""" self._version = version - self._phase = UpdatePhase.DOWNLOADING - self.phase_changed.emit(self._phase) + self._transition(UpdatePhase.DOWNLOADING) def set_ready(self, version: str) -> None: """Enter the *READY* phase for *version*.""" self._version = version - self._phase = UpdatePhase.READY - self.phase_changed.emit(self._phase) + self._transition(UpdatePhase.READY) def set_error(self, message: str) -> None: """Enter the *ERROR* phase with *message*.""" self._error_message = message - self._phase = UpdatePhase.ERROR - self.phase_changed.emit(self._phase) + self._transition(UpdatePhase.ERROR) def set_idle(self) -> None: """Return to the *IDLE* phase.""" - self._phase = UpdatePhase.IDLE - self.phase_changed.emit(self._phase) + self._transition(UpdatePhase.IDLE) def set_progress(self, percentage: int) -> None: """Update download progress (0--100).""" diff --git a/synodic_client/application/workers.py b/synodic_client/application/workers.py deleted file mode 100644 index 2af7a25..0000000 --- a/synodic_client/application/workers.py +++ /dev/null @@ -1,20 +0,0 @@ -"""Async background workers for the Synodic Client application. - -.. deprecated:: - All worker functions have been moved to the operations layer - (``synodic_client.operations.tool`` and - ``synodic_client.operations.update``). This module is kept for - backward compatibility but will be removed in a future release. -""" - -from synodic_client.operations.tool import update_all_tools as run_tool_updates -from synodic_client.operations.tool import update_runtime_plugin as run_runtime_package_updates -from synodic_client.operations.update import check_self_update as check_for_update -from synodic_client.operations.update import download_self_update as download_update - -__all__ = [ - 'check_for_update', - 'download_update', - 'run_runtime_package_updates', - 'run_tool_updates', -] diff --git a/synodic_client/cli/install.py b/synodic_client/cli/install.py index 9b98a32..356f6d9 100644 --- a/synodic_client/cli/install.py +++ b/synodic_client/cli/install.py @@ -86,18 +86,12 @@ async def _process_install_stream( """Run the install stream and collect results.""" from porringer.schema import ActionCompletedEvent, ActionStartedEvent, ManifestLoadedEvent - from synodic_client.operations.install import execute_install + from synodic_client.operations.install import collect_install - results: list[SetupActionResult] = [] action_count = 0 - async for stage, event in execute_install( - porringer, - manifest_path, - project_directory=project_directory, - strategy=strategy, - prerelease_packages=prerelease_packages, - ): + def _on_progress(stage: str, event: object) -> None: + nonlocal action_count if stage == 'manifest_loaded' and isinstance(event, ManifestLoadedEvent): action_count = len(event.manifest.actions) if not json_output: @@ -106,14 +100,22 @@ async def _process_install_stream( if not json_output: typer.echo(f' Starting: {event.action.description}') elif stage == 'action_completed' and isinstance(event, ActionCompletedEvent): - results.append(event.result) if not json_output: status = 'OK' if event.result.success else 'FAILED' if event.result.skipped: status = 'SKIPPED' typer.echo(f' {status}: {event.action.description}') - return results, action_count + results = await collect_install( + porringer, + manifest_path, + project_directory=project_directory, + strategy=strategy, + prerelease_packages=prerelease_packages, + on_progress=_on_progress, + ) + + return list(results.results), action_count async def _process_post_sync_stream( @@ -126,28 +128,27 @@ async def _process_post_sync_stream( """Run the post-sync stream and collect results.""" from porringer.schema import ActionCompletedEvent, ActionStartedEvent - from synodic_client.operations.install import execute_post_sync - - results: list[SetupActionResult] = [] + from synodic_client.operations.install import collect_post_sync if not json_output: typer.echo('Running post-sync commands...') - async for stage, event in execute_post_sync( - porringer, - manifest_path, - project_directory=project_directory, - ): + def _on_progress(stage: str, event: object) -> None: if stage == 'action_started' and isinstance(event, ActionStartedEvent): if not json_output: typer.echo(f' Running: {event.action.description}') - elif stage == 'action_completed' and isinstance(event, ActionCompletedEvent): - results.append(event.result) - if not json_output: - status = 'OK' if event.result.success else 'FAILED' - typer.echo(f' {status}: {event.action.description}') + elif stage == 'action_completed' and isinstance(event, ActionCompletedEvent) and not json_output: + status = 'OK' if event.result.success else 'FAILED' + typer.echo(f' {status}: {event.action.description}') + + results = await collect_post_sync( + porringer, + manifest_path, + project_directory=project_directory, + on_progress=_on_progress, + ) - return results + return list(results.results) async def _run( @@ -175,17 +176,13 @@ async def _run( json_output=json_output, ) - # Post-sync phase - has_post_sync = manifest_path.read_text(encoding='utf-8').find('"post_sync"') != -1 - post_sync_results = ( - await _process_post_sync_stream( - porringer, - manifest_path, - project_directory=project_directory, - json_output=json_output, - ) - if has_post_sync - else [] + # Post-sync phase — execute_post_sync no-ops when the manifest + # has no post_sync block, so we always call it. + post_sync_results = await _process_post_sync_stream( + porringer, + manifest_path, + project_directory=project_directory, + json_output=json_output, ) summary = format_install_summary( diff --git a/synodic_client/operations/config.py b/synodic_client/operations/config.py index 0a22e7b..e374503 100644 --- a/synodic_client/operations/config.py +++ b/synodic_client/operations/config.py @@ -12,6 +12,15 @@ from synodic_client.resolution import resolve_config, update_user_config from synodic_client.schema import ResolvedConfig +_VALID_KEYS: frozenset[str] = frozenset(f.name for f in dataclasses.fields(ResolvedConfig)) + + +def _validate_key(key: str) -> None: + """Raise :class:`KeyError` if *key* is not a recognised config field.""" + if key not in _VALID_KEYS: + msg = f'Unknown config key: {key!r}. Valid keys: {sorted(_VALID_KEYS)}' + raise KeyError(msg) + def get_config() -> ResolvedConfig: """Load and return the current resolved configuration. @@ -34,10 +43,7 @@ def get_config_value(key: str) -> object: Raises: KeyError: If *key* is not a recognised config field. """ - valid_keys = {f.name for f in dataclasses.fields(ResolvedConfig)} - if key not in valid_keys: - msg = f'Unknown config key: {key!r}. Valid keys: {sorted(valid_keys)}' - raise KeyError(msg) + _validate_key(key) config = resolve_config() return getattr(config, key) @@ -56,10 +62,7 @@ def set_config(key: str, value: object) -> ResolvedConfig: Raises: KeyError: If *key* is not a recognised config field. """ - valid_keys = {f.name for f in dataclasses.fields(ResolvedConfig)} - if key not in valid_keys: - msg = f'Unknown config key: {key!r}. Valid keys: {sorted(valid_keys)}' - raise KeyError(msg) + _validate_key(key) return update_user_config(**{key: value}) @@ -79,11 +82,8 @@ def update_config(**changes: object) -> ResolvedConfig: Raises: KeyError: If any key is not a recognised config field. """ - valid_keys = {f.name for f in dataclasses.fields(ResolvedConfig)} for key in changes: - if key not in valid_keys: - msg = f'Unknown config key: {key!r}. Valid keys: {sorted(valid_keys)}' - raise KeyError(msg) + _validate_key(key) return update_user_config(**changes) diff --git a/synodic_client/operations/install.py b/synodic_client/operations/install.py index 734c44e..d2e9bff 100644 --- a/synodic_client/operations/install.py +++ b/synodic_client/operations/install.py @@ -9,7 +9,7 @@ import logging import os import tempfile -from collections.abc import AsyncIterator +from collections.abc import AsyncIterator, Callable from pathlib import Path from typing import TYPE_CHECKING @@ -22,7 +22,9 @@ PluginsDiscoveredEvent, ProgressEvent, SetupAction, + SetupActionResult, SetupParameters, + SetupResults, SubActionProgressEvent, SyncStrategy, ) @@ -343,9 +345,99 @@ async def execute_post_sync( # --------------------------------------------------------------------------- -# Internal helpers for manifest filtering +# Batch collect helpers — consume a stream and return SetupResults # --------------------------------------------------------------------------- +InstallProgressCallback = Callable[[str, ProgressEvent], None] + + +async def collect_install( + porringer: API, + manifest_path: Path, + *, + project_directory: Path | None = None, + strategy: SyncStrategy = SyncStrategy.MINIMAL, + prerelease_packages: set[str] | None = None, + discovered: DiscoveredPlugins | None = None, + exclude_post_sync: bool = False, + on_progress: InstallProgressCallback | None = None, +) -> SetupResults: + """Execute install actions and return collected results. + + Consumes :func:`execute_install` internally, collecting action + results and optionally forwarding each ``(stage, event)`` to + *on_progress* for UI updates. + """ + actions: list[SetupAction] = [] + collected: list[SetupActionResult] = [] + manifest_result: SetupResults | None = None + + async for stage, event in execute_install( + porringer, + manifest_path, + project_directory=project_directory, + strategy=strategy, + prerelease_packages=prerelease_packages, + discovered=discovered, + exclude_post_sync=exclude_post_sync, + ): + if stage == 'manifest_loaded' and isinstance(event, ManifestLoadedEvent): + manifest_result = event.manifest + actions = list(event.manifest.actions) + elif stage == 'action_completed' and isinstance(event, ActionCompletedEvent): + collected.append(event.result) + + if on_progress is not None: + on_progress(stage, event) + + return SetupResults( + actions=actions, + results=collected, + manifest_path=manifest_result.manifest_path if manifest_result else None, + metadata=manifest_result.metadata if manifest_result else None, + ) + + +async def collect_post_sync( + porringer: API, + manifest_path: Path, + *, + project_directory: Path | None = None, + discovered: DiscoveredPlugins | None = None, + on_progress: InstallProgressCallback | None = None, +) -> SetupResults: + """Execute post-sync commands and return collected results. + + Consumes :func:`execute_post_sync` internally, collecting action + results and optionally forwarding each ``(stage, event)`` to + *on_progress* for UI updates. + """ + actions: list[SetupAction] = [] + collected: list[SetupActionResult] = [] + manifest_result: SetupResults | None = None + + async for stage, event in execute_post_sync( + porringer, + manifest_path, + project_directory=project_directory, + discovered=discovered, + ): + if stage == 'manifest_loaded' and isinstance(event, ManifestLoadedEvent): + manifest_result = event.manifest + actions = list(event.manifest.actions) + elif stage == 'action_completed' and isinstance(event, ActionCompletedEvent): + collected.append(event.result) + + if on_progress is not None: + on_progress(stage, event) + + return SetupResults( + actions=actions, + results=collected, + manifest_path=manifest_result.manifest_path if manifest_result else None, + metadata=manifest_result.metadata if manifest_result else None, + ) + def _strip_post_sync(manifest_path: Path) -> Path: """Return a temp manifest copy with ``post_sync`` cleared. diff --git a/synodic_client/operations/schema.py b/synodic_client/operations/schema.py index 7437880..54dd21d 100644 --- a/synodic_client/operations/schema.py +++ b/synodic_client/operations/schema.py @@ -81,9 +81,6 @@ def classify_status(status: str) -> str: # Install plan computation # --------------------------------------------------------------------------- -#: Statuses that mean the action is already handled — nothing to install. -_SATISFIED_STATUSES: frozenset[str] = frozenset({'Already installed', 'Already latest'}) - @dataclass(frozen=True, slots=True) class ActionCheckResult: diff --git a/synodic_client/operations/tool.py b/synodic_client/operations/tool.py index 0cfd921..499233b 100644 --- a/synodic_client/operations/tool.py +++ b/synodic_client/operations/tool.py @@ -17,6 +17,7 @@ from porringer.schema import ( ActionCompletedEvent, ActionStartedEvent, + SetupActionResult, SetupParameters, SkipReason, SyncStrategy, @@ -48,10 +49,10 @@ def parse_plugin_key(name: str) -> tuple[str, str | None]: return name, None -def _capture_versions(result: UpdateResult, pkg_name: str, action_result: object) -> None: +def _capture_versions(result: UpdateResult, pkg_name: str, action_result: SetupActionResult) -> None: """Store before/after version info from *action_result* into *result*.""" - old_ver = getattr(action_result, 'installed_version', '') or '' - new_ver = getattr(action_result, 'available_version', '') or '' + old_ver = action_result.installed_version or '' + new_ver = action_result.available_version or '' if pkg_name and (old_ver or new_ver): result.version_map[pkg_name] = (old_ver, new_ver) @@ -316,17 +317,17 @@ async def remove_package( def _record_completed_event( result: UpdateResult, - ar: object, + ar: SetupActionResult, pkg_name: str, on_package_completed: Callable[[str, bool, bool], None] | None, ) -> None: """Record a single completed-event into *result* and fire the callback.""" - if ar.skipped: # type: ignore[union-attr] - if ar.skip_reason in {SkipReason.ALREADY_LATEST, SkipReason.ALREADY_INSTALLED}: # type: ignore[union-attr] + if ar.skipped: + if ar.skip_reason in {SkipReason.ALREADY_LATEST, SkipReason.ALREADY_INSTALLED}: result.already_latest.append(pkg_name) if on_package_completed is not None and pkg_name: on_package_completed(pkg_name, False, True) - elif ar.success: # type: ignore[union-attr] + elif ar.success: result.packages_updated.append(pkg_name) if pkg_name: result.updated_packages.add(pkg_name) diff --git a/synodic_client/resolution.py b/synodic_client/resolution.py index b1483da..3d24a92 100644 --- a/synodic_client/resolution.py +++ b/synodic_client/resolution.py @@ -1,211 +1,184 @@ -"""Configuration resolution for the Synodic Client. - -Combines ``BuildConfig`` (read-only, next to exe) and ``UserConfig`` -(read-write, ``%LOCALAPPDATA%``) into an immutable ``ResolvedConfig`` -dataclass, then derives runtime objects like ``UpdateConfig``. - -Key design rules: -- ``ResolvedConfig`` is frozen — no mutation after construction. -- ``UserConfig`` always saves *all* fields (no sparse writes). -- ``BuildConfig`` seeds user config once, then stays out of the way. -- Settings UI calls ``update_user_config()`` to persist changes and - receive a new ``ResolvedConfig``. -""" - -from __future__ import annotations - -import logging -import sys - -from synodic_client.config import ( - load_build_config, - load_user_config, - save_user_config, -) -from synodic_client.schema import ( - DEFAULT_AUTO_UPDATE_INTERVAL_MINUTES, - DEFAULT_TOOL_UPDATE_INTERVAL_MINUTES, - GITHUB_REPO_URL, - ResolvedConfig, - UpdateChannel, - UpdateConfig, - UserConfig, -) -from synodic_client.updater import github_release_asset_url - -logger = logging.getLogger(__name__) - - -# --------------------------------------------------------------------------- -# Seed — one-time build → user config propagation -# --------------------------------------------------------------------------- - - -def seed_user_config_from_build() -> None: - """Copy ``BuildConfig`` fields into ``UserConfig`` when they are still at defaults. - - Called once during bootstrap (before the UI) so that the build's - channel/source are persisted into the user config. If the user - has already customised a field, the build value is ignored. - """ - build = load_build_config() - if build is None: - return - - user = load_user_config() - changed = False - - if build.update_source is not None and user.update_source is None: - user.update_source = build.update_source - changed = True - - if build.update_channel is not None and user.update_channel is None: - user.update_channel = build.update_channel - changed = True - - if changed: - save_user_config(user) - logger.info( - 'Seeded user config from build config: source=%s, channel=%s', - user.update_source, - user.update_channel, - ) - - -# --------------------------------------------------------------------------- -# Resolution -# --------------------------------------------------------------------------- - - -def _default_channel() -> str: - """Return the default channel based on whether we're running frozen.""" - return 'stable' if getattr(sys, 'frozen', False) else 'dev' - - -def resolve_config() -> ResolvedConfig: - """Load user config and return an immutable :class:`ResolvedConfig`. - - Build config is *not* consulted here — it should already have been - seeded via :func:`seed_user_config_from_build` at startup. - - Returns: - A fully resolved, immutable configuration snapshot. - """ - user = load_user_config() - return _resolve_from_user(user) - - -def _resolve_from_user(user: UserConfig) -> ResolvedConfig: - """Derive a ``ResolvedConfig`` from a ``UserConfig``. - - Resolves every ``None`` field to its concrete default. - """ - channel = user.update_channel or _default_channel() - - # Note: intervals use explicit None-checks because 0 is a valid - # value meaning "disabled" and `or` would incorrectly skip it. - auto_interval = user.auto_update_interval_minutes - if auto_interval is None: - auto_interval = DEFAULT_AUTO_UPDATE_INTERVAL_MINUTES - - tool_interval = user.tool_update_interval_minutes - if tool_interval is None: - tool_interval = DEFAULT_TOOL_UPDATE_INTERVAL_MINUTES - - auto_apply = user.auto_apply if user.auto_apply is not None else True - auto_start = user.auto_start if user.auto_start is not None else True - debug_logging = user.debug_logging if user.debug_logging is not None else False - - return ResolvedConfig( - update_source=user.update_source, - update_channel=channel, - auto_update_interval_minutes=auto_interval, - tool_update_interval_minutes=tool_interval, - plugin_auto_update=user.plugin_auto_update, - prerelease_packages=user.prerelease_packages, - auto_apply=auto_apply, - auto_start=auto_start, - debug_logging=debug_logging, - last_client_update=user.last_client_update, - last_tool_updates=user.last_tool_updates, - ) - - -# --------------------------------------------------------------------------- -# Mutation — save and re-resolve -# --------------------------------------------------------------------------- - - -def update_user_config(**changes: object) -> ResolvedConfig: - """Load user config, apply *changes*, save, and return a new ``ResolvedConfig``. - - This is the primary write path for the Settings UI. Each keyword - argument corresponds to a :class:`UserConfig` field name. - - Args: - **changes: Field-name / value pairs to apply to the user config. - - Returns: - A fresh :class:`ResolvedConfig` reflecting the saved state. - """ - user = load_user_config() - for field_name, value in changes.items(): - setattr(user, field_name, value) - save_user_config(user) - return _resolve_from_user(user) - - -# --------------------------------------------------------------------------- -# Derived helpers -# --------------------------------------------------------------------------- - - -def resolve_update_config(config: ResolvedConfig) -> UpdateConfig: - """Derive an ``UpdateConfig`` from resolved configuration values. - - Args: - config: A resolved configuration snapshot. - - Returns: - An ``UpdateConfig`` ready to initialise the updater. - """ - channel = UpdateChannel.DEVELOPMENT if config.update_channel == 'dev' else UpdateChannel.STABLE - - repo_url = github_release_asset_url( - config.update_source or GITHUB_REPO_URL, - channel, - ) - - return UpdateConfig( - channel=channel, - repo_url=repo_url, - auto_update_interval_minutes=config.auto_update_interval_minutes, - tool_update_interval_minutes=config.tool_update_interval_minutes, - ) - - -def resolve_auto_update_scope( - config: ResolvedConfig, - all_plugin_names: list[str], - manifest_packages: dict[str, set[str]] | None = None, -) -> tuple[set[str] | None, set[str] | None]: - """Derive plugin and package include-lists for auto-update. - - Convenience wrapper around - :func:`synodic_client.operations.tool.resolve_auto_update_scope` - that extracts ``plugin_auto_update`` from a resolved config. - - Args: - config: A resolved configuration snapshot. - all_plugin_names: Every known (installed) plugin name. - manifest_packages: Mapping of ``plugin_name`` → set of package - names declared in cached manifests. ``None`` means treat - all packages as manifest-referenced (conservative default). - - Returns: - A ``(enabled_plugins, include_packages)`` tuple. Either element - may be ``None`` meaning "no filtering". - """ - from synodic_client.operations.tool import resolve_auto_update_scope as _resolve - - return _resolve(config.plugin_auto_update, all_plugin_names, manifest_packages) +"""Configuration resolution for the Synodic Client. + +Combines ``BuildConfig`` (read-only, next to exe) and ``UserConfig`` +(read-write, ``%LOCALAPPDATA%``) into an immutable ``ResolvedConfig`` +dataclass, then derives runtime objects like ``UpdateConfig``. + +Key design rules: +- ``ResolvedConfig`` is frozen — no mutation after construction. +- ``UserConfig`` always saves *all* fields (no sparse writes). +- ``BuildConfig`` seeds user config once, then stays out of the way. +- Settings UI calls ``update_user_config()`` to persist changes and + receive a new ``ResolvedConfig``. +""" + +from __future__ import annotations + +import logging +import sys + +from synodic_client.config import ( + load_build_config, + load_user_config, + save_user_config, +) +from synodic_client.schema import ( + DEFAULT_AUTO_UPDATE_INTERVAL_MINUTES, + DEFAULT_TOOL_UPDATE_INTERVAL_MINUTES, + GITHUB_REPO_URL, + ResolvedConfig, + UpdateChannel, + UpdateConfig, + UserConfig, +) +from synodic_client.updater import github_release_asset_url + +logger = logging.getLogger(__name__) + + +# --------------------------------------------------------------------------- +# Seed — one-time build → user config propagation +# --------------------------------------------------------------------------- + + +def seed_user_config_from_build() -> None: + """Copy ``BuildConfig`` fields into ``UserConfig`` when they are still at defaults. + + Called once during bootstrap (before the UI) so that the build's + channel/source are persisted into the user config. If the user + has already customised a field, the build value is ignored. + """ + build = load_build_config() + if build is None: + return + + user = load_user_config() + changed = False + + if build.update_source is not None and user.update_source is None: + user.update_source = build.update_source + changed = True + + if build.update_channel is not None and user.update_channel is None: + user.update_channel = build.update_channel + changed = True + + if changed: + save_user_config(user) + logger.info( + 'Seeded user config from build config: source=%s, channel=%s', + user.update_source, + user.update_channel, + ) + + +# --------------------------------------------------------------------------- +# Resolution +# --------------------------------------------------------------------------- + + +def _default_channel() -> str: + """Return the default channel based on whether we're running frozen.""" + return 'stable' if getattr(sys, 'frozen', False) else 'dev' + + +def resolve_config() -> ResolvedConfig: + """Load user config and return an immutable :class:`ResolvedConfig`. + + Build config is *not* consulted here — it should already have been + seeded via :func:`seed_user_config_from_build` at startup. + + Returns: + A fully resolved, immutable configuration snapshot. + """ + user = load_user_config() + return _resolve_from_user(user) + + +def _resolve_from_user(user: UserConfig) -> ResolvedConfig: + """Derive a ``ResolvedConfig`` from a ``UserConfig``. + + Resolves every ``None`` field to its concrete default. + """ + channel = user.update_channel or _default_channel() + + # Note: intervals use explicit None-checks because 0 is a valid + # value meaning "disabled" and `or` would incorrectly skip it. + auto_interval = user.auto_update_interval_minutes + if auto_interval is None: + auto_interval = DEFAULT_AUTO_UPDATE_INTERVAL_MINUTES + + tool_interval = user.tool_update_interval_minutes + if tool_interval is None: + tool_interval = DEFAULT_TOOL_UPDATE_INTERVAL_MINUTES + + auto_apply = user.auto_apply if user.auto_apply is not None else True + auto_start = user.auto_start if user.auto_start is not None else True + debug_logging = user.debug_logging if user.debug_logging is not None else False + + return ResolvedConfig( + update_source=user.update_source, + update_channel=channel, + auto_update_interval_minutes=auto_interval, + tool_update_interval_minutes=tool_interval, + plugin_auto_update=user.plugin_auto_update, + prerelease_packages=user.prerelease_packages, + auto_apply=auto_apply, + auto_start=auto_start, + debug_logging=debug_logging, + last_client_update=user.last_client_update, + last_tool_updates=user.last_tool_updates, + ) + + +# --------------------------------------------------------------------------- +# Mutation — save and re-resolve +# --------------------------------------------------------------------------- + + +def update_user_config(**changes: object) -> ResolvedConfig: + """Load user config, apply *changes*, save, and return a new ``ResolvedConfig``. + + This is the primary write path for the Settings UI. Each keyword + argument corresponds to a :class:`UserConfig` field name. + + Args: + **changes: Field-name / value pairs to apply to the user config. + + Returns: + A fresh :class:`ResolvedConfig` reflecting the saved state. + """ + user = load_user_config() + for field_name, value in changes.items(): + setattr(user, field_name, value) + save_user_config(user) + return _resolve_from_user(user) + + +# --------------------------------------------------------------------------- +# Derived helpers +# --------------------------------------------------------------------------- + + +def resolve_update_config(config: ResolvedConfig) -> UpdateConfig: + """Derive an ``UpdateConfig`` from resolved configuration values. + + Args: + config: A resolved configuration snapshot. + + Returns: + An ``UpdateConfig`` ready to initialise the updater. + """ + channel = UpdateChannel.DEVELOPMENT if config.update_channel == 'dev' else UpdateChannel.STABLE + + repo_url = github_release_asset_url( + config.update_source or GITHUB_REPO_URL, + channel, + ) + + return UpdateConfig( + channel=channel, + repo_url=repo_url, + auto_update_interval_minutes=config.auto_update_interval_minutes, + tool_update_interval_minutes=config.tool_update_interval_minutes, + ) diff --git a/tests/unit/operations/test_tool.py b/tests/unit/operations/test_tool.py index 5e5cd45..89a340a 100644 --- a/tests/unit/operations/test_tool.py +++ b/tests/unit/operations/test_tool.py @@ -163,9 +163,8 @@ def test_single_package_success_no_version_data() -> None: action_result = MagicMock() action_result.skipped = False action_result.success = True - # Simulate older porringer without version attributes - del action_result.installed_version - del action_result.available_version + action_result.installed_version = None + action_result.available_version = None api.package.upgrade = AsyncMock(return_value=action_result) result = asyncio.run(update_tool(api, 'pip', 'requests')) diff --git a/tests/unit/qt/test_install_preview.py b/tests/unit/qt/test_install_preview.py index 4a0840c..cd9dcfc 100644 --- a/tests/unit/qt/test_install_preview.py +++ b/tests/unit/qt/test_install_preview.py @@ -28,8 +28,14 @@ skip_reason_label, ) from synodic_client.application.screen.install_workers import run_install, run_preview -from synodic_client.application.screen.schema import InstallConfig, PreviewCallbacks, PreviewConfig +from synodic_client.application.screen.schema import InstallConfig, PreviewConfig from synodic_client.application.uri import normalize_manifest_key, resolve_local_path +from synodic_client.operations.schema import ( + PreviewActionChecked, + PreviewManifestParsed, + PreviewPluginsQueried, + PreviewReady, +) _DOWNLOAD_PATCH = 'synodic_client.application.screen.install_workers.API.download' _EXPECTED_CHECKED_COUNT = 2 @@ -288,8 +294,8 @@ async def mock_stream(*args: Any, **kwargs: Any) -> Any: run_preview( porringer, str(manifest), - callbacks=PreviewCallbacks( - on_preview_ready=lambda r, p, t: results.append((r, p, t)), + on_event=lambda e: ( + isinstance(e, PreviewReady) and results.append((e.manifest, e.manifest_path, e.temp_dir)) ), ), ) @@ -364,8 +370,8 @@ async def mock_stream(*args: Any, **kwargs: Any) -> Any: run_preview( porringer, 'https://example.com/good.json', - callbacks=PreviewCallbacks( - on_preview_ready=lambda r, p, t: results.append((r, p, t)), + on_event=lambda e: ( + isinstance(e, PreviewReady) and results.append((e.manifest, e.manifest_path, e.temp_dir)) ), ), ) @@ -412,9 +418,9 @@ async def _run() -> None: await run_preview( porringer, str(manifest), - callbacks=PreviewCallbacks( - on_preview_ready=lambda p, m, t: ready_calls.append((p, m, t)), - on_action_checked=lambda row, r, s: checked.append((row, r, s)), + on_event=lambda e: ( + (isinstance(e, PreviewReady) and ready_calls.append((e.manifest, e.manifest_path, e.temp_dir))) + or (isinstance(e, PreviewActionChecked) and checked.append((e.index, e.result, e.status))) ), ) finished = True @@ -496,8 +502,8 @@ async def mock_stream(*args: Any, **kwargs: Any) -> Any: run_preview( porringer, str(manifest), - callbacks=PreviewCallbacks( - on_action_checked=lambda row, r, s: checked.append((row, r, s)), + on_event=lambda e: ( + isinstance(e, PreviewActionChecked) and checked.append((e.index, e.result, e.status)) ), ), ) @@ -555,8 +561,8 @@ async def mock_stream(*args: Any, **kwargs: Any) -> Any: run_preview( porringer, str(manifest), - callbacks=PreviewCallbacks( - on_plugins_queried=lambda avail, caps: captured.append((avail, caps)), + on_event=lambda e: ( + isinstance(e, PreviewPluginsQueried) and captured.append((e.availability, e.capabilities)) ), ), ) @@ -590,9 +596,9 @@ async def mock_stream(*args: Any, **kwargs: Any) -> Any: run_preview( porringer, str(manifest), - callbacks=PreviewCallbacks( - on_plugins_queried=lambda _avail, _caps: order.append('plugins'), - on_preview_ready=lambda *_: order.append('preview'), + on_event=lambda e: ( + (isinstance(e, PreviewPluginsQueried) and order.append('plugins')) + or (isinstance(e, PreviewReady) and order.append('preview')) ), ), ) @@ -625,8 +631,9 @@ async def mock_stream(*args: Any, **kwargs: Any) -> Any: run_preview( porringer, str(manifest), - callbacks=PreviewCallbacks( - on_manifest_parsed=lambda *a: parsed_data.append(a), + on_event=lambda e: ( + isinstance(e, PreviewManifestParsed) + and parsed_data.append((e.manifest, e.manifest_path, e.temp_dir)) ), ), ) @@ -665,10 +672,10 @@ async def mock_stream(*args: Any, **kwargs: Any) -> Any: run_preview( porringer, str(manifest), - callbacks=PreviewCallbacks( - on_manifest_parsed=lambda *_: order.append('parsed'), - on_plugins_queried=lambda _avail, _caps: order.append('plugins'), - on_preview_ready=lambda *_: order.append('ready'), + on_event=lambda e: ( + (isinstance(e, PreviewManifestParsed) and order.append('parsed')) + or (isinstance(e, PreviewPluginsQueried) and order.append('plugins')) + or (isinstance(e, PreviewReady) and order.append('ready')) ), ), ) @@ -866,8 +873,8 @@ async def mock_stream(*args: Any, **kwargs: Any) -> Any: porringer, str(manifest), config=PreviewConfig(project_directory=tmp_path), - callbacks=PreviewCallbacks( - on_action_checked=lambda row, r, s: checked.append((row, r, s)), + on_event=lambda e: ( + isinstance(e, PreviewActionChecked) and checked.append((e.index, e.result, e.status)) ), ), ) @@ -912,8 +919,8 @@ async def mock_stream(*args: Any, **kwargs: Any) -> Any: porringer, str(manifest), config=PreviewConfig(project_directory=tmp_path), - callbacks=PreviewCallbacks( - on_action_checked=lambda row, r, s: checked.append((row, r, s)), + on_event=lambda e: ( + isinstance(e, PreviewActionChecked) and checked.append((e.index, e.result, e.status)) ), ), ) diff --git a/tests/unit/qt/test_tray_window_show.py b/tests/unit/qt/test_tray_window_show.py index b3233db..6ba740b 100644 --- a/tests/unit/qt/test_tray_window_show.py +++ b/tests/unit/qt/test_tray_window_show.py @@ -7,7 +7,7 @@ import pytest from synodic_client.application.config_store import ConfigStore -from synodic_client.application.schema import UpdateTarget +from synodic_client.application.screen.schema import UpdateTarget from synodic_client.application.screen.tray import TrayScreen from synodic_client.operations.schema import UpdateResult from synodic_client.resolution import ResolvedConfig diff --git a/tests/unit/test_resolution.py b/tests/unit/test_resolution.py index 00008aa..e32ef78 100644 --- a/tests/unit/test_resolution.py +++ b/tests/unit/test_resolution.py @@ -4,9 +4,9 @@ from typing import Any from unittest.mock import patch +from synodic_client.operations.tool import resolve_auto_update_scope from synodic_client.resolution import ( ResolvedConfig, - resolve_auto_update_scope, resolve_config, resolve_update_config, seed_user_config_from_build, @@ -231,7 +231,7 @@ class TestResolveAutoUpdateScope: def test_no_mapping_returns_none_pair() -> None: """Verify (None, None) when plugin_auto_update is unset.""" config = _make_resolved() - plugins, packages = resolve_auto_update_scope(config, ['pip', 'uv']) + plugins, packages = resolve_auto_update_scope(config.plugin_auto_update, ['pip', 'uv']) assert plugins is None assert packages is None @@ -239,7 +239,7 @@ def test_no_mapping_returns_none_pair() -> None: def test_all_enabled_returns_none_pair() -> None: """Verify (None, None) when all entries are True.""" config = _make_resolved(plugin_auto_update={'pip': True, 'uv': True}) - plugins, packages = resolve_auto_update_scope(config, ['pip', 'uv']) + plugins, packages = resolve_auto_update_scope(config.plugin_auto_update, ['pip', 'uv']) assert plugins is None assert packages is None @@ -247,7 +247,7 @@ def test_all_enabled_returns_none_pair() -> None: def test_plugin_disabled() -> None: """Verify a disabled plugin is excluded.""" config = _make_resolved(plugin_auto_update={'pip': False}) - plugins, packages = resolve_auto_update_scope(config, ['pip', 'uv']) + plugins, packages = resolve_auto_update_scope(config.plugin_auto_update, ['pip', 'uv']) assert plugins is not None assert 'pip' not in plugins assert 'uv' in plugins @@ -261,7 +261,7 @@ def test_nested_dict_filters_packages() -> None: plugin_auto_update={'uv': {'ruff': True, 'mypy': False}}, ) plugins, packages = resolve_auto_update_scope( - config, + config.plugin_auto_update, ['uv', 'pip'], manifest_packages={'uv': {'ruff', 'black'}}, ) @@ -284,7 +284,7 @@ def test_mixed_entries() -> None: }, ) plugins, packages = resolve_auto_update_scope( - config, + config.plugin_auto_update, ['pip', 'uv', 'git'], ) assert plugins is not None @@ -305,7 +305,7 @@ def test_composite_keys_ignored() -> None: }, ) plugins, packages = resolve_auto_update_scope( - config, + config.plugin_auto_update, ['pip', 'uv'], ) # Neither bare plugin should be disabled From e4b8a4129f763e489ce1a4422c1e9f39d34d5a8e Mon Sep 17 00:00:00 2001 From: Asher Norland Date: Wed, 18 Mar 2026 17:47:43 -0700 Subject: [PATCH 5/5] Test Consolidation --- tests/unit/operations/test_tool.py | 60 ----- tests/unit/qt/conftest.py | 101 +++++++++ tests/unit/qt/test_action_card.py | 285 ++++++++++-------------- tests/unit/qt/test_gather_packages.py | 125 ++++------- tests/unit/qt/test_log_panel.py | 138 ++++-------- tests/unit/qt/test_preview_model.py | 61 ++--- tests/unit/qt/test_settings.py | 88 +++----- tests/unit/qt/test_sidebar.py | 49 ++-- tests/unit/qt/test_tray_window_show.py | 21 +- tests/unit/qt/test_update_controller.py | 51 +---- tests/unit/test_resolution.py | 19 ++ tests/unit/test_updater.py | 199 ++++------------- tests/unit/windows/conftest.py | 10 + tests/unit/windows/test_protocol.py | 12 +- tests/unit/windows/test_startup.py | 60 ++--- 15 files changed, 480 insertions(+), 799 deletions(-) diff --git a/tests/unit/operations/test_tool.py b/tests/unit/operations/test_tool.py index 89a340a..8eeb562 100644 --- a/tests/unit/operations/test_tool.py +++ b/tests/unit/operations/test_tool.py @@ -12,7 +12,6 @@ from synodic_client.operations.tool import ( check_tool_updates, remove_package, - resolve_auto_update_scope, update_tool, ) @@ -193,62 +192,3 @@ def test_failure() -> None: api = MagicMock() api.package.uninstall = AsyncMock(return_value=MagicMock(success=False)) assert asyncio.run(remove_package(api, 'pip', 'requests')) is False - - -# --------------------------------------------------------------------------- -# resolve_auto_update_scope -# --------------------------------------------------------------------------- - - -class TestResolveAutoUpdateScope: - """Tests for resolve_auto_update_scope().""" - - @staticmethod - def test_none_config_returns_none_none() -> None: - """No config → (None, None) meaning all plugins, all packages.""" - plugins, packages = resolve_auto_update_scope(None, ['pip', 'npm']) - assert plugins is None - assert packages is None - - @staticmethod - def test_disabled_plugin_excluded() -> None: - """A plugin set to False is excluded from the enabled set.""" - plugins, _ = resolve_auto_update_scope( - {'pip': False, 'npm': True}, - ['pip', 'npm', 'cargo'], - ) - assert plugins is not None - assert 'pip' not in plugins - assert 'npm' in plugins - assert 'cargo' in plugins - - @staticmethod - def test_per_package_entries() -> None: - """Per-package granularity builds the include set.""" - _, packages = resolve_auto_update_scope( - {'pip': {'requests': True, 'flask': False}}, - ['pip'], - manifest_packages=None, - ) - assert packages is not None - assert 'requests' in packages - assert 'flask' not in packages - - @staticmethod - def test_manifest_packages_included() -> None: - """Manifest packages are added to the include set.""" - _, packages = resolve_auto_update_scope( - {}, - ['pip'], - manifest_packages={'pip': {'requests', 'flask'}}, - ) - assert packages is not None - assert 'requests' in packages - assert 'flask' in packages - - @staticmethod - def test_empty_config_returns_none_none() -> None: - """Empty mapping → (None, None).""" - plugins, packages = resolve_auto_update_scope({}, ['pip']) - assert plugins is None - assert packages is None diff --git a/tests/unit/qt/conftest.py b/tests/unit/qt/conftest.py index d0f4f3e..7c7c1c8 100644 --- a/tests/unit/qt/conftest.py +++ b/tests/unit/qt/conftest.py @@ -10,6 +10,8 @@ import os import sys +from typing import Any +from unittest.mock import AsyncMock, MagicMock # Force offscreen rendering *before* any PySide6 import. os.environ.setdefault('QT_QPA_PLATFORM', 'offscreen') @@ -18,7 +20,106 @@ pytest.importorskip('PySide6.QtWidgets', reason='PySide6 requires system Qt libraries') +from porringer.schema import SetupAction, SetupActionResult, SkipReason +from porringer.schema.plugin import PluginKind from PySide6.QtWidgets import QApplication +from synodic_client.application.config_store import ConfigStore +from synodic_client.resolution import ResolvedConfig +from synodic_client.schema import DEFAULT_AUTO_UPDATE_INTERVAL_MINUTES, DEFAULT_TOOL_UPDATE_INTERVAL_MINUTES + # Single shared QApplication for all Qt tests in this directory. _app = QApplication.instance() or QApplication(sys.argv) + + +# --------------------------------------------------------------------------- +# Shared test factories +# --------------------------------------------------------------------------- + + +def make_resolved_config(**overrides: Any) -> ResolvedConfig: + """Build a ``ResolvedConfig`` with sensible defaults and optional overrides.""" + defaults: dict[str, Any] = { + '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, + 'prerelease_packages': None, + 'auto_apply': True, + 'auto_start': True, + 'debug_logging': False, + 'last_client_update': None, + 'last_tool_updates': None, + } + defaults.update(overrides) + return ResolvedConfig(**defaults) + + +def make_config_store(config: ResolvedConfig | None = None) -> ConfigStore: + """Build a ``ConfigStore`` seeded with *config*.""" + return ConfigStore(config or make_resolved_config()) + + +def make_mock_porringer() -> MagicMock: + """Build a ``MagicMock`` standing in for the porringer ``API``.""" + mock = MagicMock() + mock.plugin.list = AsyncMock(return_value=[]) + mock.package.list = AsyncMock(return_value=[]) + mock.cache.list_directories.return_value = [] + return mock + + +def make_action( + *, + kind: PluginKind | None = PluginKind.PACKAGE, + description: str = 'Install requests', + installer: str | None = 'pip', + package: str = 'requests', + **overrides: Any, +) -> SetupAction: + """Create a mock ``SetupAction`` with sensible defaults. + + Extra keyword arguments are set as attributes on the mock, supporting + ``package_description``, ``include_prereleases``, ``command``, + ``cli_command``, ``constraint``, and ``plugin_target``. + """ + action = MagicMock(spec=SetupAction) + action.kind = kind + action.description = description + action.installer = installer + pkg_mock = MagicMock() + pkg_mock.name = package + pkg_mock.constraint = overrides.get('constraint') + pkg_mock.configure_mock(**{'__str__': MagicMock(return_value=package)}) + action.package = pkg_mock + action.package_description = overrides.get('package_description', description) + action.command = overrides.get('command') + action.include_prereleases = overrides.get('include_prereleases', False) + action.plugin_target = overrides.get('plugin_target') + return action + + +def make_result( + *, + success: bool = True, + skipped: bool = False, + skip_reason: SkipReason | None = None, + message: str | None = None, + **overrides: Any, +) -> SetupActionResult: + """Create a ``SetupActionResult``. + + Extra keyword arguments (``action``, ``installed_version``, + ``available_version``, ``cli_command``) are forwarded to the constructor. + """ + return SetupActionResult( + action=overrides.get('action') or make_action(), + success=success, + skipped=skipped, + skip_reason=skip_reason, + message=message, + installed_version=overrides.get('installed_version'), + available_version=overrides.get('available_version'), + cli_command=overrides.get('cli_command'), + ) diff --git a/tests/unit/qt/test_action_card.py b/tests/unit/qt/test_action_card.py index 70caece..28dbb11 100644 --- a/tests/unit/qt/test_action_card.py +++ b/tests/unit/qt/test_action_card.py @@ -2,11 +2,7 @@ from __future__ import annotations -from typing import Any -from unittest.mock import MagicMock - from porringer.schema import ( - SetupAction, SetupActionResult, SkipReason, ) @@ -35,64 +31,7 @@ ) from synodic_client.operations.schema import resolve_action_status -# --------------------------------------------------------------------------- -# Helpers -# --------------------------------------------------------------------------- - - -def _make_action( - *, - kind: PluginKind | None = PluginKind.PACKAGE, - description: str = 'Install requests', - installer: str | None = 'pip', - package: str = 'requests', - **overrides: Any, -) -> SetupAction: - """Create a mock SetupAction with sensible defaults. - - Extra keyword arguments are set as attributes on the mock, supporting - ``package_description``, ``include_prereleases``, ``command``, - ``cli_command``, and ``plugin_target``. - """ - action = MagicMock(spec=SetupAction) - action.kind = kind - action.description = description - action.installer = installer - pkg_mock = MagicMock() - pkg_mock.name = package - pkg_mock.constraint = overrides.get('constraint') - pkg_mock.configure_mock(**{'__str__': MagicMock(return_value=package)}) - action.package = pkg_mock - action.package_description = overrides.get('package_description', description) - action.command = overrides.get('command') - action.include_prereleases = overrides.get('include_prereleases', False) - action.plugin_target = overrides.get('plugin_target') - return action - - -def _make_result( - *, - success: bool = True, - skipped: bool = False, - skip_reason: SkipReason | None = None, - message: str | None = None, - **overrides: Any, -) -> SetupActionResult: - """Create a SetupActionResult. - - Extra keyword arguments (``action``, ``installed_version``, - ``available_version``, ``cli_command``) are forwarded to the constructor. - """ - return SetupActionResult( - action=overrides.get('action') or _make_action(), - success=success, - skipped=skipped, - skip_reason=skip_reason, - message=message, - installed_version=overrides.get('installed_version'), - available_version=overrides.get('available_version'), - cli_command=overrides.get('cli_command'), - ) +from .conftest import make_action, make_result def _check(card: ActionCard, result: SetupActionResult) -> None: @@ -126,7 +65,7 @@ def test_skeleton_card_status_text_is_empty() -> None: def test_skeleton_populate_does_nothing() -> None: """Calling populate on a skeleton card is a no-op.""" card = ActionCard(skeleton=True) - action = _make_action() + action = make_action() card.populate(action) assert not card.status_text() @@ -134,7 +73,7 @@ def test_skeleton_populate_does_nothing() -> None: def test_skeleton_set_check_result_does_nothing() -> None: """set_check_result on skeleton is a no-op.""" card = ActionCard(skeleton=True) - result = _make_result() + result = make_result() card.set_check_result(result, 'Needed') assert not card.status_text() @@ -158,7 +97,7 @@ class TestActionCardPopulated: def test_populate_shows_package_name() -> None: """populate() fills the package label.""" card = ActionCard() - action = _make_action(package='ruff') + action = make_action(package='ruff') card.populate(action) assert card._package_label.text() == 'ruff' @@ -166,7 +105,7 @@ def test_populate_shows_package_name() -> None: def test_populate_shows_type_badge() -> None: """populate() sets the type badge.""" card = ActionCard() - action = _make_action(kind=PluginKind.TOOL) + action = make_action(kind=PluginKind.TOOL) card.populate(action) assert card._type_badge.text() == 'Tool' @@ -174,7 +113,7 @@ def test_populate_shows_type_badge() -> None: def test_populate_shows_description() -> None: """populate() sets the description label.""" card = ActionCard() - action = _make_action(package='ruff', package_description='A fast Python linter') + action = make_action(package='ruff', package_description='A fast Python linter') card.populate(action) assert card._desc_label.text() == 'A fast Python linter' @@ -182,7 +121,7 @@ def test_populate_shows_description() -> None: def test_initial_status_is_checking() -> None: """Card starts with 'Checking…' status (via spinner) after populate.""" card = ActionCard() - action = _make_action() + action = make_action() card.populate(action) assert card.status_text() == 'Checking\u2026' assert card._checking @@ -193,7 +132,7 @@ def test_initial_status_is_checking() -> None: def test_installer_missing_shows_not_installed() -> None: """Card shows 'Not installed' when the plugin is missing.""" card = ActionCard() - action = _make_action(installer='uv') + action = make_action(installer='uv') card.populate(action, plugin_installed={'uv': False}) assert card.status_text() == 'Not installed' @@ -201,7 +140,7 @@ def test_installer_missing_shows_not_installed() -> None: def test_installer_present_shows_checking() -> None: """Card shows spinner (Checking) when the plugin is installed.""" card = ActionCard() - action = _make_action(installer='pip') + action = make_action(installer='pip') card.populate(action, plugin_installed={'pip': True}) assert card.status_text() == 'Checking\u2026' assert card._checking @@ -210,7 +149,7 @@ def test_installer_present_shows_checking() -> None: def test_prerelease_checkbox_shown_for_packages() -> None: """Pre-release checkbox is visible for package actions.""" card = ActionCard() - action = _make_action(package='requests') + action = make_action(package='requests') card.populate(action) assert not card._prerelease_cb.isHidden() @@ -218,7 +157,7 @@ def test_prerelease_checkbox_shown_for_packages() -> None: def test_prerelease_checkbox_locked_by_manifest() -> None: """Pre-release checkbox locked if manifest enables it and no user override.""" card = ActionCard() - action = _make_action(include_prereleases=True) + action = make_action(include_prereleases=True) card.populate(action, prerelease_overrides=set()) assert card._prerelease_cb.isChecked() assert not card._prerelease_cb.isEnabled() @@ -227,7 +166,7 @@ def test_prerelease_checkbox_locked_by_manifest() -> None: def test_prerelease_checkbox_unlocked_if_user_override() -> None: """Pre-release checkbox unlocked when user has an override.""" card = ActionCard() - action = _make_action(package='requests', include_prereleases=True) + action = make_action(package='requests', include_prereleases=True) card.populate(action, prerelease_overrides={'requests'}) assert card._prerelease_cb.isChecked() assert card._prerelease_cb.isEnabled() @@ -245,8 +184,8 @@ class TestActionCardCheckResult: def test_needed_status() -> None: """Non-skipped result shows 'Needed'.""" card = ActionCard() - card.populate(_make_action()) - result = _make_result(success=True, skipped=False) + card.populate(make_action()) + result = make_result(success=True, skipped=False) _check(card, result) assert card.status_text() == 'Needed' assert ACTION_CARD_STATUS_NEEDED in card._status_label.styleSheet() @@ -255,8 +194,8 @@ def test_needed_status() -> None: def test_already_installed_status() -> None: """Skipped ALREADY_INSTALLED shows '\u2713 Already installed'.""" card = ActionCard() - card.populate(_make_action()) - result = _make_result( + card.populate(make_action()) + result = make_result( skipped=True, skip_reason=SkipReason.ALREADY_INSTALLED, installed_version='3.5.2', @@ -269,8 +208,8 @@ def test_already_installed_status() -> None: def test_update_available_status() -> None: """Skipped UPDATE_AVAILABLE shows 'Update available'.""" card = ActionCard() - card.populate(_make_action()) - result = _make_result( + card.populate(make_action()) + result = make_result( skipped=True, skip_reason=SkipReason.UPDATE_AVAILABLE, installed_version='1.0.0', @@ -285,8 +224,8 @@ def test_update_available_status() -> None: def test_version_transition_shown() -> None: """Version label shows 'old → new' for update-available actions.""" card = ActionCard() - card.populate(_make_action()) - result = _make_result( + card.populate(make_action()) + result = make_result( skipped=True, skip_reason=SkipReason.UPDATE_AVAILABLE, installed_version='1.0.0', @@ -301,8 +240,8 @@ def test_version_transition_shown() -> None: def test_installed_version_shown() -> None: """Version label shows installed version for satisfied actions.""" card = ActionCard() - card.populate(_make_action()) - result = _make_result( + card.populate(make_action()) + result = make_result( skipped=True, skip_reason=SkipReason.ALREADY_INSTALLED, installed_version='3.5.2', @@ -314,8 +253,8 @@ def test_installed_version_shown() -> None: def test_available_version_only_shown() -> None: """Version label shows '→ target' when only available_version is set.""" card = ActionCard() - card.populate(_make_action()) - result = _make_result( + card.populate(make_action()) + result = make_result( success=True, skipped=False, available_version='1.2.0', @@ -328,7 +267,7 @@ def test_available_version_only_shown() -> None: def test_finalize_checking_resolves_to_needed() -> None: """finalize_checking stops spinner and changes to 'Needed'.""" card = ActionCard() - card.populate(_make_action()) + card.populate(make_action()) assert card.status_text() == 'Checking\u2026' assert card._checking card.finalize_checking() @@ -340,7 +279,7 @@ def test_finalize_checking_resolves_to_needed() -> None: def test_finalize_checking_leaves_not_installed() -> None: """finalize_checking does not change 'Not installed'.""" card = ActionCard() - card.populate(_make_action(installer='uv'), plugin_installed={'uv': False}) + card.populate(make_action(installer='uv'), plugin_installed={'uv': False}) assert card.status_text() == 'Not installed' card.finalize_checking() assert card.status_text() == 'Not installed' @@ -358,8 +297,8 @@ class TestActionCardCheckFailure: def test_failed_check_shows_failed_status() -> None: """A check result with success=False shows 'Failed'.""" card = ActionCard() - card.populate(_make_action(kind=PluginKind.SCM, package='mypackage', installer='git')) - result = _make_result( + card.populate(make_action(kind=PluginKind.SCM, package='mypackage', installer='git')) + result = make_result( success=False, skipped=False, message="No SCM plugin was found for ecosystem 'git'.", @@ -372,10 +311,10 @@ def test_failed_check_shows_failed_status() -> None: def test_failed_check_shows_error_tooltip() -> None: """A failed check result surfaces the error message as a tooltip.""" card = ActionCard() - action = _make_action(kind=PluginKind.SCM, package='repo', installer=None) + action = make_action(kind=PluginKind.SCM, package='repo', installer=None) card.populate(action) msg = "SCM environment 'None' is not available" - result = _make_result(success=False, skipped=False, message=msg) + result = make_result(success=False, skipped=False, message=msg) _check(card, result) assert card._status_label.toolTip() == msg @@ -383,9 +322,9 @@ def test_failed_check_shows_error_tooltip() -> None: def test_failed_check_stops_spinner() -> None: """A failed check result stops the inline spinner.""" card = ActionCard() - card.populate(_make_action()) + card.populate(make_action()) assert card._checking - result = _make_result(success=False, skipped=False, message='error') + result = make_result(success=False, skipped=False, message='error') _check(card, result) assert not card._checking assert not card._spinner_timer.isActive() @@ -394,8 +333,8 @@ def test_failed_check_stops_spinner() -> None: def test_failed_check_not_update_available() -> None: """A failed check is not considered 'Update available'.""" card = ActionCard() - card.populate(_make_action()) - result = _make_result(success=False, skipped=False, message='backend missing') + card.populate(make_action()) + result = make_result(success=False, skipped=False, message='backend missing') _check(card, result) assert not card.is_update_available() @@ -403,8 +342,8 @@ def test_failed_check_not_update_available() -> None: def test_success_true_still_needed() -> None: """A non-skipped, successful result still shows 'Needed'.""" card = ActionCard() - card.populate(_make_action()) - result = _make_result(success=True, skipped=False) + card.populate(make_action()) + result = make_result(success=True, skipped=False) _check(card, result) assert card.status_text() == 'Needed' assert ACTION_CARD_STATUS_NEEDED in card._status_label.styleSheet() @@ -422,7 +361,7 @@ class TestActionCardExecution: def test_set_executing_shows_running() -> None: """set_executing changes status to 'Running…'.""" card = ActionCard() - card.populate(_make_action()) + card.populate(make_action()) card.set_executing() assert card.status_text() == 'Running\u2026' assert ACTION_CARD_STATUS_RUNNING in card._status_label.styleSheet() @@ -431,7 +370,7 @@ def test_set_executing_shows_running() -> None: def test_set_executing_changes_border_style() -> None: """set_executing applies the executing card style.""" card = ActionCard() - card.populate(_make_action()) + card.populate(make_action()) card.set_executing() assert ACTION_CARD_EXECUTING_STYLE in card.styleSheet() @@ -439,9 +378,9 @@ def test_set_executing_changes_border_style() -> None: def test_set_result_success() -> None: """Successful result shows 'Done' status.""" card = ActionCard() - card.populate(_make_action()) + card.populate(make_action()) card.set_executing() - result = _make_result(success=True, message='Installed ruff-0.8.0') + result = make_result(success=True, message='Installed ruff-0.8.0') card.set_result(result) assert card.status_text() == 'Done' assert ACTION_CARD_STATUS_DONE in card._status_label.styleSheet() @@ -451,9 +390,9 @@ def test_set_result_success() -> None: def test_set_result_failure() -> None: """Failed result shows 'Failed' with error in log.""" card = ActionCard() - card.populate(_make_action()) + card.populate(make_action()) card.set_executing() - result = _make_result(success=False, message='Network timeout') + result = make_result(success=False, message='Network timeout') card.set_result(result) assert card.status_text() == 'Failed' assert ACTION_CARD_STATUS_FAILED in card._status_label.styleSheet() @@ -462,9 +401,9 @@ def test_set_result_failure() -> None: def test_set_result_skipped() -> None: """Skipped result shows skip reason.""" card = ActionCard() - card.populate(_make_action()) + card.populate(make_action()) card.set_executing() - result = _make_result( + result = make_result( success=True, skipped=True, skip_reason=SkipReason.ALREADY_INSTALLED, @@ -477,9 +416,9 @@ def test_set_result_skipped() -> None: def test_set_result_updates_version_on_upgrade() -> None: """Successful upgrade updates version label to new version.""" card = ActionCard() - card.populate(_make_action()) + card.populate(make_action()) card.set_executing() - result = _make_result( + result = make_result( success=True, installed_version='1.0.0', available_version='2.0.0', @@ -515,7 +454,7 @@ def test_populate_replaces_skeletons() -> None: card_list.show_skeletons(3) action_count = 2 - actions = [_make_action(package=f'pkg-{i}') for i in range(action_count)] + actions = [make_action(package=f'pkg-{i}') for i in range(action_count)] card_list.populate(actions) assert card_list.card_count() == action_count for i in range(action_count): @@ -527,8 +466,8 @@ def test_populate_replaces_skeletons() -> None: def test_populate_includes_command_actions() -> None: """Populate includes actions with kind=None.""" card_list = ActionCardList() - a1 = _make_action(package='pkg1') - a2 = _make_action(package='pkg2', kind=None) + a1 = make_action(package='pkg1') + a2 = make_action(package='pkg2', kind=None) actions = [a1, a2] card_list.populate(actions) assert card_list.card_count() == len(actions) @@ -537,8 +476,8 @@ def test_populate_includes_command_actions() -> None: def test_get_card_by_stable_key() -> None: """get_card finds the correct card by stable content key.""" card_list = ActionCardList() - a1 = _make_action(package='first') - a2 = _make_action(package='second') + a1 = make_action(package='first') + a2 = make_action(package='second') card_list.populate([a1, a2]) c1 = card_list.get_card(a1) @@ -553,14 +492,14 @@ def test_get_card_by_stable_key() -> None: def test_get_card_returns_none_for_unknown() -> None: """get_card returns None for an unknown action.""" card_list = ActionCardList() - card_list.populate([_make_action()]) - unknown = _make_action(package='unknown') + card_list.populate([make_action()]) + unknown = make_action(package='unknown') assert card_list.get_card(unknown) is None @staticmethod def test_clear_removes_all() -> None: """Clear removes all cards.""" - actions = [_make_action(package='pkg1'), _make_action(package='pkg2')] + actions = [make_action(package='pkg1'), make_action(package='pkg2')] card_list = ActionCardList() card_list.populate(actions) assert card_list.card_count() == len(actions) @@ -572,8 +511,8 @@ def test_clear_removes_all() -> None: def test_finalize_all_checking() -> None: """finalize_all_checking resolves pending cards to 'Needed'.""" card_list = ActionCardList() - a1 = _make_action(package='pkg1') - a2 = _make_action(package='pkg2') + a1 = make_action(package='pkg1') + a2 = make_action(package='pkg2') card_list.populate([a1, a2]) # Simulate: a1 gets a check result, a2 stays as 'Checking…' @@ -581,7 +520,7 @@ def test_finalize_all_checking() -> None: assert c1 is not None _check( c1, - _make_result( + make_result( skipped=True, skip_reason=SkipReason.ALREADY_INSTALLED, ), @@ -598,7 +537,7 @@ def test_finalize_all_checking() -> None: def test_prerelease_signal_forwarded() -> None: """prerelease_toggled from a card is forwarded through the list.""" card_list = ActionCardList() - action = _make_action(package='requests') + action = make_action(package='requests') card_list.populate([action]) received: list[tuple[str, bool]] = [] @@ -624,11 +563,11 @@ class TestActionCardKindStatus: def test_bare_command_shows_pending_after_check() -> None: """Bare command (kind=None) keeps 'Pending' after dry-run check.""" card = ActionCard() - action = _make_action(kind=None, package='post_sync', installer=None, command=('echo', 'done')) + action = make_action(kind=None, package='post_sync', installer=None, command=('echo', 'done')) card.populate(action) assert card.status_text() == 'Pending' - result = _make_result(success=True, skipped=False) + result = make_result(success=True, skipped=False) _check(card, result) assert card.status_text() == 'Pending' assert ACTION_CARD_STATUS_PENDING in card._status_label.styleSheet() @@ -637,10 +576,10 @@ def test_bare_command_shows_pending_after_check() -> None: def test_project_shows_ready_after_check() -> None: """PROJECT action shows 'Ready' after dry-run check.""" card = ActionCard() - action = _make_action(kind=PluginKind.PROJECT, package='myproject') + action = make_action(kind=PluginKind.PROJECT, package='myproject') card.populate(action) - result = _make_result(success=True, skipped=False) + result = make_result(success=True, skipped=False) _check(card, result) assert card.status_text() == 'Ready' assert ACTION_CARD_STATUS_SATISFIED in card._status_label.styleSheet() @@ -649,8 +588,8 @@ def test_project_shows_ready_after_check() -> None: def test_package_still_shows_needed() -> None: """A PACKAGE action with success=True still shows 'Needed'.""" card = ActionCard() - card.populate(_make_action(kind=PluginKind.PACKAGE)) - result = _make_result(success=True, skipped=False) + card.populate(make_action(kind=PluginKind.PACKAGE)) + result = make_result(success=True, skipped=False) _check(card, result) assert card.status_text() == 'Needed' @@ -667,8 +606,8 @@ class TestActionCardVersionSpecifier: def test_specifier_available_version_shows_requires() -> None: """available_version with a specifier shows 'requires …'.""" card = ActionCard() - card.populate(_make_action()) - result = _make_result( + card.populate(make_action()) + result = make_result( success=True, skipped=False, available_version='>=0.8.0', @@ -681,8 +620,8 @@ def test_specifier_available_version_shows_requires() -> None: def test_resolved_available_version_shows_arrow() -> None: """available_version with a plain version shows '→ X.Y.Z'.""" card = ActionCard() - card.populate(_make_action()) - result = _make_result( + card.populate(make_action()) + result = make_result( success=True, skipped=False, available_version='1.2.0', @@ -694,9 +633,9 @@ def test_resolved_available_version_shows_arrow() -> None: def test_satisfied_with_constraint_shows_tooltip() -> None: """Satisfied action with constraint shows 'satisfies ...' tooltip.""" card = ActionCard() - action = _make_action(constraint='>=0.8.0') + action = make_action(constraint='>=0.8.0') card.populate(action) - result = _make_result( + result = make_result( skipped=True, skip_reason=SkipReason.ALREADY_INSTALLED, installed_version='0.9.1', @@ -709,9 +648,9 @@ def test_satisfied_with_constraint_shows_tooltip() -> None: def test_satisfied_without_constraint_no_tooltip() -> None: """Satisfied action without constraint has no version tooltip.""" card = ActionCard() - action = _make_action() + action = make_action() card.populate(action) - result = _make_result( + result = make_result( skipped=True, skip_reason=SkipReason.ALREADY_INSTALLED, installed_version='0.9.1', @@ -774,7 +713,7 @@ class TestActionCardCommandLabel: def test_default_package_command() -> None: """Package actions show 'installer install package' by default.""" card = ActionCard() - action = _make_action(package='ruff', installer='pip') + action = make_action(package='ruff', installer='pip') card.populate(action) assert card._command_label.text() == 'pip install ruff' assert not card._command_row.isHidden() @@ -783,11 +722,11 @@ def test_default_package_command() -> None: def test_explicit_cli_command_from_result() -> None: """set_check_result with cli_command updates the command label.""" card = ActionCard() - action = _make_action(package='ruff', installer='pip') + action = make_action(package='ruff', installer='pip') card.populate(action) assert card._command_label.text() == 'pip install ruff' - result = _make_result( + result = make_result( action=action, cli_command=('uv', 'tool', 'install', 'ruff'), ) @@ -798,7 +737,7 @@ def test_explicit_cli_command_from_result() -> None: def test_command_label_selectable() -> None: """Command label text is selectable by mouse.""" card = ActionCard() - action = _make_action() + action = make_action() card.populate(action) flags = card._command_label.textInteractionFlags() assert flags & Qt.TextInteractionFlag.TextSelectableByMouse @@ -807,11 +746,11 @@ def test_command_label_selectable() -> None: def test_set_check_result_updates_command_label() -> None: """set_check_result with result cli_command updates the command label.""" card = ActionCard() - action = _make_action(package='ruff', installer='pip') + action = make_action(package='ruff', installer='pip') card.populate(action) assert card._command_label.text() == 'pip install ruff' - result = _make_result( + result = make_result( action=action, cli_command=('uv', 'tool', 'install', 'ruff'), ) @@ -823,7 +762,7 @@ def test_set_check_result_updates_command_label() -> None: def test_copy_button_copies_command(monkeypatch: object) -> None: """Clicking the copy button copies the command text to the clipboard.""" card = ActionCard() - action = _make_action( + action = make_action( package='ruff', installer='uv', command=('uv', 'tool', 'install', 'ruff'), @@ -840,7 +779,7 @@ def test_copy_button_copies_command(monkeypatch: object) -> None: def test_copy_button_shows_feedback() -> None: """Clicking copy shows a check-mark on the button.""" card = ActionCard() - action = _make_action( + action = make_action( package='ruff', installer='uv', command=('uv', 'tool', 'install', 'ruff'), @@ -863,7 +802,7 @@ class TestActionCardSpinner: def test_spinner_active_during_checking() -> None: """Spinner timer is active while card is checking.""" card = ActionCard() - card.populate(_make_action()) + card.populate(make_action()) assert card._checking assert card._spinner_timer.isActive() @@ -871,9 +810,9 @@ def test_spinner_active_during_checking() -> None: def test_spinner_stops_on_check_result() -> None: """set_check_result stops the spinner.""" card = ActionCard() - card.populate(_make_action()) + card.populate(make_action()) assert card._checking - _check(card, _make_result()) + _check(card, make_result()) assert not card._checking assert not card._spinner_timer.isActive() assert card._spinner_canvas.isHidden() @@ -882,7 +821,7 @@ def test_spinner_stops_on_check_result() -> None: def test_spinner_stops_on_finalize() -> None: """finalize_checking stops the spinner.""" card = ActionCard() - card.populate(_make_action()) + card.populate(make_action()) assert card._checking card.finalize_checking() assert not card._checking @@ -892,7 +831,7 @@ def test_spinner_stops_on_finalize() -> None: def test_spinner_stops_on_executing() -> None: """set_executing stops the spinner if still checking.""" card = ActionCard() - card.populate(_make_action()) + card.populate(make_action()) assert card._checking card.set_executing() assert not card._checking @@ -902,7 +841,7 @@ def test_spinner_stops_on_executing() -> None: def test_no_spinner_for_unavailable() -> None: """Unavailable (plugin missing) actions don't spin.""" card = ActionCard() - card.populate(_make_action(installer='uv'), plugin_installed={'uv': False}) + card.populate(make_action(installer='uv'), plugin_installed={'uv': False}) assert not card._checking assert not card._spinner_timer.isActive() @@ -918,29 +857,29 @@ class TestActionSortKey: @staticmethod def test_runtime_before_package() -> None: """Runtime actions sort before packages.""" - runtime = _make_action(kind=PluginKind.RUNTIME, package='python') - package = _make_action(kind=PluginKind.PACKAGE, package='numpy') + runtime = make_action(kind=PluginKind.RUNTIME, package='python') + package = make_action(kind=PluginKind.PACKAGE, package='numpy') assert action_sort_key(runtime) < action_sort_key(package) @staticmethod def test_tool_before_scm() -> None: """Tool actions sort before SCM (matches execution phase order).""" - tool = _make_action(kind=PluginKind.TOOL, package='ruff') - scm = _make_action(kind=PluginKind.SCM, package='git') + tool = make_action(kind=PluginKind.TOOL, package='ruff') + scm = make_action(kind=PluginKind.SCM, package='git') assert action_sort_key(tool) < action_sort_key(scm) @staticmethod def test_package_before_tool() -> None: """Package actions sort before tools (matches execution phase order).""" - package = _make_action(kind=PluginKind.PACKAGE, package='numpy') - tool = _make_action(kind=PluginKind.TOOL, package='ruff') + package = make_action(kind=PluginKind.PACKAGE, package='numpy') + tool = make_action(kind=PluginKind.TOOL, package='ruff') assert action_sort_key(package) < action_sort_key(tool) @staticmethod def test_same_kind_returns_equal_key() -> None: """Same-kind actions get equal sort keys so stable sort preserves order.""" - alpha = _make_action(package='alpha') - beta = _make_action(package='beta') + alpha = make_action(package='alpha') + beta = make_action(package='beta') assert action_sort_key(alpha) == action_sort_key(beta) @@ -956,10 +895,10 @@ class TestActionCardListOrdering: def test_cards_grouped_by_kind_preserving_order() -> None: """Cards are grouped by execution phase, preserving porringer order within.""" card_list = ActionCardList() - a_pkg_b = _make_action(kind=PluginKind.PACKAGE, package='beta') - a_tool = _make_action(kind=PluginKind.TOOL, package='ruff') - a_pkg_a = _make_action(kind=PluginKind.PACKAGE, package='alpha') - a_runtime = _make_action(kind=PluginKind.RUNTIME, package='python') + a_pkg_b = make_action(kind=PluginKind.PACKAGE, package='beta') + a_tool = make_action(kind=PluginKind.TOOL, package='ruff') + a_pkg_a = make_action(kind=PluginKind.PACKAGE, package='alpha') + a_runtime = make_action(kind=PluginKind.RUNTIME, package='python') # Populate in porringer's execution order card_list.populate([a_pkg_b, a_tool, a_pkg_a, a_runtime]) @@ -978,8 +917,8 @@ def test_cards_grouped_by_kind_preserving_order() -> None: def test_bare_commands_included() -> None: """Actions with kind=None are included in the card list.""" card_list = ActionCardList() - pkg = _make_action(kind=PluginKind.PACKAGE, package='requests') - cmd = _make_action(kind=None, package='run-something') + pkg = make_action(kind=PluginKind.PACKAGE, package='requests') + cmd = make_action(kind=None, package='run-something') actions = [pkg, cmd] card_list.populate(actions) assert card_list.card_count() == len(actions) @@ -988,9 +927,9 @@ def test_bare_commands_included() -> None: def test_bare_commands_sort_last() -> None: """Bare-command actions sort after all PluginKind phases.""" card_list = ActionCardList() - cmd = _make_action(kind=None, package='post-cmd') - a_pkg = _make_action(kind=PluginKind.PACKAGE, package='requests') - a_scm = _make_action(kind=PluginKind.SCM, package='my-repo') + cmd = make_action(kind=None, package='post-cmd') + a_pkg = make_action(kind=PluginKind.PACKAGE, package='requests') + a_scm = make_action(kind=PluginKind.SCM, package='my-repo') actions = [cmd, a_pkg, a_scm] card_list.populate(actions) @@ -1010,7 +949,7 @@ def test_bare_commands_sort_last() -> None: def test_bare_command_shows_pending_status() -> None: """Bare-command card shows 'Pending' status with the pending style.""" card = ActionCard() - card.populate(_make_action(kind=None, package='echo-hello')) + card.populate(make_action(kind=None, package='echo-hello')) assert card.status_text() == 'Pending' assert ACTION_CARD_STATUS_PENDING in card._status_label.styleSheet() @@ -1027,10 +966,10 @@ class TestActionCardAlreadyLatest: def test_already_latest_shows_satisfied_style() -> None: """ALREADY_LATEST check result uses the satisfied style.""" card = ActionCard() - card.populate(_make_action()) + card.populate(make_action()) _check( card, - _make_result( + make_result( skipped=True, skip_reason=SkipReason.ALREADY_LATEST, ), @@ -1042,10 +981,10 @@ def test_already_latest_shows_satisfied_style() -> None: def test_already_latest_shows_version() -> None: """ALREADY_LATEST preserves the installed version label.""" card = ActionCard() - card.populate(_make_action()) + card.populate(make_action()) _check( card, - _make_result( + make_result( skipped=True, skip_reason=SkipReason.ALREADY_LATEST, installed_version='1.2.0', @@ -1057,20 +996,20 @@ def test_already_latest_shows_version() -> None: def test_already_installed_vs_already_latest() -> None: """ALREADY_INSTALLED and ALREADY_LATEST both use satisfied style.""" card_installed = ActionCard() - card_installed.populate(_make_action(package='a')) + card_installed.populate(make_action(package='a')) _check( card_installed, - _make_result( + make_result( skipped=True, skip_reason=SkipReason.ALREADY_INSTALLED, ), ) card_latest = ActionCard() - card_latest.populate(_make_action(package='b')) + card_latest.populate(make_action(package='b')) _check( card_latest, - _make_result( + make_result( skipped=True, skip_reason=SkipReason.ALREADY_LATEST, ), diff --git a/tests/unit/qt/test_gather_packages.py b/tests/unit/qt/test_gather_packages.py index 0c044b9..b60efd7 100644 --- a/tests/unit/qt/test_gather_packages.py +++ b/tests/unit/qt/test_gather_packages.py @@ -12,7 +12,6 @@ from porringer.schema.plugin import PluginInfo, PluginKind, RuntimePackageResult from PySide6.QtWidgets import QLabel, QPushButton -from synodic_client.application.config_store import ConfigStore from synodic_client.application.screen.plugin_row import ( FilterChip, PluginKindHeader, @@ -28,7 +27,8 @@ PLUGIN_PROVIDER_RUNTIME_TAG_DEFAULT_STYLE, PLUGIN_PROVIDER_RUNTIME_TAG_STYLE, ) -from synodic_client.resolution import ResolvedConfig + +from .conftest import make_config_store, make_mock_porringer # Named constants for expected counts (avoids PLR2004) _EXPECTED_PROJECT_INSTANCES = 2 @@ -36,37 +36,6 @@ _EXPECTED_VISIBLE_ROWS_PIPX = 2 -def _make_config() -> ResolvedConfig: - """Build a minimal ResolvedConfig for tests.""" - return ResolvedConfig( - update_source=None, - update_channel='stable', - auto_update_interval_minutes=60, - tool_update_interval_minutes=60, - plugin_auto_update=None, - prerelease_packages=None, - auto_apply=True, - auto_start=False, - debug_logging=False, - last_client_update=None, - last_tool_updates=None, - ) - - -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() - mock.plugin.list = AsyncMock(return_value=[]) - mock.package.list = AsyncMock(return_value=[]) - mock.cache.list_directories.return_value = [] - return mock - - # --------------------------------------------------------------------------- # _gather_packages # --------------------------------------------------------------------------- @@ -78,7 +47,7 @@ class TestGatherPackages: @staticmethod def test_global_query_returns_packages_with_no_directories() -> None: """Packages from the global query appear even when no directories are cached.""" - porringer = _make_porringer() + porringer = make_mock_porringer() porringer.package.list = AsyncMock( return_value=[ Package(name='pdm', version='2.22.4'), @@ -86,7 +55,7 @@ def test_global_query_returns_packages_with_no_directories() -> None: ], ) - view = ToolsView(porringer, _make_store()) + view = ToolsView(porringer, make_config_store()) result = asyncio.run(view._gather_packages('pipx', [])) assert {e.name for e in result} == {'pdm', 'cppython'} @@ -94,14 +63,14 @@ def test_global_query_returns_packages_with_no_directories() -> None: @staticmethod def test_global_query_returns_packages_with_empty_project_path() -> None: """Packages from the global query should have an empty project_path.""" - porringer = _make_porringer() + porringer = make_mock_porringer() porringer.package.list = AsyncMock( return_value=[ Package(name='pdm', version='2.22.4'), ], ) - view = ToolsView(porringer, _make_store()) + view = ToolsView(porringer, make_config_store()) result = asyncio.run(view._gather_packages('pipx', [])) matching = [e for e in result if e.name == 'pdm'] @@ -111,10 +80,10 @@ def test_global_query_returns_packages_with_empty_project_path() -> None: @staticmethod def test_global_query_called_without_project_path() -> None: """The global query must call list_packages with no project_path arg.""" - porringer = _make_porringer() + porringer = make_mock_porringer() porringer.package.list = AsyncMock(return_value=[]) - view = ToolsView(porringer, _make_store()) + view = ToolsView(porringer, make_config_store()) asyncio.run(view._gather_packages('pipx', [])) # At least one call should have been made with only plugin_name (no path) @@ -131,7 +100,7 @@ def test_global_query_called_without_project_path() -> None: @staticmethod def test_per_directory_queries_still_work() -> None: """Per-directory queries continue to run alongside the global query.""" - porringer = _make_porringer() + porringer = make_mock_porringer() async def _mock_list(plugin_name: str, project_path: Path | None = None, **kwargs) -> list[Package]: if project_path is None: @@ -141,7 +110,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_store()) + view = ToolsView(porringer, make_config_store()) result = asyncio.run(view._gather_packages('pipx', [directory])) names = {entry.name for entry in result} @@ -151,7 +120,7 @@ async def _mock_list(plugin_name: str, project_path: Path | None = None, **kwarg @staticmethod def test_per_directory_packages_carry_project_path() -> None: """Per-directory packages should include the directory path as project_path.""" - porringer = _make_porringer() + porringer = make_mock_porringer() async def _mock_list(plugin_name: str, project_path: Path | None = None, **kwargs) -> list[Package]: if project_path is None: @@ -161,7 +130,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_store()) + view = ToolsView(porringer, make_config_store()) result = asyncio.run(view._gather_packages('pipx', [directory])) matching = [e for e in result if e.name == 'mylib'] @@ -171,12 +140,12 @@ async def _mock_list(plugin_name: str, project_path: Path | None = None, **kwarg @staticmethod def test_global_packages_have_empty_project_label() -> None: """Packages from the global query should have an empty project label.""" - porringer = _make_porringer() + porringer = make_mock_porringer() porringer.package.list = AsyncMock( return_value=[Package(name='cppython', version='0.5.0')], ) - view = ToolsView(porringer, _make_store()) + view = ToolsView(porringer, make_config_store()) result = asyncio.run(view._gather_packages('pipx', [])) matching = [e for e in result if e.name == 'cppython'] @@ -187,7 +156,7 @@ def test_global_packages_have_empty_project_label() -> None: @staticmethod def test_global_query_failure_does_not_block_directory_queries() -> None: """If the global query fails, per-directory results still come through.""" - porringer = _make_porringer() + porringer = make_mock_porringer() call_count = 0 async def _mock_list(plugin_name: str, project_path: Path | None = None, **kwargs) -> list[Package]: @@ -200,7 +169,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_store()) + view = ToolsView(porringer, make_config_store()) result = asyncio.run(view._gather_packages('pipx', [directory])) names = {entry.name for entry in result} @@ -211,7 +180,7 @@ async def _mock_list(plugin_name: str, project_path: Path | None = None, **kwarg @staticmethod def test_relation_host_extracted_into_host_tool() -> None: """Packages with a PackageRelation populate the host_tool element.""" - porringer = _make_porringer() + porringer = make_mock_porringer() porringer.package.list = AsyncMock( return_value=[ Package( @@ -226,7 +195,7 @@ def test_relation_host_extracted_into_host_tool() -> None: ], ) - view = ToolsView(porringer, _make_store()) + view = ToolsView(porringer, make_config_store()) result = asyncio.run(view._gather_packages('pipx', [])) by_name = {entry.name: entry.host_tool for entry in result} @@ -245,8 +214,8 @@ class TestGatherToolPlugins: @staticmethod 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_store()) + porringer = make_mock_porringer() + view = ToolsView(porringer, make_config_store()) # Mock _discover_plugin_managers to return a fake manager mock_manager = MagicMock() @@ -278,8 +247,8 @@ def test_returns_plugins_keyed_by_host_tool(monkeypatch) -> None: @staticmethod def test_empty_when_no_managers(monkeypatch) -> None: """Returns empty dict when no PluginManager instances are discovered.""" - porringer = _make_porringer() - view = ToolsView(porringer, _make_store()) + porringer = make_mock_porringer() + view = ToolsView(porringer, make_config_store()) monkeypatch.setattr( ToolsView, '_discover_plugin_managers', @@ -292,8 +261,8 @@ def test_empty_when_no_managers(monkeypatch) -> None: @staticmethod 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_store()) + porringer = make_mock_porringer() + view = ToolsView(porringer, make_config_store()) mock_manager = MagicMock() mock_manager.installed_plugins = AsyncMock(side_effect=RuntimeError('boom')) @@ -310,8 +279,8 @@ def test_manager_failure_does_not_propagate(monkeypatch) -> None: @staticmethod def test_multiple_managers(monkeypatch) -> None: """Multiple PluginManager instances are queried in parallel.""" - porringer = _make_porringer() - view = ToolsView(porringer, _make_store()) + porringer = make_mock_porringer() + view = ToolsView(porringer, make_config_store()) mgr_pdm = MagicMock() mgr_pdm.installed_plugins = AsyncMock( @@ -592,9 +561,8 @@ class TestSearchFilter: @staticmethod def _make_view() -> ToolsView: """Build a ToolsView with a mock porringer.""" - porringer = _make_porringer() - config = _make_config() - return ToolsView(porringer, _make_store(config)) + porringer = make_mock_porringer() + return ToolsView(porringer, make_config_store()) @staticmethod def _populate_section_widgets(view: ToolsView) -> None: @@ -759,9 +727,8 @@ class TestFilterPanel: @staticmethod def _make_view() -> ToolsView: """Build a ToolsView with a mock porringer.""" - porringer = _make_porringer() - config = _make_config() - return ToolsView(porringer, _make_store(config)) + porringer = make_mock_porringer() + return ToolsView(porringer, make_config_store()) def test_panel_starts_collapsed(self) -> None: """The filter panel is hidden and has zero max-height on init.""" @@ -874,7 +841,7 @@ def test_runtime_excluded_when_empty() -> None: @staticmethod def test_runtime_skips_per_directory_queries() -> None: """RUNTIME plugins only get a global query, not per-directory queries.""" - porringer = _make_porringer() + porringer = make_mock_porringer() call_paths: list[Path | None] = [] @@ -896,7 +863,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_store()) + view = ToolsView(porringer, make_config_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', [])) @@ -968,7 +935,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_store()) + view = ToolsView(make_mock_porringer(), make_config_store()) default_exe = Path('C:/Python314/python.exe') plugin = self._pip_plugin() rt_results = self._make_runtime_results(default_exe) @@ -988,7 +955,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_store()) + view = ToolsView(make_mock_porringer(), make_config_store()) default_exe = Path('C:/Python314/python.exe') plugin = self._pip_plugin() rt_results = self._make_runtime_results(default_exe) @@ -1010,7 +977,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_store()) + view = ToolsView(make_mock_porringer(), make_config_store()) default_exe = Path('C:/Python314/python.exe') plugin = self._pip_plugin() rt_results = self._make_runtime_results(default_exe) @@ -1033,7 +1000,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_store()) + view = ToolsView(make_mock_porringer(), make_config_store()) default_exe = Path('C:/Python314/python.exe') plugin = self._pip_plugin() rt_results = self._make_runtime_results(default_exe) @@ -1055,7 +1022,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_store()) + view = ToolsView(make_mock_porringer(), make_config_store()) default_exe = Path('C:/Python314/python.exe') plugin = self._pip_plugin() rt_results = self._make_runtime_results(default_exe) @@ -1086,7 +1053,7 @@ def test_venv_packages_excluded_for_runtime_probed_plugin(self) -> None: def test_runtime_tag_uses_default_style(self) -> None: """The default runtime tag uses the green highlight style.""" - view = ToolsView(_make_porringer(), _make_store()) + view = ToolsView(make_mock_porringer(), make_config_store()) default_exe = Path('C:/Python314/python.exe') plugin = self._pip_plugin() rt_results = self._make_runtime_results(default_exe) @@ -1109,7 +1076,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_store()) + view = ToolsView(make_mock_porringer(), make_config_store()) default_exe = Path('C:/Python314/python.exe') plugin = self._pip_plugin() rt_results = self._make_runtime_results(default_exe) @@ -1133,7 +1100,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_store()) + view = ToolsView(make_mock_porringer(), make_config_store()) default_exe = Path('C:/Python314/python.exe') plugin = self._pip_plugin() rt_results = self._make_runtime_results(default_exe) @@ -1159,19 +1126,19 @@ def test_filter_chips_work_with_runtime_providers(self) -> None: @staticmethod def test_gather_runtime_packages_returns_none_for_non_consumer() -> None: """_gather_runtime_packages returns None when plugin is not a RuntimeConsumer.""" - porringer = _make_porringer() + porringer = make_mock_porringer() porringer.package.list_by_runtime = AsyncMock( return_value=None, ) - view = ToolsView(porringer, _make_store()) + view = ToolsView(porringer, make_config_store()) result = asyncio.run(view._gather_runtime_packages('pipx', MagicMock())) assert result is None @staticmethod def test_gather_runtime_packages_returns_results() -> None: """_gather_runtime_packages returns list on success.""" - porringer = _make_porringer() + porringer = make_mock_porringer() expected = [ RuntimePackageResult( provider='pim', @@ -1182,14 +1149,14 @@ def test_gather_runtime_packages_returns_results() -> None: ] porringer.package.list_by_runtime = AsyncMock(return_value=expected) - view = ToolsView(porringer, _make_store()) + view = ToolsView(porringer, make_config_store()) result = asyncio.run(view._gather_runtime_packages('pip', MagicMock())) assert result == expected @staticmethod def test_skip_global_flag_skips_global_query() -> None: """_gather_packages with skip_global=True only runs per-directory queries.""" - porringer = _make_porringer() + porringer = make_mock_porringer() call_paths: list[Path | None] = [] async def _mock_list(plugin_name: str, project_path: Path | None = None, **kwargs) -> list[Package]: @@ -1201,7 +1168,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_store()) + view = ToolsView(porringer, make_config_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_log_panel.py b/tests/unit/qt/test_log_panel.py index 2c83a1c..2b7eb1a 100644 --- a/tests/unit/qt/test_log_panel.py +++ b/tests/unit/qt/test_log_panel.py @@ -10,14 +10,11 @@ ActionCompletedEvent, ActionStartedEvent, ManifestLoadedEvent, - SetupAction, - SetupActionResult, SetupResults, SkipReason, SubActionProgress, SubActionProgressEvent, ) -from porringer.schema.plugin import PluginKind from synodic_client.application.screen.install_workers import run_install from synodic_client.application.screen.log_panel import ( @@ -39,50 +36,9 @@ LOG_STATUS_SUCCESS, ) -_EXPECTED_SECTION_COUNT = 2 - -# --------------------------------------------------------------------------- -# Helpers -# --------------------------------------------------------------------------- +from .conftest import make_action, make_result - -def _make_action( - kind: PluginKind = PluginKind.PACKAGE, - description: str = 'Install requests', - installer: str = 'pip', - package: str = 'requests', - package_description: str | None = None, -) -> SetupAction: - """Create a mock SetupAction with sensible defaults.""" - action = MagicMock(spec=SetupAction) - action.kind = kind - action.description = description - action.installer = installer - pkg_mock = MagicMock() - pkg_mock.name = package - action.package = pkg_mock - action.package_description = package_description or description - action.command = None - action.plugin_target = None - return action - - -def _make_result( - action: SetupAction | None = None, - *, - success: bool = True, - skipped: bool = False, - skip_reason: SkipReason | None = None, - message: str | None = None, -) -> SetupActionResult: - """Create a SetupActionResult.""" - return SetupActionResult( - action=action or _make_action(), - success=success, - skipped=skipped, - skip_reason=skip_reason, - message=message, - ) +_EXPECTED_SECTION_COUNT = 2 def _output_html(section: ActionLogSection) -> str: @@ -101,7 +57,7 @@ class TestActionLogSection: @staticmethod def test_initial_status_shows_running() -> None: """Section header starts with 'Running…' status.""" - action = _make_action() + action = make_action() section = ActionLogSection(action, index=1) assert section._status_label.text() == 'Running…' assert LOG_STATUS_RUNNING in section._status_label.styleSheet() @@ -109,7 +65,7 @@ def test_initial_status_shows_running() -> None: @staticmethod def test_initial_chevron_is_down() -> None: """Section starts expanded with the down-pointing chevron.""" - action = _make_action() + action = make_action() section = ActionLogSection(action, index=1) assert section._chevron.text() == CHEVRON_DOWN assert section._expanded is True @@ -118,7 +74,7 @@ def test_initial_chevron_is_down() -> None: @staticmethod def test_toggle_collapses_and_expands() -> None: """Toggling collapses the output, toggling again restores it.""" - action = _make_action() + action = make_action() section = ActionLogSection(action, index=1) section._toggle() @@ -134,7 +90,7 @@ def test_toggle_collapses_and_expands() -> None: @staticmethod def test_append_stdout_output() -> None: """Stdout lines are coloured with the stdout colour.""" - action = _make_action() + action = make_action() section = ActionLogSection(action, index=1) section.append_output('hello world', 'stdout') @@ -145,7 +101,7 @@ def test_append_stdout_output() -> None: @staticmethod def test_append_stderr_output() -> None: """Stderr lines use the stderr/amber colour.""" - action = _make_action() + action = make_action() section = ActionLogSection(action, index=1) section.append_output('warning: something', 'stderr') @@ -156,7 +112,7 @@ def test_append_stderr_output() -> None: @staticmethod def test_append_output_none_stream_uses_phase_colour() -> None: """Lines with stream=None use the phase/muted colour.""" - action = _make_action() + action = make_action() section = ActionLogSection(action, index=1) section.append_output('Verifying checksums', None) @@ -167,7 +123,7 @@ def test_append_output_none_stream_uses_phase_colour() -> None: @staticmethod def test_append_phase_uses_phase_colour() -> None: """Phase messages (stream=None) use the muted grey colour.""" - action = _make_action() + action = make_action() section = ActionLogSection(action, index=1) section.append_output('downloading', None) @@ -178,7 +134,7 @@ def test_append_phase_uses_phase_colour() -> None: @staticmethod def test_html_escaping_in_output() -> None: """Special HTML characters are escaped in output lines.""" - action = _make_action() + action = make_action() section = ActionLogSection(action, index=1) section.append_output('', 'stdout') @@ -190,7 +146,7 @@ def test_html_escaping_in_output() -> None: @staticmethod def test_html_escaping_in_phase() -> None: """Special HTML characters are escaped in phase messages.""" - action = _make_action() + action = make_action() section = ActionLogSection(action, index=1) section.append_output('Step <1> & "done"', None) @@ -201,9 +157,9 @@ def test_html_escaping_in_phase() -> None: @staticmethod def test_set_result_success() -> None: """Successful result sets 'Done' status with green colour.""" - action = _make_action() + action = make_action() section = ActionLogSection(action, index=1) - result = _make_result(action, success=True, message='Installed ruff-0.8.0') + result = make_result(action=action, success=True, message='Installed ruff-0.8.0') section.set_result(result) @@ -216,9 +172,9 @@ def test_set_result_success() -> None: @staticmethod def test_set_result_success_default_message() -> None: """Successful result without message shows default text.""" - action = _make_action() + action = make_action() section = ActionLogSection(action, index=1) - result = _make_result(action, success=True) + result = make_result(action=action, success=True) section.set_result(result) @@ -228,9 +184,9 @@ def test_set_result_success_default_message() -> None: @staticmethod def test_set_result_failure() -> None: """Failed result sets 'Failed' status with red colour.""" - action = _make_action() + action = make_action() section = ActionLogSection(action, index=1) - result = _make_result(action, success=False, message='Network timeout') + result = make_result(action=action, success=False, message='Network timeout') section.set_result(result) @@ -243,9 +199,9 @@ def test_set_result_failure() -> None: @staticmethod def test_set_result_failure_default_message() -> None: """Failed result without message shows 'Unknown error'.""" - action = _make_action() + action = make_action() section = ActionLogSection(action, index=1) - result = _make_result(action, success=False) + result = make_result(action=action, success=False) section.set_result(result) @@ -255,10 +211,10 @@ def test_set_result_failure_default_message() -> None: @staticmethod def test_set_result_skipped() -> None: """Skipped result sets the skip reason as status text.""" - action = _make_action() + action = make_action() section = ActionLogSection(action, index=1) - result = _make_result( - action, + result = make_result( + action=action, success=True, skipped=True, skip_reason=SkipReason.ALREADY_INSTALLED, @@ -274,7 +230,7 @@ def test_set_result_skipped() -> None: @staticmethod def test_multiple_output_lines_accumulate() -> None: """Multiple append_output calls accumulate in the text edit.""" - action = _make_action() + action = make_action() section = ActionLogSection(action, index=1) section.append_output('line 1', 'stdout') section.append_output('line 2', 'stderr') @@ -298,7 +254,7 @@ class TestExecutionLogPanel: def test_add_section_returns_section() -> None: """add_section returns an ActionLogSection widget.""" panel = ExecutionLogPanel() - action = _make_action() + action = make_action() section = panel.add_section(action) assert isinstance(section, ActionLogSection) @@ -306,8 +262,8 @@ def test_add_section_returns_section() -> None: def test_add_section_increments_index() -> None: """Section indices increment with each add_section call.""" panel = ExecutionLogPanel() - a1 = _make_action(description='First', package='first') - a2 = _make_action(description='Second', package='second') + a1 = make_action(description='First', package='first') + a2 = make_action(description='Second', package='second') panel.add_section(a1) panel.add_section(a2) @@ -318,7 +274,7 @@ def test_add_section_increments_index() -> None: def test_get_section_returns_correct_section() -> None: """get_section finds the section by the same action object.""" panel = ExecutionLogPanel() - action = _make_action() + action = make_action() expected = panel.add_section(action) found = panel.get_section(action) @@ -328,14 +284,14 @@ def test_get_section_returns_correct_section() -> None: def test_get_section_returns_none_for_unknown() -> None: """get_section returns None for actions not in the panel.""" panel = ExecutionLogPanel() - action = _make_action() + action = make_action() assert panel.get_section(action) is None @staticmethod def test_on_sub_progress_with_output() -> None: """on_sub_progress routes output lines to the correct section.""" panel = ExecutionLogPanel() - action = _make_action() + action = make_action() panel.add_section(action) progress = SubActionProgress( @@ -355,7 +311,7 @@ def test_on_sub_progress_with_output() -> None: def test_on_sub_progress_with_phase_message() -> None: """on_sub_progress routes phase messages when no output is set.""" panel = ExecutionLogPanel() - action = _make_action() + action = make_action() panel.add_section(action) progress = SubActionProgress( @@ -374,7 +330,7 @@ def test_on_sub_progress_with_phase_message() -> None: def test_on_sub_progress_ignores_unknown_action() -> None: """on_sub_progress does nothing when the action has no section.""" panel = ExecutionLogPanel() - action = _make_action() + action = make_action() progress = SubActionProgress(action=action, phase='installing') # Should not raise @@ -384,10 +340,10 @@ def test_on_sub_progress_ignores_unknown_action() -> None: def test_on_action_completed_updates_section() -> None: """on_action_completed calls set_result on the correct section.""" panel = ExecutionLogPanel() - action = _make_action() + action = make_action() panel.add_section(action) - result = _make_result(action, success=True, message='Done') + result = make_result(action=action, success=True, message='Done') panel.on_action_completed(action, result) section = panel.get_section(action) @@ -398,8 +354,8 @@ def test_on_action_completed_updates_section() -> None: def test_on_action_completed_ignores_unknown_action() -> None: """on_action_completed does nothing for unknown actions.""" panel = ExecutionLogPanel() - action = _make_action() - result = _make_result(action, success=True) + action = make_action() + result = make_result(action=action, success=True) # Should not raise panel.on_action_completed(action, result) @@ -408,8 +364,8 @@ def test_on_action_completed_ignores_unknown_action() -> None: def test_clear_removes_all_sections() -> None: """clear() removes all sections and resets the counter.""" panel = ExecutionLogPanel() - a1 = _make_action(description='First', package='first') - a2 = _make_action(description='Second', package='second') + a1 = make_action(description='First', package='first') + a2 = make_action(description='Second', package='second') panel.add_section(a1) panel.add_section(a2) @@ -424,8 +380,8 @@ def test_clear_removes_all_sections() -> None: def test_multiple_actions_tracked_independently() -> None: """Different actions get independent sections with separate output.""" panel = ExecutionLogPanel() - a1 = _make_action(description='First', package='first') - a2 = _make_action(description='Second', package='second') + a1 = make_action(description='First', package='first') + a2 = make_action(description='Second', package='second') panel.add_section(a1) panel.add_section(a2) @@ -462,12 +418,12 @@ def test_emits_action_started() -> None: porringer = MagicMock() manifest_path = Path('/tmp/test/porringer.json') - action = _make_action() + action = make_action() manifest = SetupResults(actions=[action]) manifest_event = ManifestLoadedEvent(manifest=manifest) started_event = ActionStartedEvent(action=action) - result = _make_result(action) + result = make_result(action=action) completed_event = ActionCompletedEvent( action=action, result=result, @@ -499,7 +455,7 @@ def test_emits_sub_progress() -> None: porringer = MagicMock() manifest_path = Path('/tmp/test/porringer.json') - action = _make_action() + action = make_action() manifest = SetupResults(actions=[action]) sub = SubActionProgress( action=action, @@ -513,7 +469,7 @@ def test_emits_sub_progress() -> None: action=action, sub_action=sub, ) - result = _make_result(action) + result = make_result(action=action) completed_event = ActionCompletedEvent( action=action, result=result, @@ -548,13 +504,13 @@ def test_non_sub_progress_events_ignored_by_sub_callback() -> None: porringer = MagicMock() manifest_path = Path('/tmp/test/porringer.json') - action = _make_action() + action = make_action() manifest = SetupResults(actions=[action]) manifest_event = ManifestLoadedEvent(manifest=manifest) # ActionStartedEvent should not trigger sub_progress callback started_event = ActionStartedEvent(action=action) - result = _make_result(action) + result = make_result(action=action) completed_event = ActionCompletedEvent( action=action, result=result, @@ -587,11 +543,11 @@ def test_non_started_events_ignored_by_started_callback() -> None: porringer = MagicMock() manifest_path = Path('/tmp/test/porringer.json') - action = _make_action() + action = make_action() manifest = SetupResults(actions=[action]) manifest_event = ManifestLoadedEvent(manifest=manifest) - result = _make_result(action) + result = make_result(action=action) completed_event = ActionCompletedEvent( action=action, result=result, diff --git a/tests/unit/qt/test_preview_model.py b/tests/unit/qt/test_preview_model.py index 8476103..d3af4c5 100644 --- a/tests/unit/qt/test_preview_model.py +++ b/tests/unit/qt/test_preview_model.py @@ -2,44 +2,11 @@ from __future__ import annotations -from typing import Any -from unittest.mock import MagicMock - -from porringer.schema import SetupAction -from porringer.schema.plugin import PluginKind - from synodic_client.application.screen.schema import ActionState, PreviewModel, PreviewPhase from synodic_client.application.uri import normalize_manifest_key from synodic_client.operations.schema import InstallPlan, SyncStrategy -# --------------------------------------------------------------------------- -# Helpers -# --------------------------------------------------------------------------- - - -def _make_action( - *, - kind: PluginKind | None = PluginKind.PACKAGE, - description: str = 'Install requests', - installer: str = 'pip', - package: str = 'requests', - **overrides: Any, -) -> SetupAction: - """Create a mock SetupAction with sensible defaults.""" - action = MagicMock(spec=SetupAction) - action.kind = kind - action.description = description - action.installer = installer - pkg_mock = MagicMock() - pkg_mock.name = package - pkg_mock.configure_mock(**{'__str__': MagicMock(return_value=package)}) - action.package = pkg_mock - action.package_description = overrides.get('package_description', description) - action.command = overrides.get('command') - action.include_prereleases = overrides.get('include_prereleases', False) - action.plugin_target = overrides.get('plugin_target') - return action - +from .conftest import make_action # --------------------------------------------------------------------------- # ActionState @@ -52,7 +19,7 @@ class TestActionState: @staticmethod def test_defaults() -> None: """Defaults should be sensible for a freshly-created state.""" - act = _make_action() + act = make_action() state = ActionState(action=act) assert state.status == 'Checking\u2026' assert state.log_lines == [] @@ -60,8 +27,8 @@ def test_defaults() -> None: @staticmethod def test_log_lines_are_independent() -> None: """Each ActionState should have its own independent log list.""" - a = ActionState(action=_make_action()) - b = ActionState(action=_make_action(package='ruff')) + a = ActionState(action=make_action()) + b = ActionState(action=make_action(package='ruff')) a.log_lines.append(('hello', None)) assert b.log_lines == [] @@ -91,7 +58,7 @@ def test_install_enabled_true_when_ready_with_needed_actions() -> None: """Install should be enabled when READY and install_plan says so.""" model = PreviewModel() model.phase = PreviewPhase.READY - state = ActionState(action=_make_action()) + state = ActionState(action=make_action()) state.status = 'Needed' model.action_states.append(state) model.install_plan = InstallPlan( @@ -114,7 +81,7 @@ def test_install_enabled_false_when_only_upgradable() -> None: """ model = PreviewModel() model.phase = PreviewPhase.READY - state = ActionState(action=_make_action()) + state = ActionState(action=make_action()) state.status = 'Update available' model.action_states.append(state) model.install_plan = InstallPlan( @@ -134,7 +101,7 @@ def test_install_enabled_false_when_ready_but_all_satisfied() -> None: """Install should be disabled when all actions are satisfied.""" model = PreviewModel() model.phase = PreviewPhase.READY - state = ActionState(action=_make_action()) + state = ActionState(action=make_action()) state.status = 'Already installed' model.action_states.append(state) assert model.install_enabled is False @@ -144,7 +111,7 @@ def test_has_post_sync_for_command_actions() -> None: """Command actions (kind=None) are tracked as post-sync.""" model = PreviewModel() model.phase = PreviewPhase.READY - state = ActionState(action=_make_action(kind=None, description='Run setup')) + state = ActionState(action=make_action(kind=None, description='Run setup')) state.status = 'Pending' model.action_states.append(state) model.install_plan = InstallPlan( @@ -165,7 +132,7 @@ def test_install_enabled_false_when_installing() -> None: """Install should be disabled during installation.""" model = PreviewModel() model.phase = PreviewPhase.INSTALLING - state = ActionState(action=_make_action()) + state = ActionState(action=make_action()) state.status = 'Needed' model.action_states.append(state) assert model.install_enabled is False @@ -175,11 +142,11 @@ def test_install_plan_indices_partition() -> None: """Install plan correctly partitions actions.""" model = PreviewModel() model.phase = PreviewPhase.READY - needed = ActionState(action=_make_action(package='a')) + needed = ActionState(action=make_action(package='a')) needed.status = 'Needed' - satisfied = ActionState(action=_make_action(package='b')) + satisfied = ActionState(action=make_action(package='b')) satisfied.status = 'Already installed' - upgradable = ActionState(action=_make_action(package='c')) + upgradable = ActionState(action=make_action(package='c')) upgradable.status = 'Update available' model.action_states = [needed, satisfied, upgradable] model.install_plan = InstallPlan( @@ -201,7 +168,7 @@ def test_install_plan_indices_partition() -> None: def test_action_state_for_found() -> None: """action_state_for should find a state by matching action key.""" model = PreviewModel() - act = _make_action(package='ruff') + act = make_action(package='ruff') state = ActionState(action=act) model.action_states.append(state) found = model.action_state_for(act) @@ -211,7 +178,7 @@ def test_action_state_for_found() -> None: def test_action_state_for_not_found() -> None: """action_state_for should return None for unknown actions.""" model = PreviewModel() - act = _make_action(package='ruff') + act = make_action(package='ruff') assert model.action_state_for(act) is None @staticmethod diff --git a/tests/unit/qt/test_settings.py b/tests/unit/qt/test_settings.py index 2fe16c9..7cf17c8 100644 --- a/tests/unit/qt/test_settings.py +++ b/tests/unit/qt/test_settings.py @@ -2,48 +2,20 @@ from __future__ import annotations -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.application.update_model import UpdateModel from synodic_client.resolution import ResolvedConfig from synodic_client.schema import DEFAULT_AUTO_UPDATE_INTERVAL_MINUTES, DEFAULT_TOOL_UPDATE_INTERVAL_MINUTES -# --------------------------------------------------------------------------- -# Helpers -# --------------------------------------------------------------------------- - - -def _make_config(**overrides: Any) -> ResolvedConfig: - """Create a ``ResolvedConfig`` with sensible defaults and optional overrides.""" - defaults: dict[str, Any] = { - '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, - 'prerelease_packages': None, - 'auto_apply': True, - 'auto_start': True, - 'debug_logging': False, - 'last_client_update': None, - 'last_tool_updates': None, - } - defaults.update(overrides) - return ResolvedConfig(**defaults) - - -def _make_store(config: ResolvedConfig | None = None) -> ConfigStore: - """Create a ``ConfigStore`` seeded with *config*.""" - return ConfigStore(config or _make_config()) +from .conftest import make_config_store, make_resolved_config def _make_window(config: ResolvedConfig | None = None, version: str = '0.0.0.test') -> SettingsWindow: """Create a ``SettingsWindow`` without showing it.""" - store = _make_store(config) + store = make_config_store(config) window = SettingsWindow(store, version=version) return window @@ -87,7 +59,7 @@ class TestSyncFromConfig: @staticmethod def test_channel_stable_default() -> None: """Default config selects the Stable channel.""" - window = _make_window(_make_config()) + window = _make_window(make_resolved_config()) window.sync_from_config() assert window._channel_combo.currentIndex() == 0 assert window._channel_combo.currentText() == 'Stable' @@ -95,7 +67,7 @@ def test_channel_stable_default() -> None: @staticmethod def test_channel_dev() -> None: """Config with update_channel='dev' selects Development.""" - window = _make_window(_make_config(update_channel='dev')) + window = _make_window(make_resolved_config(update_channel='dev')) window.sync_from_config() assert window._channel_combo.currentIndex() == 1 assert window._channel_combo.currentText() == 'Development' @@ -103,21 +75,21 @@ def test_channel_dev() -> None: @staticmethod def test_update_source_blank() -> None: """Default config leaves the source field empty.""" - window = _make_window(_make_config()) + window = _make_window(make_resolved_config()) window.sync_from_config() assert not window._source_edit.text() @staticmethod def test_update_source_set() -> None: """Custom update source is reflected in the line edit.""" - window = _make_window(_make_config(update_source='https://example.com')) + window = _make_window(make_resolved_config(update_source='https://example.com')) window.sync_from_config() assert window._source_edit.text() == 'https://example.com' @staticmethod def test_auto_update_interval_default() -> None: """Default auto-update interval shows the module default.""" - window = _make_window(_make_config()) + window = _make_window(make_resolved_config()) window.sync_from_config() assert window._auto_update_spin.value() == DEFAULT_AUTO_UPDATE_INTERVAL_MINUTES @@ -125,14 +97,14 @@ def test_auto_update_interval_default() -> None: def test_auto_update_interval_custom() -> None: """Custom auto-update interval is shown in the spinbox.""" custom_interval = 60 - window = _make_window(_make_config(auto_update_interval_minutes=custom_interval)) + window = _make_window(make_resolved_config(auto_update_interval_minutes=custom_interval)) window.sync_from_config() assert window._auto_update_spin.value() == custom_interval @staticmethod def test_tool_update_interval_default() -> None: """Default tool-update interval shows the module default.""" - window = _make_window(_make_config()) + window = _make_window(make_resolved_config()) window.sync_from_config() assert window._tool_update_spin.value() == DEFAULT_TOOL_UPDATE_INTERVAL_MINUTES @@ -140,14 +112,14 @@ def test_tool_update_interval_default() -> None: def test_tool_update_interval_custom() -> None: """Custom tool-update interval is shown in the spinbox.""" custom_interval = 45 - window = _make_window(_make_config(tool_update_interval_minutes=custom_interval)) + window = _make_window(make_resolved_config(tool_update_interval_minutes=custom_interval)) window.sync_from_config() assert window._tool_update_spin.value() == custom_interval @staticmethod def test_auto_start_reflects_registry() -> None: """Auto-start checkbox mirrors the OS registration state.""" - window = _make_window(_make_config()) + window = _make_window(make_resolved_config()) with patch('synodic_client.application.screen.settings.is_startup_registered', return_value=True): window.sync_from_config() assert window._auto_start_check.isChecked() is True @@ -164,10 +136,10 @@ class TestSettingsCallbacks: @staticmethod def test_channel_change_to_dev() -> None: """Switching to dev calls store.update with update_channel='dev'.""" - config = _make_config() + config = make_resolved_config() window = _make_window(config) - new_config = _make_config(update_channel='dev') + new_config = make_resolved_config(update_channel='dev') with patch.object(window._store, 'update', return_value=new_config) as mock_update: window._channel_combo.setCurrentIndex(1) @@ -176,11 +148,11 @@ def test_channel_change_to_dev() -> None: @staticmethod def test_channel_change_to_stable() -> None: """Switching from dev to stable persists 'stable'.""" - config = _make_config(update_channel='dev') + config = make_resolved_config(update_channel='dev') window = _make_window(config) window.sync_from_config() - new_config = _make_config(update_channel='stable') + new_config = make_resolved_config(update_channel='stable') with patch.object(window._store, 'update', return_value=new_config) as mock_update: window._channel_combo.setCurrentIndex(0) @@ -189,10 +161,10 @@ def test_channel_change_to_stable() -> None: @staticmethod def test_source_change() -> None: """Editing the source saves via the store.""" - config = _make_config() + config = make_resolved_config() window = _make_window(config) - new_config = _make_config(update_source='https://custom.example.com') + new_config = make_resolved_config(update_source='https://custom.example.com') 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() @@ -202,10 +174,10 @@ def test_source_change() -> None: @staticmethod def test_source_blank_sets_none() -> None: """Clearing the source field stores None.""" - config = _make_config(update_source='https://old.example.com') + config = make_resolved_config(update_source='https://old.example.com') window = _make_window(config) - new_config = _make_config(update_source=None) + new_config = make_resolved_config(update_source=None) with patch.object(window._store, 'update', return_value=new_config) as mock_update: window._source_edit.setText('') window._on_source_changed() @@ -216,7 +188,7 @@ def test_source_blank_sets_none() -> None: def test_auto_update_interval_change() -> None: """Changing auto-update interval saves via the store.""" new_interval = 90 - config = _make_config() + config = make_resolved_config() window = _make_window(config) with patch.object(window._store, 'update') as mock_update: @@ -228,7 +200,7 @@ def test_auto_update_interval_change() -> None: def test_tool_update_interval_change() -> None: """Changing tool-update interval saves via the store.""" new_interval = 120 - config = _make_config() + config = make_resolved_config() window = _make_window(config) with patch.object(window._store, 'update') as mock_update: @@ -239,10 +211,10 @@ def test_tool_update_interval_change() -> None: @staticmethod def test_auto_start_registers_startup_when_frozen() -> None: """Enabling auto-start calls sync_startup.""" - config = _make_config() + config = make_resolved_config() window = _make_window(config) - new_config = _make_config(auto_start=True) + new_config = make_resolved_config(auto_start=True) with ( patch.object(window._store, 'update', return_value=new_config), patch('synodic_client.application.screen.settings.sync_startup') as mock_sync, @@ -255,14 +227,14 @@ def test_auto_start_registers_startup_when_frozen() -> None: @staticmethod def test_auto_start_removes_startup_when_frozen() -> None: """Disabling auto-start calls sync_startup with auto_start=False.""" - config = _make_config(auto_start=True) + config = make_resolved_config(auto_start=True) window = _make_window(config) # Manually set initial state without triggering signals window._auto_start_check.blockSignals(True) window._auto_start_check.setChecked(True) window._auto_start_check.blockSignals(False) - new_config = _make_config(auto_start=False) + new_config = make_resolved_config(auto_start=False) with ( patch.object(window._store, 'update', return_value=new_config), patch('synodic_client.application.screen.settings.sync_startup') as mock_sync, @@ -274,10 +246,10 @@ def test_auto_start_removes_startup_when_frozen() -> None: @staticmethod def test_auto_start_skips_registry_when_not_frozen() -> None: """Auto-start toggle persists config and delegates to sync_startup.""" - config = _make_config() + config = make_resolved_config() window = _make_window(config) - new_config = _make_config(auto_start=True) + new_config = make_resolved_config(auto_start=True) with ( patch.object(window._store, 'update', return_value=new_config), patch('synodic_client.application.screen.settings.sync_startup') as mock_sync, @@ -300,7 +272,7 @@ class TestSyncDoesNotEmit: @staticmethod def test_sync_no_signal() -> None: """Syncing controls from config must not emit store.changed.""" - config = _make_config(update_channel='dev', auto_update_interval_minutes=60) + config = make_resolved_config(update_channel='dev', auto_update_interval_minutes=60) window = _make_window(config) signal_spy = MagicMock() window._store.changed.connect(signal_spy) @@ -388,10 +360,10 @@ class TestStoreConfig: @staticmethod 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)) + window = _make_window(make_resolved_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') + new_config = make_resolved_config(last_client_update='2026-03-09T12:00:00+00:00') window._store.set(new_config) window.sync_from_config() diff --git a/tests/unit/qt/test_sidebar.py b/tests/unit/qt/test_sidebar.py index 69cc933..240f0a0 100644 --- a/tests/unit/qt/test_sidebar.py +++ b/tests/unit/qt/test_sidebar.py @@ -4,6 +4,8 @@ from pathlib import Path +import pytest + from synodic_client.application.screen.schema import PreviewPhase from synodic_client.application.screen.sidebar import ManifestItem, ManifestSidebar from synodic_client.application.theme import SIDEBAR_WIDTH @@ -73,40 +75,23 @@ class TestManifestItemPhase: """Phase indicator updates.""" @staticmethod - def test_phase_loading(tmp_path: Path) -> None: - """Verify LOADING phase shows the loading indicator.""" + @pytest.mark.parametrize( + ('phase', 'expected_text'), + [ + (PreviewPhase.LOADING, 'Loading\u2026'), + (PreviewPhase.READY, 'Ready'), + (PreviewPhase.ERROR, 'Error'), + (PreviewPhase.INSTALLING, 'Installing\u2026'), + (PreviewPhase.DONE, 'Done'), + ], + ids=['loading', 'ready', 'error', 'installing', 'done'], + ) + def test_phase_label(tmp_path: Path, phase: PreviewPhase, expected_text: str) -> None: + """Verify phase label text matches the phase enum.""" item = ManifestItem(tmp_path, 'proj') - item.set_phase(PreviewPhase.LOADING) + item.set_phase(phase) assert not item._phase_label.isHidden() - assert item._phase_label.text() == 'Loading…' - - @staticmethod - def test_phase_ready(tmp_path: Path) -> None: - """Verify READY phase label text.""" - item = ManifestItem(tmp_path, 'proj') - item.set_phase(PreviewPhase.READY) - assert item._phase_label.text() == 'Ready' - - @staticmethod - def test_phase_error(tmp_path: Path) -> None: - """Verify ERROR phase label text.""" - item = ManifestItem(tmp_path, 'proj') - item.set_phase(PreviewPhase.ERROR) - assert item._phase_label.text() == 'Error' - - @staticmethod - def test_phase_installing(tmp_path: Path) -> None: - """Verify INSTALLING phase label text.""" - item = ManifestItem(tmp_path, 'proj') - item.set_phase(PreviewPhase.INSTALLING) - assert item._phase_label.text() == 'Installing…' - - @staticmethod - def test_phase_done(tmp_path: Path) -> None: - """Verify DONE phase label text.""" - item = ManifestItem(tmp_path, 'proj') - item.set_phase(PreviewPhase.DONE) - assert item._phase_label.text() == 'Done' + assert item._phase_label.text() == expected_text class TestManifestItemSignals: diff --git a/tests/unit/qt/test_tray_window_show.py b/tests/unit/qt/test_tray_window_show.py index 6ba740b..06e2646 100644 --- a/tests/unit/qt/test_tray_window_show.py +++ b/tests/unit/qt/test_tray_window_show.py @@ -6,28 +6,11 @@ import pytest -from synodic_client.application.config_store import ConfigStore from synodic_client.application.screen.schema import UpdateTarget from synodic_client.application.screen.tray import TrayScreen from synodic_client.operations.schema import UpdateResult -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, - prerelease_packages=None, - auto_apply=True, - auto_start=True, - debug_logging=False, - last_client_update=None, - last_tool_updates=None, - ) +from .conftest import make_config_store @pytest.fixture @@ -46,7 +29,7 @@ def tray_screen(): app = MagicMock() client = MagicMock() window = MagicMock() - store = ConfigStore(_make_config()) + store = make_config_store() with patch('synodic_client.application.screen.tray.SettingsWindow'): ts = TrayScreen(app, client, window, store=store) diff --git a/tests/unit/qt/test_update_controller.py b/tests/unit/qt/test_update_controller.py index 8366368..6d79660 100644 --- a/tests/unit/qt/test_update_controller.py +++ b/tests/unit/qt/test_update_controller.py @@ -2,10 +2,8 @@ from __future__ import annotations -from typing import Any from unittest.mock import MagicMock, patch -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, @@ -15,11 +13,8 @@ from synodic_client.application.update_controller import UpdateController from synodic_client.application.update_model import UpdateModel from synodic_client.operations.schema import UpdateCheckResult -from synodic_client.resolution import ResolvedConfig -from synodic_client.schema import ( - DEFAULT_AUTO_UPDATE_INTERVAL_MINUTES, - DEFAULT_TOOL_UPDATE_INTERVAL_MINUTES, -) + +from .conftest import make_config_store, make_resolved_config # --------------------------------------------------------------------------- # Helpers @@ -42,30 +37,6 @@ def __init__(self, model: UpdateModel) -> None: model.last_checked_changed.connect(self.last_checked.append) -# --------------------------------------------------------------------------- -# Helpers -# --------------------------------------------------------------------------- - - -def _make_config(**overrides: Any) -> ResolvedConfig: - """Create a ``ResolvedConfig`` with sensible defaults and optional overrides.""" - defaults: dict[str, Any] = { - '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, - 'prerelease_packages': None, - 'auto_apply': True, - 'auto_start': True, - 'debug_logging': False, - 'last_client_update': None, - 'last_tool_updates': None, - } - defaults.update(overrides) - return ResolvedConfig(**defaults) - - def _make_controller( *, auto_apply: bool = True, @@ -76,7 +47,7 @@ def _make_controller( Returns (controller, app_mock, client_mock, banner, model). """ - config = _make_config( + config = make_resolved_config( auto_apply=auto_apply, auto_update_interval_minutes=auto_update_interval_minutes, ) @@ -87,7 +58,7 @@ def _make_controller( model = UpdateModel() banner = UpdateBanner() banner.connect_model(model) - store = ConfigStore(config) + store = make_config_store(config) with patch('synodic_client.application.update_controller.resolve_update_config') as mock_ucfg: mock_ucfg.return_value = MagicMock( @@ -395,7 +366,7 @@ def test_config_changed_triggers_reinit_and_check() -> None: """Changing config should reinitialise the updater and check.""" ctrl, app, client, banner, model = _make_controller() - new_config = _make_config(update_channel='dev') + new_config = make_resolved_config(update_channel='dev') with ( patch.object(ctrl, '_reinitialize_updater') as mock_reinit, @@ -411,7 +382,7 @@ def test_config_changed_updates_auto_apply() -> None: """Changing config should update the auto_apply flag.""" ctrl, app, client, banner, model = _make_controller(auto_apply=True) - new_config = _make_config(auto_apply=False) + new_config = make_resolved_config(auto_apply=False) with ( patch.object(ctrl, '_reinitialize_updater'), @@ -470,7 +441,7 @@ def test_persist_updates_store() -> None: ctrl, _app, _client, _banner, model = _make_controller() spy = ModelSpy(model) - fake_resolved = _make_config(last_client_update='2026-03-09T00:00:00+00:00') + fake_resolved = make_resolved_config(last_client_update='2026-03-09T00:00:00+00:00') with patch.object(ctrl._store, 'update', return_value=fake_resolved) as mock_update: ctrl._persist_check_timestamp() mock_update.assert_called_once() @@ -483,7 +454,7 @@ def test_on_check_finished_success_syncs_via_store() -> None: ctrl, _app, _client, _banner, model = _make_controller() result = UpdateCheckResult(available=False, current_version='1.0.0') - fake_resolved = _make_config(last_client_update='2026-03-09T00:00:00+00:00') + fake_resolved = make_resolved_config(last_client_update='2026-03-09T00:00:00+00:00') with patch.object(ctrl._store, 'update', return_value=fake_resolved) as mock_update: ctrl._on_check_finished(result, silent=True) mock_update.assert_called_once() @@ -494,7 +465,7 @@ def test_download_finished_syncs_via_store() -> None: ctrl, _app, _client, _banner, model = _make_controller(auto_apply=False) spy = ModelSpy(model) - fake_resolved = _make_config(last_client_update='2026-03-09T00:00:00+00:00') + fake_resolved = make_resolved_config(last_client_update='2026-03-09T00:00:00+00:00') 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() @@ -520,7 +491,7 @@ def test_reinit_resets_state_and_cancels_task() -> None: fake_task.done.return_value = False ctrl._update_task = fake_task - new_config = _make_config(update_channel='dev') + new_config = make_resolved_config(update_channel='dev') with patch('synodic_client.application.update_controller.resolve_update_config') as mock_ucfg: mock_ucfg.return_value = MagicMock( auto_update_interval_minutes=0, @@ -541,7 +512,7 @@ def test_reinit_then_check_redownloads_same_version() -> None: ctrl, _app, _client, _banner, _model = _make_controller(auto_apply=False) ctrl._pending_version = '2.0.0' - new_config = _make_config(update_channel='dev') + new_config = make_resolved_config(update_channel='dev') with patch('synodic_client.application.update_controller.resolve_update_config') as mock_ucfg: mock_ucfg.return_value = MagicMock( auto_update_interval_minutes=0, diff --git a/tests/unit/test_resolution.py b/tests/unit/test_resolution.py index e32ef78..eb4bfc7 100644 --- a/tests/unit/test_resolution.py +++ b/tests/unit/test_resolution.py @@ -313,6 +313,25 @@ def test_composite_keys_ignored() -> None: # No per-package filtering from composite keys assert packages is None + @staticmethod + def test_empty_config_returns_none_pair() -> None: + """Empty mapping → (None, None).""" + plugins, packages = resolve_auto_update_scope({}, ['pip']) + assert plugins is None + assert packages is None + + @staticmethod + def test_manifest_packages_included() -> None: + """Manifest packages are added to the include set.""" + _, packages = resolve_auto_update_scope( + {}, + ['pip'], + manifest_packages={'pip': {'requests', 'flask'}}, + ) + assert packages is not None + assert 'requests' in packages + assert 'flask' in packages + # --------------------------------------------------------------------------- # resolve_update_config diff --git a/tests/unit/test_updater.py b/tests/unit/test_updater.py index ac932c2..e927a90 100644 --- a/tests/unit/test_updater.py +++ b/tests/unit/test_updater.py @@ -16,6 +16,32 @@ ) +def _setup_downloaded_state(updater: Updater) -> MagicMock: + """Put *updater* into DOWNLOADED state and return the mock velopack UpdateInfo.""" + mock_velopack_info = MagicMock(spec=velopack.UpdateInfo) + updater._update_info = UpdateInfo( + available=True, + current_version=Version('1.0.0'), + latest_version=Version('2.0.0'), + _velopack_info=mock_velopack_info, + ) + updater._state = UpdateState.DOWNLOADED + return mock_velopack_info + + +def _setup_update_available_state(updater: Updater) -> MagicMock: + """Put *updater* into UPDATE_AVAILABLE state and return the mock velopack UpdateInfo.""" + mock_velopack_info = MagicMock(spec=velopack.UpdateInfo) + updater._update_info = UpdateInfo( + available=True, + current_version=Version('1.0.0'), + latest_version=Version('2.0.0'), + _velopack_info=mock_velopack_info, + ) + updater._state = UpdateState.UPDATE_AVAILABLE + return mock_velopack_info + + class TestUpdateConfig: """Tests for UpdateConfig dataclass.""" @@ -238,21 +264,12 @@ def test_check_preserves_downloaded_state(updater: Updater) -> None: """ mock_target = MagicMock(spec=velopack.VelopackAsset) mock_target.Version = '2.0.0' - mock_velopack_info = MagicMock(spec=velopack.UpdateInfo) + mock_velopack_info = _setup_downloaded_state(updater) mock_velopack_info.TargetFullRelease = mock_target mock_manager = MagicMock(spec=velopack.UpdateManager) mock_manager.check_for_updates.return_value = mock_velopack_info - # Simulate: download already completed - updater._state = UpdateState.DOWNLOADED - updater._update_info = UpdateInfo( - available=True, - current_version=Version('1.0.0'), - latest_version=Version('2.0.0'), - _velopack_info=mock_velopack_info, - ) - with patch.object(updater, '_get_velopack_manager', return_value=mock_manager): info = updater.check_for_update() @@ -283,14 +300,7 @@ def test_download_no_update_available(updater: Updater) -> None: @staticmethod def test_download_success(updater: Updater) -> None: """Verify download_update succeeds with valid update info.""" - mock_velopack_info = MagicMock(spec=velopack.UpdateInfo) - updater._update_info = UpdateInfo( - available=True, - current_version=Version('1.0.0'), - latest_version=Version('2.0.0'), - _velopack_info=mock_velopack_info, - ) - updater._state = UpdateState.UPDATE_AVAILABLE + mock_velopack_info = _setup_update_available_state(updater) mock_manager = MagicMock(spec=velopack.UpdateManager) @@ -307,14 +317,7 @@ def test_download_success(updater: Updater) -> None: @staticmethod def test_download_with_progress_callback(updater: Updater) -> None: """Verify download_update passes progress callback.""" - mock_velopack_info = MagicMock(spec=velopack.UpdateInfo) - updater._update_info = UpdateInfo( - available=True, - current_version=Version('1.0.0'), - latest_version=Version('2.0.0'), - _velopack_info=mock_velopack_info, - ) - updater._state = UpdateState.UPDATE_AVAILABLE + mock_velopack_info = _setup_update_available_state(updater) mock_manager = MagicMock(spec=velopack.UpdateManager) progress_cb = MagicMock() @@ -331,14 +334,7 @@ def test_download_with_progress_callback(updater: Updater) -> None: @staticmethod def test_download_error(updater: Updater) -> None: """Verify download_update handles errors gracefully.""" - mock_velopack_info = MagicMock(spec=velopack.UpdateInfo) - updater._update_info = UpdateInfo( - available=True, - current_version=Version('1.0.0'), - latest_version=Version('2.0.0'), - _velopack_info=mock_velopack_info, - ) - updater._state = UpdateState.UPDATE_AVAILABLE + _setup_update_available_state(updater) mock_manager = MagicMock(spec=velopack.UpdateManager) mock_manager.download_updates.side_effect = Exception('Download failed') @@ -351,6 +347,7 @@ def test_download_error(updater: Updater) -> None: assert result is False assert updater.state == UpdateState.FAILED + assert updater._update_info is not None assert updater._update_info.error == 'Download failed' @@ -376,45 +373,39 @@ def test_apply_on_exit_no_downloaded_update(updater: Updater) -> None: updater.apply_update_on_exit() @staticmethod - def test_apply_on_exit_with_restart(updater: Updater) -> None: - """Verify apply_update_on_exit(restart=True) stages update via wait_exit_then_apply_updates.""" - mock_velopack_info = MagicMock(spec=velopack.UpdateInfo) - updater._update_info = UpdateInfo( - available=True, - current_version=Version('1.0.0'), - latest_version=Version('2.0.0'), - _velopack_info=mock_velopack_info, - ) - updater._state = UpdateState.DOWNLOADED - + @pytest.mark.parametrize( + ('restart', 'silent'), + [ + (True, False), + (False, False), + (True, True), + (False, True), + ], + ids=['restart', 'no-restart', 'silent-restart', 'silent-no-restart'], + ) + def test_apply_on_exit_matrix(updater: Updater, *, restart: bool, silent: bool) -> None: + """Verify apply_update_on_exit stages the update with the correct restart/silent flags.""" + mock_velopack_info = _setup_downloaded_state(updater) mock_manager = MagicMock(spec=velopack.UpdateManager) with ( patch.object(Updater, 'is_installed', new_callable=PropertyMock, return_value=True), patch.object(updater, '_get_velopack_manager', return_value=mock_manager), ): - updater.apply_update_on_exit(restart=True) + updater.apply_update_on_exit(restart=restart, silent=silent) assert updater.state == UpdateState.APPLYING mock_manager.wait_exit_then_apply_updates.assert_called_once_with( mock_velopack_info, - silent=False, - restart=True, + silent=silent, + restart=restart, restart_args=[], ) @staticmethod def test_apply_on_exit_with_restart_args(updater: Updater) -> None: """Verify restart_args are forwarded to wait_exit_then_apply_updates.""" - mock_velopack_info = MagicMock(spec=velopack.UpdateInfo) - updater._update_info = UpdateInfo( - available=True, - current_version=Version('1.0.0'), - latest_version=Version('2.0.0'), - _velopack_info=mock_velopack_info, - ) - updater._state = UpdateState.DOWNLOADED - + mock_velopack_info = _setup_downloaded_state(updater) mock_manager = MagicMock(spec=velopack.UpdateManager) with ( @@ -431,102 +422,10 @@ def test_apply_on_exit_with_restart_args(updater: Updater) -> None: restart_args=['--minimized'], ) - @staticmethod - def test_apply_on_exit_no_restart(updater: Updater) -> None: - """Verify apply_update_on_exit(restart=False) stages update without restart.""" - mock_velopack_info = MagicMock(spec=velopack.UpdateInfo) - updater._update_info = UpdateInfo( - available=True, - current_version=Version('1.0.0'), - latest_version=Version('2.0.0'), - _velopack_info=mock_velopack_info, - ) - updater._state = UpdateState.DOWNLOADED - - mock_manager = MagicMock(spec=velopack.UpdateManager) - - with ( - patch.object(Updater, 'is_installed', new_callable=PropertyMock, return_value=True), - patch.object(updater, '_get_velopack_manager', return_value=mock_manager), - ): - updater.apply_update_on_exit(restart=False) - - assert updater.state == UpdateState.APPLYING - mock_manager.wait_exit_then_apply_updates.assert_called_once_with( - mock_velopack_info, - silent=False, - restart=False, - restart_args=[], - ) - - @staticmethod - def test_apply_on_exit_silent_restart(updater: Updater) -> None: - """Verify silent=True suppresses the splash window.""" - mock_velopack_info = MagicMock(spec=velopack.UpdateInfo) - updater._update_info = UpdateInfo( - available=True, - current_version=Version('1.0.0'), - latest_version=Version('2.0.0'), - _velopack_info=mock_velopack_info, - ) - updater._state = UpdateState.DOWNLOADED - - mock_manager = MagicMock(spec=velopack.UpdateManager) - - with ( - patch.object(Updater, 'is_installed', new_callable=PropertyMock, return_value=True), - patch.object(updater, '_get_velopack_manager', return_value=mock_manager), - ): - updater.apply_update_on_exit(restart=True, silent=True) - - assert updater.state == UpdateState.APPLYING - mock_manager.wait_exit_then_apply_updates.assert_called_once_with( - mock_velopack_info, - silent=True, - restart=True, - restart_args=[], - ) - - @staticmethod - def test_apply_on_exit_silent_no_restart(updater: Updater) -> None: - """Verify silent=True, restart=False stages update silently.""" - mock_velopack_info = MagicMock(spec=velopack.UpdateInfo) - updater._update_info = UpdateInfo( - available=True, - current_version=Version('1.0.0'), - latest_version=Version('2.0.0'), - _velopack_info=mock_velopack_info, - ) - updater._state = UpdateState.DOWNLOADED - - mock_manager = MagicMock(spec=velopack.UpdateManager) - - with ( - patch.object(Updater, 'is_installed', new_callable=PropertyMock, return_value=True), - patch.object(updater, '_get_velopack_manager', return_value=mock_manager), - ): - updater.apply_update_on_exit(restart=False, silent=True) - - assert updater.state == UpdateState.APPLYING - mock_manager.wait_exit_then_apply_updates.assert_called_once_with( - mock_velopack_info, - silent=True, - restart=False, - restart_args=[], - ) - @staticmethod def test_apply_on_exit_silent_with_restart_args(updater: Updater) -> None: """Verify silent mode forwards restart_args.""" - mock_velopack_info = MagicMock(spec=velopack.UpdateInfo) - updater._update_info = UpdateInfo( - available=True, - current_version=Version('1.0.0'), - latest_version=Version('2.0.0'), - _velopack_info=mock_velopack_info, - ) - updater._state = UpdateState.DOWNLOADED - + mock_velopack_info = _setup_downloaded_state(updater) mock_manager = MagicMock(spec=velopack.UpdateManager) with ( diff --git a/tests/unit/windows/conftest.py b/tests/unit/windows/conftest.py index 199e0ba..361c29e 100644 --- a/tests/unit/windows/conftest.py +++ b/tests/unit/windows/conftest.py @@ -4,6 +4,16 @@ skipped automatically on non-Windows platforms. """ +from unittest.mock import MagicMock + import pytest pytest.importorskip('winreg', reason='winreg is only available on Windows') + + +def make_registry_key() -> MagicMock: + """Build a ``MagicMock`` that behaves as a context-managed registry key.""" + key = MagicMock() + key.__enter__ = MagicMock(return_value=key) + key.__exit__ = MagicMock(return_value=False) + return key diff --git a/tests/unit/windows/test_protocol.py b/tests/unit/windows/test_protocol.py index 91a230d..d9ed3a5 100644 --- a/tests/unit/windows/test_protocol.py +++ b/tests/unit/windows/test_protocol.py @@ -2,12 +2,14 @@ import winreg from pathlib import Path -from unittest.mock import MagicMock, patch +from unittest.mock import patch import pytest from synodic_client.protocol import PROTOCOL_NAME, register_protocol, remove_protocol +from .conftest import make_registry_key + _EXPECTED_REGISTRY_KEY_COUNT = 2 _TEST_PROTOCOL = f'{PROTOCOL_NAME}_test' @@ -20,9 +22,7 @@ class TestRegisterProtocol: @staticmethod def test_writes_registry_keys() -> None: """Verify correct registry keys are written on Windows.""" - mock_key = MagicMock() - mock_key.__enter__ = MagicMock(return_value=mock_key) - mock_key.__exit__ = MagicMock(return_value=False) + mock_key = make_registry_key() with ( patch.object(winreg, 'CreateKey', return_value=mock_key) as mock_create, @@ -44,9 +44,7 @@ def test_writes_registry_keys() -> None: @staticmethod def test_sets_url_protocol_value() -> None: """Verify the 'URL Protocol' value is set.""" - mock_key = MagicMock() - mock_key.__enter__ = MagicMock(return_value=mock_key) - mock_key.__exit__ = MagicMock(return_value=False) + mock_key = make_registry_key() with patch.object(winreg, 'CreateKey', return_value=mock_key), patch.object(winreg, 'SetValueEx') as mock_set: register_protocol(r'C:\Program Files\Synodic\synodic.exe') diff --git a/tests/unit/windows/test_startup.py b/tests/unit/windows/test_startup.py index 19d0643..b16c72a 100644 --- a/tests/unit/windows/test_startup.py +++ b/tests/unit/windows/test_startup.py @@ -17,6 +17,8 @@ sync_startup, ) +from .conftest import make_registry_key + class TestRegisterStartup: """Tests for register_startup.""" @@ -24,9 +26,7 @@ class TestRegisterStartup: @staticmethod def test_writes_registry_value() -> None: """Verify correct registry value is written on Windows.""" - mock_key = MagicMock() - mock_key.__enter__ = MagicMock(return_value=mock_key) - mock_key.__exit__ = MagicMock(return_value=False) + mock_key = make_registry_key() with ( patch.object(winreg, 'OpenKey', return_value=mock_key) as mock_open, @@ -52,13 +52,9 @@ def test_writes_registry_value() -> None: @staticmethod def test_writes_startup_approved_enabled() -> None: """Verify the StartupApproved enabled flag is written.""" - mock_run_key = MagicMock() - mock_run_key.__enter__ = MagicMock(return_value=mock_run_key) - mock_run_key.__exit__ = MagicMock(return_value=False) + mock_run_key = make_registry_key() - mock_approved_key = MagicMock() - mock_approved_key.__enter__ = MagicMock(return_value=mock_approved_key) - mock_approved_key.__exit__ = MagicMock(return_value=False) + mock_approved_key = make_registry_key() with ( patch.object(winreg, 'OpenKey', return_value=mock_run_key), @@ -93,9 +89,7 @@ class TestRemoveStartup: @staticmethod def test_deletes_registry_value() -> None: """Verify the startup value is deleted.""" - mock_key = MagicMock() - mock_key.__enter__ = MagicMock(return_value=mock_key) - mock_key.__exit__ = MagicMock(return_value=False) + mock_key = make_registry_key() with ( patch.object(winreg, 'OpenKey', return_value=mock_key), @@ -108,13 +102,9 @@ def test_deletes_registry_value() -> None: @staticmethod def test_clears_startup_approved() -> None: """Verify the StartupApproved flag is also deleted.""" - mock_run_key = MagicMock() - mock_run_key.__enter__ = MagicMock(return_value=mock_run_key) - mock_run_key.__exit__ = MagicMock(return_value=False) + mock_run_key = make_registry_key() - mock_approved_key = MagicMock() - mock_approved_key.__enter__ = MagicMock(return_value=mock_approved_key) - mock_approved_key.__exit__ = MagicMock(return_value=False) + mock_approved_key = make_registry_key() def _open_key_side_effect(_root: int, path: str, _reserved: int, _access: int) -> MagicMock: if 'Explorer' in path: @@ -136,9 +126,7 @@ def _open_key_side_effect(_root: int, path: str, _reserved: int, _access: int) - @staticmethod def test_handles_missing_value_gracefully() -> None: """Verify no error when startup value doesn't exist.""" - mock_key = MagicMock() - mock_key.__enter__ = MagicMock(return_value=mock_key) - mock_key.__exit__ = MagicMock(return_value=False) + mock_key = make_registry_key() with ( patch.object(winreg, 'OpenKey', return_value=mock_key), @@ -161,16 +149,14 @@ class TestIsStartupRegistered: @staticmethod def test_returns_true_when_present() -> None: """Verify True when the value exists and no approval override.""" - mock_key = MagicMock() - mock_key.__enter__ = MagicMock(return_value=mock_key) - mock_key.__exit__ = MagicMock(return_value=False) + mock_key = make_registry_key() def _open_key_side_effect(_root: int, path: str, _reserved: int, _access: int) -> MagicMock: return mock_key def _query_side_effect(key: MagicMock, name: str) -> tuple[object, int]: # Run key exists; StartupApproved key raises FileNotFoundError - # (no override → treated as enabled) + # (no override → treated as enabled) raise FileNotFoundError with ( @@ -189,9 +175,7 @@ def _query_side_effect(key: MagicMock, name: str) -> tuple[object, int]: @staticmethod def test_returns_true_when_startup_approved_enabled() -> None: """Verify True when the StartupApproved byte is 0x02 (enabled).""" - mock_key = MagicMock() - mock_key.__enter__ = MagicMock(return_value=mock_key) - mock_key.__exit__ = MagicMock(return_value=False) + mock_key = make_registry_key() enabled_data = b'\x02' + b'\x00' * 11 @@ -211,9 +195,7 @@ def test_returns_true_when_startup_approved_enabled() -> None: @staticmethod def test_returns_false_when_startup_approved_disabled() -> None: """Verify False when the StartupApproved byte is 0x03 (disabled).""" - mock_key = MagicMock() - mock_key.__enter__ = MagicMock(return_value=mock_key) - mock_key.__exit__ = MagicMock(return_value=False) + mock_key = make_registry_key() disabled_data = b'\x03' + b'\x00' * 11 @@ -233,9 +215,7 @@ def test_returns_false_when_startup_approved_disabled() -> None: @staticmethod def test_returns_false_when_missing() -> None: """Verify False when the value does not exist.""" - mock_key = MagicMock() - mock_key.__enter__ = MagicMock(return_value=mock_key) - mock_key.__exit__ = MagicMock(return_value=False) + mock_key = make_registry_key() with ( patch.object(winreg, 'OpenKey', return_value=mock_key), @@ -250,9 +230,7 @@ class TestGetRegisteredStartupPath: @staticmethod def test_returns_unquoted_path() -> None: """Verify the returned path has surrounding quotes stripped.""" - mock_key = MagicMock() - mock_key.__enter__ = MagicMock(return_value=mock_key) - mock_key.__exit__ = MagicMock(return_value=False) + mock_key = make_registry_key() with ( patch.object(winreg, 'OpenKey', return_value=mock_key), @@ -267,9 +245,7 @@ def test_returns_unquoted_path() -> None: @staticmethod def test_returns_none_when_missing() -> None: """Verify None when the registry value does not exist.""" - mock_key = MagicMock() - mock_key.__enter__ = MagicMock(return_value=mock_key) - mock_key.__exit__ = MagicMock(return_value=False) + mock_key = make_registry_key() with ( patch.object(winreg, 'OpenKey', return_value=mock_key), @@ -280,9 +256,7 @@ def test_returns_none_when_missing() -> None: @staticmethod def test_returns_none_on_os_error() -> None: """Verify None when an OSError prevents reading the registry.""" - mock_key = MagicMock() - mock_key.__enter__ = MagicMock(return_value=mock_key) - mock_key.__exit__ = MagicMock(return_value=False) + mock_key = make_registry_key() with ( patch.object(winreg, 'OpenKey', return_value=mock_key),