diff --git a/.github/workflows/tox.yml b/.github/workflows/tox.yml index ee06d5aee..6903f8235 100644 --- a/.github/workflows/tox.yml +++ b/.github/workflows/tox.yml @@ -25,17 +25,17 @@ jobs: - tox_env: lint - tox_env: docs - tox_env: py36 - PREFIX: PYTEST_REQPASS=433 + PREFIX: PYTEST_REQPASS=427 - tox_env: py37 - PREFIX: PYTEST_REQPASS=433 + PREFIX: PYTEST_REQPASS=427 - tox_env: py38 - PREFIX: PYTEST_REQPASS=433 + PREFIX: PYTEST_REQPASS=427 - tox_env: py39 - PREFIX: PYTEST_REQPASS=433 + PREFIX: PYTEST_REQPASS=427 - tox_env: py36-devel - PREFIX: PYTEST_REQPASS=433 + PREFIX: PYTEST_REQPASS=427 - tox_env: py39-devel - PREFIX: PYTEST_REQPASS=433 + PREFIX: PYTEST_REQPASS=427 - tox_env: packaging - tox_env: dockerfile - tox_env: build-containers diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index bc6657a06..34316ccdf 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -52,6 +52,7 @@ repos: additional_dependencies: - packaging - rich + - subprocess-tee - repo: https://github.com/pre-commit/mirrors-pylint rev: v2.6.0 hooks: @@ -59,4 +60,5 @@ repos: additional_dependencies: - ansible-base - rich + - subprocess-tee - testinfra diff --git a/.yamllint b/.yamllint index 415452f5c..93da95683 100644 --- a/.yamllint +++ b/.yamllint @@ -1,10 +1,10 @@ --- # Based on ansible-lint config extends: default - ignore: | - *{{cookiecutter** - + lib/molecule/cookiecutter/scenario/driver/delegated/{{cookiecutter.molecule_directory}} + lib/molecule/cookiecutter/molecule/{{cookiecutter.role_name}} + lib/molecule/cookiecutter/scenario/verifier/ansible/{{cookiecutter.molecule_directory}} rules: braces: max-spaces-inside: 1 diff --git a/lib/molecule/constants.py b/lib/molecule/constants.py index d0cc70a59..afd4d6eb2 100644 --- a/lib/molecule/constants.py +++ b/lib/molecule/constants.py @@ -4,5 +4,9 @@ RC_SUCCESS = 0 RC_TIMEOUT = 3 RC_SETUP_ERROR = 4 # Broken setup, like missing Ansible +RC_UNKNOWN_ERROR = ( + 5 # Unexpected errors for which we do not have more specific codes, yet +) + MOLECULE_HEADER = "# Molecule managed" diff --git a/lib/molecule/dependency/ansible_galaxy/base.py b/lib/molecule/dependency/ansible_galaxy/base.py index feae0357d..5862369fc 100644 --- a/lib/molecule/dependency/ansible_galaxy/base.py +++ b/lib/molecule/dependency/ansible_galaxy/base.py @@ -23,8 +23,6 @@ import copy import os -import sh - from molecule import logger, util from molecule.dependency import base @@ -106,10 +104,11 @@ def bake(self): options = self.options verbose_flag = util.verbose_flag(options) - self._sh_command = getattr(sh, self.command) - self._sh_command = self._sh_command.bake(*self.COMMANDS) - self._sh_command = self._sh_command.bake( - options, *verbose_flag, _env=self.env, _out=LOG.out, _err=LOG.error + self._sh_command = util.BakedCommand( + cmd=[self.command, *self.COMMANDS, *util.dict2args(options), *verbose_flag], + env=self.env, + stdout=LOG.out, + stderr=LOG.error, ) def execute(self): diff --git a/lib/molecule/dependency/base.py b/lib/molecule/dependency/base.py index f2f5077d5..c6c13fac6 100644 --- a/lib/molecule/dependency/base.py +++ b/lib/molecule/dependency/base.py @@ -23,9 +23,7 @@ import os import time -import sh - -from molecule import util +from molecule import constants, util from molecule.logger import get_logger LOG = get_logger(__name__) @@ -54,11 +52,12 @@ def execute_with_retries(self): exception = None try: + # print(555, self._sh_command) util.run_command(self._sh_command, debug=self._config.debug) msg = "Dependency completed successfully." LOG.success(msg) return - except sh.ErrorReturnCode: + except Exception: pass for counter in range(1, (self.RETRY + 1)): @@ -75,10 +74,11 @@ def execute_with_retries(self): msg = "Dependency completed successfully." LOG.success(msg) return - except sh.ErrorReturnCode as _exception: + except Exception as _exception: exception = _exception - util.sysexit(exception.exit_code) + LOG.error(str(exception), self._sh_command) + util.sysexit(getattr(exception, "exit_code", constants.RC_UNKNOWN_ERROR)) @abc.abstractmethod def execute(self): # pragma: no cover diff --git a/lib/molecule/dependency/shell.py b/lib/molecule/dependency/shell.py index 9c780c015..0e23a4f36 100644 --- a/lib/molecule/dependency/shell.py +++ b/lib/molecule/dependency/shell.py @@ -19,12 +19,9 @@ # DEALINGS IN THE SOFTWARE. """Shell Dependency Module.""" -import shlex - -import sh - from molecule import logger from molecule.dependency import base +from molecule.util import BakedCommand LOG = logger.get_logger(__name__) @@ -86,20 +83,9 @@ def command(self): def default_options(self): return {} - def bake(self): - """ - Bake a ``shell`` command so it's ready to execute and returns None. - - :return: None - """ - command_list = shlex.split(self.command) - command, args = command_list[0], command_list[1:] - - self._sh_command = getattr(sh, command) - # Reconstruct command with remaining args. - self._sh_command = self._sh_command.bake( - args, _env=self.env, _out=LOG.out, _err=LOG.error - ) + def bake(self) -> None: + """Bake a ``shell`` command so it's ready to execute.""" + self._sh_command = BakedCommand(cmd=self.command, env=self.env) def execute(self): if not self.enabled: diff --git a/lib/molecule/provisioner/ansible_playbook.py b/lib/molecule/provisioner/ansible_playbook.py index 7edb9584a..6f432c8e1 100644 --- a/lib/molecule/provisioner/ansible_playbook.py +++ b/lib/molecule/provisioner/ansible_playbook.py @@ -19,8 +19,6 @@ # DEALINGS IN THE SOFTWARE. """Ansible-Playbook Provisioner Module.""" -import sh - from molecule import logger, util LOG = logger.get_logger(__name__) @@ -69,23 +67,28 @@ def bake(self): if options.get("become"): del options["become"] - self._ansible_command = sh.ansible_playbook.bake( - options, - self._playbook, - *verbose_flag, - _cwd=self._config.scenario.directory, - _env=self._env, - _out=self._out, - _err=self._err - ) - ansible_args = list(self._config.provisioner.ansible_args) + list( self._config.ansible_args ) - if ansible_args: - if self._config.action not in ["create", "destroy"]: - self._ansible_command = self._ansible_command.bake(ansible_args) + # if ansible_args: + # if self._config.action not in ["create", "destroy"]: + # # inserts ansible_args at index 1 + # self._ansible_command.cmd.extend(ansible_args) + + self._ansible_command = util.BakedCommand( + cmd=[ + "ansible-playbook", + *util.dict2args(options), + *util.bool2args(verbose_flag), + *ansible_args, + self._playbook, # must always go last + ], + cwd=self._config.scenario.directory, + env=self._env, + stdout=self._out, + stderr=self._err, + ) def execute(self): """ @@ -100,13 +103,12 @@ def execute(self): LOG.warning("Skipping, %s action has no playbook." % self._config.action) return - try: - self._config.driver.sanity_checks() - cmd = util.run_command(self._ansible_command, debug=self._config.debug) - return cmd.stdout.decode("utf-8") - except sh.ErrorReturnCode as e: - out = e.stdout.decode("utf-8") - util.sysexit_with_message(str(out), e.exit_code) + self._config.driver.sanity_checks() + result = util.run_command(self._ansible_command, debug=self._config.debug) + if result.returncode != 0: + util.sysexit_with_message(result.stdout, result.returncode) + + return result.stdout def add_cli_arg(self, name, value): """ diff --git a/lib/molecule/test/conftest.py b/lib/molecule/test/conftest.py index 929bf7da0..ceaf950ef 100644 --- a/lib/molecule/test/conftest.py +++ b/lib/molecule/test/conftest.py @@ -33,13 +33,14 @@ @pytest.helpers.register def run_command(cmd, env=os.environ, log=True): - if log: - cmd = _rebake_command(cmd, env) + if cmd.__class__.__name__ == "Command": + if log: + cmd = _rebake_command(cmd, env) - # Never let sh truncate exceptions in testing - cmd = cmd.bake(_truncate_exc=False) + # Never let sh truncate exceptions in testing + cmd = cmd.bake(_truncate_exc=False) - return util.run_command(cmd) + return util.run_command(cmd, env=env) def _rebake_command(cmd, env, out=LOG.out, err=LOG.error): diff --git a/lib/molecule/test/functional/conftest.py b/lib/molecule/test/functional/conftest.py index 9a7a3e872..60cf69785 100644 --- a/lib/molecule/test/functional/conftest.py +++ b/lib/molecule/test/functional/conftest.py @@ -27,7 +27,6 @@ import pexpect import pkg_resources import pytest -import sh from molecule import logger, util from molecule.config import ansible_version @@ -73,8 +72,7 @@ def with_scenario(request, scenario_to_test, driver_name, scenario_name, skip_te if scenario_name: msg = "CLEANUP: Destroying instances for all scenario(s)" LOG.out(msg) - options = {"driver_name": driver_name, "all": True} - cmd = sh.molecule.bake("destroy", **options) + cmd = ["molecule", "destroy", "--driver-name", driver_name, "--all"] pytest.helpers.run_command(cmd) @@ -94,16 +92,13 @@ def skip_test(request, driver_name): @pytest.helpers.register def idempotence(scenario_name): - options = {"scenario_name": scenario_name} - cmd = sh.molecule.bake("create", **options) + cmd = ["molecule", "create", "--scenario-name", scenario_name] pytest.helpers.run_command(cmd) - options = {"scenario_name": scenario_name} - cmd = sh.molecule.bake("converge", **options) + cmd = ["molecule", "converge", "--scenario-name", scenario_name] pytest.helpers.run_command(cmd) - options = {"scenario_name": scenario_name} - cmd = sh.molecule.bake("idempotence", **options) + cmd = ["molecule", "idempotence", "--scenario-name", scenario_name] pytest.helpers.run_command(cmd) @@ -111,13 +106,12 @@ def idempotence(scenario_name): def init_role(temp_dir, driver_name): role_directory = os.path.join(temp_dir.strpath, "test-init") - cmd = sh.molecule.bake("init", "role", "test-init", {"driver-name": driver_name}) + cmd = ["molecule", "init", "role", "test-init", "--driver-name", driver_name] pytest.helpers.run_command(cmd) pytest.helpers.metadata_lint_update(role_directory) with change_dir_to(role_directory): - options = {"all": True} - cmd = sh.molecule.bake("test", **options) + cmd = ["molecule", "test", "--all"] pytest.helpers.run_command(cmd) @@ -125,7 +119,7 @@ def init_role(temp_dir, driver_name): def init_scenario(temp_dir, driver_name): # Create role role_directory = os.path.join(temp_dir.strpath, "test-init") - cmd = sh.molecule.bake("init", "role", "test-init", {"driver-name": driver_name}) + cmd = ["molecule", "init", "role", "test-init", "--driver-name", driver_name] pytest.helpers.run_command(cmd) pytest.helpers.metadata_lint_update(role_directory) @@ -134,14 +128,21 @@ def init_scenario(temp_dir, driver_name): molecule_directory = pytest.helpers.molecule_directory() scenario_directory = os.path.join(molecule_directory, "test-scenario") - options = {"role-name": "test-init", "driver-name": driver_name} - cmd = sh.molecule.bake("init", "scenario", "test-scenario", **options) + cmd = [ + "molecule", + "init", + "scenario", + "test-scenario", + "--role-name", + "test-init", + "--driver-name", + driver_name, + ] pytest.helpers.run_command(cmd) assert os.path.isdir(scenario_directory) - options = {"scenario_name": "test-scenario", "all": True} - cmd = sh.molecule.bake("test", **options) + cmd = ["molecule", "test", "--scenario-name", "test-scenario", "--all"] pytest.helpers.run_command(cmd) @@ -163,13 +164,13 @@ def metadata_lint_update(role_directory): # of the role directory and pointed at the role directory to ensure # the customize ansible-lint config is used. with change_dir_to(role_directory): - cmd = sh.ansible_lint.bake(".") + cmd = ["ansible-lint", "."] pytest.helpers.run_command(cmd) @pytest.helpers.register def list(x): - cmd = sh.molecule.bake("list") + cmd = ["molecule", "list"] out = pytest.helpers.run_command(cmd, log=False) out = out.stdout.decode("utf-8") out = util.strip_ansi_color(out) @@ -180,10 +181,9 @@ def list(x): @pytest.helpers.register def list_with_format_plain(x): - cmd = sh.molecule.bake("list", {"format": "plain"}) - out = pytest.helpers.run_command(cmd, log=False) - out = out.stdout.decode("utf-8") - out = util.strip_ansi_color(out) + cmd = ["molecule", "list", "--format", "plain"] + result = util.run_command(cmd) + out = util.strip_ansi_color(result.stdout) for l in x.splitlines(): assert l in out @@ -191,12 +191,10 @@ def list_with_format_plain(x): @pytest.helpers.register def login(login_args, scenario_name="default"): - options = {"scenario_name": scenario_name} - cmd = sh.molecule.bake("destroy", **options) + cmd = ["molecule", "destroy", "--scenario-name", scenario_name] pytest.helpers.run_command(cmd) - options = {"scenario_name": scenario_name} - cmd = sh.molecule.bake("create", **options) + cmd = ["molecule", "create", "--scenario-name", scenario_name] pytest.helpers.run_command(cmd) for instance, regexp in login_args: @@ -214,31 +212,25 @@ def login(login_args, scenario_name="default"): @pytest.helpers.register def test(driver_name, scenario_name="default", parallel=False): - options = { - "scenario_name": scenario_name, - "all": scenario_name is None, - "parallel": parallel, - } - - if driver_name == "delegated": - options = {"scenario_name": scenario_name} + cmd = ["molecule", "test", "--scenario-name", scenario_name] + if driver_name != "delegated": + if scenario_name is None: + cmd.append("--all") + if parallel: + cmd.append("--parallel") - cmd = sh.molecule.bake("test", **options) pytest.helpers.run_command(cmd) @pytest.helpers.register def verify(scenario_name="default"): - options = {"scenario_name": scenario_name} - cmd = sh.molecule.bake("create", **options) + cmd = ["molecule", "create", "--scenario-name", scenario_name] pytest.helpers.run_command(cmd) - options = {"scenario_name": scenario_name} - cmd = sh.molecule.bake("converge", **options) + cmd = ["molecule", "converge", "--scenario-name", scenario_name] pytest.helpers.run_command(cmd) - options = {"scenario_name": scenario_name} - cmd = sh.molecule.bake("verify", **options) + cmd = ["molecule", "verify", "--scenario-name", scenario_name] pytest.helpers.run_command(cmd) diff --git a/lib/molecule/test/functional/test_command.py b/lib/molecule/test/functional/test_command.py index 3193e2915..b4dc8d9b6 100644 --- a/lib/molecule/test/functional/test_command.py +++ b/lib/molecule/test/functional/test_command.py @@ -19,7 +19,8 @@ # DEALINGS IN THE SOFTWARE. import pytest -import sh + +from molecule import util @pytest.fixture @@ -49,8 +50,7 @@ def driver_name(request): indirect=["scenario_to_test", "driver_name", "scenario_name"], ) def test_command_check(scenario_to_test, with_scenario, scenario_name): - options = {"scenario_name": scenario_name} - cmd = sh.molecule.bake("check", **options) + cmd = ["molecule", "check", "--scenario-name", scenario_name] pytest.helpers.run_command(cmd) @@ -63,8 +63,7 @@ def test_command_check(scenario_to_test, with_scenario, scenario_name): indirect=["scenario_to_test", "driver_name", "scenario_name"], ) def test_command_cleanup(scenario_to_test, with_scenario, scenario_name): - options = {"scenario_name": scenario_name} - cmd = sh.molecule.bake("cleanup", **options) + cmd = ["molecule", "cleanup", "--scenario-name", scenario_name] pytest.helpers.run_command(cmd) @@ -77,8 +76,7 @@ def test_command_cleanup(scenario_to_test, with_scenario, scenario_name): indirect=["scenario_to_test", "driver_name", "scenario_name"], ) def test_command_converge(scenario_to_test, with_scenario, scenario_name): - options = {"scenario_name": scenario_name} - cmd = sh.molecule.bake("converge", **options) + cmd = ["molecule", "converge", "--scenario-name", scenario_name] pytest.helpers.run_command(cmd) @@ -91,8 +89,7 @@ def test_command_converge(scenario_to_test, with_scenario, scenario_name): indirect=["scenario_to_test", "driver_name", "scenario_name"], ) def test_command_create(scenario_to_test, with_scenario, scenario_name): - options = {"scenario_name": scenario_name} - cmd = sh.molecule.bake("create", **options) + cmd = ["molecule", "create", "--scenario-name", scenario_name] pytest.helpers.run_command(cmd) @@ -105,15 +102,14 @@ def test_command_create(scenario_to_test, with_scenario, scenario_name): indirect=["scenario_to_test", "driver_name", "scenario_name"], ) def test_command_dependency(request, scenario_to_test, with_scenario, scenario_name): - options = {"scenario_name": scenario_name} - cmd = sh.molecule.bake("dependency", **options) - result = pytest.helpers.run_command(cmd, log=False) - assert result.exit_code == 0 + cmd = ["molecule", "dependency", "--scenario-name", scenario_name] + result = util.run_command(cmd, echo=True) + assert result.returncode == 0 - # Validate that depdendency worked by running converge, which make use - cmd = sh.molecule.bake("converge", **options) - result = pytest.helpers.run_command(cmd, log=False) - assert result.exit_code == 0 + # Validate that dependency worked by running converge, which make use + cmd = ["molecule", "converge", "--scenario-name", scenario_name] + result = util.run_command(cmd, echo=True) + assert result.returncode == 0 @pytest.mark.extensive @@ -123,8 +119,7 @@ def test_command_dependency(request, scenario_to_test, with_scenario, scenario_n indirect=["scenario_to_test", "driver_name", "scenario_name"], ) def test_command_destroy(scenario_to_test, with_scenario, scenario_name): - options = {"scenario_name": scenario_name} - cmd = sh.molecule.bake("destroy", **options) + cmd = ["molecule", "destroy", "--scenario-name", scenario_name] pytest.helpers.run_command(cmd) @@ -159,8 +154,7 @@ def test_command_init_scenario(temp_dir, driver_name, skip_test): indirect=["scenario_to_test", "driver_name", "scenario_name"], ) def test_command_lint(scenario_to_test, with_scenario, scenario_name): - options = {"scenario_name": scenario_name} - cmd = sh.molecule.bake("lint", **options) + cmd = ["molecule", "lint", "--scenario-name", scenario_name] pytest.helpers.run_command(cmd) @@ -204,12 +198,10 @@ def test_command_list_with_format_plain(scenario_to_test, with_scenario, expecte indirect=["scenario_to_test", "driver_name", "scenario_name"], ) def test_command_prepare(scenario_to_test, with_scenario, scenario_name): - options = {"scenario_name": scenario_name} - - cmd = sh.molecule.bake("create", **options) + cmd = ["molecule", "create", "--scenario-name", scenario_name] pytest.helpers.run_command(cmd) - cmd = sh.molecule.bake("prepare", **options) + cmd = ["molecule", "prepare", "--scenario-name", scenario_name] pytest.helpers.run_command(cmd) @@ -222,8 +214,7 @@ def test_command_prepare(scenario_to_test, with_scenario, scenario_name): indirect=["scenario_to_test", "driver_name", "scenario_name"], ) def test_command_side_effect(scenario_to_test, with_scenario, scenario_name): - options = {"scenario_name": scenario_name} - cmd = sh.molecule.bake("side-effect", **options) + cmd = ["molecule", "side-effect", "--scenario-name", scenario_name] pytest.helpers.run_command(cmd) @@ -236,8 +227,7 @@ def test_command_side_effect(scenario_to_test, with_scenario, scenario_name): indirect=["scenario_to_test", "driver_name", "scenario_name"], ) def test_command_syntax(scenario_to_test, with_scenario, scenario_name): - options = {"scenario_name": scenario_name} - cmd = sh.molecule.bake("syntax", **options) + cmd = ["molecule", "syntax", "--scenario-name", scenario_name] pytest.helpers.run_command(cmd) diff --git a/lib/molecule/test/unit/command/test_base.py b/lib/molecule/test/unit/command/test_base.py index b4f853234..387eb6c5c 100644 --- a/lib/molecule/test/unit/command/test_base.py +++ b/lib/molecule/test/unit/command/test_base.py @@ -46,11 +46,6 @@ def _instance(_base_class, config_instance): return _base_class(config_instance) -@pytest.fixture -def _patched_verify_configs(mocker): - return mocker.patch("molecule.command.base._verify_configs") - - @pytest.fixture def _patched_base_setup(mocker): return mocker.patch("molecule.test.unit.command.test_base.ExtendedBase._setup") @@ -262,12 +257,6 @@ def test_get_configs(config_instance): assert isinstance(result[0], config.Config) -def test_get_configs_calls_verify_configs(_patched_verify_configs): - base.get_configs({}, {}) - - _patched_verify_configs.assert_called_once_with([], "molecule/*/molecule.yml") - - def test_verify_configs(config_instance): configs = [config_instance] diff --git a/lib/molecule/test/unit/conftest.py b/lib/molecule/test/unit/conftest.py index 8da53dd04..703933040 100644 --- a/lib/molecule/test/unit/conftest.py +++ b/lib/molecule/test/unit/conftest.py @@ -23,6 +23,7 @@ import os import shutil from pathlib import Path +from subprocess import CompletedProcess from uuid import uuid4 import pytest @@ -194,7 +195,9 @@ def patched_logger_success(mocker): @pytest.fixture def patched_run_command(mocker): m = mocker.patch("molecule.util.run_command") - m.return_value = mocker.Mock(stdout=b"patched-run-command-stdout") + m.return_value = CompletedProcess( + args="foo", returncode=0, stdout="patched-run-command-stdout", stderr="" + ) return m diff --git a/lib/molecule/test/unit/cookiecutter/test_molecule.py b/lib/molecule/test/unit/cookiecutter/test_molecule.py index f3ef70fa1..7306c2806 100644 --- a/lib/molecule/test/unit/cookiecutter/test_molecule.py +++ b/lib/molecule/test/unit/cookiecutter/test_molecule.py @@ -21,7 +21,6 @@ import os import pytest -import sh from molecule import util from molecule.command.init import base @@ -73,5 +72,5 @@ def test_valid(temp_dir, _molecule_file, _role_directory, _command_args, _instan assert {} == schema_v3.validate(data) - cmd = sh.yamllint.bake("-s", _molecule_file) + cmd = ["yamllint", "-s", _molecule_file] pytest.helpers.run_command(cmd) diff --git a/lib/molecule/test/unit/dependency/ansible_galaxy/test_collections.py b/lib/molecule/test/unit/dependency/ansible_galaxy/test_collections.py index 909ac8eb0..6794881f2 100644 --- a/lib/molecule/test/unit/dependency/ansible_galaxy/test_collections.py +++ b/lib/molecule/test/unit/dependency/ansible_galaxy/test_collections.py @@ -21,7 +21,6 @@ import os import pytest -import sh from molecule import config from molecule.dependency.ansible_galaxy import collections @@ -129,21 +128,22 @@ def test_env_property(_instance): @pytest.mark.parametrize("config_instance", ["_dependency_section_data"], indirect=True) -def test_bake(_instance, role_file, roles_path): +def test_collections_bake(_instance, role_file, roles_path): _instance.bake() - x = [ - str(sh.ansible_galaxy), + args = [ + "ansible-galaxy", "collection", "install", - "--requirements-file={}".format(role_file), - "--collections-path={}".format(roles_path), + "--collections-path", + roles_path, + "--foo", + "bar", "--force", - "--foo=bar", + "--requirements-file", + role_file, "-v", ] - result = str(_instance._sh_command).split() - - assert sorted(x) == sorted(result) + assert _instance._sh_command.cmd == args def test_execute( @@ -206,10 +206,10 @@ def test_execute_bakes( assert 1 == patched_run_command.call_count -def test_executes_catches_and_exits_return_code( +def test_collections_executes_catches_and_exits_return_code( patched_run_command, _patched_ansible_galaxy_has_requirements_file, _instance ): - patched_run_command.side_effect = sh.ErrorReturnCode_1(sh.ansible_galaxy, b"", b"") + patched_run_command.side_effect = SystemExit(1) with pytest.raises(SystemExit) as e: _instance.execute() diff --git a/lib/molecule/test/unit/dependency/ansible_galaxy/test_roles.py b/lib/molecule/test/unit/dependency/ansible_galaxy/test_roles.py index aa9f80ce3..1b2065f3d 100644 --- a/lib/molecule/test/unit/dependency/ansible_galaxy/test_roles.py +++ b/lib/molecule/test/unit/dependency/ansible_galaxy/test_roles.py @@ -21,7 +21,6 @@ import os import pytest -import sh from molecule import config from molecule.dependency.ansible_galaxy import roles @@ -126,20 +125,21 @@ def test_env_property(_instance): @pytest.mark.parametrize("config_instance", ["_dependency_section_data"], indirect=True) -def test_bake(_instance, role_file, roles_path): +def test_galaxy_bake(_instance, role_file, roles_path): _instance.bake() - x = [ - str(sh.ansible_galaxy), + args = [ + "ansible-galaxy", "install", - "--role-file={}".format(role_file), - "--roles-path={}".format(roles_path), + "--foo", + "bar", "--force", - "--foo=bar", + "--role-file", + role_file, + "--roles-path", + roles_path, "-v", ] - result = str(_instance._sh_command).split() - - assert sorted(x) == sorted(result) + assert _instance._sh_command.cmd == args def test_execute( @@ -202,10 +202,10 @@ def test_execute_bakes( assert 1 == patched_run_command.call_count -def test_executes_catches_and_exits_return_code( +def test_galaxy_executes_catches_and_exits_return_code( patched_run_command, _patched_ansible_galaxy_has_requirements_file, _instance ): - patched_run_command.side_effect = sh.ErrorReturnCode_1(sh.ansible_galaxy, b"", b"") + patched_run_command.side_effect = SystemExit(1) with pytest.raises(SystemExit) as e: _instance.execute() diff --git a/lib/molecule/test/unit/dependency/test_shell.py b/lib/molecule/test/unit/dependency/test_shell.py index 16af402bc..52c2df322 100644 --- a/lib/molecule/test/unit/dependency/test_shell.py +++ b/lib/molecule/test/unit/dependency/test_shell.py @@ -19,7 +19,6 @@ # DEALINGS IN THE SOFTWARE. import pytest -import sh from molecule import config from molecule.dependency import shell @@ -91,43 +90,6 @@ def test_env_property(_instance): assert "bar" == _instance.env["FOO"] -@pytest.mark.parametrize("config_instance", ["_dependency_section_data"], indirect=True) -def test_bake(_instance): - _instance.bake() - - x = [str(sh.ls), "-l", "-a", "/tmp"] - result = str(_instance._sh_command).split() - - assert sorted(x) == sorted(result) - - -@pytest.mark.parametrize("config_instance", ["_dependency_section_data"], indirect=True) -@pytest.mark.parametrize( - "command,words", - { - "ls -l -a /tmp": ["ls", "-l", "-a", "/tmp"], - 'sh -c "echo hello world"': ["sh", "-c", "echo hello world"], - 'echo "hello world"': ["echo", "hello world"], - """printf "%s\\n" foo "bar baz" 'a b' c""": [ - "printf", - r"%s\n", - "foo", - "bar baz", - "a b", - "c", - ], - }.items(), -) -def test_bake2(_instance, command, words): - _instance._config.config["dependency"]["command"] = command - _instance.bake() - baked_exe = _instance._sh_command._path.decode() - baked_args = [w.decode() for w in _instance._sh_command._partial_baked_args] - - assert baked_exe.endswith(words[0]) - assert baked_args == words[1:] - - def test_execute(patched_run_command, patched_logger_success, _instance): _instance._sh_command = "patched-command" _instance.execute() @@ -159,12 +121,11 @@ def test_execute_bakes(patched_run_command, _instance): @pytest.mark.parametrize("config_instance", ["_dependency_section_data"], indirect=True) -def test_executes_catches_and_exits_return_code(patched_run_command, _instance): - patched_run_command.side_effect = sh.ErrorReturnCode_1(sh.ls, b"", b"") +def test_dep_executes_catches_and_exits_return_code(patched_run_command, _instance): + patched_run_command.side_effect = SystemExit(1) with pytest.raises(SystemExit) as e: _instance.execute() - - assert 1 == e.value.code + assert e.value.code == 1 def test_has_command_configured(_instance): diff --git a/lib/molecule/test/unit/provisioner/test_ansible_playbook.py b/lib/molecule/test/unit/provisioner/test_ansible_playbook.py index 515968709..e8035ac31 100644 --- a/lib/molecule/test/unit/provisioner/test_ansible_playbook.py +++ b/lib/molecule/test/unit/provisioner/test_ansible_playbook.py @@ -18,8 +18,9 @@ # FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER # DEALINGS IN THE SOFTWARE. +from subprocess import CompletedProcess + import pytest -import sh from molecule import config from molecule.provisioner import ansible_playbook @@ -54,16 +55,17 @@ def test_bake(_inventory_directory, _instance): _instance._playbook = pb _instance.bake() - x = [ - str(sh.ansible_playbook), + args = [ + "ansible-playbook", "--become", - "--inventory={}".format(_inventory_directory), - "--skip-tags=molecule-notest,notest", + "--inventory", + _inventory_directory, + "--skip-tags", + "molecule-notest,notest", pb, ] - result = str(_instance._ansible_command).split() - assert sorted(x) == sorted(result) + assert _instance._ansible_command.cmd == args def test_bake_removes_non_interactive_options_from_non_converge_playbooks( @@ -71,16 +73,16 @@ def test_bake_removes_non_interactive_options_from_non_converge_playbooks( ): _instance.bake() - x = [ - str(sh.ansible_playbook), - "--inventory={}".format(_inventory_directory), - "--skip-tags=molecule-notest,notest", + args = [ + "ansible-playbook", + "--inventory", + _inventory_directory, + "--skip-tags", + "molecule-notest,notest", "playbook", ] - result = str(_instance._ansible_command).split() - - assert sorted(x) == sorted(result) + assert _instance._ansible_command.cmd == args def test_bake_has_ansible_args(_inventory_directory, _instance): @@ -88,20 +90,20 @@ def test_bake_has_ansible_args(_inventory_directory, _instance): _instance._config.config["provisioner"]["ansible_args"] = ("frob", "nitz") _instance.bake() - x = [ - str(sh.ansible_playbook), - "--inventory={}".format(_inventory_directory), - "--skip-tags=molecule-notest,notest", - "playbook", + args = [ + "ansible-playbook", + "--inventory", + _inventory_directory, + "--skip-tags", + "molecule-notest,notest", "frob", "nitz", "foo", "bar", + "playbook", ] - result = str(_instance._ansible_command).split() - - assert sorted(x) == sorted(result) + assert _instance._ansible_command.cmd == args def test_bake_does_not_have_ansible_args(_inventory_directory, _instance): @@ -110,32 +112,34 @@ def test_bake_does_not_have_ansible_args(_inventory_directory, _instance): _instance._config.action = action _instance.bake() - x = [ - str(sh.ansible_playbook), - "--inventory={}".format(_inventory_directory), - "--skip-tags=molecule-notest,notest", + args = [ + "ansible-playbook", + "--inventory", + _inventory_directory, + "--skip-tags", + "molecule-notest,notest", + "foo", + "bar", "playbook", ] - result = str(_instance._ansible_command).split() - - assert sorted(x) == sorted(result) + assert _instance._ansible_command.cmd == args def test_bake_idem_does_have_skip_tag(_inventory_directory, _instance): _instance._config.action = "idempotence" _instance.bake() - x = [ - str(sh.ansible_playbook), - "--inventory={}".format(_inventory_directory), - "--skip-tags=molecule-notest,notest,molecule-idempotence-notest", + args = [ + "ansible-playbook", + "--inventory", + _inventory_directory, + "--skip-tags", + "molecule-notest,notest,molecule-idempotence-notest", "playbook", ] - result = str(_instance._ansible_command).split() - - assert sorted(x) == sorted(result) + assert _instance._ansible_command.cmd == args def test_execute(patched_run_command, _instance): @@ -151,46 +155,52 @@ def test_execute_bakes(_inventory_directory, patched_run_command, _instance): assert _instance._ansible_command is not None - x = [ - str(sh.ansible_playbook), - "--inventory={}".format(_inventory_directory), - "--skip-tags=molecule-notest,notest", + args = [ + "ansible-playbook", + "--inventory", + _inventory_directory, + "--skip-tags", + "molecule-notest,notest", "playbook", ] - result = str(patched_run_command.mock_calls[0][1][0]).split() + # result = str(patched_run_command.mock_calls[0][1][0]).split() - assert sorted(x) == sorted(result) + assert _instance._ansible_command.cmd == args def test_execute_bakes_with_ansible_args( _inventory_directory, patched_run_command, _instance ): - _instance._config.ansible_args = ("--foo", "--bar") + _instance._config.ansible_args = ("-o", "--syntax-check") _instance.execute() assert _instance._ansible_command is not None - x = [ - str(sh.ansible_playbook), - "--inventory={}".format(_inventory_directory), - "--skip-tags=molecule-notest,notest", + args = [ + "ansible-playbook", + "--inventory", + _inventory_directory, + "--skip-tags", + "molecule-notest,notest", + # "--foo", + # "--bar", + "-o", + "--syntax-check", "playbook", - "--foo", - "--bar", ] - result = str(patched_run_command.mock_calls[0][1][0]).split() - - assert sorted(x) == sorted(result) + assert _instance._ansible_command.cmd == args def test_executes_catches_and_exits_return_code_with_stdout( patched_run_command, patched_logger_critical, _instance ): - patched_run_command.side_effect = sh.ErrorReturnCode_1( - sh.ansible_playbook, b"out", b"err" - ) + patched_run_command.side_effect = [ + CompletedProcess( + args="ansible-playbook", returncode=1, stdout="out", stderr="err" + ) + ] with pytest.raises(SystemExit) as e: _instance.execute() diff --git a/lib/molecule/test/unit/test_api.py b/lib/molecule/test/unit/test_api.py index dc058973d..90260c498 100644 --- a/lib/molecule/test/unit/test_api.py +++ b/lib/molecule/test/unit/test_api.py @@ -19,10 +19,7 @@ # FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER # DEALINGS IN THE SOFTWARE. -import pytest -import sh - -from molecule import api +from molecule import api, util def test_api_molecule_drivers_as_attributes(): @@ -47,10 +44,10 @@ def test_api_verifiers(): def test_cli_mol(): - cmd_molecule = sh.molecule.bake("--version") - x = pytest.helpers.run_command(cmd_molecule, log=False) - cmd_mol = sh.mol.bake("--version") - y = pytest.helpers.run_command(cmd_mol, log=False) - assert x.exit_code == y.exit_code + cmd_molecule = ["molecule", "--version"] + x = util.run_command(cmd_molecule) + cmd_mol = ["mol", "--version"] + y = util.run_command(cmd_mol) + assert x.returncode == y.returncode assert x.stderr == y.stderr assert x.stdout == y.stdout diff --git a/lib/molecule/test/unit/test_util.py b/lib/molecule/test/unit/test_util.py index 462ed4571..92aebd5c4 100644 --- a/lib/molecule/test/unit/test_util.py +++ b/lib/molecule/test/unit/test_util.py @@ -26,7 +26,6 @@ import colorama import pytest -import sh from molecule import util from molecule.constants import MOLECULE_HEADER @@ -171,34 +170,29 @@ def test_sysexit_with_message_and_custom_code(patched_logger_critical): def test_run_command(): - cmd = sh.ls.bake() + cmd = ["ls"] x = util.run_command(cmd) - assert 0 == x.exit_code + assert 0 == x.returncode def test_run_command_with_debug(mocker, patched_print_debug): - cmd = sh.ls.bake(_env={"ANSIBLE_FOO": "foo", "MOLECULE_BAR": "bar"}) - util.run_command(cmd, debug=True) + env = {"ANSIBLE_FOO": "foo", "MOLECULE_BAR": "bar"} + util.run_command(["ls"], debug=True, env=env) x = [ mocker.call("ANSIBLE ENVIRONMENT", "---\nANSIBLE_FOO: foo\n"), mocker.call("MOLECULE ENVIRONMENT", "---\nMOLECULE_BAR: bar\n"), mocker.call("SHELL REPLAY", "ANSIBLE_FOO=foo MOLECULE_BAR=bar"), - mocker.call("COMMAND", sh.which("ls")), ] assert x == patched_print_debug.mock_calls def test_run_command_with_debug_handles_no_env(mocker, patched_print_debug): - cmd = sh.ls.bake() + cmd = "ls" util.run_command(cmd, debug=True) - x = [ - mocker.call("ANSIBLE ENVIRONMENT", "--- {}\n"), - mocker.call("MOLECULE ENVIRONMENT", "--- {}\n"), - mocker.call("SHELL REPLAY", ""), - mocker.call("COMMAND", sh.which("ls")), - ] + # when env is empty we expect not to print anything + x = [] assert x == patched_print_debug.mock_calls diff --git a/lib/molecule/test/unit/verifier/test_testinfra.py b/lib/molecule/test/unit/verifier/test_testinfra.py index 96c8ec05b..0f479d69d 100644 --- a/lib/molecule/test/unit/verifier/test_testinfra.py +++ b/lib/molecule/test/unit/verifier/test_testinfra.py @@ -21,7 +21,6 @@ import os import pytest -import sh from molecule import config, util from molecule.verifier import testinfra @@ -201,21 +200,23 @@ def test_bake(_patched_testinfra_get_tests, inventory_file, _instance): util.write_file(file1_file, "") _instance.bake() - x = [ - str(sh.Command("pytest")), - "--ansible-inventory={}".format(inventory_file), - "--connection=ansible", - "-v", - "--foo=bar", - "foo.py", - "bar.py", + args = [ + "pytest", + "--ansible-inventory", + inventory_file, + "--connection", + "ansible", + "--foo", + "bar", "-p", "no:cacheprovider", + "foo.py", + "bar.py", + "-v", file1_file, ] - result = str(_instance._testinfra_command).split() - assert sorted(x) == sorted(result) + assert _instance._testinfra_command.cmd == args def test_execute( @@ -268,10 +269,10 @@ def test_execute_bakes(patched_run_command, _patched_testinfra_get_tests, _insta assert 1 == patched_run_command.call_count -def test_executes_catches_and_exits_return_code( +def test_testinfra_executes_catches_and_exits_return_code( patched_run_command, _patched_testinfra_get_tests, _instance ): - patched_run_command.side_effect = sh.ErrorReturnCode_1(sh.pytest, b"", b"") + patched_run_command.side_effect = SystemExit(1) with pytest.raises(SystemExit) as e: _instance.execute() diff --git a/lib/molecule/util.py b/lib/molecule/util.py index f8c4341ff..07901a7ff 100644 --- a/lib/molecule/util.py +++ b/lib/molecule/util.py @@ -26,12 +26,15 @@ import re import sys from collections.abc import Mapping +from dataclasses import dataclass from functools import lru_cache # noqa -from typing import Any, Dict, Optional +from subprocess import CompletedProcess +from typing import Any, Dict, List, Optional, Union import colorama import jinja2 import yaml +from subprocess_tee import run from molecule.constants import MOLECULE_HEADER from molecule.logger import get_logger @@ -46,10 +49,10 @@ def increase_indent(self, flow=False, indentless=False): return super(SafeDumper, self).increase_indent(flow, False) -def print_debug(title, data): +def print_debug(title: str, data: str) -> None: """Print debug information.""" title = "DEBUG: {}".format(title) - title = [ + title_list = [ colorama.Back.WHITE, colorama.Style.BRIGHT, colorama.Fore.BLACK, @@ -58,18 +61,18 @@ def print_debug(title, data): colorama.Back.RESET, colorama.Style.RESET_ALL, ] - print("".join(title)) - data = [ + print("".join(title_list)) + data_list = [ colorama.Fore.BLACK, colorama.Style.BRIGHT, data, colorama.Style.RESET_ALL, colorama.Fore.RESET, ] - print("".join(data)) + print("".join(data_list)) -def print_environment_vars(env): +def print_environment_vars(env: Optional[Dict[str, str]]) -> None: """ Print ``Ansible`` and ``Molecule`` environment variables and returns None. @@ -77,19 +80,20 @@ def print_environment_vars(env): ``os.environ``. :return: None """ - ansible_env = {k: v for (k, v) in env.items() if "ANSIBLE_" in k} - print_debug("ANSIBLE ENVIRONMENT", safe_dump(ansible_env)) - - molecule_env = {k: v for (k, v) in env.items() if "MOLECULE_" in k} - print_debug("MOLECULE ENVIRONMENT", safe_dump(molecule_env)) - - combined_env = ansible_env.copy() - combined_env.update(molecule_env) - print_debug( - "SHELL REPLAY", - " ".join(["{}={}".format(k, v) for (k, v) in sorted(combined_env.items())]), - ) - print() + if env: + ansible_env = {k: v for (k, v) in env.items() if "ANSIBLE_" in k} + print_debug("ANSIBLE ENVIRONMENT", safe_dump(ansible_env)) + + molecule_env = {k: v for (k, v) in env.items() if "MOLECULE_" in k} + print_debug("MOLECULE ENVIRONMENT", safe_dump(molecule_env)) + + combined_env = ansible_env.copy() + combined_env.update(molecule_env) + print_debug( + "SHELL REPLAY", + " ".join(["{}={}".format(k, v) for (k, v) in sorted(combined_env.items())]), + ) + print() def sysexit(code: int = 1) -> None: @@ -113,21 +117,36 @@ def sysexit_with_message( sysexit(code) -def run_command(cmd, debug=False): +def run_command(cmd, env=None, debug=False, echo=False) -> CompletedProcess: """ Execute the given command and returns None. - :param cmd: A ``sh.Command`` object to execute. + :param cmd: : + - a string or list of strings (similar to subprocess.run) + - a BakedCommand object ( :param debug: An optional bool to toggle debug output. :return: ``sh`` object """ + args = [] + stdout = None + stderr = None + if cmd.__class__.__name__ == "Command": + raise RuntimeError( + "Molecule 3.2.0 dropped use of sh library, update plugin code to use new API. " + "See https://github.com/ansible-community/molecule/issues/2678" + ) + elif cmd.__class__.__name__ == "BakedCommand": + env = cmd.env if not env else cmd.env.copy().update(env) + args = cmd.cmd + stdout = cmd.stdout + stderr = cmd.stderr + else: + args = cmd + if debug: - # WARN(retr0h): Uses an internal ``sh`` data structure to dig - # the environment out of the ``sh.command`` object. - print_environment_vars(cmd._partial_call_args.get("env", {})) - print_debug("COMMAND", str(cmd)) - print() - return cmd(_truncate_exc=False) + print_environment_vars(env) + + return run(args, env=env, stdout=stdout, stderr=stderr, echo=echo or debug) def os_walk(directory, pattern, excludes=[]): @@ -393,3 +412,33 @@ def boolean(value: Any, strict=True) -> bool: "The value '%s' is not a valid boolean. Valid booleans include: %s" % (str(value), ", ".join(repr(i) for i in BOOLEANS)) ) + + +@dataclass +class BakedCommand: + """Define a subprocess command to be executed.""" + + cmd: Union[str, List[str]] + env: Optional[Dict] + cwd: Optional[str] = None + stdout: Any = None + stderr: Any = None + + +def dict2args(data: Dict) -> List[str]: + """Convert a dictionary of options to command like arguments.""" + result = [] + # keep sorting in order to achieve a predictable behavior + for k, v in sorted(data.items()): + if v is not False: + prefix = "-" if len(k) == 1 else "--" + result.append(f"{prefix}{k}".replace("_", "-")) + if v is not True: + # { foo: True } should produce --foo without any values + result.append(v) + return result + + +def bool2args(data: bool) -> List[str]: + """Convert a boolean value to command line argument (flag).""" + return [] diff --git a/lib/molecule/verifier/testinfra.py b/lib/molecule/verifier/testinfra.py index 3cf78755e..63f896943 100644 --- a/lib/molecule/verifier/testinfra.py +++ b/lib/molecule/verifier/testinfra.py @@ -22,8 +22,6 @@ import glob import os -import sh - from molecule import logger, util from molecule.api import Verifier @@ -159,15 +157,14 @@ def bake(self): verbose_flag = util.verbose_flag(options) args = verbose_flag + self.additional_files_or_dirs - self._testinfra_command = sh.Command("pytest").bake( - options, - self._tests, - *args, - _cwd=self._config.scenario.directory, - _env=self.env, - _out=LOG.out, - _err=LOG.error + self._testinfra_command = util.BakedCommand( + cmd=["pytest", *util.dict2args(options), *self._tests, *args], + cwd=self._config.scenario.directory, + env=self.env, + stdout=LOG.out, + stderr=LOG.error, ) + # print(self._testinfra_command.cmd) def execute(self): if not self.enabled: @@ -186,13 +183,12 @@ def execute(self): msg = "Executing Testinfra tests found in {}/...".format(self.directory) LOG.info(msg) - try: - util.run_command(self._testinfra_command, debug=self._config.debug) + result = util.run_command(self._testinfra_command, debug=self._config.debug) + if result.returncode == 0: msg = "Verifier completed successfully." LOG.success(msg) - - except sh.ErrorReturnCode as e: - util.sysexit(e.exit_code) + else: + util.sysexit(result.returncode) def _get_tests(self): """ diff --git a/mypy.ini b/mypy.ini index 101f5588c..af02e7394 100644 --- a/mypy.ini +++ b/mypy.ini @@ -43,7 +43,7 @@ ignore_missing_imports = True [mypy-setuptools] ignore_missing_imports = True -[mypy-sh] +[mypy-subprocess_tee] ignore_missing_imports = True [mypy-tree_format] diff --git a/setup.cfg b/setup.cfg index 385262b31..ad2a91994 100644 --- a/setup.cfg +++ b/setup.cfg @@ -79,7 +79,7 @@ install_requires = pluggy >= 0.7.1, < 1.0 PyYAML >= 5.1, < 6 rich >= 6.0 - sh >= 1.13.1, < 1.14 + subprocess-tee >= 0.1.2 setuptools >= 42 # for pkg_resources tree-format >= 0.1.2 yamllint >= 1.15.0, < 2