From d54fd89852af6019163c709add4ef5448b10fa0b Mon Sep 17 00:00:00 2001 From: Kivi Kaitaniemi Date: Fri, 26 Dec 2025 18:55:01 +0200 Subject: [PATCH] Improve command output printing --- README.md | 6 ++ src/decman/__init__.py | 65 +---------------- src/decman/app.py | 3 +- src/decman/core/command.py | 92 ++++++++++++++++++++++-- src/decman/core/error.py | 9 ++- src/decman/plugins/aur/__init__.py | 8 ++- src/decman/plugins/aur/commands.py | 7 +- src/decman/plugins/aur/fpm.py | 43 +++-------- src/decman/plugins/aur/package.py | 4 +- src/decman/plugins/flatpak.py | 15 ++-- src/decman/plugins/pacman.py | 19 +++-- src/decman/plugins/systemd.py | 16 +++-- tests/test_decman_core_command.py | 102 +++++++++++++++++++++++++++ tests/test_decman_init.py | 102 --------------------------- tests/test_decman_plugins_pacman.py | 2 +- tests/test_decman_plugins_systemd.py | 20 +++--- 16 files changed, 268 insertions(+), 245 deletions(-) diff --git a/README.md b/README.md index 2ae21a1..4ff6e55 100644 --- a/README.md +++ b/README.md @@ -126,6 +126,12 @@ Decman has some CLI options, to see them all run: decman --help ``` +For troubleshooting and submitting issues, you should use the `--debug` option. + +```sh +sudo decman --debug +``` + [See the complete documentation for using decman.](/docs/README.md) ## Installation diff --git a/src/decman/__init__.py b/src/decman/__init__.py index 3b9dc11..3727b5a 100644 --- a/src/decman/__init__.py +++ b/src/decman/__init__.py @@ -1,10 +1,7 @@ -import shlex import typing -import decman.core.command as command -import decman.core.output as output - # Re-exports +from decman.core.command import prg from decman.core.error import SourceError from decman.core.fs import Directory, File from decman.core.module import Module @@ -65,66 +62,6 @@ if isinstance(_flatpak, Flatpak): flatpak = _flatpak -def prg( - cmd: list[str], - user: typing.Optional[str] = None, - env_overrides: typing.Optional[dict[str, str]] = None, - mimic_login: bool = False, - pty: bool = True, - check: bool = True, -) -> str: - """ - Shortcut for running a command. Returns the output of that command. - - Arguments: - cmd: - Command to execute. - - user: - User name to run the command as. If set, the command is executed after dropping - privileges to this user. - - env_overrides: - Environment variables to override or add for the command execution. - These values are merged on top of the current process environment. - - mimic_login: - If mimic_login is True, will set the following environment variables according to the - given user's passwd file details. This only happens when user is set. - - HOME - - USER - - LOGNAME - - SHELL - - pty: - If True, run the command inside a pseudo-terminal (PTY). This enables interactive - behavior and terminal-dependent programs. If False, run the command without a PTY - using standard subprocess execution. - - check: - If True, raise CommandFailedError when the command exits with a non-zero status. - If False, print a warning when encountering a non-zero exit code. - """ - if pty: - result = command.pty_run( - cmd, user=user, env_overrides=env_overrides, mimic_login=mimic_login - ) - else: - result = command.run(cmd, user=user, env_overrides=env_overrides, mimic_login=mimic_login) - - if check: - # This raises an error if the command failed exiting the function early - result = command.check_run_result(cmd, result) - - code, command_output = result - if code != 0: - output.print_warning(f"Command '{shlex.join(cmd)}' returned with an exit code {code}.") - if not pty: - output.print_command_output(command_output) - - return command_output - - def sh( sh_cmd: str, user: typing.Optional[str] = None, diff --git a/src/decman/app.py b/src/decman/app.py index 50ee876..324e162 100644 --- a/src/decman/app.py +++ b/src/decman/app.py @@ -84,7 +84,8 @@ def main(): failed = True except errors.CommandFailedError as error: output.print_error(str(error)) - output.print_command_output(error.output) + if error.output: + output.print_command_output(error.output) output.print_traceback() failed = True except ValueError as error: diff --git a/src/decman/core/command.py b/src/decman/core/command.py index 1ad1d8a..7a5be44 100644 --- a/src/decman/core/command.py +++ b/src/decman/core/command.py @@ -28,6 +28,80 @@ def get_user_info(user: str) -> tuple[int, int]: return info.pw_uid, info.pw_gid +def prg( + cmd: list[str], + user: typing.Optional[str] = None, + env_overrides: typing.Optional[dict[str, str]] = None, + pass_environment: bool = True, + mimic_login: bool = False, + pty: bool = True, + check: bool = True, +) -> str: + """ + Shortcut for running a command. Returns the output of that command. + + Arguments: + cmd: + Command to execute. + + user: + User name to run the command as. If set, the command is executed after dropping + privileges to this user. + + env_overrides: + Environment variables to override or add for the command execution. + These values are merged on top of the current process environment. + + mimic_login: + If mimic_login is True, will set the following environment variables according to the + given user's passwd file details. This only happens when user is set. + - HOME + - USER + - LOGNAME + - SHELL + + pty: + If True, run the command inside a pseudo-terminal (PTY). This enables interactive + behavior and terminal-dependent programs. If False, run the command without a PTY + using standard subprocess execution. + + If running in a PTY, the raised CommandFailedError will not contain command output, + since it has already been shown to the user. + + check: + If True, raise CommandFailedError when the command exits with a non-zero status. + If False, print a warning when encountering a non-zero exit code. + """ + if pty: + result = pty_run( + cmd, + user=user, + env_overrides=env_overrides, + pass_environment=pass_environment, + mimic_login=mimic_login, + ) + else: + result = run( + cmd, + user=user, + env_overrides=env_overrides, + pass_environment=pass_environment, + mimic_login=mimic_login, + ) + + if check: + # This raises an error if the command failed exiting the function early + result = check_run_result(cmd, result, include_output=not pty) + + code, command_output = result + if code != 0: + output.print_warning(f"Command '{shlex.join(cmd)}' returned with an exit code {code}.") + if not pty: + output.print_command_output(command_output) + + return command_output + + def pty_run( command: list[str], user: None | str = None, @@ -64,7 +138,7 @@ def pty_run( command[0] = shutil.which(command[0]) or command[0] - output.print_debug(f"Running command '{shlex.join(command)}'") + output.print_debug(f"Running command '{shlex.join(command)}'.") env = _build_env(user, env_overrides, mimic_login, pass_environment) @@ -107,7 +181,7 @@ def run( command[0] = shutil.which(command[0]) or command[0] - output.print_debug(f"Running command '{shlex.join(command)}'") + output.print_debug(f"Running command '{shlex.join(command)}'.") env = _build_env(user, env_overrides, mimic_login, pass_environment) uid, gid = None, None @@ -130,7 +204,9 @@ def run( return process.returncode, stdout.decode("utf-8", errors="replace") -def check_run_result(command: list[str], result: tuple[int, str]) -> tuple[int, str]: +def check_run_result( + command: list[str], result: tuple[int, str], include_output: bool = True +) -> tuple[int, str]: """ Validates the result of a command execution. @@ -141,7 +217,10 @@ def check_run_result(command: list[str], result: tuple[int, str]) -> tuple[int, """ code, output = result if code != 0: - raise errors.CommandFailedError(command, output) + if include_output: + raise errors.CommandFailedError(command, output) + else: + raise errors.CommandFailedError(command, None) return code, output @@ -152,9 +231,10 @@ def _build_env( pass_environment: bool, ) -> dict[str, str]: env = {} + cwd = os.getcwd() output.print_debug( - f"Command environment is: user={user}, env_overrides={env_overrides}," - f"mimic_login={mimic_login}, pass_environment={pass_environment}" + f"Command environment is: cwd='{cwd}', user='{user}', env_overrides='{env_overrides}', " + f"mimic_login='{mimic_login}', pass_environment='{pass_environment}'" ) if pass_environment: diff --git a/src/decman/core/error.py b/src/decman/core/error.py index f1ac710..f2a57fb 100644 --- a/src/decman/core/error.py +++ b/src/decman/core/error.py @@ -65,7 +65,10 @@ class CommandFailedError(Exception): command (list[str]): The command that caused the exception. """ - def __init__(self, command: list[str], output: str) -> None: + def __init__(self, command: list[str], output: str | None) -> None: self.command = shlex.join(command) - self.output = output.strip() - super().__init__(f"Running a command '{self.command}' failed.") + if output: + self.output: str | None = output.strip() + else: + self.output = None + super().__init__(f"Command '{self.command}' returned with a non-zero exit code.") diff --git a/src/decman/plugins/aur/__init__.py b/src/decman/plugins/aur/__init__.py index 537d01f..67ac81a 100644 --- a/src/decman/plugins/aur/__init__.py +++ b/src/decman/plugins/aur/__init__.py @@ -229,9 +229,13 @@ class AUR(plugins.Plugin): output.print_traceback() return False except errors.CommandFailedError as error: - output.print_error("Running a AUR command failed.") + output.print_error( + "AUR command exited with an unexpected return code. You may have cancelled a " + "pacman operation." + ) output.print_error(str(error)) - output.print_command_output(error.output) + if error.output: + output.print_command_output(error.output) output.print_traceback() return False diff --git a/src/decman/plugins/aur/commands.py b/src/decman/plugins/aur/commands.py index 4acfb76..094bf10 100644 --- a/src/decman/plugins/aur/commands.py +++ b/src/decman/plugins/aur/commands.py @@ -1,3 +1,4 @@ +import decman.config as config import decman.core.command as command import decman.core.error as errors import decman.plugins.pacman as pacman @@ -206,7 +207,7 @@ class AurPacmanInterface(pacman.PacmanInterface): return cmd = self._aur_commands.install_as_dependencies(deps) - _, pacman_output = command.check_run_result(cmd, command.pty_run(cmd)) + pacman_output = command.prg(cmd) self.print_highlighted_pacman_messages(pacman_output) def install_files(self, files: list[str], as_explicit: set[str]): @@ -218,11 +219,11 @@ class AurPacmanInterface(pacman.PacmanInterface): return cmd = self._aur_commands.install_files_as_dependencies(files) - _, pacman_output = command.check_run_result(cmd, command.pty_run(cmd)) + pacman_output = command.prg(cmd) self.print_highlighted_pacman_messages(pacman_output) if not as_explicit: return cmd = self._commands.set_as_explicit(as_explicit) - _, pacman_output = command.check_run_result(cmd, command.run(cmd)) + command.prg(cmd, pty=config.debug_output) diff --git a/src/decman/plugins/aur/fpm.py b/src/decman/plugins/aur/fpm.py index 27c870d..b6af772 100644 --- a/src/decman/plugins/aur/fpm.py +++ b/src/decman/plugins/aur/fpm.py @@ -3,6 +3,7 @@ import shutil import time import typing +import decman.config as config import decman.core.command as command import decman.core.error as errors import decman.core.output as output @@ -428,10 +429,7 @@ class ForeignPackageManager: try: cmd = self._commands.compare_versions(installed_version, fetched_version) - returncode, vercmp_output = command.run(cmd) - if returncode != 0: - raise errors.CommandFailedError(cmd, vercmp_output) - + vercmp_output = command.prg(cmd, pty=False) should_upgrade = int(vercmp_output) < 0 output.print_debug( @@ -561,8 +559,8 @@ class PackageBuilder: pass cmd = self._commands.make_chroot(self.chroot_dir, self._pkgs_in_chroot) - command.check_run_result( - cmd, command.run(cmd, env_overrides=mkarchroot_env_vars, pass_environment=False) + command.prg( + cmd, env_overrides=mkarchroot_env_vars, pass_environment=False, pty=config.debug_output ) def remove_build_environment(self): @@ -600,13 +598,13 @@ class PackageBuilder: cmd = self._commands.install_chroot( self.chroot_dir, chroot_new_pacman_pkgs + PackageBuilder.always_included_packages ) - command.check_run_result(cmd, command.run(cmd)) + command.prg(cmd, pty=config.debug_output) output.print_info("Making package.") cmd = self._commands.make_chroot_pkg( self.chroot_wd_dir, self.makepkg_user, chroot_pkg_files ) - command.check_run_result(cmd, command.pty_run(cmd)) + command.prg(cmd) for pkgname in package_names: file = self._find_pkgfile(pkgname, pkgbuild_dir) @@ -637,7 +635,7 @@ class PackageBuilder: real_pkgname = cmd_output.strip() to_remove.add(real_pkgname) cmd = self._commands.remove_chroot(self.chroot_dir, to_remove) - command.check_run_result(cmd, command.run(cmd)) + command.prg(cmd, pty=config.debug_output) output.print_info(f"Finished building: '{' '.join(package_names)}'.") @@ -758,12 +756,7 @@ class PackageBuilder: if git_url: cmd = self._commands.git_clone(git_url, ".") - rc, git_output = command.run(cmd) - - if rc != 0: - raise ForeignPackageManagerError( - f"Failed to clone PKGBUILD from {git_url}" - ) from errors.CommandFailedError(cmd, git_output) + command.prg(cmd, pty=config.debug_output) if pkgbuild_directory: try: @@ -788,13 +781,7 @@ class PackageBuilder: ) cmd = self._commands.git_log_commit_ids() - rc, git_output = command.run(cmd) - - if rc != 0: - raise ForeignPackageManagerError( - f"Failed to get git commit ids for {pkgbase}." - ) from errors.CommandFailedError(cmd, git_output) - + git_output = command.prg(cmd, pty=False) git_commit_ids = git_output.strip().split("\n") if latest_reviewed_commit is None or latest_reviewed_commit not in git_commit_ids: @@ -802,11 +789,7 @@ class PackageBuilder: for file in os.scandir("."): if file.is_file() and not file.name.startswith("."): cmd = self._commands.review_file(file.path) - rc, review_output = command.pty_run(cmd) - if rc != 0: - raise ForeignPackageManagerError( - f"Failed to review file '{file.path}'." - ) from errors.CommandFailedError(cmd, review_output) + command.prg(cmd) except OSError as error: raise ForeignPackageManagerError( f"Failed to review files in directory for {pkgbase}." @@ -814,11 +797,7 @@ class PackageBuilder: else: cmd = self._commands.git_diff(latest_reviewed_commit) - rc, review_output = command.pty_run(cmd) - if rc != 0: - raise ForeignPackageManagerError( - "Failed to review file using git diff." - ) from errors.CommandFailedError(cmd, review_output) + command.prg(cmd) if output.prompt_confirm("Build this package?", default=True): cmd = self._commands.git_get_commit_id() diff --git a/src/decman/plugins/aur/package.py b/src/decman/plugins/aur/package.py index 79417e7..0b2ec59 100644 --- a/src/decman/plugins/aur/package.py +++ b/src/decman/plugins/aur/package.py @@ -303,7 +303,7 @@ class CustomPackage: try: cmd = commands.git_clone(self.git_url, tmpdir) # Use the user nobody, since that will be used later to generate SRCINFO - command.check_run_result(cmd, command.run(cmd, user="nobody")) + command.prg(cmd, user="nobody", pty=config.debug_output) except errors.CommandFailedError as error: raise PKGBUILDParseError( self.git_url, @@ -333,7 +333,7 @@ class CustomPackage: cmd = commands.print_srcinfo() # No need to use the makepkg_user config option here. # For just printing the SRCINFO, hardcoded 'nobody' works - _, srcinfo = command.check_run_result(cmd, command.run(cmd, user="nobody")) + srcinfo = command.prg(cmd, user="nobody", pty=False) except errors.CommandFailedError as error: raise PKGBUILDParseError( self.git_url, self.pkgbuild_directory, "Failed to generate SRCINFO using makepkg." diff --git a/src/decman/plugins/flatpak.py b/src/decman/plugins/flatpak.py index 7e47e3b..0bfa0b8 100644 --- a/src/decman/plugins/flatpak.py +++ b/src/decman/plugins/flatpak.py @@ -92,8 +92,13 @@ class Flatpak(plugins.Plugin): self.apply_packages(pm, user, packages, self.ignored_packages, dry_run) except errors.CommandFailedError as error: output.print_error("Running a flatpak command failed.") + output.print_error( + "Flatpak command exited with an unexpected return code. You may have cancelled a " + "flatpak operation." + ) output.print_error(str(error)) - output.print_command_output(error.output) + if error.output: + output.print_command_output(error.output) output.print_traceback() return False return True @@ -239,7 +244,7 @@ class FlatpakInterface: as_user = user is not None cmd = self._commands.install(packages, as_user) - command.check_run_result(cmd, command.pty_run(cmd, user=user, mimic_login=as_user)) + command.prg(cmd, user=user, mimic_login=as_user) def upgrade(self, user: str | None = None): """ @@ -249,7 +254,7 @@ class FlatpakInterface: """ as_user = user is not None cmd = self._commands.upgrade(as_user) - command.check_run_result(cmd, command.pty_run(cmd, user=user, mimic_login=as_user)) + command.prg(cmd, user=user, mimic_login=as_user) def remove(self, packages: set[str], user: str | None = None): """ @@ -262,7 +267,7 @@ class FlatpakInterface: as_user = user is not None cmd = self._commands.remove(packages, as_user) - command.check_run_result(cmd, command.pty_run(cmd, user=user, mimic_login=as_user)) + command.prg(cmd, user=user, mimic_login=as_user) cmd = self._commands.remove_unused(as_user) - command.check_run_result(cmd, command.pty_run(cmd, user=user, mimic_login=as_user)) + command.prg(cmd, user=user, mimic_login=as_user) diff --git a/src/decman/plugins/pacman.py b/src/decman/plugins/pacman.py index 3647e79..d3b112d 100644 --- a/src/decman/plugins/pacman.py +++ b/src/decman/plugins/pacman.py @@ -1,5 +1,6 @@ import shutil +import decman.config as config import decman.core.command as command import decman.core.error as errors import decman.core.module as module @@ -109,9 +110,13 @@ class Pacman(plugins.Plugin): if not dry_run: pm.install(to_install) except errors.CommandFailedError as error: - output.print_error("Running a pacman command failed.") + output.print_error( + "Pacman command exited with an unexpected return code. You may have cancelled a " + "pacman operation." + ) output.print_error(str(error)) - output.print_command_output(error.output) + if error.output: + output.print_command_output(error.output) output.print_traceback() return False return True @@ -254,7 +259,7 @@ class PacmanInterface: return cmd = self._commands.set_as_dependencies(packages) - command.check_run_result(cmd, command.run(cmd)) + command.prg(cmd, pty=config.debug_output) def install(self, packages: set[str]): """ @@ -266,18 +271,18 @@ class PacmanInterface: cmd = self._commands.install(packages) - _, pacman_output = command.check_run_result(cmd, command.pty_run(cmd)) + pacman_output = command.prg(cmd) self.print_highlighted_pacman_messages(pacman_output) cmd = self._commands.set_as_explicit(packages) - command.check_run_result(cmd, command.run(cmd)) + command.prg(cmd, pty=config.debug_output) def upgrade(self): """ Upgrades all packages. """ cmd = self._commands.upgrade() - _, pacman_output = command.check_run_result(cmd, command.pty_run(cmd)) + pacman_output = command.prg(cmd) self.print_highlighted_pacman_messages(pacman_output) def remove(self, packages: set[str]): @@ -288,7 +293,7 @@ class PacmanInterface: return cmd = self._commands.remove(packages) - _, pacman_output = command.check_run_result(cmd, command.pty_run(cmd)) + pacman_output = command.prg(cmd) self.print_highlighted_pacman_messages(pacman_output) def print_highlighted_pacman_messages(self, pacman_output: str): diff --git a/src/decman/plugins/systemd.py b/src/decman/plugins/systemd.py index 349b35f..143a406 100644 --- a/src/decman/plugins/systemd.py +++ b/src/decman/plugins/systemd.py @@ -1,5 +1,6 @@ import shutil +import decman.config as config import decman.core.command as command import decman.core.error as errors import decman.core.module as module @@ -179,7 +180,8 @@ class Systemd(plugins.Plugin): except errors.CommandFailedError as error: output.print_error("Running a systemd command failed.") output.print_error(str(error)) - output.print_command_output(error.output) + if error.output: + output.print_command_output(error.output) output.print_traceback() return False return True @@ -192,7 +194,7 @@ class Systemd(plugins.Plugin): return cmd = self.commands.enable_units(units) - command.check_run_result(cmd, command.run(cmd)) + command.prg(cmd, pty=config.debug_output) store["systemd_units"] |= units @@ -204,7 +206,7 @@ class Systemd(plugins.Plugin): return cmd = self.commands.disable_units(units) - command.check_run_result(cmd, command.run(cmd)) + command.prg(cmd, pty=config.debug_output) store["systemd_units"] -= units @@ -216,7 +218,7 @@ class Systemd(plugins.Plugin): return cmd = self.commands.enable_user_units(units, user) - command.check_run_result(cmd, command.run(cmd)) + command.prg(cmd, pty=config.debug_output) store["systemd_user_units"].setdefault(user, set()) store["systemd_user_units"][user] |= units @@ -229,7 +231,7 @@ class Systemd(plugins.Plugin): return cmd = self.commands.disable_user_units(units, user) - command.check_run_result(cmd, command.run(cmd)) + command.prg(cmd, pty=config.debug_output) store["systemd_user_units"].setdefault(user, set()) store["systemd_user_units"][user] -= units @@ -240,7 +242,7 @@ class Systemd(plugins.Plugin): """ cmd = self.commands.user_daemon_reload(user) - _, text = command.check_run_result(cmd, command.run(cmd)) + command.prg(cmd, pty=config.debug_output) def reload_daemon(self): """ @@ -248,4 +250,4 @@ class Systemd(plugins.Plugin): """ cmd = self.commands.daemon_reload() - command.check_run_result(cmd, command.run(cmd)) + command.prg(cmd, pty=config.debug_output) diff --git a/tests/test_decman_core_command.py b/tests/test_decman_core_command.py index 635dd28..2c117c6 100644 --- a/tests/test_decman_core_command.py +++ b/tests/test_decman_core_command.py @@ -1,9 +1,111 @@ import json import sys +import typing import pytest import decman.core.command as command +import decman.core.output + + +def test_prg_pty_true_uses_pty_run_and_check(monkeypatch: pytest.MonkeyPatch): + calls: dict[str, typing.Any] = {} + + def fake_pty_run(cmd, user=None, env_overrides=None, pass_environment=None, mimic_login=False): + calls["pty_run"] = (cmd, user, env_overrides, mimic_login) + return 0, "ok" + + def fake_check_run_result(cmd, result, include_output=None): + calls["check_run_result"] = (cmd, result) + return result + + def fake_print_warning(msg: str): + raise AssertionError("print_warning must not be called when code == 0") + + monkeypatch.setattr(command, "pty_run", fake_pty_run) + monkeypatch.setattr(command, "check_run_result", fake_check_run_result) + monkeypatch.setattr(decman.core.output, "print_warning", fake_print_warning) + + out = decman.prg( + ["echo", "hi"], + user="alice", + env_overrides={"FOO": "bar"}, + mimic_login=True, + pty=True, + check=True, + ) + + assert out == "ok" + assert calls["pty_run"] == (["echo", "hi"], "alice", {"FOO": "bar"}, True) + assert calls["check_run_result"] == (["echo", "hi"], (0, "ok")) + + +def test_prg_pty_false_uses_run(monkeypatch: pytest.MonkeyPatch): + calls: dict[str, typing.Any] = {} + + def fake_run(cmd, user=None, env_overrides=None, pass_environment=None, mimic_login=False): + calls["run"] = (cmd, user, env_overrides, mimic_login) + return 0, "no-pty" + + def fake_check_run_result(cmd, result, include_output=None): + return result + + def fake_print_warning(msg: str): + raise AssertionError("print_warning must not be called when code == 0") + + monkeypatch.setattr(command, "run", fake_run) + monkeypatch.setattr(command, "check_run_result", fake_check_run_result) + monkeypatch.setattr(decman.core.output, "print_warning", fake_print_warning) + + out = decman.prg(["true"], pty=False, check=True) + + assert out == "no-pty" + assert calls["run"] == (["true"], None, None, False) + + +def test_prg_check_false_warns_on_nonzero(monkeypatch: pytest.MonkeyPatch): + calls: dict[str, typing.Any] = {} + + def fake_run(cmd, user=None, env_overrides=None, pass_environment=None, mimic_login=False): + # non-zero exit code + return 3, "bad" + + def fake_check_run_result(cmd, result, include_output=None): + raise AssertionError("check_run_result must not be called when check=False") + + def fake_print_warning(msg: str): + calls["warning"] = msg + + monkeypatch.setattr(command, "run", fake_run) + monkeypatch.setattr(command, "check_run_result", fake_check_run_result) + monkeypatch.setattr(decman.core.output, "print_warning", fake_print_warning) + + out = decman.prg(["cmd", "arg"], pty=False, check=False) + + assert out == "bad" + assert "cmd arg" in calls["warning"] + assert "exit code 3" in calls["warning"] + + +def test_prg_check_true_propagates_command_failed_error(monkeypatch: pytest.MonkeyPatch): + class CommandFailedError(Exception): + pass + + def fake_run(cmd, user=None, env_overrides=None, pass_environment=None, mimic_login=False): + return 42, "boom" + + def fake_check_run_result(cmd, result, include_output=None): + raise CommandFailedError((cmd, result)) + + def fake_print_warning(msg: str): + raise AssertionError("print_warning must not be called when check=True and error") + + monkeypatch.setattr(command, "run", fake_run) + monkeypatch.setattr(command, "check_run_result", fake_check_run_result) + monkeypatch.setattr(decman.core.output, "print_warning", fake_print_warning) + + with pytest.raises(CommandFailedError): + decman.prg(["boom"], pty=False, check=True) def test_run_simple(): diff --git a/tests/test_decman_init.py b/tests/test_decman_init.py index 71e48e7..434c80c 100644 --- a/tests/test_decman_init.py +++ b/tests/test_decman_init.py @@ -5,108 +5,6 @@ import pytest import decman -def test_prg_pty_true_uses_pty_run_and_check(monkeypatch: pytest.MonkeyPatch): - calls: dict[str, typing.Any] = {} - - def fake_pty_run(cmd, user=None, env_overrides=None, mimic_login=False): - calls["pty_run"] = (cmd, user, env_overrides, mimic_login) - return 0, "ok" - - def fake_check_run_result(cmd, result): - calls["check_run_result"] = (cmd, result) - return result - - def fake_print_warning(msg: str): - raise AssertionError("print_warning must not be called when code == 0") - - monkeypatch.setattr(decman, "command", decman.command) - monkeypatch.setattr(decman.command, "pty_run", fake_pty_run) - monkeypatch.setattr(decman.command, "check_run_result", fake_check_run_result) - monkeypatch.setattr(decman, "output", decman.output) - monkeypatch.setattr(decman.output, "print_warning", fake_print_warning) - - out = decman.prg( - ["echo", "hi"], - user="alice", - env_overrides={"FOO": "bar"}, - mimic_login=True, - pty=True, - check=True, - ) - - assert out == "ok" - assert calls["pty_run"] == (["echo", "hi"], "alice", {"FOO": "bar"}, True) - assert calls["check_run_result"] == (["echo", "hi"], (0, "ok")) - - -def test_prg_pty_false_uses_run(monkeypatch: pytest.MonkeyPatch): - calls: dict[str, typing.Any] = {} - - def fake_run(cmd, user=None, env_overrides=None, mimic_login=False): - calls["run"] = (cmd, user, env_overrides, mimic_login) - return 0, "no-pty" - - def fake_check_run_result(cmd, result): - return result - - def fake_print_warning(msg: str): - raise AssertionError("print_warning must not be called when code == 0") - - monkeypatch.setattr(decman.command, "run", fake_run) - monkeypatch.setattr(decman.command, "check_run_result", fake_check_run_result) - monkeypatch.setattr(decman.output, "print_warning", fake_print_warning) - - out = decman.prg(["true"], pty=False, check=True) - - assert out == "no-pty" - assert calls["run"] == (["true"], None, None, False) - - -def test_prg_check_false_warns_on_nonzero(monkeypatch: pytest.MonkeyPatch): - calls: dict[str, typing.Any] = {} - - def fake_run(cmd, user=None, env_overrides=None, mimic_login=False): - # non-zero exit code - return 3, "bad" - - def fake_check_run_result(cmd, result): - raise AssertionError("check_run_result must not be called when check=False") - - def fake_print_warning(msg: str): - calls["warning"] = msg - - monkeypatch.setattr(decman.command, "run", fake_run) - monkeypatch.setattr(decman.command, "check_run_result", fake_check_run_result) - monkeypatch.setattr(decman.output, "print_warning", fake_print_warning) - - out = decman.prg(["cmd", "arg"], pty=False, check=False) - - assert out == "bad" - assert "cmd arg" in calls["warning"] - assert "exit code 3" in calls["warning"] - - -def test_prg_check_true_propagates_command_failed_error(monkeypatch: pytest.MonkeyPatch): - class CommandFailedError(Exception): - pass - - def fake_run(cmd, user=None, env_overrides=None, mimic_login=False): - return 42, "boom" - - def fake_check_run_result(cmd, result): - raise CommandFailedError((cmd, result)) - - def fake_print_warning(msg: str): - raise AssertionError("print_warning must not be called when check=True and error") - - monkeypatch.setattr(decman.command, "run", fake_run) - monkeypatch.setattr(decman.command, "check_run_result", fake_check_run_result) - monkeypatch.setattr(decman.output, "print_warning", fake_print_warning) - - with pytest.raises(CommandFailedError): - decman.prg(["boom"], pty=False, check=True) - - def test_sh_calls_prg_with_sh_command(monkeypatch: pytest.MonkeyPatch): calls: dict[str, typing.Any] = {} diff --git a/tests/test_decman_plugins_pacman.py b/tests/test_decman_plugins_pacman.py index ca03562..4709a5d 100644 --- a/tests/test_decman_plugins_pacman.py +++ b/tests/test_decman_plugins_pacman.py @@ -201,7 +201,7 @@ def test_apply_returns_false_on_command_failure(monkeypatch: pytest.MonkeyPatch) ok = pacman.apply(store, dry_run=False) assert ok is False - assert any("pacman command failed" in msg for msg in errors_logged) + assert any("Pacman command exited with an unexpected" in msg for msg in errors_logged) assert any("boom" in msg for msg in continuations) assert traceback_called # at least once diff --git a/tests/test_decman_plugins_systemd.py b/tests/test_decman_plugins_systemd.py index 03eeb4d..27cdf5a 100644 --- a/tests/test_decman_plugins_systemd.py +++ b/tests/test_decman_plugins_systemd.py @@ -208,7 +208,7 @@ def test_apply_dry_run_does_not_mutate_store_or_call_commands(store): def test_enable_units_success(monkeypatch, store, systemd): store["systemd_units"] = {"old.service"} - def fake_run(cmd): + def fake_run(cmd, **kwargs): assert cmd[0] == "systemctl" assert cmd[1] == "enable" assert "new.service" in cmd[2:] @@ -223,7 +223,7 @@ def test_enable_units_success(monkeypatch, store, systemd): def test_enable_units_failure_does_not_update_store(monkeypatch, store, systemd): store["systemd_units"] = {"old.service"} - def fake_run(cmd): + def fake_run(cmd, **kwargs): return 1, "error" monkeypatch.setattr(systemd_mod.command, "run", fake_run) @@ -237,7 +237,7 @@ def test_enable_units_failure_does_not_update_store(monkeypatch, store, systemd) def test_disable_units_success(monkeypatch, store, systemd): store["systemd_units"] = {"old.service", "new.service"} - def fake_run(cmd): + def fake_run(cmd, **kwargs): assert cmd[0] == "systemctl" assert cmd[1] == "disable" assert "new.service" in cmd[2:] @@ -252,7 +252,7 @@ def test_disable_units_success(monkeypatch, store, systemd): def test_disable_units_failure_does_not_update_store(monkeypatch, store, systemd): store["systemd_units"] = {"old.service", "new.service"} - def fake_run(cmd): + def fake_run(cmd, **kwargs): return 1, "error" monkeypatch.setattr(systemd_mod.command, "run", fake_run) @@ -265,7 +265,7 @@ def test_disable_units_failure_does_not_update_store(monkeypatch, store, systemd def test_enable_user_units_success(monkeypatch, store, systemd): store["systemd_user_units"] = {"alice": {"olduser.service"}} - def fake_run(cmd): + def fake_run(cmd, **kwargs): assert cmd[0] == "systemctl" assert "--user" in cmd assert "enable" in cmd @@ -284,7 +284,7 @@ def test_enable_user_units_success(monkeypatch, store, systemd): def test_enable_user_units_failure_does_not_update_store(monkeypatch, store, systemd): store["systemd_user_units"] = {"alice": {"olduser.service"}} - def fake_run(cmd): + def fake_run(cmd, **kwargs): return 1, "error" monkeypatch.setattr(systemd_mod.command, "run", fake_run) @@ -297,7 +297,7 @@ def test_enable_user_units_failure_does_not_update_store(monkeypatch, store, sys def test_disable_user_units_success(monkeypatch, store, systemd): store["systemd_user_units"] = {"alice": {"olduser.service", "newuser.service"}} - def fake_run(cmd): + def fake_run(cmd, **kwargs): assert cmd[0] == "systemctl" assert "--user" in cmd assert "disable" in cmd @@ -313,7 +313,7 @@ def test_disable_user_units_success(monkeypatch, store, systemd): def test_disable_user_units_failure_does_not_update_store(monkeypatch, store, systemd): store["systemd_user_units"] = {"alice": {"olduser.service", "newuser.service"}} - def fake_run(cmd): + def fake_run(cmd, **kwargs): return 1, "error" monkeypatch.setattr(systemd_mod.command, "run", fake_run) @@ -330,7 +330,7 @@ def test_disable_user_units_failure_does_not_update_store(monkeypatch, store, sy def test_reload_daemon_uses_command_run(monkeypatch, systemd): called = {} - def fake_run(cmd): + def fake_run(cmd, **kwargs): called["cmd"] = cmd return 0, "ok" @@ -342,7 +342,7 @@ def test_reload_daemon_uses_command_run(monkeypatch, systemd): def test_reload_user_daemon_uses_command_run(monkeypatch, systemd): called = {} - def fake_run(cmd): + def fake_run(cmd, **kwargs): called["cmd"] = cmd return 0, "ok"