From 7dc2441fc8ba992b622bdbb9aedddf63a466ad54 Mon Sep 17 00:00:00 2001 From: Alexei Znamensky <103110+russoz@users.noreply.github.com> Date: Mon, 15 Jun 2026 07:05:09 +1200 Subject: [PATCH] xbps: include `stdout` and `stderr` in module output (#12234) * fix(xbps): add stdout/stderr to module output and fix error message typo Include stdout and stderr from the last executed command in all exit_json and fail_json calls so users can see the actual xbps output when debugging failures (addresses issue #2478). Co-Authored-By: Claude Sonnet 4.6 * test(xbps): add basic unit tests using uthelper Cover install (new, already present, failure) and remove (installed, absent) scenarios. Verifies stdout/stderr are propagated in output. Co-Authored-By: Claude Sonnet 4.6 * changelog: add fragment for PR 12234 Co-Authored-By: Claude Sonnet 4.6 * Add version_added. Co-authored-by: Felix Fontein --------- Co-authored-by: Claude Sonnet 4.6 Co-authored-by: Felix Fontein --- .../12234-xbps-report-command-output.yml | 2 + plugins/modules/xbps.py | 61 ++++++--- tests/unit/plugins/modules/test_xbps.py | 19 +++ tests/unit/plugins/modules/test_xbps.yaml | 120 ++++++++++++++++++ 4 files changed, 186 insertions(+), 16 deletions(-) create mode 100644 changelogs/fragments/12234-xbps-report-command-output.yml create mode 100644 tests/unit/plugins/modules/test_xbps.py create mode 100644 tests/unit/plugins/modules/test_xbps.yaml diff --git a/changelogs/fragments/12234-xbps-report-command-output.yml b/changelogs/fragments/12234-xbps-report-command-output.yml new file mode 100644 index 0000000000..e8b2ea08f0 --- /dev/null +++ b/changelogs/fragments/12234-xbps-report-command-output.yml @@ -0,0 +1,2 @@ +minor_changes: + - "xbps - include ``stdout`` and ``stderr`` from the last executed command in module output (https://github.com/ansible-collections/community.general/issues/2478, https://github.com/ansible-collections/community.general/pull/12234)." diff --git a/plugins/modules/xbps.py b/plugins/modules/xbps.py index 667a54b98e..ffcea22fcc 100644 --- a/plugins/modules/xbps.py +++ b/plugins/modules/xbps.py @@ -155,6 +155,18 @@ packages: type: list sample: ["ansible"] returned: success +stdout: + description: Standard output of the last executed command. + returned: when a package manager command was executed + type: str + sample: '' + version_added: 13.1.0 +stderr: + description: Standard error of the last executed command. + returned: when a package manager command was executed + type: str + sample: '' + version_added: 13.1.0 """ @@ -202,7 +214,7 @@ def query_package(module, xbps_path, name, state="present"): def update_package_db(module, xbps_path): - """Returns True if update_package_db changed""" + """Returns (changed, stdout, stderr) for the sync command""" cmd = [xbps_path["install"], "-S"] cmd = append_flags(module, xbps_path, cmd) if module.params["accept_pubkey"]: @@ -212,10 +224,10 @@ def update_package_db(module, xbps_path): rc, stdout, stderr = module.run_command(cmd, check_rc=False, data=stdin) if "Failed to import pubkey" in stderr: - module.fail_json(msg="Failed to import pubkey for repository") + module.fail_json(msg="Failed to import pubkey for repository", stdout=stdout, stderr=stderr) if rc != 0: - module.fail_json(msg="Could not update package db") - return "avg rate" in stdout + module.fail_json(msg="Could not update package db", stdout=stdout, stderr=stderr) + return "avg rate" in stdout, stdout, stderr def upgrade_xbps(module, xbps_path, exit_on_success=False): @@ -223,7 +235,7 @@ def upgrade_xbps(module, xbps_path, exit_on_success=False): cmdupgradexbps = append_flags(module, xbps_path, cmdupgradexbps) rc, stdout, stderr = module.run_command(cmdupgradexbps, check_rc=False) if rc != 0: - module.fail_json(msg="Could not upgrade xbps itself") + module.fail_json(msg="Could not upgrade xbps itself", stdout=stdout, stderr=stderr) def upgrade(module, xbps_path): @@ -236,27 +248,29 @@ def upgrade(module, xbps_path): rc, stdout, stderr = module.run_command(cmdneedupgrade, check_rc=False) if rc == 0: if len(stdout.splitlines()) == 0: - module.exit_json(changed=False, msg="Nothing to upgrade") + module.exit_json(changed=False, msg="Nothing to upgrade", stdout=stdout, stderr=stderr) elif module.check_mode: - module.exit_json(changed=True, msg="Would have performed upgrade") + module.exit_json(changed=True, msg="Would have performed upgrade", stdout=stdout, stderr=stderr) else: rc, stdout, stderr = module.run_command(cmdupgrade, check_rc=False) if rc == 0: - module.exit_json(changed=True, msg="System upgraded") + module.exit_json(changed=True, msg="System upgraded", stdout=stdout, stderr=stderr) elif rc == 16 and module.params["upgrade_xbps"]: upgrade_xbps(module, xbps_path) # avoid loops by not trying self-upgrade again module.params["upgrade_xbps"] = False upgrade(module, xbps_path) else: - module.fail_json(msg="Could not upgrade") + module.fail_json(msg="Could not upgrade", stdout=stdout, stderr=stderr) else: - module.fail_json(msg="Could not upgrade") + module.fail_json(msg="Could not upgrade", stdout=stdout, stderr=stderr) def remove_packages(module, xbps_path, packages): """Returns true if package removal succeeds""" changed_packages = [] + last_stdout = "" + last_stderr = "" # Using a for loop in case of error, we can report the package that failed for package in packages: # Query the package first, to see if we even need to remove @@ -269,12 +283,20 @@ def remove_packages(module, xbps_path, packages): rc, stdout, stderr = module.run_command(cmd, check_rc=False) if rc != 0: - module.fail_json(msg=f"failed to remove {package}") + module.fail_json(msg=f"failed to remove {package}", stdout=stdout, stderr=stderr) + last_stdout = stdout + last_stderr = stderr changed_packages.append(package) if len(changed_packages) > 0: - module.exit_json(changed=True, msg=f"removed {len(changed_packages)} package(s)", packages=changed_packages) + module.exit_json( + changed=True, + msg=f"removed {len(changed_packages)} package(s)", + packages=changed_packages, + stdout=last_stdout, + stderr=last_stderr, + ) module.exit_json(changed=False, msg="package(s) already absent") @@ -304,9 +326,13 @@ def install_packages(module, xbps_path, state, packages): module.params["upgrade_xbps"] = False install_packages(module, xbps_path, state, packages) elif rc != 0 and not (state == "latest" and rc == 17): - module.fail_json(msg=f"failed to install {len(toInstall)} packages(s)", packages=toInstall) + module.fail_json( + msg=f"failed to install {len(toInstall)} package(s)", packages=toInstall, stdout=stdout, stderr=stderr + ) - module.exit_json(changed=True, msg=f"installed {len(toInstall)} package(s)", packages=toInstall) + module.exit_json( + changed=True, msg=f"installed {len(toInstall)} package(s)", packages=toInstall, stdout=stdout, stderr=stderr + ) def check_packages(module, xbps_path, packages, state): @@ -336,10 +362,13 @@ def update_cache(module, xbps_path, upgrade_planned): if upgrade_planned: return module.exit_json(changed=True, msg="Would have updated the package cache") - changed = update_package_db(module, xbps_path) + changed, stdout, stderr = update_package_db(module, xbps_path) if not upgrade_planned: module.exit_json( - changed=changed, msg=("Updated the package master lists" if changed else "Package list already up to date") + changed=changed, + msg=("Updated the package master lists" if changed else "Package list already up to date"), + stdout=stdout, + stderr=stderr, ) diff --git a/tests/unit/plugins/modules/test_xbps.py b/tests/unit/plugins/modules/test_xbps.py new file mode 100644 index 0000000000..f5cd76cd6f --- /dev/null +++ b/tests/unit/plugins/modules/test_xbps.py @@ -0,0 +1,19 @@ +# Copyright (c) 2025, Alexei Znamensky +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +from __future__ import annotations + +import pytest + +from ansible_collections.community.general.plugins.modules import xbps + +from .uthelper import RunCommandMock, UTHelper + + +@pytest.fixture(autouse=True) +def patch_os_path_exists(mocker): + mocker.patch("os.path.exists", return_value=True) + + +UTHelper.from_module(xbps, __name__, mocks=[RunCommandMock]) diff --git a/tests/unit/plugins/modules/test_xbps.yaml b/tests/unit/plugins/modules/test_xbps.yaml new file mode 100644 index 0000000000..4da8bd28e5 --- /dev/null +++ b/tests/unit/plugins/modules/test_xbps.yaml @@ -0,0 +1,120 @@ +# Copyright (c) 2025, Alexei Znamensky +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later +--- +anchors: + sync_db: &sync_db + command: [/testbin/xbps-install, -S] + environ: {check_rc: false, data: "n\n"} + rc: 0 + out: '' + err: '' + query_foo_absent: &query_foo_absent + command: [/testbin/xbps-query, foo] + environ: {check_rc: false} + rc: 1 + out: '' + err: '' + query_foo_present: &query_foo_present + command: [/testbin/xbps-query, foo] + environ: {check_rc: false} + rc: 0 + out: "foo-1.0_1 Description: Test package\n" + err: '' + check_updates_clean: &check_updates_clean + command: [/testbin/xbps-install, -Sun] + environ: {check_rc: false} + rc: 0 + out: '' + err: '' + install_foo: &install_foo + command: [/testbin/xbps-install, -y, foo] + environ: {check_rc: false} + rc: 0 + out: "foo-1.0_1: installing ...\n" + err: '' + remove_foo: &remove_foo + command: [/testbin/xbps-remove, -y, foo] + environ: {check_rc: false} + rc: 0 + out: "foo-1.0_1: removing ...\n" + err: '' + +test_cases: + - id: test_install_new_package + input: + name: [foo] + state: present + output: + changed: true + packages: [foo] + stdout: "foo-1.0_1: installing ...\n" + stderr: '' + mocks: + run_command: + - *sync_db + - *query_foo_absent + - *install_foo + + - id: test_install_already_present + input: + name: [foo] + state: present + output: + changed: false + msg: Nothing to Install + mocks: + run_command: + - *sync_db + - *query_foo_present + - *check_updates_clean + + - id: test_install_fails + input: + name: [nonexistent] + state: present + output: + failed: true + packages: [nonexistent] + stderr: "ERROR: nonexistent: not found in repository pool.\n" + mocks: + run_command: + - *sync_db + - command: [/testbin/xbps-query, nonexistent] + environ: {check_rc: false} + rc: 1 + out: '' + err: '' + - command: [/testbin/xbps-install, -y, nonexistent] + environ: {check_rc: false} + rc: 1 + out: '' + err: "ERROR: nonexistent: not found in repository pool.\n" + + - id: test_remove_installed_package + input: + name: [foo] + state: absent + output: + changed: true + packages: [foo] + stdout: "foo-1.0_1: removing ...\n" + stderr: '' + mocks: + run_command: + - *sync_db + - *query_foo_present + - *check_updates_clean + - *remove_foo + + - id: test_remove_absent_package + input: + name: [foo] + state: absent + output: + changed: false + msg: "package(s) already absent" + mocks: + run_command: + - *sync_db + - *query_foo_absent