From 141f4fb00ac700e90ccf8024eca5500bbc3482b3 Mon Sep 17 00:00:00 2001 From: samypr100 <3933065+samypr100@users.noreply.github.com> Date: Fri, 17 Feb 2023 15:45:26 -0500 Subject: [PATCH 1/7] Warning when invalid groups are referenced --- docs/managing-dependencies.md | 10 +++++- src/poetry/console/commands/group_command.py | 32 +++++++++++++++++ tests/console/commands/test_install.py | 36 ++++++++++++++++++++ 3 files changed, 77 insertions(+), 1 deletion(-) diff --git a/docs/managing-dependencies.md b/docs/managing-dependencies.md index 16622624857..c6c4bbba7bc 100644 --- a/docs/managing-dependencies.md +++ b/docs/managing-dependencies.md @@ -162,8 +162,16 @@ the `--only` option. poetry install --only docs ``` +{{% warning %}} + +Groups provided to `--with`, `--without`, and `--only` should at least be declared in `pyproject.toml`. +Poetry will display a **Warning** if a group that is not declared is used, letting the user know of a +potential mistake. + +{{% /warning %}} + {{% note %}} -If you only want to install the project's runtime dependencies, you can do so with the +If you only want to install the project's runtime dependencies, you can do so with the `--only main` notation: ```bash diff --git a/src/poetry/console/commands/group_command.py b/src/poetry/console/commands/group_command.py index 27438bf0c07..5450f45cfb9 100644 --- a/src/poetry/console/commands/group_command.py +++ b/src/poetry/console/commands/group_command.py @@ -1,5 +1,6 @@ from __future__ import annotations +from collections import defaultdict from typing import TYPE_CHECKING from cleo.helpers import option @@ -78,6 +79,7 @@ def activated_groups(self) -> set[str]: for groups in self.option(key, "") for group in groups.split(",") } + self.validate_groups(groups) for opt, new, group in [ ("no-dev", "only", MAIN_GROUP), @@ -107,3 +109,33 @@ def project_with_activated_groups_only(self) -> ProjectPackage: return self.poetry.package.with_dependency_groups( list(self.activated_groups), only=True ) + + def validate_groups(self, group_options: dict[str, set[str]]) -> bool: + """ + Currently issues a warning if it detects that a group is + not part of pyproject.toml + + Can be overridden to adapt behavior. + """ + invalid_options = defaultdict(set) + for opt, groups in group_options.items(): + for group in groups: + try: + self.poetry.package.dependency_group(group) + except ValueError: + invalid_options[opt].add(group) + if invalid_options: + line_err = ( + "The --with, " + "--without, " + "and --only " + "options may only have valid groups." + ) + for opt, invalid_groups in invalid_options.items(): + line_err += ( + " Invalid" + f" {','.join(sorted(invalid_groups))} provided to --{opt}." + ) + line_err += "" + self.line_error(line_err) + return len(invalid_options) == 0 diff --git a/tests/console/commands/test_install.py b/tests/console/commands/test_install.py index 0200a452bfa..317ac84f542 100644 --- a/tests/console/commands/test_install.py +++ b/tests/console/commands/test_install.py @@ -257,6 +257,42 @@ def test_only_root_conflicts_with_without_only( ) +@pytest.mark.parametrize( + ("options", "valid_groups"), + [ + ({"--with": MAIN_GROUP}, {MAIN_GROUP}), + ({"--with": "spam"}, set()), + ({"--with": "spam,foo"}, {"foo"}), + ({"--without": "spam"}, set()), + ({"--without": "spam,bar"}, {"bar"}), + ({"--with": "eggs,ham", "--without": "spam"}, set()), + ({"--with": "eggs,ham", "--without": "spam,baz"}, {"baz"}), + ({"--only": "spam"}, set()), + ({"--only": "bim"}, {"bim"}), + ({"--only": MAIN_GROUP}, {MAIN_GROUP}), + ], +) +def test_invalid_groups_with_without_only( + tester: CommandTester, + mocker: MockerFixture, + options: dict[str, str], + valid_groups: set[str], +): + mocker.patch.object(tester.command.installer, "run", return_value=0) + + cmd_args = " ".join(f"{flag} {groups}" for (flag, groups) in options.items()) + tester.execute(cmd_args) + + assert tester.status_code == 0 + + io_error = tester.io.fetch_error() + for opt, groups in options.items(): + group_list = groups.split(",") + invalid_groups = ",".join(sorted(set(group_list) - valid_groups)) + if invalid_groups: + assert f"Invalid {invalid_groups} provided to {opt}." in io_error + + def test_remove_untracked_outputs_deprecation_warning( tester: CommandTester, mocker: MockerFixture, From 70207f932ada1ca24c9856bdf10617d7dfd21c24 Mon Sep 17 00:00:00 2001 From: samypr100 <3933065+samypr100@users.noreply.github.com> Date: Thu, 23 Mar 2023 09:24:12 -0400 Subject: [PATCH 2/7] Update src/poetry/console/commands/group_command.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Randy Döring <30527984+radoering@users.noreply.github.com> --- src/poetry/console/commands/group_command.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/poetry/console/commands/group_command.py b/src/poetry/console/commands/group_command.py index 5450f45cfb9..14e11fc9049 100644 --- a/src/poetry/console/commands/group_command.py +++ b/src/poetry/console/commands/group_command.py @@ -120,9 +120,7 @@ def validate_groups(self, group_options: dict[str, set[str]]) -> bool: invalid_options = defaultdict(set) for opt, groups in group_options.items(): for group in groups: - try: - self.poetry.package.dependency_group(group) - except ValueError: + if not self.poetry.package.has_dependency_group(group): invalid_options[opt].add(group) if invalid_options: line_err = ( From 90c54899d649ab1d8fd0eb5b20ea129e9881567f Mon Sep 17 00:00:00 2001 From: samypr100 <3933065+samypr100@users.noreply.github.com> Date: Mon, 3 Apr 2023 16:28:24 -0400 Subject: [PATCH 3/7] Raise error when encountering invalid groups --- docs/managing-dependencies.md | 2 +- src/poetry/console/commands/group_command.py | 19 ++++++---- src/poetry/exceptions.py | 4 +++ tests/console/commands/test_install.py | 38 ++++++++++++-------- 4 files changed, 41 insertions(+), 22 deletions(-) diff --git a/docs/managing-dependencies.md b/docs/managing-dependencies.md index c6c4bbba7bc..1434a5b5072 100644 --- a/docs/managing-dependencies.md +++ b/docs/managing-dependencies.md @@ -165,7 +165,7 @@ poetry install --only docs {{% warning %}} Groups provided to `--with`, `--without`, and `--only` should at least be declared in `pyproject.toml`. -Poetry will display a **Warning** if a group that is not declared is used, letting the user know of a +Poetry will raise an **Error** if a group that is not declared is used, letting the user know of a potential mistake. {{% /warning %}} diff --git a/src/poetry/console/commands/group_command.py b/src/poetry/console/commands/group_command.py index 14e11fc9049..039e378a323 100644 --- a/src/poetry/console/commands/group_command.py +++ b/src/poetry/console/commands/group_command.py @@ -1,12 +1,14 @@ from __future__ import annotations from collections import defaultdict +from itertools import chain from typing import TYPE_CHECKING from cleo.helpers import option from poetry.core.packages.dependency_group import MAIN_GROUP from poetry.console.commands.command import Command +from poetry.exceptions import GroupNotFound if TYPE_CHECKING: @@ -79,7 +81,7 @@ def activated_groups(self) -> set[str]: for groups in self.option(key, "") for group in groups.split(",") } - self.validate_groups(groups) + self.validate_group_options(groups) for opt, new, group in [ ("no-dev", "only", MAIN_GROUP), @@ -110,9 +112,9 @@ def project_with_activated_groups_only(self) -> ProjectPackage: list(self.activated_groups), only=True ) - def validate_groups(self, group_options: dict[str, set[str]]) -> bool: + def validate_group_options(self, group_options: dict[str, set[str]]) -> bool: """ - Currently issues a warning if it detects that a group is + Currently raises en error if it detects that a group is not part of pyproject.toml Can be overridden to adapt behavior. @@ -124,7 +126,7 @@ def validate_groups(self, group_options: dict[str, set[str]]) -> bool: invalid_options[opt].add(group) if invalid_options: line_err = ( - "The --with, " + "The --with, " "--without, " "and --only " "options may only have valid groups." @@ -132,8 +134,13 @@ def validate_groups(self, group_options: dict[str, set[str]]) -> bool: for opt, invalid_groups in invalid_options.items(): line_err += ( " Invalid" - f" {','.join(sorted(invalid_groups))} provided to --{opt}." + f" [{', '.join(sorted(invalid_groups))}] provided to --{opt}." ) - line_err += "" + line_err += "" self.line_error(line_err) + if len(invalid_options) != 0: + invalid_groups_str = ", ".join( + sorted(set(chain(*invalid_options.values()))) + ) + raise GroupNotFound(f"Group(s) [{invalid_groups_str}] were not found.") return len(invalid_options) == 0 diff --git a/src/poetry/exceptions.py b/src/poetry/exceptions.py index 0d755667594..7096b28c22b 100644 --- a/src/poetry/exceptions.py +++ b/src/poetry/exceptions.py @@ -7,3 +7,7 @@ class PoetryException(Exception): class InvalidProjectFile(PoetryException): pass + + +class GroupNotFound(PoetryException): + pass diff --git a/tests/console/commands/test_install.py b/tests/console/commands/test_install.py index 317ac84f542..9b5d9370b9c 100644 --- a/tests/console/commands/test_install.py +++ b/tests/console/commands/test_install.py @@ -7,6 +7,8 @@ from poetry.core.masonry.utils.module import ModuleOrPackageNotFound from poetry.core.packages.dependency_group import MAIN_GROUP +from poetry.exceptions import GroupNotFound + if TYPE_CHECKING: from cleo.testers.command_tester import CommandTester @@ -258,18 +260,18 @@ def test_only_root_conflicts_with_without_only( @pytest.mark.parametrize( - ("options", "valid_groups"), + ("options", "valid_groups", "should_raise"), [ - ({"--with": MAIN_GROUP}, {MAIN_GROUP}), - ({"--with": "spam"}, set()), - ({"--with": "spam,foo"}, {"foo"}), - ({"--without": "spam"}, set()), - ({"--without": "spam,bar"}, {"bar"}), - ({"--with": "eggs,ham", "--without": "spam"}, set()), - ({"--with": "eggs,ham", "--without": "spam,baz"}, {"baz"}), - ({"--only": "spam"}, set()), - ({"--only": "bim"}, {"bim"}), - ({"--only": MAIN_GROUP}, {MAIN_GROUP}), + ({"--with": MAIN_GROUP}, {MAIN_GROUP}, False), + ({"--with": "spam"}, set(), True), + ({"--with": "spam,foo"}, {"foo"}, True), + ({"--without": "spam"}, set(), True), + ({"--without": "spam,bar"}, {"bar"}, True), + ({"--with": "eggs,ham", "--without": "spam"}, set(), True), + ({"--with": "eggs,ham", "--without": "spam,baz"}, {"baz"}, True), + ({"--only": "spam"}, set(), True), + ({"--only": "bim"}, {"bim"}, False), + ({"--only": MAIN_GROUP}, {MAIN_GROUP}, False), ], ) def test_invalid_groups_with_without_only( @@ -277,20 +279,26 @@ def test_invalid_groups_with_without_only( mocker: MockerFixture, options: dict[str, str], valid_groups: set[str], + should_raise: bool, ): mocker.patch.object(tester.command.installer, "run", return_value=0) cmd_args = " ".join(f"{flag} {groups}" for (flag, groups) in options.items()) - tester.execute(cmd_args) - assert tester.status_code == 0 + if not should_raise: + tester.execute(cmd_args) + assert tester.status_code == 0 + else: + with pytest.raises(GroupNotFound, match=r"^Group\(s\) \[.*] were not found"): + tester.execute(cmd_args) + assert tester.status_code is None io_error = tester.io.fetch_error() for opt, groups in options.items(): group_list = groups.split(",") - invalid_groups = ",".join(sorted(set(group_list) - valid_groups)) + invalid_groups = ", ".join(sorted(set(group_list) - valid_groups)) if invalid_groups: - assert f"Invalid {invalid_groups} provided to {opt}." in io_error + assert f"Invalid [{invalid_groups}] provided to {opt}." in io_error def test_remove_untracked_outputs_deprecation_warning( From bcd5401276374371fd1f3b1d08e78e60c03592cd Mon Sep 17 00:00:00 2001 From: samypr100 <3933065+samypr100@users.noreply.github.com> Date: Tue, 4 Apr 2023 09:52:48 -0400 Subject: [PATCH 4/7] Addressing PR feedback Removing extraneous docs Collapsing error output into single Exception Migrating GroupNotFound to console.exceptions --- docs/managing-dependencies.md | 8 ------ src/poetry/console/commands/group_command.py | 30 +++++++++----------- src/poetry/console/exceptions.py | 4 +++ src/poetry/exceptions.py | 4 --- tests/console/commands/test_install.py | 22 ++++++++------ 5 files changed, 30 insertions(+), 38 deletions(-) diff --git a/docs/managing-dependencies.md b/docs/managing-dependencies.md index 1434a5b5072..14932d43df9 100644 --- a/docs/managing-dependencies.md +++ b/docs/managing-dependencies.md @@ -162,14 +162,6 @@ the `--only` option. poetry install --only docs ``` -{{% warning %}} - -Groups provided to `--with`, `--without`, and `--only` should at least be declared in `pyproject.toml`. -Poetry will raise an **Error** if a group that is not declared is used, letting the user know of a -potential mistake. - -{{% /warning %}} - {{% note %}} If you only want to install the project's runtime dependencies, you can do so with the `--only main` notation: diff --git a/src/poetry/console/commands/group_command.py b/src/poetry/console/commands/group_command.py index 039e378a323..ab5dc21f31e 100644 --- a/src/poetry/console/commands/group_command.py +++ b/src/poetry/console/commands/group_command.py @@ -8,7 +8,7 @@ from poetry.core.packages.dependency_group import MAIN_GROUP from poetry.console.commands.command import Command -from poetry.exceptions import GroupNotFound +from poetry.console.exceptions import GroupNotFound if TYPE_CHECKING: @@ -81,7 +81,7 @@ def activated_groups(self) -> set[str]: for groups in self.option(key, "") for group in groups.split(",") } - self.validate_group_options(groups) + self._validate_group_options(groups) for opt, new, group in [ ("no-dev", "only", MAIN_GROUP), @@ -112,12 +112,9 @@ def project_with_activated_groups_only(self) -> ProjectPackage: list(self.activated_groups), only=True ) - def validate_group_options(self, group_options: dict[str, set[str]]) -> bool: + def _validate_group_options(self, group_options: dict[str, set[str]]) -> bool: """ - Currently raises en error if it detects that a group is - not part of pyproject.toml - - Can be overridden to adapt behavior. + Raises en error if it detects that a group is not part of pyproject.toml """ invalid_options = defaultdict(set) for opt, groups in group_options.items(): @@ -125,22 +122,21 @@ def validate_group_options(self, group_options: dict[str, set[str]]) -> bool: if not self.poetry.package.has_dependency_group(group): invalid_options[opt].add(group) if invalid_options: - line_err = ( - "The --with, " + error_details = ( + "The --with, " "--without, " "and --only " "options may only have valid groups." ) for opt, invalid_groups in invalid_options.items(): - line_err += ( - " Invalid" - f" [{', '.join(sorted(invalid_groups))}] provided to --{opt}." + error_details += ( + f" Invalid [{', '.join(sorted(invalid_groups))}] provided to" + f" --{opt}." ) - line_err += "" - self.line_error(line_err) - if len(invalid_options) != 0: - invalid_groups_str = ", ".join( + all_invalid_groups_str = ", ".join( sorted(set(chain(*invalid_options.values()))) ) - raise GroupNotFound(f"Group(s) [{invalid_groups_str}] were not found.") + raise GroupNotFound( + f"Group(s) [{all_invalid_groups_str}] were not found. {error_details}" + ) return len(invalid_options) == 0 diff --git a/src/poetry/console/exceptions.py b/src/poetry/console/exceptions.py index aadc8c17e7f..2cc359ddb75 100644 --- a/src/poetry/console/exceptions.py +++ b/src/poetry/console/exceptions.py @@ -5,3 +5,7 @@ class PoetryConsoleError(CleoError): pass + + +class GroupNotFound(PoetryConsoleError): + pass diff --git a/src/poetry/exceptions.py b/src/poetry/exceptions.py index 7096b28c22b..0d755667594 100644 --- a/src/poetry/exceptions.py +++ b/src/poetry/exceptions.py @@ -7,7 +7,3 @@ class PoetryException(Exception): class InvalidProjectFile(PoetryException): pass - - -class GroupNotFound(PoetryException): - pass diff --git a/tests/console/commands/test_install.py b/tests/console/commands/test_install.py index 9b5d9370b9c..cf5afebbcee 100644 --- a/tests/console/commands/test_install.py +++ b/tests/console/commands/test_install.py @@ -7,7 +7,7 @@ from poetry.core.masonry.utils.module import ModuleOrPackageNotFound from poetry.core.packages.dependency_group import MAIN_GROUP -from poetry.exceptions import GroupNotFound +from poetry.console.exceptions import GroupNotFound if TYPE_CHECKING: @@ -289,16 +289,20 @@ def test_invalid_groups_with_without_only( tester.execute(cmd_args) assert tester.status_code == 0 else: - with pytest.raises(GroupNotFound, match=r"^Group\(s\) \[.*] were not found"): + with pytest.raises( + GroupNotFound, match=r"^Group\(s\) \[.*] were not found." + ) as e: tester.execute(cmd_args) assert tester.status_code is None - - io_error = tester.io.fetch_error() - for opt, groups in options.items(): - group_list = groups.split(",") - invalid_groups = ", ".join(sorted(set(group_list) - valid_groups)) - if invalid_groups: - assert f"Invalid [{invalid_groups}] provided to {opt}." in io_error + for opt, groups in options.items(): + group_list = groups.split(",") + invalid_groups = ", ".join(sorted(set(group_list) - valid_groups)) + if invalid_groups: + assert ( + f"Invalid [{invalid_groups}] provided to" + f" {opt}." + in str(e.value) + ) def test_remove_untracked_outputs_deprecation_warning( From 08ab3eae4d694f96c6885f046d74c7a7e7ee2d60 Mon Sep 17 00:00:00 2001 From: samypr100 <3933065+samypr100@users.noreply.github.com> Date: Tue, 4 Apr 2023 11:29:45 -0400 Subject: [PATCH 5/7] Simplifying Exception Message --- src/poetry/console/commands/group_command.py | 28 +++++++------------- tests/console/commands/test_install.py | 14 +++++----- 2 files changed, 16 insertions(+), 26 deletions(-) diff --git a/src/poetry/console/commands/group_command.py b/src/poetry/console/commands/group_command.py index ab5dc21f31e..20b560b07d3 100644 --- a/src/poetry/console/commands/group_command.py +++ b/src/poetry/console/commands/group_command.py @@ -1,7 +1,6 @@ from __future__ import annotations from collections import defaultdict -from itertools import chain from typing import TYPE_CHECKING from cleo.helpers import option @@ -120,23 +119,16 @@ def _validate_group_options(self, group_options: dict[str, set[str]]) -> bool: for opt, groups in group_options.items(): for group in groups: if not self.poetry.package.has_dependency_group(group): - invalid_options[opt].add(group) + invalid_options[group].add(opt) if invalid_options: - error_details = ( - "The --with, " - "--without, " - "and --only " - "options may only have valid groups." - ) - for opt, invalid_groups in invalid_options.items(): - error_details += ( - f" Invalid [{', '.join(sorted(invalid_groups))}] provided to" - f" --{opt}." + invalid_groups = sorted(invalid_options.keys()) + error_msg = "Group(s) not found:" + for group in invalid_groups: + opts = ", ".join( + f"--{opt}" + for opt in sorted(invalid_options[group]) ) - all_invalid_groups_str = ", ".join( - sorted(set(chain(*invalid_options.values()))) - ) - raise GroupNotFound( - f"Group(s) [{all_invalid_groups_str}] were not found. {error_details}" - ) + error_msg += f" {group} (via {opts})," + error_msg = error_msg.rstrip(",") + raise GroupNotFound(error_msg) return len(invalid_options) == 0 diff --git a/tests/console/commands/test_install.py b/tests/console/commands/test_install.py index cf5afebbcee..f39081872d0 100644 --- a/tests/console/commands/test_install.py +++ b/tests/console/commands/test_install.py @@ -1,5 +1,7 @@ from __future__ import annotations +import re + from typing import TYPE_CHECKING import pytest @@ -289,19 +291,15 @@ def test_invalid_groups_with_without_only( tester.execute(cmd_args) assert tester.status_code == 0 else: - with pytest.raises( - GroupNotFound, match=r"^Group\(s\) \[.*] were not found." - ) as e: + with pytest.raises(GroupNotFound, match=r"^Group\(s\) not found:") as e: tester.execute(cmd_args) assert tester.status_code is None for opt, groups in options.items(): group_list = groups.split(",") - invalid_groups = ", ".join(sorted(set(group_list) - valid_groups)) - if invalid_groups: + invalid_groups = sorted(set(group_list) - valid_groups) + for group in invalid_groups: assert ( - f"Invalid [{invalid_groups}] provided to" - f" {opt}." - in str(e.value) + re.search(rf"{group} \(via .*{opt}.*\)", str(e.value)) is not None ) From fe83b6366ca886a339018e05474eb5b152d16440 Mon Sep 17 00:00:00 2001 From: samypr100 <3933065+samypr100@users.noreply.github.com> Date: Wed, 5 Apr 2023 09:19:31 -0400 Subject: [PATCH 6/7] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Randy Döring <30527984+radoering@users.noreply.github.com> --- src/poetry/console/commands/group_command.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/poetry/console/commands/group_command.py b/src/poetry/console/commands/group_command.py index 20b560b07d3..2a2b0a3ae44 100644 --- a/src/poetry/console/commands/group_command.py +++ b/src/poetry/console/commands/group_command.py @@ -111,7 +111,7 @@ def project_with_activated_groups_only(self) -> ProjectPackage: list(self.activated_groups), only=True ) - def _validate_group_options(self, group_options: dict[str, set[str]]) -> bool: + def _validate_group_options(self, group_options: dict[str, set[str]]) -> None: """ Raises en error if it detects that a group is not part of pyproject.toml """ @@ -121,14 +121,12 @@ def _validate_group_options(self, group_options: dict[str, set[str]]) -> bool: if not self.poetry.package.has_dependency_group(group): invalid_options[group].add(opt) if invalid_options: - invalid_groups = sorted(invalid_options.keys()) - error_msg = "Group(s) not found:" - for group in invalid_groups: + message_parts = [] + for group in sorted(invalid_options): opts = ", ".join( f"--{opt}" for opt in sorted(invalid_options[group]) ) - error_msg += f" {group} (via {opts})," - error_msg = error_msg.rstrip(",") - raise GroupNotFound(error_msg) + message_parts.append(f"{group} (via {opts})") + raise GroupNotFound(f"Group(s) not found: {', '.join(message_parts)}") return len(invalid_options) == 0 From 25ae7c767ed4803c67c228fb10ae6d72b755ef70 Mon Sep 17 00:00:00 2001 From: samypr100 <3933065+samypr100@users.noreply.github.com> Date: Wed, 5 Apr 2023 09:24:15 -0400 Subject: [PATCH 7/7] Post feedback cleanup --- src/poetry/console/commands/group_command.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/poetry/console/commands/group_command.py b/src/poetry/console/commands/group_command.py index 2a2b0a3ae44..88b9a1ce39a 100644 --- a/src/poetry/console/commands/group_command.py +++ b/src/poetry/console/commands/group_command.py @@ -129,4 +129,3 @@ def _validate_group_options(self, group_options: dict[str, set[str]]) -> None: ) message_parts.append(f"{group} (via {opts})") raise GroupNotFound(f"Group(s) not found: {', '.join(message_parts)}") - return len(invalid_options) == 0