From dad1991196931a621ec26494e9293bff14dc93a9 Mon Sep 17 00:00:00 2001 From: Tiago Nobrega Date: Tue, 10 Dec 2024 12:11:08 -0300 Subject: [PATCH 1/5] chore: add remote-build command from Charmcraft This is a 'chore' commit because the command is added verbatim, including references to charmcraft entities. Also includes a helper function and its tests, both also unchanged. --- craft_application/commands/remote.py | 202 +++++++++++++++++++++++++++ craft_application/util/__init__.py | 2 + craft_application/util/cli.py | 48 +++++++ tests/unit/util/test_cli.py | 95 +++++++++++++ 4 files changed, 347 insertions(+) create mode 100644 craft_application/commands/remote.py create mode 100644 craft_application/util/cli.py create mode 100644 tests/unit/util/test_cli.py diff --git a/craft_application/commands/remote.py b/craft_application/commands/remote.py new file mode 100644 index 00000000..de857ea8 --- /dev/null +++ b/craft_application/commands/remote.py @@ -0,0 +1,202 @@ +# Copyright 2024 Canonical Ltd. +# +# This program is free software: you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License version 3, as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, +# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. +# See the GNU Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License along +# with this program. If not, see . +"""Build a project remotely on Launchpad.""" + +import argparse +import os +import pathlib +import time +from collections.abc import Collection +from typing import Any, cast + +from craft_application.commands import ExtensibleCommand +from craft_application.launchpad.models import Build, BuildState +from craft_application.remote.utils import get_build_id +from craft_cli import emit +from overrides import override # pyright: ignore[reportUnknownVariableType] + +from charmcraft import models, utils + +OVERVIEW = """\ +Command remote-build sends the current project to be built +remotely. After the build is complete, packages for each +architecture are retrieved and will be available in the +local filesystem. + +Interrupted remote builds can be resumed using the --recover +option, followed by the build number informed when the remote +build was originally dispatched. The current state of the +remote build for each architecture can be checked using the +--status option. + +To set a timeout on the remote-build command, use the option +``--launchpad-timeout=``. The timeout is local, so +the build on launchpad will continue even if the local instance +is interrupted or times out. +""" + +_CONFIRMATION_PROMPT = ( + "All data sent to remote builders will be publicly available. " + "Are you sure you want to continue?" +) + + +class RemoteBuild(ExtensibleCommand): + """Run analysis on a built charm.""" + + name = "remote-build" + help_msg = "Build a charm remotely on Launchpad." + overview = OVERVIEW + always_load_project = True + + @override + def _fill_parser(self, parser: argparse.ArgumentParser) -> None: + parser.add_argument( + "--recover", action="store_true", help="recover an interrupted build" + ) + parser.add_argument( + "--launchpad-accept-public-upload", + action="store_true", + help="acknowledge that uploaded code will be publicly available.", + ) + parser.add_argument( + "--launchpad-timeout", + type=int, + default=0, + metavar="", + help="Time in seconds to wait for launchpad to build.", + ) + + def _run( + self, parsed_args: argparse.Namespace, **kwargs: Any + ) -> int | None: # noqa: ANN401 + """Run the remote-build command. + + :param parsed_args: parsed argument namespace from craft_cli. + + :raises AcceptPublicUploadError: If the user does not agree to upload data. + """ + if os.getenv("SUDO_USER") and os.geteuid() == 0: + emit.progress( + "Running with 'sudo' may cause permission errors and is discouraged.", + permanent=True, + ) + # Give the user a bit of time to process this before proceeding. + time.sleep(1) + + emit.progress( + "remote-build is experimental and is subject to change. Use with caution.", + permanent=True, + ) + + if not parsed_args.launchpad_accept_public_upload: + if not utils.confirm_with_user(_CONFIRMATION_PROMPT, default=False): + emit.message("Cannot proceed without accepting a public upload.") + return 77 # permission denied from sysexits.h + + builder = self._services.remote_build + project = cast(models.Charm, self._services.project) + config = cast(dict[str, Any], self.config) + project_dir = ( + pathlib.Path(config.get("global_args", {}).get("project_dir") or ".") + .expanduser() + .resolve() + ) + emit.trace(f"Project directory: {project_dir}") + + if parsed_args.launchpad_timeout: + emit.debug(f"Setting timeout to {parsed_args.launchpad_timeout} seconds") + builder.set_timeout(parsed_args.launchpad_timeout) + + build_id = get_build_id(self._app.name, project.name, project_dir) + if parsed_args.recover: + emit.progress(f"Recovering build {build_id}") + builds = builder.resume_builds(build_id) + else: + emit.progress( + "Starting new build. It may take a while to upload large projects." + ) + builds = builder.start_builds(project_dir) + + try: + returncode = self._monitor_and_complete(build_id, builds) + except KeyboardInterrupt: + if utils.confirm_with_user("Cancel builds?", default=True): + emit.progress("Cancelling builds.") + builder.cancel_builds() + emit.progress("Cleaning up") + builder.cleanup() + returncode = 0 + else: + emit.progress("Cleaning up") + builder.cleanup() + return returncode + + def _monitor_and_complete( + self, build_id: str | None, builds: Collection[Build] + ) -> int: + builder = self._services.remote_build + emit.progress("Monitoring build") + try: + for states in builder.monitor_builds(): + building: set[str] = set() + succeeded: set[str] = set() + uploading: set[str] = set() + not_building: set[str] = set() + for arch, build_state in states.items(): + if build_state.is_running: + building.add(arch) + elif build_state == BuildState.SUCCESS: + succeeded.add(arch) + elif build_state == BuildState.UPLOADING: + uploading.add(arch) + else: + not_building.add(arch) + progress_parts: list[str] = [] + if not_building: + progress_parts.append("Stopped: " + ",".join(sorted(not_building))) + if building: + progress_parts.append("Building: " + ", ".join(sorted(building))) + if uploading: + progress_parts.append("Uploading: " + ",".join(sorted(uploading))) + if succeeded: + progress_parts.append("Succeeded: " + ", ".join(sorted(succeeded))) + emit.progress("; ".join(progress_parts)) + except TimeoutError: + if build_id: + resume_command = ( + f"{self._app.name} remote-build --recover --build-id={build_id}" + ) + else: + resume_command = f"{self._app.name} remote-build --recover" + emit.message( + f"Timed out waiting for build.\nTo resume, run {resume_command!r}" + ) + return 75 # Temporary failure + + emit.progress(f"Fetching {len(builds)} build logs...") + logs = builder.fetch_logs(pathlib.Path.cwd()) + + emit.progress("Fetching build artifacts...") + artifacts = builder.fetch_artifacts(pathlib.Path.cwd()) + + log_names = sorted(path.name for path in logs.values() if path) + artifact_names = sorted(path.name for path in artifacts) + + emit.message( + "Build completed.\n" + f"Log files: {', '.join(log_names)}\n" + f"Artifacts: {', '.join(artifact_names)}" + ) + return 0 diff --git a/craft_application/util/__init__.py b/craft_application/util/__init__.py index 6bd33ead..f9753b03 100644 --- a/craft_application/util/__init__.py +++ b/craft_application/util/__init__.py @@ -16,6 +16,7 @@ """Utilities for craft-application.""" from craft_application.util.callbacks import get_unique_callbacks +from craft_application.util.cli import confirm_with_user from craft_application.util.docs import render_doc_url from craft_application.util.logging import setup_loggers from craft_application.util.paths import get_filename_from_url_path, get_managed_logpath @@ -37,6 +38,7 @@ __all__ = [ "get_unique_callbacks", + "confirm_with_user", "render_doc_url", "setup_loggers", "get_filename_from_url_path", diff --git a/craft_application/util/cli.py b/craft_application/util/cli.py new file mode 100644 index 00000000..c52c8069 --- /dev/null +++ b/craft_application/util/cli.py @@ -0,0 +1,48 @@ +# This file is part of craft-application. +# +# Copyright 2024 Canonical Ltd. +# +# This program is free software: you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License version 3, as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, +# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. +# See the GNU Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with this program. If not, see . +"""Utilities related to the command line.""" +import sys + +from craft_cli import emit + + +def confirm_with_user(prompt: str, default: bool = False) -> bool: + """Query user for yes/no answer. + + If stdin is not a tty, the default value is returned. + + If user returns an empty answer, the default value is returned. + + :returns: True if answer starts with [yY], False if answer starts with [nN], + otherwise the default. + """ + if is_charmcraft_running_in_managed_mode(): + raise RuntimeError("confirmation not yet supported in managed-mode") + + if not sys.stdin.isatty(): + return default + + choices = " [Y/n]: " if default else " [y/N]: " + + with emit.pause(): + reply = input(prompt + choices).lower().strip() + + if reply and reply[0] == "y": + return True + elif reply and reply[0] == "n": + return False + else: + return default diff --git a/tests/unit/util/test_cli.py b/tests/unit/util/test_cli.py new file mode 100644 index 00000000..850d5693 --- /dev/null +++ b/tests/unit/util/test_cli.py @@ -0,0 +1,95 @@ +# This file is part of craft-application. +# +# Copyright 2024 Canonical Ltd. +# +# This program is free software: you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License version 3, as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, +# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. +# See the GNU Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with this program. If not, see . +"""Tests for cli functions.""" +from unittest.mock import call, patch + +import pytest +from craft_application.util import confirm_with_user + + +@pytest.fixture +def mock_isatty(): + with patch("sys.stdin.isatty", return_value=True) as mock_isatty: + yield mock_isatty + + +@pytest.fixture +def mock_input(): + with patch("charmcraft.utils.cli.input", return_value="") as mock_input: + yield mock_input + + +def test_confirm_with_user_defaults_with_tty(mock_input, mock_isatty): + mock_input.return_value = "" + mock_isatty.return_value = True + + assert confirm_with_user("prompt", default=True) is True + assert mock_input.mock_calls == [call("prompt [Y/n]: ")] + mock_input.reset_mock() + + assert confirm_with_user("prompt", default=False) is False + assert mock_input.mock_calls == [call("prompt [y/N]: ")] + + +def test_confirm_with_user_defaults_without_tty(mock_input, mock_isatty): + mock_isatty.return_value = False + + assert confirm_with_user("prompt", default=True) is True + assert confirm_with_user("prompt", default=False) is False + + assert mock_input.mock_calls == [] + + +@pytest.mark.parametrize( + ("user_input", "expected"), + [ + ("y", True), + ("Y", True), + ("yes", True), + ("YES", True), + ("n", False), + ("N", False), + ("no", False), + ("NO", False), + ], +) +def test_confirm_with_user(user_input, expected, mock_input, mock_isatty): + mock_input.return_value = user_input + + assert confirm_with_user("prompt") == expected + assert mock_input.mock_calls == [call("prompt [y/N]: ")] + + +def test_confirm_with_user_errors_in_managed_mode( + mock_is_charmcraft_running_in_managed_mode, +): + mock_is_charmcraft_running_in_managed_mode.return_value = True + + with pytest.raises(RuntimeError): + confirm_with_user("prompt") + + +def test_confirm_with_user_pause_emitter(mock_isatty, emitter): + """The emitter should be paused when using the terminal.""" + mock_isatty.return_value = True + + def fake_input(prompt): + """Check if the Emitter is paused.""" + assert emitter.paused + return "" + + with patch("charmcraft.utils.cli.input", fake_input): + confirm_with_user("prompt") From 699920d7776cda17caafa0d5684e9de83c6198fe Mon Sep 17 00:00:00 2001 From: Tiago Nobrega Date: Tue, 10 Dec 2024 15:19:43 -0300 Subject: [PATCH 2/5] feat: add a 'remote-build' command The implementation comes from Charmcraft. This commit has the code changes to make the command craft-tool-agnostic, plus some general test and linting fixes and improvements. --- craft_application/commands/__init__.py | 2 + craft_application/commands/remote.py | 38 +++++---- craft_application/util/cli.py | 10 +-- docs/reference/changelog.rst | 6 ++ tests/conftest.py | 13 ++++ tests/unit/commands/test_remote.py | 104 +++++++++++++++++++++++++ tests/unit/util/test_cli.py | 32 +++----- 7 files changed, 160 insertions(+), 45 deletions(-) create mode 100644 tests/unit/commands/test_remote.py diff --git a/craft_application/commands/__init__.py b/craft_application/commands/__init__.py index d636c58a..90f7ea89 100644 --- a/craft_application/commands/__init__.py +++ b/craft_application/commands/__init__.py @@ -20,11 +20,13 @@ from .init import InitCommand from .lifecycle import get_lifecycle_command_group, LifecycleCommand from .other import get_other_command_group +from .remote import RemoteBuild # Not part of the default commands. __all__ = [ "AppCommand", "ExtensibleCommand", "InitCommand", + "RemoteBuild", "lifecycle", "LifecycleCommand", "get_lifecycle_command_group", diff --git a/craft_application/commands/remote.py b/craft_application/commands/remote.py index de857ea8..84681374 100644 --- a/craft_application/commands/remote.py +++ b/craft_application/commands/remote.py @@ -20,13 +20,13 @@ from collections.abc import Collection from typing import Any, cast -from craft_application.commands import ExtensibleCommand -from craft_application.launchpad.models import Build, BuildState -from craft_application.remote.utils import get_build_id from craft_cli import emit from overrides import override # pyright: ignore[reportUnknownVariableType] -from charmcraft import models, utils +from craft_application import models, util +from craft_application.commands import ExtensibleCommand +from craft_application.launchpad.models import Build, BuildState +from craft_application.remote.utils import get_build_id OVERVIEW = """\ Command remote-build sends the current project to be built @@ -53,10 +53,10 @@ class RemoteBuild(ExtensibleCommand): - """Run analysis on a built charm.""" + """Build a project on Launchpad.""" name = "remote-build" - help_msg = "Build a charm remotely on Launchpad." + help_msg = "Build a project remotely on Launchpad." overview = OVERVIEW always_load_project = True @@ -79,8 +79,10 @@ def _fill_parser(self, parser: argparse.ArgumentParser) -> None: ) def _run( - self, parsed_args: argparse.Namespace, **kwargs: Any - ) -> int | None: # noqa: ANN401 + self, + parsed_args: argparse.Namespace, + **_kwargs: Any, # noqa: ANN401 (use of Any) + ) -> int | None: """Run the remote-build command. :param parsed_args: parsed argument namespace from craft_cli. @@ -100,13 +102,15 @@ def _run( permanent=True, ) - if not parsed_args.launchpad_accept_public_upload: - if not utils.confirm_with_user(_CONFIRMATION_PROMPT, default=False): - emit.message("Cannot proceed without accepting a public upload.") - return 77 # permission denied from sysexits.h + if ( + not parsed_args.launchpad_accept_public_upload + and not util.confirm_with_user(_CONFIRMATION_PROMPT, default=False) + ): + emit.message("Cannot proceed without accepting a public upload.") + return 77 # permission denied from sysexits.h builder = self._services.remote_build - project = cast(models.Charm, self._services.project) + project = cast(models.Project, self._services.project) config = cast(dict[str, Any], self.config) project_dir = ( pathlib.Path(config.get("global_args", {}).get("project_dir") or ".") @@ -132,7 +136,7 @@ def _run( try: returncode = self._monitor_and_complete(build_id, builds) except KeyboardInterrupt: - if utils.confirm_with_user("Cancel builds?", default=True): + if util.confirm_with_user("Cancel builds?", default=True): emit.progress("Cancelling builds.") builder.cancel_builds() emit.progress("Cleaning up") @@ -143,7 +147,7 @@ def _run( builder.cleanup() return returncode - def _monitor_and_complete( + def _monitor_and_complete( # noqa: PLR0912 (too many branches) self, build_id: str | None, builds: Collection[Build] ) -> int: builder = self._services.remote_build @@ -165,11 +169,11 @@ def _monitor_and_complete( not_building.add(arch) progress_parts: list[str] = [] if not_building: - progress_parts.append("Stopped: " + ",".join(sorted(not_building))) + progress_parts.append("Stopped: " + ", ".join(sorted(not_building))) if building: progress_parts.append("Building: " + ", ".join(sorted(building))) if uploading: - progress_parts.append("Uploading: " + ",".join(sorted(uploading))) + progress_parts.append("Uploading: " + ", ".join(sorted(uploading))) if succeeded: progress_parts.append("Succeeded: " + ", ".join(sorted(succeeded))) emit.progress("; ".join(progress_parts)) diff --git a/craft_application/util/cli.py b/craft_application/util/cli.py index c52c8069..018f9aec 100644 --- a/craft_application/util/cli.py +++ b/craft_application/util/cli.py @@ -19,7 +19,7 @@ from craft_cli import emit -def confirm_with_user(prompt: str, default: bool = False) -> bool: +def confirm_with_user(prompt: str, *, default: bool = False) -> bool: """Query user for yes/no answer. If stdin is not a tty, the default value is returned. @@ -29,9 +29,6 @@ def confirm_with_user(prompt: str, default: bool = False) -> bool: :returns: True if answer starts with [yY], False if answer starts with [nN], otherwise the default. """ - if is_charmcraft_running_in_managed_mode(): - raise RuntimeError("confirmation not yet supported in managed-mode") - if not sys.stdin.isatty(): return default @@ -42,7 +39,6 @@ def confirm_with_user(prompt: str, default: bool = False) -> bool: if reply and reply[0] == "y": return True - elif reply and reply[0] == "n": + if reply and reply[0] == "n": return False - else: - return default + return default diff --git a/docs/reference/changelog.rst b/docs/reference/changelog.rst index d607022b..f7c44f67 100644 --- a/docs/reference/changelog.rst +++ b/docs/reference/changelog.rst @@ -7,6 +7,12 @@ Changelog 4.6.0 (YYYY-MMM-DD) ------------------- +Commands +======== + +- Add a ``remote-build`` command. This command is not registered by default, + but is available for application use. + Git === diff --git a/tests/conftest.py b/tests/conftest.py index bed92b32..d825f7c1 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -23,6 +23,7 @@ from dataclasses import dataclass from importlib import metadata from typing import TYPE_CHECKING, Any +from unittest.mock import Mock import craft_application import craft_parts @@ -315,6 +316,16 @@ def _get_loader(self, template_dir: pathlib.Path) -> jinja2.BaseLoader: return FakeInitService +@pytest.fixture +def fake_remote_build_service_class(): + class FakeRemoteBuild(services.RemoteBuildService): + @override + def _get_lp_client(self) -> launchpad.Launchpad: + return Mock(spec=launchpad.Launchpad) + + return FakeRemoteBuild + + @pytest.fixture def fake_services( tmp_path, @@ -323,10 +334,12 @@ def fake_services( fake_lifecycle_service_class, fake_package_service_class, fake_init_service_class, + fake_remote_build_service_class, ): services.ServiceFactory.register("package", fake_package_service_class) services.ServiceFactory.register("lifecycle", fake_lifecycle_service_class) services.ServiceFactory.register("init", fake_init_service_class) + services.ServiceFactory.register("remote_build", fake_remote_build_service_class) factory = services.ServiceFactory(app_metadata, project=fake_project) factory.update_kwargs( "lifecycle", work_dir=tmp_path, cache_dir=tmp_path / "cache", build_plan=[] diff --git a/tests/unit/commands/test_remote.py b/tests/unit/commands/test_remote.py new file mode 100644 index 00000000..f5f46351 --- /dev/null +++ b/tests/unit/commands/test_remote.py @@ -0,0 +1,104 @@ +# This file is part of craft-application. +# +# Copyright 2024 Canonical Ltd. +# +# This program is free software: you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License version 3, as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, +# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. +# See the GNU Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License along +# with this program. If not, see . +"""Tests for remote-build commands.""" +import argparse + +import pytest +from craft_application import util +from craft_application.commands import RemoteBuild +from craft_application.launchpad.models import BuildState + + +@pytest.fixture +def remote_build( + app_metadata, + fake_services, +): + config = {"app": app_metadata, "services": fake_services} + return RemoteBuild(config) + + +def test_remote_build_no_accept_upload(remote_build, mocker): + parsed_args = argparse.Namespace(launchpad_accept_public_upload=False) + + mocker.patch.object(util, "confirm_with_user", return_value=False) + assert remote_build.run(parsed_args) == 77 # noqa: PLR2004 (magic value) + + +def test_remote_build_run(remote_build, mocker, fake_services, tmp_path, emitter): + builder = fake_services.remote_build + + build_states = [ + # All 3 builds still pending + { + "arch1": BuildState.PENDING, + "arch2": BuildState.PENDING, + "arch3": BuildState.PENDING, + }, + # 2 builds running, 1 pending + { + "arch1": BuildState.BUILDING, + "arch2": BuildState.BUILDING, + "arch3": BuildState.PENDING, + }, + # 1 uploading, 1 building, 1 pending + { + "arch1": BuildState.UPLOADING, + "arch2": BuildState.BUILDING, + "arch3": BuildState.PENDING, + }, + # All 3 succeeded + { + "arch1": BuildState.SUCCESS, + "arch2": BuildState.SUCCESS, + "arch3": BuildState.SUCCESS, + }, + ] + + mocker.patch.object( + builder, "start_builds", return_value=["arch1", "arch2", "arch3"] + ) + mocker.patch.object(builder, "monitor_builds", side_effect=[build_states]) + + logs = { + "arch1": tmp_path / "log1.txt", + "arch2": tmp_path / "log2.txt", + "arch3": tmp_path / "log3.txt", + } + mocker.patch.object(builder, "fetch_logs", return_value=logs) + + artifacts = [tmp_path / "art1.zip", tmp_path / "art2.zip", tmp_path / "art3.zip"] + mocker.patch.object(builder, "fetch_artifacts", return_value=artifacts) + + parsed_args = argparse.Namespace( + launchpad_accept_public_upload=True, launchpad_timeout=None, recover=False + ) + assert remote_build.run(parsed_args) is None + + emitter.assert_progress( + "Starting new build. It may take a while to upload large projects." + ) + emitter.assert_progress("Stopped: arch1, arch2, arch3") + emitter.assert_progress("Stopped: arch3; Building: arch1, arch2") + emitter.assert_progress("Stopped: arch3; Building: arch2; Uploading: arch1") + emitter.assert_progress("Succeeded: arch1, arch2, arch3") + emitter.assert_progress("Fetching 3 build logs...") + emitter.assert_progress("Fetching build artifacts...") + emitter.assert_message( + "Build completed.\n" + "Log files: log1.txt, log2.txt, log3.txt\n" + "Artifacts: art1.zip, art2.zip, art3.zip" + ) diff --git a/tests/unit/util/test_cli.py b/tests/unit/util/test_cli.py index 850d5693..bbe7401e 100644 --- a/tests/unit/util/test_cli.py +++ b/tests/unit/util/test_cli.py @@ -14,22 +14,20 @@ # You should have received a copy of the GNU Lesser General Public License # along with this program. If not, see . """Tests for cli functions.""" -from unittest.mock import call, patch +from unittest.mock import call import pytest from craft_application.util import confirm_with_user @pytest.fixture -def mock_isatty(): - with patch("sys.stdin.isatty", return_value=True) as mock_isatty: - yield mock_isatty +def mock_isatty(mocker): + return mocker.patch("sys.stdin.isatty", return_value=True) @pytest.fixture -def mock_input(): - with patch("charmcraft.utils.cli.input", return_value="") as mock_input: - yield mock_input +def mock_input(mocker): + return mocker.patch("builtins.input", return_value="") def test_confirm_with_user_defaults_with_tty(mock_input, mock_isatty): @@ -66,30 +64,22 @@ def test_confirm_with_user_defaults_without_tty(mock_input, mock_isatty): ("NO", False), ], ) -def test_confirm_with_user(user_input, expected, mock_input, mock_isatty): +@pytest.mark.usefixtures("mock_isatty") +def test_confirm_with_user(user_input, expected, mock_input): mock_input.return_value = user_input assert confirm_with_user("prompt") == expected assert mock_input.mock_calls == [call("prompt [y/N]: ")] -def test_confirm_with_user_errors_in_managed_mode( - mock_is_charmcraft_running_in_managed_mode, -): - mock_is_charmcraft_running_in_managed_mode.return_value = True - - with pytest.raises(RuntimeError): - confirm_with_user("prompt") - - -def test_confirm_with_user_pause_emitter(mock_isatty, emitter): +def test_confirm_with_user_pause_emitter(mock_isatty, emitter, mocker): """The emitter should be paused when using the terminal.""" mock_isatty.return_value = True - def fake_input(prompt): + def fake_input(_prompt): """Check if the Emitter is paused.""" assert emitter.paused return "" - with patch("charmcraft.utils.cli.input", fake_input): - confirm_with_user("prompt") + mocker.patch("builtins.input", fake_input) + confirm_with_user("prompt") From f459a321430b44a876cc10f0a04f47ab4780d0b4 Mon Sep 17 00:00:00 2001 From: Tiago Nobrega Date: Thu, 12 Dec 2024 18:46:42 -0300 Subject: [PATCH 3/5] fixup! feat: add a 'remote-build' command --- craft_application/commands/remote.py | 8 ++++---- tests/unit/util/test_cli.py | 6 ++---- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/craft_application/commands/remote.py b/craft_application/commands/remote.py index 84681374..6a8ebb48 100644 --- a/craft_application/commands/remote.py +++ b/craft_application/commands/remote.py @@ -30,7 +30,7 @@ OVERVIEW = """\ Command remote-build sends the current project to be built -remotely. After the build is complete, packages for each +remotely. After the build is complete, packages for each architecture are retrieved and will be available in the local filesystem. @@ -189,12 +189,12 @@ def _monitor_and_complete( # noqa: PLR0912 (too many branches) ) return 75 # Temporary failure - emit.progress(f"Fetching {len(builds)} build logs...") - logs = builder.fetch_logs(pathlib.Path.cwd()) - emit.progress("Fetching build artifacts...") artifacts = builder.fetch_artifacts(pathlib.Path.cwd()) + emit.progress(f"Fetching {len(builds)} build logs...") + logs = builder.fetch_logs(pathlib.Path.cwd()) + log_names = sorted(path.name for path in logs.values() if path) artifact_names = sorted(path.name for path in artifacts) diff --git a/tests/unit/util/test_cli.py b/tests/unit/util/test_cli.py index bbe7401e..c26b5eef 100644 --- a/tests/unit/util/test_cli.py +++ b/tests/unit/util/test_cli.py @@ -30,10 +30,8 @@ def mock_input(mocker): return mocker.patch("builtins.input", return_value="") -def test_confirm_with_user_defaults_with_tty(mock_input, mock_isatty): - mock_input.return_value = "" - mock_isatty.return_value = True - +@pytest.mark.usefixtures("mock_isatty") +def test_confirm_with_user_defaults_with_tty(mock_input): assert confirm_with_user("prompt", default=True) is True assert mock_input.mock_calls == [call("prompt [Y/n]: ")] mock_input.reset_mock() From c3754b105cf8a943ea57de413da434b93b2bf7cd Mon Sep 17 00:00:00 2001 From: Tiago Nobrega Date: Fri, 13 Dec 2024 10:53:18 -0300 Subject: [PATCH 4/5] refactor: use emit.confirm() instead --- craft_application/commands/remote.py | 9 ++- craft_application/util/__init__.py | 2 - craft_application/util/cli.py | 44 --------------- tests/unit/commands/test_remote.py | 4 +- tests/unit/util/test_cli.py | 83 ---------------------------- 5 files changed, 6 insertions(+), 136 deletions(-) delete mode 100644 craft_application/util/cli.py delete mode 100644 tests/unit/util/test_cli.py diff --git a/craft_application/commands/remote.py b/craft_application/commands/remote.py index 6a8ebb48..07c52b63 100644 --- a/craft_application/commands/remote.py +++ b/craft_application/commands/remote.py @@ -23,7 +23,7 @@ from craft_cli import emit from overrides import override # pyright: ignore[reportUnknownVariableType] -from craft_application import models, util +from craft_application import models from craft_application.commands import ExtensibleCommand from craft_application.launchpad.models import Build, BuildState from craft_application.remote.utils import get_build_id @@ -102,9 +102,8 @@ def _run( permanent=True, ) - if ( - not parsed_args.launchpad_accept_public_upload - and not util.confirm_with_user(_CONFIRMATION_PROMPT, default=False) + if not parsed_args.launchpad_accept_public_upload and not emit.confirm( + _CONFIRMATION_PROMPT, default=False ): emit.message("Cannot proceed without accepting a public upload.") return 77 # permission denied from sysexits.h @@ -136,7 +135,7 @@ def _run( try: returncode = self._monitor_and_complete(build_id, builds) except KeyboardInterrupt: - if util.confirm_with_user("Cancel builds?", default=True): + if emit.confirm("Cancel builds?", default=True): emit.progress("Cancelling builds.") builder.cancel_builds() emit.progress("Cleaning up") diff --git a/craft_application/util/__init__.py b/craft_application/util/__init__.py index f9753b03..6bd33ead 100644 --- a/craft_application/util/__init__.py +++ b/craft_application/util/__init__.py @@ -16,7 +16,6 @@ """Utilities for craft-application.""" from craft_application.util.callbacks import get_unique_callbacks -from craft_application.util.cli import confirm_with_user from craft_application.util.docs import render_doc_url from craft_application.util.logging import setup_loggers from craft_application.util.paths import get_filename_from_url_path, get_managed_logpath @@ -38,7 +37,6 @@ __all__ = [ "get_unique_callbacks", - "confirm_with_user", "render_doc_url", "setup_loggers", "get_filename_from_url_path", diff --git a/craft_application/util/cli.py b/craft_application/util/cli.py deleted file mode 100644 index 018f9aec..00000000 --- a/craft_application/util/cli.py +++ /dev/null @@ -1,44 +0,0 @@ -# This file is part of craft-application. -# -# Copyright 2024 Canonical Ltd. -# -# This program is free software: you can redistribute it and/or modify it -# under the terms of the GNU Lesser General Public License version 3, as -# published by the Free Software Foundation. -# -# This program is distributed in the hope that it will be useful, but WITHOUT -# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, -# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. -# See the GNU Lesser General Public License for more details. -# -# You should have received a copy of the GNU Lesser General Public License -# along with this program. If not, see . -"""Utilities related to the command line.""" -import sys - -from craft_cli import emit - - -def confirm_with_user(prompt: str, *, default: bool = False) -> bool: - """Query user for yes/no answer. - - If stdin is not a tty, the default value is returned. - - If user returns an empty answer, the default value is returned. - - :returns: True if answer starts with [yY], False if answer starts with [nN], - otherwise the default. - """ - if not sys.stdin.isatty(): - return default - - choices = " [Y/n]: " if default else " [y/N]: " - - with emit.pause(): - reply = input(prompt + choices).lower().strip() - - if reply and reply[0] == "y": - return True - if reply and reply[0] == "n": - return False - return default diff --git a/tests/unit/commands/test_remote.py b/tests/unit/commands/test_remote.py index f5f46351..bb5d5b38 100644 --- a/tests/unit/commands/test_remote.py +++ b/tests/unit/commands/test_remote.py @@ -17,9 +17,9 @@ import argparse import pytest -from craft_application import util from craft_application.commands import RemoteBuild from craft_application.launchpad.models import BuildState +from craft_cli import emit @pytest.fixture @@ -34,7 +34,7 @@ def remote_build( def test_remote_build_no_accept_upload(remote_build, mocker): parsed_args = argparse.Namespace(launchpad_accept_public_upload=False) - mocker.patch.object(util, "confirm_with_user", return_value=False) + mocker.patch.object(emit, "confirm", return_value=False) assert remote_build.run(parsed_args) == 77 # noqa: PLR2004 (magic value) diff --git a/tests/unit/util/test_cli.py b/tests/unit/util/test_cli.py deleted file mode 100644 index c26b5eef..00000000 --- a/tests/unit/util/test_cli.py +++ /dev/null @@ -1,83 +0,0 @@ -# This file is part of craft-application. -# -# Copyright 2024 Canonical Ltd. -# -# This program is free software: you can redistribute it and/or modify it -# under the terms of the GNU Lesser General Public License version 3, as -# published by the Free Software Foundation. -# -# This program is distributed in the hope that it will be useful, but WITHOUT -# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, -# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. -# See the GNU Lesser General Public License for more details. -# -# You should have received a copy of the GNU Lesser General Public License -# along with this program. If not, see . -"""Tests for cli functions.""" -from unittest.mock import call - -import pytest -from craft_application.util import confirm_with_user - - -@pytest.fixture -def mock_isatty(mocker): - return mocker.patch("sys.stdin.isatty", return_value=True) - - -@pytest.fixture -def mock_input(mocker): - return mocker.patch("builtins.input", return_value="") - - -@pytest.mark.usefixtures("mock_isatty") -def test_confirm_with_user_defaults_with_tty(mock_input): - assert confirm_with_user("prompt", default=True) is True - assert mock_input.mock_calls == [call("prompt [Y/n]: ")] - mock_input.reset_mock() - - assert confirm_with_user("prompt", default=False) is False - assert mock_input.mock_calls == [call("prompt [y/N]: ")] - - -def test_confirm_with_user_defaults_without_tty(mock_input, mock_isatty): - mock_isatty.return_value = False - - assert confirm_with_user("prompt", default=True) is True - assert confirm_with_user("prompt", default=False) is False - - assert mock_input.mock_calls == [] - - -@pytest.mark.parametrize( - ("user_input", "expected"), - [ - ("y", True), - ("Y", True), - ("yes", True), - ("YES", True), - ("n", False), - ("N", False), - ("no", False), - ("NO", False), - ], -) -@pytest.mark.usefixtures("mock_isatty") -def test_confirm_with_user(user_input, expected, mock_input): - mock_input.return_value = user_input - - assert confirm_with_user("prompt") == expected - assert mock_input.mock_calls == [call("prompt [y/N]: ")] - - -def test_confirm_with_user_pause_emitter(mock_isatty, emitter, mocker): - """The emitter should be paused when using the terminal.""" - mock_isatty.return_value = True - - def fake_input(_prompt): - """Check if the Emitter is paused.""" - assert emitter.paused - return "" - - mocker.patch("builtins.input", fake_input) - confirm_with_user("prompt") From 04dc7108e52154ce243180875e579959349db7a2 Mon Sep 17 00:00:00 2001 From: Tiago Nobrega Date: Fri, 13 Dec 2024 11:05:16 -0300 Subject: [PATCH 5/5] build(deps): bump craft-cli to >= 2.12.0 --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index ab15c0a9..8c2ec5be 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,7 +5,7 @@ dynamic = ["version", "readme"] dependencies = [ "annotated-types>=0.6.0", "craft-archives>=2.0.0", - "craft-cli>=2.10.1", + "craft-cli>=2.12.0", "craft-grammar>=2.0.0", "craft-parts>=2.1.1", "craft-platforms>=0.3.1",