From 0519ffd2a85aab83b21a6fd9e32e50a1650cf2f2 Mon Sep 17 00:00:00 2001 From: antazoey Date: Thu, 13 Jun 2024 09:40:20 -0500 Subject: [PATCH] fix: better error handling for bad dependencies: config (#2132) --- src/ape/api/config.py | 16 ++++++++++++++-- src/ape/managers/project.py | 2 ++ src/ape/pytest/plugin.py | 4 ++-- src/ape/utils/basemodel.py | 9 ++++++++- tests/functional/test_config.py | 18 ++++++++++++++++++ tests/functional/test_project.py | 2 +- 6 files changed, 45 insertions(+), 6 deletions(-) diff --git a/src/ape/api/config.py b/src/ape/api/config.py index e22a383b59..e484f9448f 100644 --- a/src/ape/api/config.py +++ b/src/ape/api/config.py @@ -1,5 +1,5 @@ import os -from collections.abc import Iterator +from collections.abc import Iterable, Iterator from enum import Enum from functools import cached_property from pathlib import Path @@ -197,7 +197,7 @@ class ApeConfig(ExtraAttributesMixin, BaseSettings, ManagerAccessMixin): @classmethod def validate_model(cls, model): model = model or {} - fixed_model = {} + fixed_model: dict = {} for key, val in model.items(): # Allows hyphens to work anywhere where underscores are. fixed_model[key.replace("-", "_")] = val @@ -207,7 +207,19 @@ def validate_model(cls, model): # problems when moving the project around (as happens in local # dependencies). fixed_deps = [] + dependencies_list = fixed_model.get("dependencies", []) + if not isinstance(dependencies_list, Iterable): + raise ConfigError( + "Expecting dependencies: to be iterable. " + f"Received: {type(dependencies_list).__name__}" + ) + for dep in fixed_model.get("dependencies", []): + if not isinstance(dep, dict): + raise ConfigError( + f"Expecting mapping for dependency. Received: {type(dep).__name__}." + ) + fixed_dep = {**dep} if "project" not in fixed_dep: fixed_dep["project"] = project diff --git a/src/ape/managers/project.py b/src/ape/managers/project.py index fd4fda0115..ed86c8b483 100644 --- a/src/ape/managers/project.py +++ b/src/ape/managers/project.py @@ -2058,6 +2058,8 @@ def __getattr__(self, item: str) -> Any: if path.stem != item: continue + if message and message[-1] not in (".", "?", "!"): + message = f"{message}." message = ( f"{message} However, there is a source file named '{path.name}', " "did you mean to reference a contract name from this source file?" diff --git a/src/ape/pytest/plugin.py b/src/ape/pytest/plugin.py index cc95dfab57..92218811a9 100644 --- a/src/ape/pytest/plugin.py +++ b/src/ape/pytest/plugin.py @@ -21,9 +21,9 @@ def add_option(*names, **kwargs): name_str = ", ".join(names) if "already added" in str(err): raise ConfigError( - f"Another pytest plugin besides `ape_test` uses an option with " + "Another pytest plugin besides `ape_test` uses an option with " f"one of '{name_str}'. Note that Ape does not support being " - f"installed alongside Brownie; please use separate environments!" + "installed alongside Brownie; please use separate environments!" ) raise ConfigError(f"Failed adding option {name_str}: {err}") from err diff --git a/src/ape/utils/basemodel.py b/src/ape/utils/basemodel.py index c4a345a5d2..96c5acb7e9 100644 --- a/src/ape/utils/basemodel.py +++ b/src/ape/utils/basemodel.py @@ -442,9 +442,16 @@ def get_attribute_with_extras(obj: Any, name: str) -> Any: if extras_checked: extras_str = ", ".join(sorted(extras_checked)) - message = f"{message}. Also checked extra(s) '{extras_str}'." + suffix = f"Also checked extra(s) '{extras_str}'" + if suffix not in message: + if message and message[-1] not in (".", "?", "!"): + message = f"{message}." + message = f"{message} {suffix}" _recursion_checker.reset(name) + if message and message[-1] not in (".", "?", "!"): + message = f"{message}." + attr_err = ApeAttributeError(message) if base_err: raise attr_err from base_err diff --git a/tests/functional/test_config.py b/tests/functional/test_config.py index 31041c281f..a9effbaf93 100644 --- a/tests/functional/test_config.py +++ b/tests/functional/test_config.py @@ -368,3 +368,21 @@ def test_write_to_disk_txt(config): path = base_path / "config.txt" with pytest.raises(ConfigError, match=f"Unsupported destination file type '{path}'."): config.write_to_disk(path) + + +def test_dependencies_not_list_of_dicts(project): + # NOTE: `project:` is a not a user-facing config, only + # for internal Ape. + data = {"dependencies": 123, "project": str(project.path)} + expected = "Expecting dependencies: to be iterable. Received: int" + with pytest.raises(ConfigError, match=expected): + _ = ApeConfig.model_validate(data) + + +def test_dependencies_list_of_non_dicts(project): + # NOTE: `project:` is a not a user-facing config, only + # for internal Ape. + data = {"dependencies": [123, 123], "project": str(project.path)} + expected = "Expecting mapping for dependency. Received: int." + with pytest.raises(ConfigError, match=expected): + _ = ApeConfig.model_validate(data) diff --git a/tests/functional/test_project.py b/tests/functional/test_project.py index 79ae7d44f8..758f6aa0e6 100644 --- a/tests/functional/test_project.py +++ b/tests/functional/test_project.py @@ -206,7 +206,7 @@ def test_getattr_same_name_as_source_file(project_with_source_files_contract): r"Also checked extra\(s\) 'contracts'\. " r"However, there is a source file named 'ContractA\.sol', " r"did you mean to reference a contract name from this source file\? " - r"Else, could it be from one of the missing compilers for extensions: ." + r"Else, could it be from one of the missing compilers for extensions: .*" ) with pytest.raises(AttributeError, match=expected): _ = project_with_source_files_contract.ContractA