From 0ad5948407af3af1d6b1f37530c001a46e5665a8 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Fri, 29 Dec 2023 19:52:43 +0100 Subject: [PATCH 1/9] Implement tests and ensure that some eager options are more eager. --- src/_pytask/click.py | 78 +++++++++++++++++++++++++++++++++------ src/_pytask/parameters.py | 64 ++++++++++++++++++++++++++++++-- src/_pytask/path.py | 1 + tests/conftest.py | 2 +- tests/test_hook_module.py | 50 +++++++++++++++++++++++++ 5 files changed, 179 insertions(+), 16 deletions(-) create mode 100644 tests/test_hook_module.py diff --git a/src/_pytask/click.py b/src/_pytask/click.py index 478eb335..9f2f3a95 100644 --- a/src/_pytask/click.py +++ b/src/_pytask/click.py @@ -1,15 +1,20 @@ """Contains code related to click.""" from __future__ import annotations -import enum import inspect +from enum import Enum from gettext import gettext as _ +from gettext import ngettext from typing import Any from typing import ClassVar +from typing import TYPE_CHECKING import click from _pytask import __version__ as version from _pytask.console import console +from click import Choice +from click import Context +from click import Parameter from click.parser import split_opt from click_default_group import DefaultGroup from rich.highlighter import RegexHighlighter @@ -17,11 +22,14 @@ from rich.table import Table from rich.text import Text +if TYPE_CHECKING: + import collections.abc as cabc + __all__ = ["ColoredCommand", "ColoredGroup", "EnumChoice"] -class EnumChoice(click.Choice): +class EnumChoice(Choice): """An enum-based choice type. The implementation is copied from https://github.com/pallets/click/pull/2210 and @@ -35,17 +43,15 @@ class EnumChoice(click.Choice): """ - def __init__(self, enum_type: type[enum.Enum], case_sensitive: bool = True) -> None: + def __init__(self, enum_type: type[Enum], case_sensitive: bool = True) -> None: super().__init__( choices=[element.value for element in enum_type], case_sensitive=case_sensitive, ) self.enum_type = enum_type - def convert( - self, value: Any, param: click.Parameter | None, ctx: click.Context | None - ) -> Any: - if isinstance(value, enum.Enum): + def convert(self, value: Any, param: Parameter | None, ctx: Context | None) -> Any: + if isinstance(value, Enum): value = value.value value = super().convert(value=value, param=param, ctx=ctx) if value is None: @@ -68,7 +74,7 @@ class ColoredGroup(DefaultGroup): def format_help( self: DefaultGroup, - ctx: click.Context, + ctx: Context, formatter: Any, # noqa: ARG002 ) -> None: """Format the help text.""" @@ -114,12 +120,60 @@ def format_help( ) +def _iter_params_for_processing( + invocation_order: cabc.Sequence[Parameter], + declaration_order: cabc.Sequence[Parameter], +) -> list[Parameter]: + def sort_key(item: Parameter) -> tuple[bool, float]: + # Hardcode the order of the config and paths parameters so that they are always + # processed first even if other eager parameters are chosen. The rest follows + # https://click.palletsprojects.com/en/8.1.x/advanced/#callback-evaluation-order. + if item.name == "paths": + return False, -2 + + if item.name == "config": + return False, -1 + + try: + idx: float = invocation_order.index(item) + except ValueError: + idx = float("inf") + + return not item.is_eager, idx + + return sorted(declaration_order, key=sort_key) + + class ColoredCommand(click.Command): """A command with colored help pages.""" + def parse_args(self, ctx: Context, args: list[str]) -> list[str]: + if not args and self.no_args_is_help and not ctx.resilient_parsing: + click.echo(ctx.get_help(), color=ctx.color) + ctx.exit() + + parser = self.make_parser(ctx) + opts, args, param_order = parser.parse_args(args=args) + + for param in _iter_params_for_processing(param_order, self.get_params(ctx)): + value, args = param.handle_parse_result(ctx, opts, args) + + if args and not ctx.allow_extra_args and not ctx.resilient_parsing: + ctx.fail( + ngettext( + "Got unexpected extra argument ({args})", + "Got unexpected extra arguments ({args})", + len(args), + ).format(args=" ".join(map(str, args))) + ) + + ctx.args = args + ctx._opt_prefixes.update(parser._opt_prefixes) + return args + def format_help( self: click.Command, - ctx: click.Context, + ctx: Context, formatter: Any, # noqa: ARG002 ) -> None: """Format the help text.""" @@ -143,7 +197,7 @@ def format_help( def _print_options( - group_or_command: click.Command | DefaultGroup, ctx: click.Context + group_or_command: click.Command | DefaultGroup, ctx: Context ) -> None: """Print options formatted with a table in a panel.""" highlighter = _OptionHighlighter() @@ -195,7 +249,7 @@ def _print_options( def _format_help_text( # noqa: C901, PLR0912, PLR0915 - param: click.Parameter, ctx: click.Context + param: Parameter, ctx: Context ) -> Text: """Format the help of a click parameter. @@ -264,7 +318,7 @@ def _format_help_text( # noqa: C901, PLR0912, PLR0915 and not default_value ): default_string = "" - elif isinstance(default_value, enum.Enum): + elif isinstance(default_value, Enum): default_string = str(default_value.value) else: default_string = str(default_value) diff --git a/src/_pytask/parameters.py b/src/_pytask/parameters.py index 36e547fe..019878be 100644 --- a/src/_pytask/parameters.py +++ b/src/_pytask/parameters.py @@ -2,15 +2,23 @@ from __future__ import annotations from pathlib import Path +from typing import Iterable +from typing import TYPE_CHECKING import click from _pytask.config_utils import set_defaults_from_config +from _pytask.path import import_path from _pytask.pluginmanager import hookimpl +from _pytask.pluginmanager import register_hook_impls_from_modules +from _pytask.pluginmanager import storage from click import Context from sqlalchemy.engine import make_url from sqlalchemy.engine import URL from sqlalchemy.exc import ArgumentError +if TYPE_CHECKING: + from pluggy import PluginManager + _CONFIG_OPTION = click.Option( ["-c", "--config"], @@ -96,20 +104,70 @@ def _database_url_callback( _DATABASE_URL_OPTION = click.Option( ["--database-url"], type=str, - help=("Url to the database."), + help="Url to the database.", default=None, show_default="sqlite:///.../.pytask/pytask.sqlite3", callback=_database_url_callback, ) +def _hook_module_callback( + ctx: Context, + name: str, # noqa: ARG001 + value: tuple[str, ...], +) -> Iterable[str | Path]: + """Register the user's hook modules from the configuration file.""" + if not value: + return value + + parsed_modules = [] + for module_name in value: + if module_name.endswith(".py"): + path = Path(module_name) + if ctx.params["config"]: + path = ctx.params["config"].parent.joinpath(path).resolve() + else: + path = Path.cwd().joinpath(path).resolve() + module = import_path(path, ctx.params["root"]) + parsed_modules.append(module.__name__) + else: + parsed_modules.append(module_name) + + # If there are hook modules, we register a hook implementation to add them. + # ``pytask_add_hooks`` is a historic hook specification, so even command line + # options can be added. + if parsed_modules: + + class HookModule: + @staticmethod + @hookimpl + def pytask_add_hooks(pm: PluginManager) -> None: + """Add hooks.""" + register_hook_impls_from_modules(pm, parsed_modules) + + pm = storage.get() + pm.register(HookModule) + + return parsed_modules + + +_HOOK_MODULE_OPTION = click.Option( + ["--hook-module"], + type=str, + help="Path to a Python module that contains hook implementations.", + multiple=True, + is_eager=True, + callback=_hook_module_callback, +) + + @hookimpl(trylast=True) def pytask_extend_command_line_interface(cli: click.Group) -> None: """Register general markers.""" for command in ("build", "clean", "collect", "dag", "profile"): - cli.commands[command].params.extend([_PATH_ARGUMENT, _DATABASE_URL_OPTION]) + cli.commands[command].params.extend((_PATH_ARGUMENT, _DATABASE_URL_OPTION)) for command in ("build", "clean", "collect", "dag", "markers", "profile"): - cli.commands[command].params.append(_CONFIG_OPTION) + cli.commands[command].params.extend((_CONFIG_OPTION, _HOOK_MODULE_OPTION)) for command in ("build", "clean", "collect", "profile"): cli.commands[command].params.extend([_IGNORE_OPTION, _EDITOR_URL_SCHEME_OPTION]) for command in ("build",): diff --git a/src/_pytask/path.py b/src/_pytask/path.py index 86968b8c..9bc74035 100644 --- a/src/_pytask/path.py +++ b/src/_pytask/path.py @@ -131,6 +131,7 @@ def import_path(path: Path, root: Path) -> ModuleType: """ module_name = _module_name_from_path(path, root) + with contextlib.suppress(KeyError): return sys.modules[module_name] diff --git a/tests/conftest.py b/tests/conftest.py index 756200e7..ee77081b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -7,10 +7,10 @@ from typing import Any import pytest +from _pytask.pluginmanager import storage from click.testing import CliRunner from packaging import version from pytask import console -from pytask import storage @pytest.fixture(autouse=True) diff --git a/tests/test_hook_module.py b/tests/test_hook_module.py new file mode 100644 index 00000000..994b97ed --- /dev/null +++ b/tests/test_hook_module.py @@ -0,0 +1,50 @@ +from __future__ import annotations + +import textwrap + +from pytask import cli +from pytask import ExitCode + + +def test_add_new_hook_via_cli(runner, tmp_path): + hooks = """ + import click + from pytask import hookimpl + + @hookimpl + def pytask_extend_command_line_interface(cli): + print("Hello World!") + cli.commands["build"].params.append(click.Option(["--new-option"])) + """ + tmp_path.joinpath("hooks.py").write_text(textwrap.dedent(hooks)) + result = runner.invoke( + cli, + [ + "build", + tmp_path.as_posix(), + "--hook-module", + tmp_path.joinpath("hooks.py").as_posix(), + "--help", + ], + ) + assert result.exit_code == ExitCode.OK + assert "--new-option" in result.output + + +def test_add_new_hook_via_config(runner, tmp_path): + tmp_path.joinpath("pyproject.toml").write_text( + "[tool.pytask.ini_options]\nhook_module = ['hooks.py']" + ) + + hooks = """ + import click + from pytask import hookimpl + + @hookimpl + def pytask_extend_command_line_interface(cli): + cli.commands["build"].params.append(click.Option(["--new-option"])) + """ + tmp_path.joinpath("hooks.py").write_text(textwrap.dedent(hooks)) + result = runner.invoke(cli, ["build", tmp_path.as_posix(), "--help"]) + assert result.exit_code == ExitCode.OK + assert "--new-option" in result.output From ae08f33531d3eb5079799934fe36bd334c008adf Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Fri, 29 Dec 2023 23:26:08 +0100 Subject: [PATCH 2/9] Fix docs. --- ..._write_a_plugin.md => extending_pytask.md} | 78 ++++++++++++++++--- docs/source/how_to_guides/index.md | 2 +- docs/source/reference_guides/configuration.md | 15 ++++ docs/source/tutorials/plugins.md | 6 +- tests/test_hook_module.py | 35 +++++---- 5 files changed, 106 insertions(+), 30 deletions(-) rename docs/source/how_to_guides/{how_to_write_a_plugin.md => extending_pytask.md} (56%) diff --git a/docs/source/how_to_guides/how_to_write_a_plugin.md b/docs/source/how_to_guides/extending_pytask.md similarity index 56% rename from docs/source/how_to_guides/how_to_write_a_plugin.md rename to docs/source/how_to_guides/extending_pytask.md index 6fc024a0..0d65c308 100644 --- a/docs/source/how_to_guides/how_to_write_a_plugin.md +++ b/docs/source/how_to_guides/extending_pytask.md @@ -1,11 +1,71 @@ -# How to write a plugin +# Extending pytask -Since pytask is based on pluggy, it is extensible. In this section, you will learn some -key concepts you need to know to write a plugin. It won't deal with pluggy in detail, -but if you are interested feel free to read [pluggy](../explanations/pluggy.md). A quick -look at the first paragraphs might be useful nonetheless. +pytask can be extended since it is built upon +[pluggy](https://pluggy.readthedocs.io/en/latest/), a plugin system for Python. -## Preparation +How does it work? Throughout the execution, pytask arrives at entrypoints, called hook +functions. When pytask calls a hook function it loops through hook implementations and +each hook implementation can alter the result of the entrypoint. + +The full list of hook functions is specified in {doc}`../reference_guides/hookspecs`. + +More general information about pluggy can be found in its +[documentation](https://pluggy.readthedocs.io/en/latest/). + +There are two ways to add new hook implementations. + +1. Using the {option}`pytask build --hook-module` commandline option or the + {confval}`hook_module` configuration value. +1. Packaging your plugin as a Python package to publish and share it. + +(hook-module)= + +## Using `--hook-module` and `hook_module` + +The easiest and quickest way to extend pytask is to create a module, for example, +`hooks.py` and register it temporarily via the commandline option or permanently via the +configuration. + +```console +pytask --hook-module hooks.py +``` + +or + +```toml +[tool.pytask.ini_options] +hook_module = ["hooks.py"] +``` + +The value can be a path. If the path is relative it is assumed to be relative to the +configuration file or relative to the current working directory as a fallback. + +The value can also be a module name. For example, if `hooks.py` lies your projects +package called `myproject` which is importable, then, you can also use + +```toml +[tool.pytask.ini_options] +hook_module = ["myproject.hooks"] +``` + +In `hooks.py` we can add another commandline option to `pytask build` by providing an +addition hook implementation for the hook specification +{func}`~_pytask.hookspecs.pytask_extend_command_line_interface`. + +```python +import click +from _pytask.pluginmanager import hookimpl + + +@hookimpl +def pytask_extend_command_line_interface(cli): + """Add parameters to the command line interface.""" + cli.commands["build"].params.append(click.Option(["--hello"])) +``` + +## Packaging a plugin + +### Preparation Before you start implementing your plugin, the following notes may help you. @@ -24,11 +84,11 @@ Before you start implementing your plugin, the following notes may help you. for your plugin to get feedback from other developers. Your proposal should be concise and explain what problem you want to solve and how. -## Writing your plugin +### Writing your plugin This section explains some steps which are required for all plugins. -### Set up the setuptools entry-point +#### Set up the setuptools entry-point pytask discovers plugins via `setuptools` entry-points. Following the approach advocated for by [setuptools_scm](https://github.com/pypa/setuptools_scm), the entry-point is @@ -65,7 +125,7 @@ For a complete example with `setuptools_scm` and `pyproject.toml` see the The entry-point for pytask is called `"pytask"` and points to a module called `pytask_plugin.plugin`. -### `plugin.py` +#### `plugin.py` `plugin.py` is the entrypoint for pytask to your package. You can put all of your hook implementations in this module, but it is recommended to imitate the structure of pytask diff --git a/docs/source/how_to_guides/index.md b/docs/source/how_to_guides/index.md index 1f8e8f74..0babca6a 100644 --- a/docs/source/how_to_guides/index.md +++ b/docs/source/how_to_guides/index.md @@ -20,7 +20,7 @@ how_to_influence_build_order hashing_inputs_of_tasks using_task_returns writing_custom_nodes -how_to_write_a_plugin +extending_pytask the_data_catalog ``` diff --git a/docs/source/reference_guides/configuration.md b/docs/source/reference_guides/configuration.md index eecc42ce..fed10f86 100644 --- a/docs/source/reference_guides/configuration.md +++ b/docs/source/reference_guides/configuration.md @@ -97,6 +97,21 @@ editor_url_scheme = "no_link" ```` +````{confval} hook_module + +Register additional modules containing hook implementations. + +```toml +hook_modules = ["myproject.hooks", "hooks.py"] +``` + +You can use module names and paths as values. Relative paths are assumed to be relative +to the configuration file or the current working directory. + +{ref}`This how-to guide ` has more information. + +```` + ````{confval} ignore pytask can ignore files and directories and exclude some tasks or reduce the duration of diff --git a/docs/source/tutorials/plugins.md b/docs/source/tutorials/plugins.md index 4a8c8b35..a7eaba42 100644 --- a/docs/source/tutorials/plugins.md +++ b/docs/source/tutorials/plugins.md @@ -19,7 +19,7 @@ You can find plugins in many places. - Search on [anaconda.org](https://anaconda.org/search?q=pytask) or [prefix.dev](https://prefix.dev) for related packages. -## How to implement your plugin +## How to extend pytask -Follow the {doc}`guide on writing a plugin <../how_to_guides/how_to_write_a_plugin>` to -write your plugin. +Follow the {doc}`guide on extending pytask <../how_to_guides/extending_pytask>` to add +your own hook implementations or write your plugin. diff --git a/tests/test_hook_module.py b/tests/test_hook_module.py index 994b97ed..f45d6247 100644 --- a/tests/test_hook_module.py +++ b/tests/test_hook_module.py @@ -1,12 +1,12 @@ from __future__ import annotations +import subprocess import textwrap -from pytask import cli from pytask import ExitCode -def test_add_new_hook_via_cli(runner, tmp_path): +def test_add_new_hook_via_cli(tmp_path): hooks = """ import click from pytask import hookimpl @@ -17,21 +17,17 @@ def pytask_extend_command_line_interface(cli): cli.commands["build"].params.append(click.Option(["--new-option"])) """ tmp_path.joinpath("hooks.py").write_text(textwrap.dedent(hooks)) - result = runner.invoke( - cli, - [ - "build", - tmp_path.as_posix(), - "--hook-module", - tmp_path.joinpath("hooks.py").as_posix(), - "--help", - ], + result = subprocess.run( + ("pytask", "build", "--hook-module", "hooks.py", "--help"), + cwd=tmp_path, + capture_output=True, + check=True, ) - assert result.exit_code == ExitCode.OK - assert "--new-option" in result.output + assert result.returncode == ExitCode.OK + assert "--new-option" in result.stdout.decode() -def test_add_new_hook_via_config(runner, tmp_path): +def test_add_new_hook_via_config(tmp_path): tmp_path.joinpath("pyproject.toml").write_text( "[tool.pytask.ini_options]\nhook_module = ['hooks.py']" ) @@ -45,6 +41,11 @@ def pytask_extend_command_line_interface(cli): cli.commands["build"].params.append(click.Option(["--new-option"])) """ tmp_path.joinpath("hooks.py").write_text(textwrap.dedent(hooks)) - result = runner.invoke(cli, ["build", tmp_path.as_posix(), "--help"]) - assert result.exit_code == ExitCode.OK - assert "--new-option" in result.output + result = subprocess.run( + ("pytask", "build", tmp_path.as_posix(), "--help"), + cwd=tmp_path, + capture_output=True, + check=True, + ) + assert result.returncode == ExitCode.OK + assert "--new-option" in result.stdout.decode() From a8c114c5fac454ae9c84002e079594391d9d9794 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Fri, 29 Dec 2023 23:29:58 +0100 Subject: [PATCH 3/9] Simplifications. --- src/_pytask/click.py | 14 ++++++-------- src/_pytask/path.py | 1 - 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/_pytask/click.py b/src/_pytask/click.py index 9f2f3a95..4692eb18 100644 --- a/src/_pytask/click.py +++ b/src/_pytask/click.py @@ -13,6 +13,7 @@ from _pytask import __version__ as version from _pytask.console import console from click import Choice +from click import Command from click import Context from click import Parameter from click.parser import split_opt @@ -23,7 +24,7 @@ from rich.text import Text if TYPE_CHECKING: - import collections.abc as cabc + from collections.abc import Sequence __all__ = ["ColoredCommand", "ColoredGroup", "EnumChoice"] @@ -121,8 +122,7 @@ def format_help( def _iter_params_for_processing( - invocation_order: cabc.Sequence[Parameter], - declaration_order: cabc.Sequence[Parameter], + invocation_order: Sequence[Parameter], declaration_order: Sequence[Parameter] ) -> list[Parameter]: def sort_key(item: Parameter) -> tuple[bool, float]: # Hardcode the order of the config and paths parameters so that they are always @@ -144,7 +144,7 @@ def sort_key(item: Parameter) -> tuple[bool, float]: return sorted(declaration_order, key=sort_key) -class ColoredCommand(click.Command): +class ColoredCommand(Command): """A command with colored help pages.""" def parse_args(self, ctx: Context, args: list[str]) -> list[str]: @@ -172,7 +172,7 @@ def parse_args(self, ctx: Context, args: list[str]) -> list[str]: return args def format_help( - self: click.Command, + self: Command, ctx: Context, formatter: Any, # noqa: ARG002 ) -> None: @@ -196,9 +196,7 @@ def format_help( ) -def _print_options( - group_or_command: click.Command | DefaultGroup, ctx: Context -) -> None: +def _print_options(group_or_command: Command | DefaultGroup, ctx: Context) -> None: """Print options formatted with a table in a panel.""" highlighter = _OptionHighlighter() diff --git a/src/_pytask/path.py b/src/_pytask/path.py index 9bc74035..86968b8c 100644 --- a/src/_pytask/path.py +++ b/src/_pytask/path.py @@ -131,7 +131,6 @@ def import_path(path: Path, root: Path) -> ModuleType: """ module_name = _module_name_from_path(path, root) - with contextlib.suppress(KeyError): return sys.modules[module_name] From 1a34c2d70c5ca86f1c1c57db99242c481624b037 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Fri, 29 Dec 2023 23:49:34 +0100 Subject: [PATCH 4/9] Add more tests. --- src/_pytask/parameters.py | 7 +++++++ tests/test_hook_module.py | 20 ++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/src/_pytask/parameters.py b/src/_pytask/parameters.py index 019878be..f1ad8eff 100644 --- a/src/_pytask/parameters.py +++ b/src/_pytask/parameters.py @@ -128,6 +128,13 @@ def _hook_module_callback( path = ctx.params["config"].parent.joinpath(path).resolve() else: path = Path.cwd().joinpath(path).resolve() + + if not path.exists(): + msg = ( + f"The hook module {path} does not exist. " + "Please provide a valid path." + ) + raise click.BadParameter(msg) module = import_path(path, ctx.params["root"]) parsed_modules.append(module.__name__) else: diff --git a/tests/test_hook_module.py b/tests/test_hook_module.py index f45d6247..49d57a9d 100644 --- a/tests/test_hook_module.py +++ b/tests/test_hook_module.py @@ -49,3 +49,23 @@ def pytask_extend_command_line_interface(cli): ) assert result.returncode == ExitCode.OK assert "--new-option" in result.stdout.decode() + + +def test_error_when_hook_module_path_does_not_exist(tmp_path): + result = subprocess.run( # noqa: PLW1510 + ("pytask", "build", "--hook-module", "hooks.py", "--help"), + cwd=tmp_path, + capture_output=True, + ) + assert result.returncode == ExitCode.CONFIGURATION_FAILED + assert b"Error: Invalid value for '--hook-module'" in result.stderr + + +def test_error_when_hook_module_module_does_not_exist(tmp_path): + result = subprocess.run( # noqa: PLW1510 + ("pytask", "build", "--hook-module", "hooks", "--help"), + cwd=tmp_path, + capture_output=True, + ) + assert result.returncode == ExitCode.FAILED + assert b"ModuleNotFoundError: No module named 'hooks'" in result.stderr From 3837b22be7f5c4c6a8a377791b421f47487e3c30 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Fri, 29 Dec 2023 23:52:27 +0100 Subject: [PATCH 5/9] To changes. --- docs/source/changes.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/source/changes.md b/docs/source/changes.md index 31eef098..5b9038fd 100644 --- a/docs/source/changes.md +++ b/docs/source/changes.md @@ -23,6 +23,8 @@ releases are available on [PyPI](https://pypi.org/project/pytask) and - {pull}`528` improves the codecov setup and coverage. - {pull}`535` reenables and fixes tests with Jupyter. - {pull}`536` allows partialed functions to be task functions. +- {pull}`539` implements the {confval}`hook_module` configuration value and + `--hook-module` commandline option to register hooks. - {pull}`540` changes the CLI entry-point and allow `pytask.build(tasks=task_func)` as the signatures suggested. - {pull}`542` refactors the plugin manager. From 13d30c916907ccf673eb4a38d068634ae256ee75 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Sat, 30 Dec 2023 11:14:12 +0100 Subject: [PATCH 6/9] Add tests with import module. --- tests/test_hook_module.py | 45 ++++++++++++++++++++++++++++----------- 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/tests/test_hook_module.py b/tests/test_hook_module.py index 49d57a9d..4c63771f 100644 --- a/tests/test_hook_module.py +++ b/tests/test_hook_module.py @@ -3,10 +3,12 @@ import subprocess import textwrap +import pytest from pytask import ExitCode -def test_add_new_hook_via_cli(tmp_path): +@pytest.mark.parametrize("module_name", [True, False]) +def test_add_new_hook_via_cli(tmp_path, module_name): hooks = """ import click from pytask import hookimpl @@ -16,20 +18,32 @@ def pytask_extend_command_line_interface(cli): print("Hello World!") cli.commands["build"].params.append(click.Option(["--new-option"])) """ - tmp_path.joinpath("hooks.py").write_text(textwrap.dedent(hooks)) - result = subprocess.run( - ("pytask", "build", "--hook-module", "hooks.py", "--help"), - cwd=tmp_path, - capture_output=True, - check=True, - ) + tmp_path.joinpath("hooks").mkdir() + tmp_path.joinpath("hooks", "hooks.py").write_text(textwrap.dedent(hooks)) + + if module_name: + args = ( + "python", + "-m", + "pytask", + "build", + "--hook-module", + "hooks.hooks", + "--help", + ) + else: + args = ("pytask", "build", "--hook-module", "hooks/hooks.py", "--help") + + result = subprocess.run(args, cwd=tmp_path, capture_output=True, check=True) + assert result.returncode == ExitCode.OK assert "--new-option" in result.stdout.decode() -def test_add_new_hook_via_config(tmp_path): +@pytest.mark.parametrize("module_name", [True, False]) +def test_add_new_hook_via_config(tmp_path, module_name): tmp_path.joinpath("pyproject.toml").write_text( - "[tool.pytask.ini_options]\nhook_module = ['hooks.py']" + "[tool.pytask.ini_options]\nhook_module = ['hooks/hooks.py']" ) hooks = """ @@ -40,9 +54,16 @@ def test_add_new_hook_via_config(tmp_path): def pytask_extend_command_line_interface(cli): cli.commands["build"].params.append(click.Option(["--new-option"])) """ - tmp_path.joinpath("hooks.py").write_text(textwrap.dedent(hooks)) + tmp_path.joinpath("hooks").mkdir() + tmp_path.joinpath("hooks", "hooks.py").write_text(textwrap.dedent(hooks)) + + if module_name: + args = ("python", "-m", "pytask", "build", "--help") + else: + args = ("pytask", "build", "--help") + result = subprocess.run( - ("pytask", "build", tmp_path.as_posix(), "--help"), + args, cwd=tmp_path, capture_output=True, check=True, From a276ccea24f399286e7787d24d998657b734f4a8 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Sat, 30 Dec 2023 11:37:43 +0100 Subject: [PATCH 7/9] Add more tests. --- src/_pytask/click.py | 5 ++++- src/_pytask/parameters.py | 8 ++++++++ tests/test_hook_module.py | 15 +++++++++++++-- 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/_pytask/click.py b/src/_pytask/click.py index 4692eb18..35e9fd9b 100644 --- a/src/_pytask/click.py +++ b/src/_pytask/click.py @@ -129,9 +129,12 @@ def sort_key(item: Parameter) -> tuple[bool, float]: # processed first even if other eager parameters are chosen. The rest follows # https://click.palletsprojects.com/en/8.1.x/advanced/#callback-evaluation-order. if item.name == "paths": - return False, -2 + return False, -3 if item.name == "config": + return False, -2 + + if item.name == "hook_module": return False, -1 try: diff --git a/src/_pytask/parameters.py b/src/_pytask/parameters.py index f1ad8eff..555327fc 100644 --- a/src/_pytask/parameters.py +++ b/src/_pytask/parameters.py @@ -1,6 +1,7 @@ """Contains common parameters for the commands of the command line interface.""" from __future__ import annotations +import importlib.util from pathlib import Path from typing import Iterable from typing import TYPE_CHECKING @@ -138,6 +139,13 @@ def _hook_module_callback( module = import_path(path, ctx.params["root"]) parsed_modules.append(module.__name__) else: + spec = importlib.util.find_spec(module_name) + if spec is None: + msg = ( + f"The hook module {module_name!r} is not importable. " + "Please provide a valid module name." + ) + raise click.BadParameter(msg) parsed_modules.append(module_name) # If there are hook modules, we register a hook implementation to add them. diff --git a/tests/test_hook_module.py b/tests/test_hook_module.py index 4c63771f..6aff209a 100644 --- a/tests/test_hook_module.py +++ b/tests/test_hook_module.py @@ -88,5 +88,16 @@ def test_error_when_hook_module_module_does_not_exist(tmp_path): cwd=tmp_path, capture_output=True, ) - assert result.returncode == ExitCode.FAILED - assert b"ModuleNotFoundError: No module named 'hooks'" in result.stderr + assert result.returncode == ExitCode.CONFIGURATION_FAILED + assert b"Error: Invalid value for '--hook-module':" in result.stderr + + +def test_error_when_hook_module_is_no_iterable(tmp_path): + tmp_path.joinpath("pyproject.toml").write_text( + "[tool.pytask.ini_options]\nhook_module = 'hooks'" + ) + result = subprocess.run( # noqa: PLW1510 + ("pytask", "build", "--help"), cwd=tmp_path, capture_output=True + ) + assert result.returncode == ExitCode.CONFIGURATION_FAILED + assert b"Error: Invalid value for '--hook-module':" in result.stderr From 20babae05c715de390bc07fbdf2d753352cb0301 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Sat, 30 Dec 2023 12:50:29 +0100 Subject: [PATCH 8/9] Skip tests on windows. --- tests/test_hook_module.py | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/tests/test_hook_module.py b/tests/test_hook_module.py index 6aff209a..3f659bd4 100644 --- a/tests/test_hook_module.py +++ b/tests/test_hook_module.py @@ -1,13 +1,28 @@ from __future__ import annotations +import os import subprocess +import sys import textwrap import pytest from pytask import ExitCode -@pytest.mark.parametrize("module_name", [True, False]) +@pytest.mark.parametrize( + "module_name", + [ + pytest.param( + True, + marks=pytest.mark.xfail( + sys.platform == "win32" and "CI" in os.environ, + reason="pytask is not found in subprocess", + ), + strict=True, + ), + False, + ], +) def test_add_new_hook_via_cli(tmp_path, module_name): hooks = """ import click @@ -40,7 +55,20 @@ def pytask_extend_command_line_interface(cli): assert "--new-option" in result.stdout.decode() -@pytest.mark.parametrize("module_name", [True, False]) +@pytest.mark.parametrize( + "module_name", + [ + pytest.param( + True, + marks=pytest.mark.xfail( + sys.platform == "win32" and "CI" in os.environ, + reason="pytask is not found in subprocess", + ), + strict=True, + ), + False, + ], +) def test_add_new_hook_via_config(tmp_path, module_name): tmp_path.joinpath("pyproject.toml").write_text( "[tool.pytask.ini_options]\nhook_module = ['hooks/hooks.py']" From db2d8f238c3b12829355ed668175da9124456237 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Sat, 30 Dec 2023 12:54:10 +0100 Subject: [PATCH 9/9] Fix test. --- tests/test_hook_module.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_hook_module.py b/tests/test_hook_module.py index 3f659bd4..24784c1d 100644 --- a/tests/test_hook_module.py +++ b/tests/test_hook_module.py @@ -17,8 +17,8 @@ marks=pytest.mark.xfail( sys.platform == "win32" and "CI" in os.environ, reason="pytask is not found in subprocess", + strict=True, ), - strict=True, ), False, ], @@ -63,8 +63,8 @@ def pytask_extend_command_line_interface(cli): marks=pytest.mark.xfail( sys.platform == "win32" and "CI" in os.environ, reason="pytask is not found in subprocess", + strict=True, ), - strict=True, ), False, ],