From fcfe9326459e53b74f270d95e74c926971ba49af Mon Sep 17 00:00:00 2001 From: Kate Case Date: Thu, 24 Oct 2024 10:08:50 -0400 Subject: [PATCH 1/5] Start commands cleanup --- .config/pydoclint-baseline.txt | 12 ------------ src/molecule/command/base.py | 20 ++++++++++++-------- 2 files changed, 12 insertions(+), 20 deletions(-) diff --git a/.config/pydoclint-baseline.txt b/.config/pydoclint-baseline.txt index 9b4090c361..1360755f88 100644 --- a/.config/pydoclint-baseline.txt +++ b/.config/pydoclint-baseline.txt @@ -1,15 +1,3 @@ -src/molecule/command/base.py - DOC303: Class `Base`: The __init__() docstring does not need a "Returns" section, because it cannot return anything - DOC302: Class `Base`: The class docstring does not need a "Returns" section, because __init__() cannot return anything - DOC501: Function `execute_cmdline_scenarios` has "raise" statements, but the docstring does not have a "Raises" section - DOC503: Function `execute_cmdline_scenarios` exceptions in the "Raises" section in the docstring do not match those in the function body Raises values in the docstring: []. Raised exceptions in the body: ['SystemExit']. - DOC201: Function `execute_subcommand` does not have a return section in docstring - DOC201: Function `click_group_ex` does not have a return section in docstring - DOC201: Function `click_command_ex` does not have a return section in docstring - DOC101: Function `result_callback`: Docstring contains fewer arguments than in function signature. - DOC106: Function `result_callback`: The option `--arg-type-hints-in-signature` is `True` but there are no argument type hints in the signature - DOC103: Function `result_callback`: 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: [**kwargs: , *args: ]. --------------------- src/molecule/command/check.py DOC101: Method `Check.execute`: Docstring contains fewer arguments than in function signature. DOC106: Method `Check.execute`: The option `--arg-type-hints-in-signature` is `True` but there are no argument type hints in the signature diff --git a/src/molecule/command/base.py b/src/molecule/command/base.py index 029468337f..67e53ec43b 100644 --- a/src/molecule/command/base.py +++ b/src/molecule/command/base.py @@ -29,7 +29,7 @@ import shutil import subprocess -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, NoReturn import click import wcmatch.pathlib @@ -56,7 +56,7 @@ MOLECULE_DEFAULT_SCENARIO_NAME = "default" -class Base(metaclass=abc.ABCMeta): +class Base(abc.ABC): """An abstract base class used to define the command interface.""" def __init__(self, c: config.Config) -> None: @@ -75,7 +75,7 @@ def __init_subclass__(cls) -> None: """Decorate execute from all subclasses.""" super().__init_subclass__() for wrapper in logger.get_section_loggers(): - cls.execute = wrapper(cls.execute) # type: ignore # noqa: PGH003 + cls.execute = wrapper(cls.execute) # type: ignore[method-assign] @abc.abstractmethod def execute( @@ -91,8 +91,9 @@ def execute( def _setup(self) -> None: """Prepare Molecule's provisioner and returns None.""" self._config.write() - self._config.provisioner.write_config() # type: ignore[union-attr] - self._config.provisioner.manage_inventory() # type: ignore[union-attr] + if self._config.provisioner is not None: + self._config.provisioner.write_config() # type: ignore[no-untyped-call] + self._config.provisioner.manage_inventory() # type: ignore[no-untyped-call] def execute_cmdline_scenarios( @@ -204,7 +205,7 @@ def execute_scenario(scenario: Scenario) -> None: scenario._remove_scenario_state_directory() # noqa: SLF001 -def filter_ignored_scenarios(scenario_paths) -> list[str]: # type: ignore[no-untyped-def] # noqa: ANN001, D103 +def filter_ignored_scenarios(scenario_paths: list[str]) -> list[str]: # noqa: D103 command = ["git", "check-ignore", *scenario_paths] with contextlib.suppress(subprocess.CalledProcessError, FileNotFoundError): @@ -295,7 +296,7 @@ def _get_subcommand(string: str) -> str: return string.split(".")[-1] -def click_group_ex(): # type: ignore[no-untyped-def] # noqa: ANN201 +def click_group_ex() -> Callable[[Callable[..., Any]], click.Command]: """Return extended version of click.group().""" # Color coding used to group command types, documented only here as we may # decide to change them later. @@ -331,7 +332,10 @@ def click_command_ex() -> Callable[[Callable[..., Any]], click.Command]: ) -def result_callback(*args, **kwargs): # type: ignore[no-untyped-def] # noqa: ANN002, ANN003, ANN201, ARG001 +def result_callback( + *args: object, # noqa: ARG001 + **kwargs: object, # noqa: ARG001 +) -> NoReturn: """Click natural exit callback.""" # We want to be used we run out custom exit code, regardless if run was # a success or failure. From 768c2c29c8d35a751e3f24c7cf31f8e970c2d0df Mon Sep 17 00:00:00 2001 From: Kate Case Date: Mon, 28 Oct 2024 09:17:32 -0400 Subject: [PATCH 2/5] Try to type click This doesn't entirely seem to be working. --- .config/pydoclint-baseline.txt | 10 ---------- src/molecule/command/base.py | 36 +++++++++++++++++++++++++--------- src/molecule/command/check.py | 33 ++++++++++++++++++++++++------- 3 files changed, 53 insertions(+), 26 deletions(-) diff --git a/.config/pydoclint-baseline.txt b/.config/pydoclint-baseline.txt index 1360755f88..e50cb0a03e 100644 --- a/.config/pydoclint-baseline.txt +++ b/.config/pydoclint-baseline.txt @@ -1,13 +1,3 @@ -src/molecule/command/check.py - DOC101: Method `Check.execute`: Docstring contains fewer arguments than in function signature. - DOC106: Method `Check.execute`: The option `--arg-type-hints-in-signature` is `True` but there are no argument type hints in the signature - DOC107: Method `Check.execute`: The option `--arg-type-hints-in-signature` is `True` but not all args in the signature have type hints - DOC103: Method `Check.execute`: 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: [action_args: ]. - DOC101: Function `check`: Docstring contains fewer arguments than in function signature. - DOC106: Function `check`: The option `--arg-type-hints-in-signature` is `True` but there are no argument type hints in the signature - DOC107: Function `check`: The option `--arg-type-hints-in-signature` is `True` but not all args in the signature have type hints - DOC103: Function `check`: 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: [ctx: , parallel: , scenario_name: ]. --------------------- src/molecule/command/cleanup.py DOC101: Method `Cleanup.execute`: Docstring contains fewer arguments than in function signature. DOC106: Method `Cleanup.execute`: The option `--arg-type-hints-in-signature` is `True` but there are no argument type hints in the signature diff --git a/src/molecule/command/base.py b/src/molecule/command/base.py index 67e53ec43b..1f1d0bf189 100644 --- a/src/molecule/command/base.py +++ b/src/molecule/command/base.py @@ -51,6 +51,8 @@ from molecule.scenario import Scenario from molecule.types import CommandArgs, MoleculeArgs + ClickGroup = Callable[[Callable[..., MoleculeArgs]], click.Command] + LOG = logging.getLogger(__name__) MOLECULE_GLOB = os.environ.get("MOLECULE_GLOB", "molecule/*/molecule.yml") MOLECULE_DEFAULT_SCENARIO_NAME = "default" @@ -64,9 +66,6 @@ def __init__(self, c: config.Config) -> None: Args: c: An instance of a Molecule config. - - Returns: - None """ self._config = c self._setup() @@ -80,7 +79,7 @@ def __init_subclass__(cls) -> None: @abc.abstractmethod def execute( self, - action_args: list[str] | None = None, + action_args: MoleculeArgs | None = None, ) -> None: # pragma: no cover """Abstract method to execute the command. @@ -115,6 +114,9 @@ def execute_cmdline_scenarios( args: ``args`` dict from ``click`` command context command_args: dict of command arguments, including the target ansible_args: Optional tuple of arguments to pass to the `ansible-playbook` command + + Raises: + SystemExit: If scenario exits prematurely. """ glob_str = MOLECULE_GLOB if scenario_name: @@ -175,6 +177,9 @@ def execute_subcommand( Args: current_config: An instance of a Molecule config. subcommand_and_args: A string representing the subcommand and arguments. + + Returns: + The result of the subcommand. """ (subcommand, *args) = subcommand_and_args.split(" ") command_module = getattr(molecule.command, subcommand) @@ -296,8 +301,12 @@ def _get_subcommand(string: str) -> str: return string.split(".")[-1] -def click_group_ex() -> Callable[[Callable[..., Any]], click.Command]: - """Return extended version of click.group().""" +def click_group_ex() -> ClickGroup: + """Return extended version of click.group(). + + Returns: + Click command group. + """ # Color coding used to group command types, documented only here as we may # decide to change them later. # green : (default) as sequence step @@ -323,8 +332,12 @@ def click_group_ex() -> Callable[[Callable[..., Any]], click.Command]: ) -def click_command_ex() -> Callable[[Callable[..., Any]], click.Command]: - """Return extended version of click.command().""" +def click_command_ex() -> ClickGroup: + """Return extended version of click.command(). + + Returns: + Click command group. + """ return click.command( cls=HelpColorsCommand, help_headers_color="yellow", @@ -336,7 +349,12 @@ def result_callback( *args: object, # noqa: ARG001 **kwargs: object, # noqa: ARG001 ) -> NoReturn: - """Click natural exit callback.""" + """Click natural exit callback. + + Args: + *args: Unused. + **kwargs: Unused. + """ # We want to be used we run out custom exit code, regardless if run was # a success or failure. util.sysexit(0) diff --git a/src/molecule/command/check.py b/src/molecule/command/check.py index 55d1a56228..bbdf3d1f59 100644 --- a/src/molecule/command/check.py +++ b/src/molecule/command/check.py @@ -32,7 +32,7 @@ if TYPE_CHECKING: - from molecule.types import CommandArgs + from molecule.types import CommandArgs, MoleculeArgs LOG = logging.getLogger(__name__) @@ -42,9 +42,17 @@ class Check(base.Base): """Check Command Class.""" - def execute(self, action_args=None): # type: ignore[no-untyped-def] # noqa: ANN001, ANN201, ARG002 - """Execute the actions necessary to perform a `molecule check` and returns None.""" - self._config.provisioner.check() # type: ignore[union-attr] + def execute( + self, + action_args: MoleculeArgs | None = None, # noqa: ARG002 + ) -> None: + """Execute the actions necessary to perform a `molecule check`. + + Args: + action_args: Molecule cli arguments. Unused. + """ + if self._config.provisioner is not None: + self._config.provisioner.check() @base.click_command_ex() @@ -60,9 +68,20 @@ def execute(self, action_args=None): # type: ignore[no-untyped-def] # noqa: AN default=MOLECULE_PARALLEL, help="Enable or disable parallel mode. Default is disabled.", ) -def check(ctx, scenario_name, parallel): # type: ignore[no-untyped-def] # pragma: no cover # noqa: ANN001, ANN201 - """Use the provisioner to perform a Dry-Run (destroy, dependency, create, prepare, converge).""" - args = ctx.obj.get("args") +def check( # pragma: no cover + ctx: click.Context, + scenario_name: str, + *, + parallel: bool, +) -> None: + """Use the provisioner to perform a Dry-Run (destroy, dependency, create, prepare, converge). + + Args: + ctx: Click context object holding commandline arguments. + scenario_name: Name of the scenario to run. + parallel: Whether the scenario(s) should be run in parallel. + """ + args: MoleculeArgs = ctx.obj.get("args") subcommand = base._get_subcommand(__name__) # noqa: SLF001 command_args: CommandArgs = {"parallel": parallel, "subcommand": subcommand} From ff7a5c814ba3ec094b5109679adc022b8d176d89 Mon Sep 17 00:00:00 2001 From: Kate Case Date: Mon, 28 Oct 2024 09:42:52 -0400 Subject: [PATCH 3/5] Fix click types --- src/molecule/command/base.py | 4 ++-- src/molecule/command/check.py | 4 ++-- tests/unit/command/test_check.py | 16 +++++++++------- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/molecule/command/base.py b/src/molecule/command/base.py index 1f1d0bf189..042187744a 100644 --- a/src/molecule/command/base.py +++ b/src/molecule/command/base.py @@ -51,7 +51,7 @@ from molecule.scenario import Scenario from molecule.types import CommandArgs, MoleculeArgs - ClickGroup = Callable[[Callable[..., MoleculeArgs]], click.Command] + ClickGroup = Callable[[Callable[..., None]], click.Command] LOG = logging.getLogger(__name__) MOLECULE_GLOB = os.environ.get("MOLECULE_GLOB", "molecule/*/molecule.yml") @@ -79,7 +79,7 @@ def __init_subclass__(cls) -> None: @abc.abstractmethod def execute( self, - action_args: MoleculeArgs | None = None, + action_args: list[str] | None = None, ) -> None: # pragma: no cover """Abstract method to execute the command. diff --git a/src/molecule/command/check.py b/src/molecule/command/check.py index bbdf3d1f59..1ae9e1309c 100644 --- a/src/molecule/command/check.py +++ b/src/molecule/command/check.py @@ -44,7 +44,7 @@ class Check(base.Base): def execute( self, - action_args: MoleculeArgs | None = None, # noqa: ARG002 + action_args: list[str] | None = None, # noqa: ARG002 ) -> None: """Execute the actions necessary to perform a `molecule check`. @@ -52,7 +52,7 @@ def execute( action_args: Molecule cli arguments. Unused. """ if self._config.provisioner is not None: - self._config.provisioner.check() + self._config.provisioner.check() # type: ignore[no-untyped-call] @base.click_command_ex() diff --git a/tests/unit/command/test_check.py b/tests/unit/command/test_check.py index 1613db70a2..e5ab15f20d 100644 --- a/tests/unit/command/test_check.py +++ b/tests/unit/command/test_check.py @@ -27,28 +27,30 @@ if TYPE_CHECKING: + from unittest.mock import MagicMock, Mock + from pytest_mock import MockerFixture from molecule import config @pytest.fixture() -def _patched_ansible_check(mocker): # type: ignore[no-untyped-def] # noqa: ANN001, ANN202 +def _patched_ansible_check(mocker: MockerFixture) -> MagicMock: return mocker.patch("molecule.provisioner.ansible.Ansible.check") # NOTE(retr0h): The use of the `patched_config_validate` fixture, disables # config.Config._validate from executing. Thus preventing odd side-effects # throughout patched.assert_called unit tests. -def test_check_execute( # type: ignore[no-untyped-def] # noqa: ANN201, D103 +def test_check_execute( # noqa: D103 mocker: MockerFixture, # noqa: ARG001 - caplog, # noqa: ANN001 - _patched_ansible_check, # noqa: ANN001, PT019 - patched_config_validate, # noqa: ANN001, ARG001 + caplog: pytest.LogCaptureFixture, + _patched_ansible_check: Mock, # noqa: PT019 + patched_config_validate: Mock, # noqa: ARG001 config_instance: config.Config, -): +) -> None: c = check.Check(config_instance) - c.execute() # type: ignore[no-untyped-call] + c.execute() assert "default" in caplog.text assert "check" in caplog.text From 1e25e4341d30266cd0832b6bd4b29c373e3a4bf5 Mon Sep 17 00:00:00 2001 From: Kate Case Date: Tue, 29 Oct 2024 10:07:15 -0400 Subject: [PATCH 4/5] Oh these are different --- src/molecule/command/base.py | 5 +++-- src/molecule/command/matrix.py | 2 +- src/molecule/shell.py | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/molecule/command/base.py b/src/molecule/command/base.py index 042187744a..9916678928 100644 --- a/src/molecule/command/base.py +++ b/src/molecule/command/base.py @@ -51,7 +51,8 @@ from molecule.scenario import Scenario from molecule.types import CommandArgs, MoleculeArgs - ClickGroup = Callable[[Callable[..., None]], click.Command] + ClickCommand = Callable[[Callable[..., None]], click.Command] + ClickGroup = Callable[[Callable[..., None]], click.Group] LOG = logging.getLogger(__name__) MOLECULE_GLOB = os.environ.get("MOLECULE_GLOB", "molecule/*/molecule.yml") @@ -332,7 +333,7 @@ def click_group_ex() -> ClickGroup: ) -def click_command_ex() -> ClickGroup: +def click_command_ex() -> ClickCommand: """Return extended version of click.command(). Returns: diff --git a/src/molecule/command/matrix.py b/src/molecule/command/matrix.py index 109ca3751a..fad1ffad24 100644 --- a/src/molecule/command/matrix.py +++ b/src/molecule/command/matrix.py @@ -37,7 +37,7 @@ LOG = logging.getLogger(__name__) -class Matrix(base.Base): # pylint: disable=abstract-method +class Matrix(base.Base): """Matrix Command Class. .. program:: molecule matrix subcommand diff --git a/src/molecule/shell.py b/src/molecule/shell.py index aa2cd8e656..cbc37db6b2 100644 --- a/src/molecule/shell.py +++ b/src/molecule/shell.py @@ -93,7 +93,7 @@ def print_version( ctx.exit() -@click_group_ex() # type: ignore[no-untyped-call, misc] +@click_group_ex() @click.option( "--debug/--no-debug", default=MOLECULE_DEBUG, From f1987ca9e03ebb2dcc382df358943200cf435ccc Mon Sep 17 00:00:00 2001 From: Kate Case Date: Tue, 29 Oct 2024 10:20:33 -0400 Subject: [PATCH 5/5] Missed an undocumented method --- src/molecule/command/base.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/molecule/command/base.py b/src/molecule/command/base.py index 9916678928..11783a9eeb 100644 --- a/src/molecule/command/base.py +++ b/src/molecule/command/base.py @@ -29,7 +29,7 @@ import shutil import subprocess -from typing import TYPE_CHECKING, Any, NoReturn +from typing import TYPE_CHECKING, Any import click import wcmatch.pathlib @@ -47,6 +47,7 @@ if TYPE_CHECKING: from collections.abc import Callable + from typing import NoReturn from molecule.scenario import Scenario from molecule.types import CommandArgs, MoleculeArgs @@ -211,7 +212,15 @@ def execute_scenario(scenario: Scenario) -> None: scenario._remove_scenario_state_directory() # noqa: SLF001 -def filter_ignored_scenarios(scenario_paths: list[str]) -> list[str]: # noqa: D103 +def filter_ignored_scenarios(scenario_paths: list[str]) -> list[str]: + """Filter out candidate scenario paths that are ignored by git. + + Args: + scenario_paths: List of candidate scenario paths. + + Returns: + Filtered list of scenario paths. + """ command = ["git", "check-ignore", *scenario_paths] with contextlib.suppress(subprocess.CalledProcessError, FileNotFoundError):