From ae2fbaa9954dfc65bc9d6ea223a7c0e2ed5f05f6 Mon Sep 17 00:00:00 2001 From: Gavin Date: Thu, 28 May 2026 15:34:27 -0700 Subject: [PATCH] fix(cli): clarify PiecesOS install completion wording --- src/pieces/core/install_pieces_os.py | 12 ++- src/pieces/install_messages.py | 32 ++++++++ tests/test_install_messages.py | 118 +++++++++++++++++++++++++++ 3 files changed, 158 insertions(+), 4 deletions(-) create mode 100644 src/pieces/install_messages.py create mode 100644 tests/test_install_messages.py diff --git a/src/pieces/core/install_pieces_os.py b/src/pieces/core/install_pieces_os.py index d55106a7..a3d27a73 100644 --- a/src/pieces/core/install_pieces_os.py +++ b/src/pieces/core/install_pieces_os.py @@ -3,6 +3,10 @@ from rich.progress import Progress, BarColumn, DownloadColumn, TransferSpeedColumn from pieces._vendor.pieces_os_client.wrapper.installation import DownloadModel, DownloadState from ..settings import Settings +from pieces.install_messages import ( + install_completed_message, + install_failed_message, +) from pieces.urls import URLs import platform @@ -36,12 +40,12 @@ def run(self): task, total=model.total_bytes, completed=model.bytes_received ) if model.state == DownloadState.FAILED: - Settings.logger.print( - "❌ Failed to install PiecesOS, Opening in your webbrowser" - ) + Settings.logger.print(install_failed_message()) self.download_docs() elif model.state == DownloadState.COMPLETED: - Settings.logger.print("✅ Installed PiecesOS successfully") + Settings.logger.print( + install_completed_message(platform.system()) + ) progress.refresh() except KeyboardInterrupt: self.installer.cancel_download() diff --git a/src/pieces/install_messages.py b/src/pieces/install_messages.py new file mode 100644 index 00000000..4b017e46 --- /dev/null +++ b/src/pieces/install_messages.py @@ -0,0 +1,32 @@ +"""User-facing PiecesOS install messages. + +Mirrors ``src/pieces/readiness_messages.py``: stdlib-only, zero ``pieces.*`` +imports, pure functions returning strings so the wording is testable without +running the installer. + +Wording constraints (do not relax without re-review): + +- Do not overclaim install state. On macOS/Windows the installer only + downloads the package and launches the OS installer; the user must still + finish the GUI install. Only Linux (snap) actually installs in-process. +- These helpers select wording only. They must not change install behavior + or control flow. +""" + + +def install_completed_message(system: str) -> str: + """Return the success message for a finished download/install. + + ``system`` is the value of ``platform.system()`` ("Linux", "Darwin", + "Windows"). Only Linux performs an in-process install (snap); macOS and + Windows merely download the package and open the OS installer, so they get + download-oriented wording that points the user at the launched installer. + """ + if system == "Linux": + return "✅ Installed PiecesOS. Run `pieces open` to launch it." + return "📥 Downloaded PiecesOS. Finish the installation in the window that just opened, then run `pieces open`." + + +def install_failed_message() -> str: + """Return the failure message shown before opening the manual install page.""" + return "❌ Couldn't install PiecesOS automatically — opening the manual install page in your browser." diff --git a/tests/test_install_messages.py b/tests/test_install_messages.py new file mode 100644 index 00000000..627310e2 --- /dev/null +++ b/tests/test_install_messages.py @@ -0,0 +1,118 @@ +"""Tests for ``src/pieces/install_messages.py``. + +These pin the wording contract of the install messages so macOS/Windows stop +overclaiming "Installed ... successfully" when the CLI only downloaded the +package and launched the OS installer. They also guard the call site against +reintroducing the old overclaiming string, and assert the helper module has no +circular-import surface (stdlib-only, no ``pieces.*`` imports). +""" + +import os +import subprocess +import sys +from pathlib import Path + +import pytest + +from pieces.install_messages import ( + install_completed_message, + install_failed_message, +) + + +REPO_ROOT = Path(__file__).resolve().parents[1] + +# The legacy overclaiming string that must not reappear at the call site. +FORBIDDEN_LEGACY_STRING = "✅ Installed PiecesOS successfully" + +CALL_SITE_PATH = "src/pieces/core/install_pieces_os.py" + + +class TestInstallCompletedMessage: + """Wording assertions for the per-OS success message.""" + + @pytest.mark.parametrize("system", ["Darwin", "Windows"]) + def test_macos_windows_say_downloaded_not_installed(self, system): + msg = install_completed_message(system) + assert "Downloaded PiecesOS" in msg + assert "pieces open" in msg + # Must not overclaim a finished install on these platforms. + assert "Installed PiecesOS successfully" not in msg + + @pytest.mark.parametrize("system", ["Darwin", "Windows"]) + def test_macos_windows_point_at_opened_installer(self, system): + assert "window that just opened" in install_completed_message(system) + + def test_linux_says_installed(self): + msg = install_completed_message("Linux") + assert "Installed PiecesOS" in msg + assert "pieces open" in msg + + def test_linux_differs_from_macos(self): + assert install_completed_message("Linux") != install_completed_message( + "Darwin" + ) + + def test_unknown_platform_falls_back_to_download_wording(self): + # Anything that is not Linux is treated as the download/launch flow. + assert "Downloaded PiecesOS" in install_completed_message("Plan9") + + def test_returns_single_line(self): + for system in ("Linux", "Darwin", "Windows"): + assert "\n" not in install_completed_message(system) + + +class TestInstallFailedMessage: + """Wording assertions for the failure message.""" + + def test_mentions_manual_install_page_in_browser(self): + msg = install_failed_message() + assert "manual install page" in msg + assert "browser" in msg + + def test_does_not_overclaim_success(self): + assert "Installed PiecesOS successfully" not in install_failed_message() + + def test_returns_single_line(self): + assert "\n" not in install_failed_message() + + +def test_call_site_drops_legacy_overclaim_string(): + """Regression guard: the install command call site must not reintroduce + the overclaiming legacy success string. Use the helper instead. + """ + contents = (REPO_ROOT / CALL_SITE_PATH).read_text(encoding="utf-8") + assert FORBIDDEN_LEGACY_STRING not in contents, ( + f"{CALL_SITE_PATH} reintroduced the overclaiming legacy string " + f"{FORBIDDEN_LEGACY_STRING!r}. Use the helpers in " + f"src/pieces/install_messages.py instead." + ) + + +def test_import_smoke_no_circular(): + """Importing the helper in a fresh interpreter must not raise. + + Runs in a subprocess so session module caching cannot mask a real cycle. + """ + src_dir = REPO_ROOT / "src" + + env = os.environ.copy() + existing_pythonpath = env.get("PYTHONPATH", "") + env["PYTHONPATH"] = ( + f"{src_dir}{os.pathsep}{existing_pythonpath}" + if existing_pythonpath + else str(src_dir) + ) + + result = subprocess.run( + [sys.executable, "-c", "import pieces.install_messages"], + capture_output=True, + text=True, + env=env, + ) + + assert result.returncode == 0, ( + f"import regression detected\n" + f"stdout:\n{result.stdout}\n" + f"stderr:\n{result.stderr}" + )