From 498a5f504b5ebd42c9fe290d5a8e69d2314c49e1 Mon Sep 17 00:00:00 2001 From: Tucker Beck Date: Sun, 13 Apr 2025 22:07:07 -0700 Subject: [PATCH 1/2] feat: add `failure_message` option for tasks (#2080) * If a `failure_message` option is provided, show that message: "Task N failed: a nice failure message" * If no `failure_message` option is provided, show the subprocess exception: "Task N failed: Command 'might-fail' returned non-zero exit status 1." Addresses Issue #2080: https://github.com/copier-org/copier/issues/2080 --- copier/_main.py | 9 ++++++++- copier/_template.py | 6 ++++++ copier/errors.py | 14 +++++++++++--- docs/configuring.md | 4 ++++ tests/test_tasks.py | 36 ++++++++++++++++++++++++++++++++++++ 5 files changed, 65 insertions(+), 4 deletions(-) diff --git a/copier/_main.py b/copier/_main.py index fde8e4170..0ba07c297 100644 --- a/copier/_main.py +++ b/copier/_main.py @@ -389,7 +389,14 @@ def _execute_tasks(self, tasks: Sequence[Task]) -> None: with local.cwd(working_directory), local.env(**extra_env): process = subprocess.run(task_cmd, shell=use_shell, env=local.env) if process.returncode: - raise TaskError.from_process(process) + if task.failure_message: + message = self._render_string(task.failure_message, extra_context) + else: + message = f"{task_cmd!r} returned non-zero exit status {process.returncode}" + raise TaskError.from_process( + process, + message=f"Task {i + 1} failed: {message}.", + ) def _render_context(self) -> AnyByStrMutableMapping: """Produce render context for Jinja.""" diff --git a/copier/_template.py b/copier/_template.py index b26a724f9..d053517bd 100644 --- a/copier/_template.py +++ b/copier/_template.py @@ -169,12 +169,17 @@ class Task: working_directory: The directory from inside where to execute the task. If `None`, the project directory will be used. + + failure_message: + Provides a message to pritn if the task fails. + If `None`, the subprocess exception message will be used. """ cmd: str | Sequence[str] extra_vars: dict[str, Any] = field(default_factory=dict) condition: str | bool = True working_directory: Path = Path() + failure_message: str | None = None @dataclass @@ -526,6 +531,7 @@ def tasks(self) -> Sequence[Task]: extra_vars=extra_vars, condition=task.get("when", "true"), working_directory=Path(task.get("working_directory", ".")), + failure_message=task.get("failure_message"), ) ) else: diff --git a/copier/errors.py b/copier/errors.py index fb61095ae..0dcce16dd 100644 --- a/copier/errors.py +++ b/copier/errors.py @@ -146,16 +146,23 @@ def __init__( returncode: int, stdout: str | bytes | None, stderr: str | bytes | None, + message: str | None = None, ): subprocess.CalledProcessError.__init__( self, returncode=returncode, cmd=command, output=stdout, stderr=stderr ) - message = f"Task {command!r} returned non-zero exit status {returncode}." - UserMessageError.__init__(self, message) + if not message: + message = subprocess.CalledProcessError.__str__(self) + UserMessageError.__init__(self, message=message) + + def __str__(self) -> str: + return self.message @classmethod def from_process( - cls, process: CompletedProcess[str] | CompletedProcess[bytes] + cls, + process: CompletedProcess[str] | CompletedProcess[bytes], + message: str | None = None, ) -> Self: """Create a TaskError from a CompletedProcess.""" return cls( @@ -163,6 +170,7 @@ def from_process( returncode=process.returncode, stdout=process.stdout, stderr=process.stderr, + message=message, ) diff --git a/docs/configuring.md b/docs/configuring.md index 65ab6e279..93da0ebeb 100644 --- a/docs/configuring.md +++ b/docs/configuring.md @@ -1572,6 +1572,8 @@ If a `dict` is given it can contain the following items: - **when** (optional): Specifies a condition that needs to hold for the task to run. - **working_directory** (optional): Specifies the directory in which the command will be run. Defaults to the destination directory. +- **failure_message** (optional): Provides a message to print if the task fails. If + not provided, the subprocess exception message will be shown. If a `str` or `List[str]` is given as a task it will be treated as `command` with all other items not present. @@ -1599,6 +1601,8 @@ Refer to the example provided below for more information. when: "{{ _copier_conf.os in ['linux', 'macos'] }}" - command: Remove-Item {{ name_of_the_project }}\\README.md when: "{{ _copier_conf.os == 'windows' }}" + - command: finalize-project + failure_message: Couldn't finalize {{ name_of_the_project }} ``` Note: the example assumes you use [Invoke](https://www.pyinvoke.org/) as diff --git a/tests/test_tasks.py b/tests/test_tasks.py index 1b664062e..fdfc8da5e 100644 --- a/tests/test_tasks.py +++ b/tests/test_tasks.py @@ -3,6 +3,7 @@ from pathlib import Path from typing import Literal +from copier.errors import TaskError import pytest import yaml @@ -180,3 +181,38 @@ def test_copier_phase_variable(tmp_path_factory: pytest.TempPathFactory) -> None ) copier.run_copy(str(src), dst, unsafe=True) assert (dst / "tasks").exists() + + +@pytest.mark.parametrize( + "task,failure_message,match,data", + [ + ("false", "Oh, dear! The task failed", r"Task \d+ failed: Oh, dear! The task failed\.", None), + ("ls non-existing-directory", None, r"Task \d+ failed: 'ls non-existing-directory' returned non-zero exit status 2\.", None), + ("false", "{{ name }} blew up", r"Task \d+ failed: Wile E. Coyote blew up\.", {"name": "Wile E. Coyote"}), + ], +) +def test_task_failure_message( + tmp_path_factory: pytest.TempPathFactory, + task: str, + failure_message: str | None, + match: str, + data: dict[str, str] | None, +) -> None: + src, dst = map(tmp_path_factory.mktemp, ("src", "dst")) + yaml_dict = { + "_tasks": [ + { + "command": task, + } + ] + } + if failure_message: + yaml_dict["_tasks"][0]["failure_message"] = failure_message + + build_file_tree( + { + (src / "copier.yml"): yaml.safe_dump(yaml_dict) + } + ) + with pytest.raises(TaskError, match=match): + copier.run_copy(str(src), dst, unsafe=True, data=data) From d489cdebc793e81852cea3dd147c0f9233214f7c Mon Sep 17 00:00:00 2001 From: Tucker Beck Date: Fri, 16 May 2025 10:18:50 -0700 Subject: [PATCH 2/2] fix: small tweaks and improvements to TaskError - Add an `index` attribute to TaskError - TaskError adds its own prefix: "Task failed: " - Fixed a typo - Updated unit tests --- copier/_main.py | 8 ++------ copier/_template.py | 2 +- copier/errors.py | 6 ++++++ tests/test_tasks.py | 6 +++--- 4 files changed, 12 insertions(+), 10 deletions(-) diff --git a/copier/_main.py b/copier/_main.py index 0ba07c297..2744305d0 100644 --- a/copier/_main.py +++ b/copier/_main.py @@ -389,14 +389,10 @@ def _execute_tasks(self, tasks: Sequence[Task]) -> None: with local.cwd(working_directory), local.env(**extra_env): process = subprocess.run(task_cmd, shell=use_shell, env=local.env) if process.returncode: + message: str | None = None if task.failure_message: message = self._render_string(task.failure_message, extra_context) - else: - message = f"{task_cmd!r} returned non-zero exit status {process.returncode}" - raise TaskError.from_process( - process, - message=f"Task {i + 1} failed: {message}.", - ) + raise TaskError.from_process(process, i, message=message) def _render_context(self) -> AnyByStrMutableMapping: """Produce render context for Jinja.""" diff --git a/copier/_template.py b/copier/_template.py index d053517bd..52effc9a8 100644 --- a/copier/_template.py +++ b/copier/_template.py @@ -171,7 +171,7 @@ class Task: If `None`, the project directory will be used. failure_message: - Provides a message to pritn if the task fails. + Provides a message to print if the task fails. If `None`, the subprocess exception message will be used. """ diff --git a/copier/errors.py b/copier/errors.py index 0dcce16dd..4ed8aa4a5 100644 --- a/copier/errors.py +++ b/copier/errors.py @@ -142,6 +142,7 @@ class TaskError(subprocess.CalledProcessError, UserMessageError): def __init__( self, + index: int, command: str | Sequence[str], returncode: int, stdout: str | bytes | None, @@ -151,8 +152,11 @@ def __init__( subprocess.CalledProcessError.__init__( self, returncode=returncode, cmd=command, output=stdout, stderr=stderr ) + self.index = index if not message: message = subprocess.CalledProcessError.__str__(self) + message = message.rstrip(".") + message = f"Task {index + 1} failed: {message}." UserMessageError.__init__(self, message=message) def __str__(self) -> str: @@ -162,10 +166,12 @@ def __str__(self) -> str: def from_process( cls, process: CompletedProcess[str] | CompletedProcess[bytes], + index: int, message: str | None = None, ) -> Self: """Create a TaskError from a CompletedProcess.""" return cls( + index, command=process.args, returncode=process.returncode, stdout=process.stdout, diff --git a/tests/test_tasks.py b/tests/test_tasks.py index fdfc8da5e..0b0c5aa35 100644 --- a/tests/test_tasks.py +++ b/tests/test_tasks.py @@ -186,9 +186,9 @@ def test_copier_phase_variable(tmp_path_factory: pytest.TempPathFactory) -> None @pytest.mark.parametrize( "task,failure_message,match,data", [ - ("false", "Oh, dear! The task failed", r"Task \d+ failed: Oh, dear! The task failed\.", None), - ("ls non-existing-directory", None, r"Task \d+ failed: 'ls non-existing-directory' returned non-zero exit status 2\.", None), - ("false", "{{ name }} blew up", r"Task \d+ failed: Wile E. Coyote blew up\.", {"name": "Wile E. Coyote"}), + ("false", "Oh, dear! The task failed", r"^Task \d+ failed: Oh, dear! The task failed\.$", None), + ("ls non-existing-directory", None, r"^Task \d+ failed: Command 'ls non-existing-directory' returned non-zero exit status 2\.$", None), + ("false", "{{ name }} blew up", r"^Task \d+ failed: Wile E. Coyote blew up\.$", {"name": "Wile E. Coyote"}), ], ) def test_task_failure_message(