diff --git a/pdm.lock b/pdm.lock index f2d9875..df4dd57 100644 --- a/pdm.lock +++ b/pdm.lock @@ -5,7 +5,7 @@ groups = ["default", "build", "lint", "test"] strategy = ["inherit_metadata"] lock_version = "4.5.0" -content_hash = "sha256:18995a70ffa148eee18c62fe7dca60cfe1028305073ededcde318b8bf056d404" +content_hash = "sha256:52b09137c7599dc609e087bb58f7ced9bd9940ac53f77e0b6401eeb30d3f6bb3" [[metadata.targets]] requires_python = ">=3.14,<3.15" @@ -336,7 +336,7 @@ files = [ [[package]] name = "porringer" -version = "0.2.1.dev78" +version = "0.2.1.dev79" requires_python = ">=3.14" summary = "" groups = ["default"] @@ -349,8 +349,8 @@ dependencies = [ "userpath>=1.9.2", ] files = [ - {file = "porringer-0.2.1.dev78-py3-none-any.whl", hash = "sha256:557edbcda23f37b7f6b3226ba7d0ec76c31b9aa8f6c1af64a12537b3f0d35433"}, - {file = "porringer-0.2.1.dev78.tar.gz", hash = "sha256:7118736c60b371c5ca8a56fd583acdc5ca263a679eb6ea85fa1fc477525f570d"}, + {file = "porringer-0.2.1.dev79-py3-none-any.whl", hash = "sha256:fa010a1887f376827d154794dfa1ec8552263fb596e987bea8fa2e0e3920e478"}, + {file = "porringer-0.2.1.dev79.tar.gz", hash = "sha256:328de37f930e0042341378037c1b3bb9e36479485748f01569d8fac0936883df"}, ] [[package]] diff --git a/pyproject.toml b/pyproject.toml index 52d5731..1eb06d0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -14,7 +14,7 @@ requires-python = ">=3.14, <3.15" dependencies = [ "pyside6>=6.10.2", "packaging>=26.0", - "porringer>=0.2.1.dev78", + "porringer>=0.2.1.dev79", "qasync>=0.28.0", "velopack>=0.0.1444.dev49733", "typer>=0.24.1", diff --git a/synodic_client/application/screen/__init__.py b/synodic_client/application/screen/__init__.py index 109ae5e..bbac2ea 100644 --- a/synodic_client/application/screen/__init__.py +++ b/synodic_client/application/screen/__init__.py @@ -8,7 +8,7 @@ from datetime import UTC, datetime -from porringer.schema import SetupAction, SkipReason +from porringer.schema import SetupAction, SetupActionResult, SkipReason from porringer.schema.plugin import PluginKind _SECONDS_PER_MINUTE = 60 @@ -63,18 +63,26 @@ def skip_reason_label(reason: SkipReason | None) -> str: return SKIP_REASON_LABELS.get(reason, reason.name.replace('_', ' ').capitalize()) -def format_cli_command(action: SetupAction, *, suppress_description: bool = False) -> str: +def format_cli_command( + action: SetupAction, + *, + result: SetupActionResult | None = None, + suppress_description: bool = False, +) -> str: """Return a human-readable CLI command string for *action*. - Prefers ``cli_command``, falls back to ``command``, then synthesises - an ``installer install `` string for package actions, and + Prefers ``result.cli_command`` (populated after dry-run), falls + back to ``action.command``, then synthesises an + ``installer install `` string for package actions, and finally returns the action description as a last resort. When *suppress_description* is ``True`` the final description fallback returns an empty string instead. """ - if parts := (action.cli_command or action.command): - return ' '.join(parts) + if result is not None and result.cli_command: + return ' '.join(result.cli_command) + if action.command: + return ' '.join(action.command) if action.kind == PluginKind.PACKAGE and action.package: return f'{action.installer or "pip"} install {action.package}' return '' if suppress_description else action.description diff --git a/synodic_client/application/screen/action_card.py b/synodic_client/application/screen/action_card.py index ca0c7bf..d5911d6 100644 --- a/synodic_client/application/screen/action_card.py +++ b/synodic_client/application/screen/action_card.py @@ -73,19 +73,6 @@ _KIND_ORDER: dict[PluginKind | None, int] = {kind: i for i, kind in enumerate(PHASE_ORDER)} -def action_key(action: SetupAction) -> tuple[object, ...]: - """Return a stable identity key for *action*. - - Uses content-based fields (kind, installer, package name, command) - so the same logical action from different ``execute_stream`` runs - resolves to the same key. - """ - pkg_name = str(action.package.name) if action.package else None - pt_name = str(action.plugin_target.name) if action.plugin_target else None - cmd = tuple(action.command) if action.command else None - return (action.kind, action.installer, pkg_name, pt_name, cmd) - - def action_sort_key(action: SetupAction) -> int: """Return a sort key that groups cards by execution phase. @@ -396,24 +383,6 @@ def populate( else: self._prerelease_cb.hide() - def update_command(self, action: SetupAction) -> None: - """Update the CLI command label after the resolved preview arrives. - - Called from the two-phase display flow once ``MANIFEST_LOADED`` - provides actions with their ``cli_command`` populated. - - Args: - action: The setup action with resolved CLI command. - """ - if self._is_skeleton: - return - cmd_text = format_cli_command(action, suppress_description=True) - if cmd_text: - self._command_label.setText(cmd_text) - self._command_row.show() - else: - self._command_row.hide() - def _populate_status( self, action: SetupAction, @@ -494,7 +463,7 @@ def set_check_result(self, result: SetupActionResult) -> None: self._status_label.setText(label) self._status_label.setStyleSheet(ACTION_CARD_STATUS_UPDATE) elif result.skipped: - label = skip_reason_label(result.skip_reason) + label = '\u2713 ' + skip_reason_label(result.skip_reason) self._status_label.setText(label) self._status_label.setStyleSheet(ACTION_CARD_STATUS_SATISFIED) elif not result.success: @@ -517,6 +486,13 @@ def set_check_result(self, result: SetupActionResult) -> None: else: self._status_label.setToolTip('') + # CLI command — update with resolved cli_command from result + assert self._action is not None + cmd_text = format_cli_command(self._action, result=result, suppress_description=True) + if cmd_text: + self._command_label.setText(cmd_text) + self._command_row.show() + # Version column self._check_available_version = result.available_version if result.installed_version and result.available_version: @@ -524,6 +500,9 @@ def set_check_result(self, result: SetupActionResult) -> None: self._version_label.setStyleSheet(ACTION_CARD_VERSION_STYLE + ' color: #d7ba7d;') elif result.installed_version: self._version_label.setText(result.installed_version) + elif result.available_version: + self._version_label.setText(f'\u2192 {result.available_version}') + self._version_label.setStyleSheet(ACTION_CARD_VERSION_STYLE + ' color: grey;') def finalize_checking(self) -> None: """Resolve a still-pending 'Checking\u2026' status to 'Needed'. @@ -611,9 +590,8 @@ def is_update_available(self) -> bool: class ActionCardList(QWidget): """Container of :class:`ActionCard` widgets. - Cards are keyed by :func:`action_key` (content-based) so that - look-ups work across different ``execute_stream`` runs where the - ``SetupAction`` objects are different Python instances. + Cards are keyed by ``SetupAction`` directly (frozen dataclass) so + that look-ups work across different ``execute_stream`` runs. """ prerelease_toggled = Signal(str, bool) @@ -629,7 +607,7 @@ def __init__(self, parent: QWidget | None = None) -> None: self._layout.addStretch() self._cards: list[ActionCard] = [] - self._action_map: dict[tuple[object, ...], ActionCard] = {} + self._action_map: dict[SetupAction, ActionCard] = {} # ------------------------------------------------------------------ # Skeleton loading @@ -679,7 +657,7 @@ def populate( card.prerelease_toggled.connect(self.prerelease_toggled.emit) self._layout.insertWidget(self._layout.count() - 1, card) self._cards.append(card) - self._action_map[action_key(act)] = card + self._action_map[act] = card # ------------------------------------------------------------------ # Card lookup @@ -696,11 +674,10 @@ def card_count(self) -> int: return len(self._cards) def get_card(self, action: SetupAction) -> ActionCard | None: - """Look up the card for a given action by stable content key. + """Look up the card for a given action. - Works across different ``execute_stream`` runs — the preview - and install phases produce different ``SetupAction`` instances - but the same logical action maps to the same card. + ``SetupAction`` is a frozen dataclass, so the same logical + action from different ``execute_stream`` runs hashes equally. Args: action: The setup action to find. @@ -708,7 +685,7 @@ def get_card(self, action: SetupAction) -> ActionCard | None: Returns: The card widget, or ``None`` if not found. """ - return self._action_map.get(action_key(action)) + return self._action_map.get(action) # ------------------------------------------------------------------ # Bulk operations diff --git a/synodic_client/application/screen/install.py b/synodic_client/application/screen/install.py index 0b4335b..0c1f432 100644 --- a/synodic_client/application/screen/install.py +++ b/synodic_client/application/screen/install.py @@ -45,7 +45,7 @@ from synodic_client.application.package_state import PackageStateStore from synodic_client.application.screen import skip_reason_label -from synodic_client.application.screen.action_card import ActionCardList, action_key +from synodic_client.application.screen.action_card import ActionCardList from synodic_client.application.screen.card import CardFrame from synodic_client.application.screen.install_workers import run_install, run_preview from synodic_client.application.screen.log_panel import ExecutionLogPanel @@ -595,11 +595,11 @@ def _on_preview_ready(self, preview: SetupResults, manifest_path: str, temp_dir_ self._install_btn.setEnabled(True) def _on_preview_resolved(self, preview: SetupResults, manifest_path: str, temp_dir_path: str) -> None: - """Handle the fully-resolved preview (CLI commands populated). + """Handle the fully-resolved preview. Called after ``MANIFEST_LOADED`` — cards are already visible - from the earlier ``_on_manifest_parsed`` handler. This only - updates CLI command text and the temp-dir reference. + from the earlier ``_on_manifest_parsed`` handler. This updates + the temp-dir reference and emits metadata. """ if self._model.preview is None: return @@ -609,19 +609,13 @@ def _on_preview_resolved(self, preview: SetupResults, manifest_path: str, temp_d if preview.metadata: self.metadata_ready.emit(preview) - for action in preview.actions: - if action.cli_command: - card = self._card_list.get_card(action) - if card is not None: - card.update_command(action) - def _on_action_checked(self, row: int, result: SetupActionResult) -> None: """Update the model and action card with a dry-run result.""" m = self._model if result.skipped and result.skip_reason == SkipReason.UPDATE_AVAILABLE: label = skip_reason_label(result.skip_reason) if 0 <= row < len(m.action_states): - m.upgradable_keys.add(action_key(m.action_states[row].action)) + m.upgradable_keys.add(m.action_states[row].action) elif result.skipped: label = skip_reason_label(result.skip_reason) elif not result.success: diff --git a/synodic_client/application/screen/install_workers.py b/synodic_client/application/screen/install_workers.py index 86bb87c..b52986b 100644 --- a/synodic_client/application/screen/install_workers.py +++ b/synodic_client/application/screen/install_workers.py @@ -159,10 +159,9 @@ def _dispatch_preview_event( ) -> None: """Route a single preview stream event to the appropriate callback. - Mutates *state* in-place with updated ``action_index`` / ``got_parsed``. + Mutates *state* in-place (``got_parsed`` flag). """ if event.kind == ProgressEventKind.MANIFEST_PARSED and event.manifest: - state.action_index = {id(a): i for i, a in enumerate(event.manifest.actions)} if cb.on_manifest_parsed is not None: cb.on_manifest_parsed(event.manifest, manifest_path, temp_dir_str) state.got_parsed = True @@ -176,16 +175,17 @@ def _dispatch_preview_event( return if event.kind == ProgressEventKind.MANIFEST_LOADED and event.manifest: - if not state.got_parsed: - state.action_index = {id(a): i for i, a in enumerate(event.manifest.actions)} if cb.on_preview_ready is not None: cb.on_preview_ready(event.manifest, manifest_path, temp_dir_str) return - if event.kind == ProgressEventKind.ACTION_COMPLETED and event.result and event.action: - row = state.action_index.get(id(event.action)) - if row is not None and cb.on_action_checked is not None: - cb.on_action_checked(row, event.result) + if ( + event.kind == ProgressEventKind.ACTION_COMPLETED + and event.result + and event.action_index is not None + and cb.on_action_checked is not None + ): + cb.on_action_checked(event.action_index, event.result) # --------------------------------------------------------------------------- diff --git a/synodic_client/application/screen/log_panel.py b/synodic_client/application/screen/log_panel.py index 83e2422..4375fa0 100644 --- a/synodic_client/application/screen/log_panel.py +++ b/synodic_client/application/screen/log_panel.py @@ -26,7 +26,6 @@ ) from synodic_client.application.screen import ACTION_KIND_LABELS, skip_reason_label -from synodic_client.application.screen.action_card import action_key from synodic_client.application.screen.card import CHEVRON_DOWN, CHEVRON_RIGHT, ClickableHeader from synodic_client.application.theme import ( LOG_CHEVRON_STYLE, @@ -186,7 +185,7 @@ def __init__(self, parent: QWidget | None = None) -> None: self._layout.addStretch() # Map action content-key → section widget for quick lookup - self._sections: dict[tuple[object, ...], ActionLogSection] = {} + self._sections: dict[SetupAction, ActionLogSection] = {} self._section_count = 0 # --- Public API --- @@ -204,7 +203,7 @@ def add_section(self, action: SetupAction) -> ActionLogSection: section = ActionLogSection(action, self._section_count, self) # Insert before the stretch self._layout.insertWidget(self._layout.count() - 1, section) - self._sections[action_key(action)] = section + self._sections[action] = section return section @@ -217,7 +216,7 @@ def get_section(self, action: SetupAction) -> ActionLogSection | None: Returns: The section widget, or ``None`` if not found. """ - return self._sections.get(action_key(action)) + return self._sections.get(action) def on_sub_progress(self, action: SetupAction, progress: SubActionProgress) -> None: """Handle a sub-action progress event. diff --git a/synodic_client/application/screen/schema.py b/synodic_client/application/screen/schema.py index 28e3002..bc88cea 100644 --- a/synodic_client/application/screen/schema.py +++ b/synodic_client/application/screen/schema.py @@ -23,7 +23,6 @@ ) from porringer.schema.plugin import RuntimePackageResult -from synodic_client.application.screen.action_card import action_key from synodic_client.application.uri import normalize_manifest_key # --------------------------------------------------------------------------- @@ -237,7 +236,6 @@ class PreviewModel: def __init__(self) -> None: """Initialise a blank preview model.""" - self._action_key = action_key self._normalize = normalize_manifest_key self.phase: PreviewPhase = PreviewPhase.IDLE @@ -248,19 +246,19 @@ def __init__(self) -> None: self.plugin_installed: dict[str, bool] = {} self.prerelease_overrides: set[str] = set() self.action_states: list[ActionState] = [] - self._action_state_map: dict[tuple[object, ...], ActionState] = {} + self._action_state_map: dict[SetupAction, ActionState] = {} self._action_state_map_len: int = 0 - self.upgradable_keys: set[tuple[object, ...]] = set() + self.upgradable_keys: set[SetupAction] = set() self.checked_count: int = 0 self.completed_count: int = 0 self.temp_dir: str | None = None # -- Computed helpers -------------------------------------------------- - def _ensure_action_state_map(self) -> dict[tuple[object, ...], ActionState]: - """Return the action-key → state lookup, rebuilding if stale.""" + def _ensure_action_state_map(self) -> dict[SetupAction, ActionState]: + """Return the action → state lookup, rebuilding if stale.""" if len(self.action_states) != self._action_state_map_len: - self._action_state_map = {self._action_key(s.action): s for s in self.action_states} + self._action_state_map = {s.action: s for s in self.action_states} self._action_state_map_len = len(self.action_states) return self._action_state_map @@ -279,8 +277,8 @@ def install_enabled(self) -> bool: return self.actionable_count > 0 or any(s.action.kind is None for s in self.action_states) def action_state_for(self, act: SetupAction) -> ActionState | None: - """Look up :class:`ActionState` by content key (O(1) amortized).""" - return self._ensure_action_state_map().get(self._action_key(act)) + """Look up :class:`ActionState` for *act* (O(1) amortized).""" + return self._ensure_action_state_map().get(act) def has_same_manifest(self, key: str) -> bool: """Return ``True`` if *key* matches the current manifest key.""" @@ -339,5 +337,4 @@ class PreviewConfig: class _DispatchState: """Mutable accumulator for :func:`_dispatch_preview_event`.""" - action_index: dict[int, int] = field(default_factory=dict) got_parsed: bool = False diff --git a/synodic_client/application/theme.py b/synodic_client/application/theme.py index e6e4e73..e96ac34 100644 --- a/synodic_client/application/theme.py +++ b/synodic_client/application/theme.py @@ -427,8 +427,8 @@ ACTION_CARD_STATUS_NEEDED = 'color: palette(text); font-size: 11px; font-weight: bold;' """Status label: Needed.""" -ACTION_CARD_STATUS_SATISFIED = 'color: grey; font-size: 11px;' -"""Status label: Already installed.""" +ACTION_CARD_STATUS_SATISFIED = 'color: #6a9955; font-size: 11px;' +"""Status label: Already installed (muted green with checkmark).""" ACTION_CARD_STATUS_UPDATE = 'color: #d7ba7d; font-size: 11px; font-weight: bold;' """Status label: Update available (amber).""" diff --git a/synodic_client/application/update_model.py b/synodic_client/application/update_model.py index 368f461..48cc33f 100644 --- a/synodic_client/application/update_model.py +++ b/synodic_client/application/update_model.py @@ -50,6 +50,7 @@ class UpdateModel(QObject): last_checked_changed = Signal(str) def __init__(self, parent: QObject | None = None) -> None: + """Initialize the update model.""" super().__init__(parent) self._phase = UpdatePhase.IDLE self._version: str = '' diff --git a/tests/unit/qt/test_action_card.py b/tests/unit/qt/test_action_card.py index b21e4ed..a62c598 100644 --- a/tests/unit/qt/test_action_card.py +++ b/tests/unit/qt/test_action_card.py @@ -17,7 +17,6 @@ from synodic_client.application.screen.action_card import ( ActionCard, ActionCardList, - action_key, action_sort_key, ) from synodic_client.application.theme import ( @@ -43,7 +42,7 @@ def _make_action( *, kind: PluginKind | None = PluginKind.PACKAGE, description: str = 'Install requests', - installer: str = 'pip', + installer: str | None = 'pip', package: str = 'requests', **overrides: Any, ) -> SetupAction: @@ -63,7 +62,6 @@ def _make_action( action.package = pkg_mock action.package_description = overrides.get('package_description', description) action.command = overrides.get('command') - action.cli_command = overrides.get('cli_command') action.include_prereleases = overrides.get('include_prereleases', False) action.plugin_target = overrides.get('plugin_target') return action @@ -80,7 +78,7 @@ def _make_result( """Create a SetupActionResult. Extra keyword arguments (``action``, ``installed_version``, - ``available_version``) are forwarded to the constructor. + ``available_version``, ``cli_command``) are forwarded to the constructor. """ return SetupActionResult( action=overrides.get('action') or _make_action(), @@ -90,6 +88,7 @@ def _make_result( message=message, installed_version=overrides.get('installed_version'), available_version=overrides.get('available_version'), + cli_command=overrides.get('cli_command'), ) @@ -244,7 +243,7 @@ def test_needed_status() -> None: @staticmethod def test_already_installed_status() -> None: - """Skipped ALREADY_INSTALLED shows 'Already installed'.""" + """Skipped ALREADY_INSTALLED shows '\u2713 Already installed'.""" card = ActionCard() card.populate(_make_action()) result = _make_result( @@ -253,7 +252,7 @@ def test_already_installed_status() -> None: installed_version='3.5.2', ) card.set_check_result(result) - assert card.status_text() == 'Already installed' + assert card.status_text() == '\u2713 Already installed' assert ACTION_CARD_STATUS_SATISFIED in card._status_label.styleSheet() @staticmethod @@ -301,6 +300,20 @@ def test_installed_version_shown() -> None: card.set_check_result(result) assert card._version_label.text() == '3.5.2' + @staticmethod + 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( + success=True, + skipped=False, + available_version='1.2.0', + ) + card.set_check_result(result) + assert '\u2192 1.2.0' in card._version_label.text() + assert 'grey' in card._version_label.styleSheet() + @staticmethod def test_finalize_checking_resolves_to_needed() -> None: """finalize_checking stops spinner and changes to 'Needed'.""" @@ -349,8 +362,7 @@ 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') - action.installer = None # Simulate an unresolved deferred action + 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) @@ -506,8 +518,7 @@ 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') - a2.kind = None # bare command + a2 = _make_action(package='pkg2', kind=None) actions = [a1, a2] card_list.populate(actions) assert card_list.card_count() == len(actions) @@ -528,21 +539,6 @@ def test_get_card_by_stable_key() -> None: assert c1._package_label.text() == 'first' assert c2._package_label.text() == 'second' - @staticmethod - def test_get_card_cross_instance() -> None: - """get_card works with a different object that has the same content.""" - card_list = ActionCardList() - original = _make_action(package='numpy', installer='pip') - card_list.populate([original]) - - # Create a separate mock with the same content fields - duplicate = _make_action(package='numpy', installer='pip') - assert original is not duplicate - - card = card_list.get_card(duplicate) - assert card is not None - assert card._package_label.text() == 'numpy' - @staticmethod def test_get_card_returns_none_for_unknown() -> None: """get_card returns None for an unknown action.""" @@ -582,7 +578,7 @@ def test_finalize_all_checking() -> None: card_list.finalize_all_checking() - assert c1.status_text() == 'Already installed' # unchanged + assert c1.status_text() == '\u2713 Already installed' # unchanged c2 = card_list.get_card(a2) assert c2 is not None assert c2.status_text() == 'Needed' # resolved @@ -612,53 +608,6 @@ def test_card_at_out_of_range() -> None: assert card_list.card_at(-1) is None -# --------------------------------------------------------------------------- -# action_key — stable identity -# --------------------------------------------------------------------------- - - -class TestActionKey: - """Tests for the action_key function.""" - - @staticmethod - def test_same_content_same_key() -> None: - """Two actions with identical content produce the same key.""" - a = _make_action(package='numpy', installer='pip') - b = _make_action(package='numpy', installer='pip') - assert a is not b - assert action_key(a) == action_key(b) - - @staticmethod - def test_different_package_different_key() -> None: - """Actions with different packages produce different keys.""" - a = _make_action(package='numpy') - b = _make_action(package='scipy') - assert action_key(a) != action_key(b) - - @staticmethod - def test_different_installer_different_key() -> None: - """Actions with different installers produce different keys.""" - a = _make_action(package='numpy', installer='pip') - b = _make_action(package='numpy', installer='uv') - assert action_key(a) != action_key(b) - - @staticmethod - def test_different_kind_different_key() -> None: - """Actions with different kinds produce different keys.""" - a = _make_action(package='ruff', kind=PluginKind.PACKAGE) - b = _make_action(package='ruff', kind=PluginKind.TOOL) - assert action_key(a) != action_key(b) - - @staticmethod - def test_command_action_key() -> None: - """Command actions include the command in the key.""" - a = _make_action(command=['echo', 'hello']) - b = _make_action(command=['echo', 'hello']) - c = _make_action(command=['echo', 'world']) - assert action_key(a) == action_key(b) - assert action_key(a) != action_key(c) - - # --------------------------------------------------------------------------- # ActionCard — CLI command label # --------------------------------------------------------------------------- @@ -677,11 +626,18 @@ def test_default_package_command() -> None: assert not card._command_row.isHidden() @staticmethod - def test_explicit_cli_command() -> None: - """Actions with cli_command show that instead of the default.""" + def test_explicit_cli_command_from_result() -> None: + """set_check_result with cli_command updates the command label.""" card = ActionCard() - action = _make_action(cli_command=['uv', 'tool', 'install', 'ruff']) + action = _make_action(package='ruff', installer='pip') card.populate(action) + assert card._command_label.text() == 'pip install ruff' + + result = _make_result( + action=action, + cli_command=('uv', 'tool', 'install', 'ruff'), + ) + card.set_check_result(result) assert card._command_label.text() == 'uv tool install ruff' @staticmethod @@ -694,46 +650,30 @@ def test_command_label_selectable() -> None: assert flags & Qt.TextInteractionFlag.TextSelectableByMouse @staticmethod - def test_update_command_updates_text() -> None: - """update_command replaces the command label text.""" + 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') card.populate(action) assert card._command_label.text() == 'pip install ruff' - resolved = _make_action(cli_command=['uv', 'tool', 'install', 'ruff']) - card.update_command(resolved) + result = _make_result( + action=action, + cli_command=('uv', 'tool', 'install', 'ruff'), + ) + card.set_check_result(result) assert card._command_label.text() == 'uv tool install ruff' assert not card._command_row.isHidden() - @staticmethod - def test_update_command_hides_label_when_empty() -> None: - """update_command hides the row when the resolved action has no command.""" - card = ActionCard() - action = _make_action(package='ruff', installer='pip') - card.populate(action) - assert not card._command_row.isHidden() - - empty_action = _make_action(kind=PluginKind.RUNTIME) - empty_action.cli_command = None - empty_action.command = None - empty_action.package = None - card.update_command(empty_action) - assert card._command_row.isHidden() - - @staticmethod - def test_update_command_noop_on_skeleton() -> None: - """update_command does nothing when card is a skeleton.""" - card = ActionCard(skeleton=True) - action = _make_action(cli_command=['uv', 'tool', 'install', 'ruff']) - # Should not raise — skeleton simply returns early - card.update_command(action) - @staticmethod 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(cli_command=['uv', 'tool', 'install', 'ruff']) + action = _make_action( + package='ruff', + installer='uv', + command=('uv', 'tool', 'install', 'ruff'), + ) card.populate(action) clipboard = QApplication.clipboard() @@ -746,7 +686,11 @@ 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(cli_command=['uv', 'tool', 'install', 'ruff']) + action = _make_action( + package='ruff', + installer='uv', + command=('uv', 'tool', 'install', 'ruff'), + ) card.populate(action) card._copy_btn.click() @@ -936,7 +880,7 @@ def test_already_latest_shows_satisfied_style() -> None: skip_reason=SkipReason.ALREADY_LATEST, ) ) - assert card.status_text() == 'Already latest' + assert card.status_text() == '\u2713 Already latest' assert ACTION_CARD_STATUS_SATISFIED in card._status_label.styleSheet() @staticmethod @@ -974,8 +918,8 @@ def test_already_installed_vs_already_latest() -> None: ) ) - assert card_installed.status_text() == 'Already installed' - assert card_latest.status_text() == 'Already latest' + assert card_installed.status_text() == '\u2713 Already installed' + assert card_latest.status_text() == '\u2713 Already latest' # Both use the satisfied stylesheet assert ACTION_CARD_STATUS_SATISFIED in card_installed._status_label.styleSheet() assert ACTION_CARD_STATUS_SATISFIED in card_latest._status_label.styleSheet() diff --git a/tests/unit/qt/test_install_preview.py b/tests/unit/qt/test_install_preview.py index bc144ec..6413899 100644 --- a/tests/unit/qt/test_install_preview.py +++ b/tests/unit/qt/test_install_preview.py @@ -49,7 +49,6 @@ def _make_action( action.installer = installer action.package = package action.command = None - action.cli_command = None return action @staticmethod @@ -85,7 +84,6 @@ def _make_action(**overrides: Any) -> MagicMock: 'description': 'Install test', 'installer': 'pip', 'package': 'requests', - 'cli_command': None, 'command': None, } defaults.update(overrides) @@ -96,19 +94,20 @@ def _make_action(**overrides: Any) -> MagicMock: action.installer = defaults['installer'] action.package = defaults['package'] action.command = defaults['command'] - action.cli_command = defaults['cli_command'] return action - def test_prefers_cli_command(self) -> None: - """Verify cli_command takes precedence over command and fallback.""" - action = self._make_action(cli_command=['uv', 'pip', 'install', 'requests']) - assert format_cli_command(action) == 'uv pip install requests' + def test_prefers_cli_command_from_result(self) -> None: + """Verify result cli_command takes precedence over command and fallback.""" + action = self._make_action() + result = MagicMock() + result.cli_command = ('uv', 'pip', 'install', 'requests') + assert format_cli_command(action, result=result) == 'uv pip install requests' def test_falls_back_to_command(self) -> None: """Verify command is used when cli_command is absent.""" action = self._make_action( kind='TOOL', - command=['echo', 'hello'], + command=('echo', 'hello'), ) assert format_cli_command(action) == 'echo hello' @@ -150,6 +149,7 @@ def test_worker_emits_finished_on_success() -> None: kind=ProgressEventKind.ACTION_COMPLETED, action=action, result=result, + action_index=0, ) async def mock_stream(*args, **kwargs): @@ -389,7 +389,12 @@ def test_emits_preview_ready_and_finished(tmp_path: Path) -> None: # Dry-run stream yields manifest loaded then one completed event manifest_event = ProgressEvent(kind=ProgressEventKind.MANIFEST_LOADED, manifest=preview) result = SetupActionResult(action=action, success=True, skipped=False, skip_reason=None) - completed_event = ProgressEvent(kind=ProgressEventKind.ACTION_COMPLETED, action=action, result=result) + completed_event = ProgressEvent( + kind=ProgressEventKind.ACTION_COMPLETED, + action=action, + result=result, + action_index=0, + ) async def mock_stream(*args: Any, **kwargs: Any) -> Any: yield manifest_event @@ -448,7 +453,7 @@ async def _run() -> None: @staticmethod def test_action_checked_maps_correct_rows(tmp_path: Path) -> None: - """Verify on_action_checked receives correct row indices via identity matching.""" + """Verify on_action_checked receives correct row indices via content-based matching.""" manifest = tmp_path / 'porringer.json' manifest.write_text('{}') @@ -466,8 +471,18 @@ def test_action_checked_maps_correct_rows(tmp_path: Path) -> None: result_a = SetupActionResult(action=action_a, success=True, skipped=False, skip_reason=None) # Stream returns in execution order (b before a), not preview order - event_b = ProgressEvent(kind=ProgressEventKind.ACTION_COMPLETED, action=action_b, result=result_b) - event_a = ProgressEvent(kind=ProgressEventKind.ACTION_COMPLETED, action=action_a, result=result_a) + event_b = ProgressEvent( + kind=ProgressEventKind.ACTION_COMPLETED, + action=action_b, + result=result_b, + action_index=1, + ) + event_a = ProgressEvent( + kind=ProgressEventKind.ACTION_COMPLETED, + action=action_a, + result=result_a, + action_index=0, + ) async def mock_stream(*args: Any, **kwargs: Any) -> Any: yield manifest_event @@ -815,7 +830,6 @@ def _make_scm_action(description: str = 'Clone repo') -> MagicMock: action.installer = 'git' action.package = None action.command = None - action.cli_command = None return action def test_scm_already_installed_emits_correct_result(self, tmp_path: Path) -> None: @@ -838,6 +852,7 @@ def test_scm_already_installed_emits_correct_result(self, tmp_path: Path) -> Non kind=ProgressEventKind.ACTION_COMPLETED, action=action, result=result, + action_index=0, ) async def mock_stream(*args: Any, **kwargs: Any) -> Any: @@ -884,6 +899,7 @@ def test_scm_needed_emits_correct_result(self, tmp_path: Path) -> None: kind=ProgressEventKind.ACTION_COMPLETED, action=action, result=result, + action_index=0, ) async def mock_stream(*args: Any, **kwargs: Any) -> Any: diff --git a/tests/unit/qt/test_log_panel.py b/tests/unit/qt/test_log_panel.py index 812f661..813bb20 100644 --- a/tests/unit/qt/test_log_panel.py +++ b/tests/unit/qt/test_log_panel.py @@ -61,7 +61,6 @@ def _make_action( action.package = pkg_mock action.package_description = package_description or description action.command = None - action.cli_command = None action.plugin_target = None return action diff --git a/tests/unit/qt/test_preview_model.py b/tests/unit/qt/test_preview_model.py index 19ef933..957cef6 100644 --- a/tests/unit/qt/test_preview_model.py +++ b/tests/unit/qt/test_preview_model.py @@ -8,7 +8,6 @@ from porringer.schema import SetupAction from porringer.schema.plugin import PluginKind -from synodic_client.application.screen.action_card import action_key from synodic_client.application.screen.schema import ActionState, PreviewModel, PreviewPhase from synodic_client.application.uri import normalize_manifest_key @@ -36,7 +35,6 @@ def _make_action( action.package = pkg_mock action.package_description = overrides.get('package_description', description) action.command = overrides.get('command') - action.cli_command = overrides.get('cli_command') action.include_prereleases = overrides.get('include_prereleases', False) action.plugin_target = overrides.get('plugin_target') return action @@ -105,7 +103,7 @@ def test_install_enabled_true_when_ready_with_upgradable() -> None: state = ActionState(action=_make_action()) state.status = 'Already installed' model.action_states.append(state) - model.upgradable_keys.add(action_key(state.action)) + model.upgradable_keys.add(state.action) assert model.install_enabled is True @staticmethod @@ -149,7 +147,7 @@ def test_actionable_count() -> None: upgradable = ActionState(action=_make_action(package='c')) upgradable.status = 'Update available' model.action_states = [needed, satisfied, upgradable] - model.upgradable_keys.add(action_key(upgradable.action)) + model.upgradable_keys.add(upgradable.action) expected_actionable = 2 # 1 needed + 1 upgradable assert model.actionable_count == expected_actionable diff --git a/tests/unit/qt/test_update_controller.py b/tests/unit/qt/test_update_controller.py index 09218d6..f3a73e4 100644 --- a/tests/unit/qt/test_update_controller.py +++ b/tests/unit/qt/test_update_controller.py @@ -48,6 +48,27 @@ def __init__(self, model: UpdateModel) -> None: # --------------------------------------------------------------------------- +class ModelSpy: + """Records signal emissions from an :class:`UpdateModel`.""" + + def __init__(self, model: UpdateModel) -> None: + """Connect to *model* signals and record emissions.""" + self.status: list[tuple[str, str]] = [] + self.check_button_enabled: list[bool] = [] + self.restart_visible: list[bool] = [] + self.last_checked: list[str] = [] + + model.status_text_changed.connect(lambda t, s: self.status.append((t, s))) + model.check_button_enabled_changed.connect(self.check_button_enabled.append) + model.restart_visible_changed.connect(self.restart_visible.append) + 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] = {