Skip to content

Commit

Permalink
Make run_command part of App class (#4359)
Browse files Browse the repository at this point in the history
  • Loading branch information
ssbarnea authored Jan 14, 2025
1 parent 097f771 commit 2f952b2
Show file tree
Hide file tree
Showing 59 changed files with 572 additions and 496 deletions.
24 changes: 24 additions & 0 deletions .config/pydoclint-baseline.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
tests/conftest.py
DOC101: Function `app`: Docstring contains fewer arguments than in function signature.
DOC103: Function `app`: Docstring arguments are different from function arguments. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Arguments in the function signature but not in the docstring: [name: str].
DOC101: Function `reset_pytest_vars`: Docstring contains fewer arguments than in function signature.
DOC106: Function `reset_pytest_vars`: The option `--arg-type-hints-in-signature` is `True` but there are no argument type hints in the signature
DOC107: Function `reset_pytest_vars`: The option `--arg-type-hints-in-signature` is `True` but not all args in the signature have type hints
Expand Down Expand Up @@ -60,6 +62,28 @@ tests/integration/conftest.py
tests/integration/test_command.py
DOC601: Class `ParamDefault`: Class docstring contains fewer class attributes than actual class attributes. (Please read https://jsh9.github.io/pydoclint/checking_class_attributes.html on how to correctly document class attributes.)
DOC603: Class `ParamDefault`: Class docstring attributes are different from actual class attributes. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Attributes in the class definition but not in the docstring: [argnames: tuple[str, str], argvalues: tuple[tuple[str, str]], ids: str, indirect: tuple[str, str]]. (Please read https://jsh9.github.io/pydoclint/checking_class_attributes.html on how to correctly document class attributes.)
DOC101: Function `test_command`: Docstring contains fewer arguments than in function signature.
DOC103: Function `test_command`: Docstring arguments are different from function arguments. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Arguments in the function signature but not in the docstring: [app: App].
DOC101: Function `test_command_idempotence`: Docstring contains fewer arguments than in function signature.
DOC103: Function `test_command_idempotence`: Docstring arguments are different from function arguments. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Arguments in the function signature but not in the docstring: [app: App].
DOC101: Function `test_command_dependency`: Docstring contains fewer arguments than in function signature.
DOC103: Function `test_command_dependency`: Docstring arguments are different from function arguments. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Arguments in the function signature but not in the docstring: [app: App].
DOC101: Function `test_command_init_scenario`: Docstring contains fewer arguments than in function signature.
DOC103: Function `test_command_init_scenario`: Docstring arguments are different from function arguments. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Arguments in the function signature but not in the docstring: [app: App].
DOC101: Function `test_command_list_with_format_plain`: Docstring contains fewer arguments than in function signature.
DOC103: Function `test_command_list_with_format_plain`: Docstring arguments are different from function arguments. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Arguments in the function signature but not in the docstring: [app: App].
DOC101: Function `test_with_missing_platform_name`: Docstring contains fewer arguments than in function signature.
DOC103: Function `test_with_missing_platform_name`: Docstring arguments are different from function arguments. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Arguments in the function signature but not in the docstring: [app: App].
DOC101: Function `test_role_name_check_one`: Docstring contains fewer arguments than in function signature.
DOC103: Function `test_role_name_check_one`: Docstring arguments are different from function arguments. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Arguments in the function signature but not in the docstring: [app: App].
DOC101: Function `test_sample_collection`: Docstring contains fewer arguments than in function signature.
DOC103: Function `test_sample_collection`: Docstring arguments are different from function arguments. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Arguments in the function signature but not in the docstring: [app: App].
DOC101: Function `test_podman`: Docstring contains fewer arguments than in function signature.
DOC103: Function `test_podman`: Docstring arguments are different from function arguments. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Arguments in the function signature but not in the docstring: [app: App].
DOC101: Function `test_docker`: Docstring contains fewer arguments than in function signature.
DOC103: Function `test_docker`: Docstring arguments are different from function arguments. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Arguments in the function signature but not in the docstring: [app: App].
DOC101: Function `test_smoke`: Docstring contains fewer arguments than in function signature.
DOC103: Function `test_smoke`: Docstring arguments are different from function arguments. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Arguments in the function signature but not in the docstring: [app: App].
--------------------
tests/unit/command/init/test_scenario.py
DOC201: Function `fixture_command_args` does not have a return section in docstring
Expand Down
6 changes: 4 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,9 @@ external = [
ignore = [
"COM812", # conflicts with ISC001 on format
"E501", # line-too-long / rely on black
"ISC001" # conflicts with COM812 on format
"ISC001", # conflicts with COM812 on format
"S603", # subprocess
"S607" # subprocess
]
select = ["ALL"]

Expand All @@ -384,7 +386,7 @@ required-imports = ["from __future__ import annotations"]

[tool.ruff.lint.per-file-ignores]
"_version.py" = ["SIM108"]
"tests/**" = ["SLF001", "S101", "S602", "T201"]
"tests/**" = ["SLF001", "S101", "S602", "T201", "D417", "ARG001", "ANN001"]

[tool.ruff.lint.pydocstyle]
convention = "google"
Expand Down
51 changes: 51 additions & 0 deletions src/molecule/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,14 @@
from __future__ import annotations

from functools import lru_cache
from pathlib import Path
from subprocess import CalledProcessError, CompletedProcess
from typing import TYPE_CHECKING

from ansible_compat.runtime import Runtime

from molecule.util import print_environment_vars


if TYPE_CHECKING:
from pathlib import Path
Expand All @@ -23,6 +27,53 @@ def __init__(self, path: Path) -> None:
"""
self.runtime = Runtime(project_dir=path, isolated=False)

def run_command( # noqa: PLR0913
self,
cmd: str | list[str],
env: dict[str, str] | None = None,
cwd: Path | None = None,
*,
debug: bool = False,
echo: bool = False, # noqa: ARG002
quiet: bool = False, # noqa: ARG002
check: bool = False,
) -> CompletedProcess[str]:
"""Execute the given command and returns None.
Args:
cmd: A list of strings containing the command to run.
env: A dict containing the shell's environment.
cwd: An optional Path to the working directory.
debug: An optional bool to toggle debug output.
echo: An optional bool to toggle command echo.
quiet: An optional bool to toggle command output.
check: An optional bool to toggle command error checking.
Returns:
A completed process object.
Raises:
CalledProcessError: If return code is nonzero and check is True.
"""
if debug:
print_environment_vars(env)

result = self.runtime.run(
args=cmd,
env=env,
cwd=cwd,
tee=True,
set_acp=False,
)
if result.returncode != 0 and check:
raise CalledProcessError(
returncode=result.returncode,
cmd=result.args,
output=result.stdout,
stderr=result.stderr,
)
return result


@lru_cache
def get_app(path: Path) -> App:
Expand Down
20 changes: 13 additions & 7 deletions src/molecule/command/init/scenario.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@

import click

from molecule import api, config, util
from molecule import api, util
from molecule.command import base as command_base
from molecule.command.init import base
from molecule.config import DEFAULT_DRIVER, MOLECULE_EMBEDDED_DATA_DIR
from molecule.config import DEFAULT_DRIVER, MOLECULE_EMBEDDED_DATA_DIR, Config, molecule_directory


if TYPE_CHECKING:
Expand Down Expand Up @@ -83,12 +83,14 @@ class Scenario(base.Base):
Initialize a new scenario using a embedded template.
"""

def __init__(self, command_args: CommandArgs) -> None:
def __init__(self, config: Config, command_args: CommandArgs) -> None:
"""Construct Scenario.
Args:
config: An instance of a Molecule config.
command_args: Arguments to pass to init-scenario playbook.
"""
self._config = config
self._command_args = command_args

def execute(self, action_args: list[str] | None = None) -> None: # noqa: ARG002
Expand All @@ -101,8 +103,8 @@ def execute(self, action_args: list[str] | None = None) -> None: # noqa: ARG002

msg = f"Initializing new scenario {scenario_name}..."
LOG.info(msg)
molecule_directory = Path(config.molecule_directory(Path.cwd()))
scenario_directory = molecule_directory / scenario_name
molecule_path = Path(molecule_directory(Path.cwd()))
scenario_directory = molecule_path / scenario_name

if scenario_directory.is_dir():
msg = f"The directory molecule/{scenario_name} exists. Cannot create new scenario."
Expand All @@ -123,7 +125,7 @@ def execute(self, action_args: list[str] | None = None) -> None: # noqa: ARG002
# it to use colors.
env["ANSIBLE_FORCE_COLOR"] = "1"
env["ANSIBLE_PYTHON_INTERPRETER"] = sys.executable
util.run_command(cmd, env=env, check=True)
self._config.app.run_command(cmd, env=env, check=True)

msg = f"Initialized scenario in {scenario_directory} successfully."
LOG.info(msg)
Expand Down Expand Up @@ -198,5 +200,9 @@ def scenario(
"subcommand": __name__,
}

s = Scenario(command_args)
config = Config(
"",
args={},
)
s = Scenario(config, command_args)
s.execute()
5 changes: 3 additions & 2 deletions src/molecule/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,8 @@ def __init__(
"MOLECULE_PROJECT_DIRECTORY",
os.getcwd(), # noqa: PTH109
)
self.runtime = get_app(Path(self.project_directory)).runtime
self.app = get_app(Path(self.project_directory))
self.runtime = self.app.runtime
self.scenario_path = Path(molecule_file).parent

# Former after_init() contents
Expand Down Expand Up @@ -245,7 +246,7 @@ def collection_directory(self) -> Path | None:
return path

# Last resort, try to find git root
show_toplevel = util.run_command("git rev-parse --show-toplevel")
show_toplevel = self.app.run_command("git rev-parse --show-toplevel")
if show_toplevel.returncode == 0:
path = Path(show_toplevel.stdout.strip())
if (path / "galaxy.yml").exists():
Expand Down
4 changes: 2 additions & 2 deletions src/molecule/dependency/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def __init__(self, config: Config) -> None:
def execute_with_retries(self) -> None:
"""Run dependency downloads with retry and timed back-off."""
try:
util.run_command(self._sh_command, debug=self._config.debug, check=True)
self._config.app.run_command(self._sh_command, debug=self._config.debug, check=True)
msg = "Dependency completed successfully."
LOG.info(msg)
return # noqa: TRY300
Expand All @@ -82,7 +82,7 @@ def execute_with_retries(self) -> None:
self.SLEEP += self.BACKOFF

try:
util.run_command(self._sh_command, debug=self._config.debug, check=True)
self._config.app.run_command(self._sh_command, debug=self._config.debug, check=True)
msg = "Dependency completed successfully."
LOG.info(msg)
return # noqa: TRY300
Expand Down
2 changes: 1 addition & 1 deletion src/molecule/provisioner/ansible_playbook.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ def execute(self, action_args: list[str] | None = None) -> str: # noqa: ARG002
warnings.filterwarnings("default", category=MoleculeRuntimeWarning)
self._config.driver.sanity_checks()
cwd = self._config.scenario_path
result = util.run_command(
result = self._config.app.run_command(
cmd=self._ansible_command,
env=self._env,
debug=self._config.debug,
Expand Down
49 changes: 0 additions & 49 deletions src/molecule/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import sys

from pathlib import Path
from subprocess import CalledProcessError, CompletedProcess
from typing import TYPE_CHECKING, overload

import jinja2
Expand All @@ -37,7 +36,6 @@
from ansible_compat.ports import cache
from rich.syntax import Syntax

from molecule.app import get_app
from molecule.console import console
from molecule.constants import MOLECULE_HEADER

Expand Down Expand Up @@ -153,53 +151,6 @@ def sysexit_with_message(
sysexit(code)


def run_command( # noqa: PLR0913
cmd: str | list[str],
env: dict[str, str] | None = None,
cwd: Path | None = None,
*,
debug: bool = False,
echo: bool = False, # noqa: ARG001
quiet: bool = False, # noqa: ARG001
check: bool = False,
) -> CompletedProcess[str]:
"""Execute the given command and returns None.
Args:
cmd: A list of strings containing the command to run.
env: A dict containing the shell's environment.
cwd: An optional Path to the working directory.
debug: An optional bool to toggle debug output.
echo: An optional bool to toggle command echo.
quiet: An optional bool to toggle command output.
check: An optional bool to toggle command error checking.
Returns:
A completed process object.
Raises:
CalledProcessError: If return code is nonzero and check is True.
"""
if debug:
print_environment_vars(env)

result = get_app(Path()).runtime.run(
args=cmd,
env=env,
cwd=cwd,
tee=True,
set_acp=False,
)
if result.returncode != 0 and check:
raise CalledProcessError(
returncode=result.returncode,
cmd=result.args,
output=result.stdout,
stderr=result.stderr,
)
return result


def os_walk(
directory: str | Path,
pattern: str,
Expand Down
2 changes: 1 addition & 1 deletion src/molecule/verifier/testinfra.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ def execute(self, action_args: list[str] | None = None) -> None:
msg = f"Executing Testinfra tests found in {self.directory}/..."
LOG.info(msg)

result = util.run_command(
result = self._config.app.run_command(
self._testinfra_command,
env=self.env,
debug=self._config.debug,
Expand Down
20 changes: 16 additions & 4 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@

import pytest

from molecule.app import App, get_app


if TYPE_CHECKING:
from collections.abc import Iterable
Expand All @@ -48,7 +50,7 @@
)


def is_subset(subset, superset): # type: ignore[no-untyped-def] # noqa: ANN001, ANN201, D103
def is_subset(subset, superset): # type: ignore[no-untyped-def] # noqa: ANN201, D103
# Checks if first dict is a subset of the second one
if isinstance(subset, dict):
return all(
Expand All @@ -66,6 +68,16 @@ def is_subset(subset, superset): # type: ignore[no-untyped-def] # noqa: ANN001
return subset == superset


@pytest.fixture(scope="session")
def app(name: str = "app") -> App:
"""Run the container.
Returns:
callable: The container executor.
"""
return get_app(Path())


@pytest.fixture(autouse=True)
def _no_color(monkeypatch: pytest.MonkeyPatch) -> None:
"""Disable coloring output.
Expand All @@ -91,7 +103,7 @@ def resources_folder_path() -> Path:
return FIXTURES_DIR / "resources"


def pytest_collection_modifyitems(items, config): # type: ignore[no-untyped-def] # noqa: ANN001, ANN201, D103
def pytest_collection_modifyitems(items, config): # type: ignore[no-untyped-def] # noqa: ANN201, D103
marker = config.getoption("-m")
is_sharded = False
shard_id = 0
Expand Down Expand Up @@ -120,7 +132,7 @@ def pytest_collection_modifyitems(items, config): # type: ignore[no-untyped-def


@pytest.fixture(autouse=True)
def reset_pytest_vars(monkeypatch): # type: ignore[no-untyped-def] # noqa: ANN001, ANN201
def reset_pytest_vars(monkeypatch): # type: ignore[no-untyped-def] # noqa: ANN201
"""Make PYTEST_* env vars inaccessible to subprocesses."""
for var_name in tuple(os.environ):
if var_name.startswith("PYTEST_"):
Expand All @@ -142,7 +154,7 @@ def test_fixture_dir(request: pytest.FixtureRequest) -> Path:
@pytest.hookimpl(hookwrapper=True, tryfirst=True)
def pytest_runtest_makereport( # type: ignore[misc]
item: Item,
call: CallInfo[None], # noqa: ARG001
call: CallInfo[None],
) -> pytest.TestReport:
"""Create a phase report for each test phase.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
).get_hosts("all")


def test_hosts_file(host): # type: ignore[no-untyped-def] # noqa: ANN001, ANN201
def test_hosts_file(host): # type: ignore[no-untyped-def] # noqa: ANN201
"""Validate host file."""
f = host.file("/etc/hosts")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@
).get_hosts("all")


def test_side_effect_removed_file(host): # type: ignore[no-untyped-def] # noqa: ANN001, ANN201
def test_side_effect_removed_file(host): # type: ignore[no-untyped-def] # noqa: ANN201
"""Validate that file was removed."""
assert not host.file("/tmp/testfile").exists # noqa: S108
Loading

0 comments on commit 2f952b2

Please sign in to comment.