From 01e0fb0fb01c0e34462a3d291f7314b037e0a79d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tin=20Tvrtkovi=C4=87?= Date: Mon, 2 Dec 2024 22:57:32 +0100 Subject: [PATCH] Improve coverage (#606) * Improve coverage * Clean up dead code * Improve `v` test coverage * Improve tagged_union coverage * Improve disambiguators coverage * Fail CI under 100% coverage --- .github/workflows/main.yml | 2 +- src/cattrs/converters.py | 1 - src/cattrs/disambiguators.py | 2 +- src/cattrs/gen/_generics.py | 9 +---- src/cattrs/strategies/_subclasses.py | 2 +- tests/strategies/test_include_subclasses.py | 31 ++++++++++++++--- tests/strategies/test_tagged_unions.py | 5 ++- tests/test_converter.py | 38 ++++++++++++++++++--- tests/test_converter_inheritance.py | 6 ++-- tests/test_gen_dict.py | 13 ++++--- tests/test_v.py | 8 ++++- 11 files changed, 86 insertions(+), 31 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 47d1d2dc..3124f030 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -79,7 +79,7 @@ jobs: echo "total=$TOTAL" >> $GITHUB_ENV # Report again and fail if under the threshold. - python -Im coverage report --fail-under=99 + python -Im coverage report --fail-under=100 - name: "Upload HTML report." uses: "actions/upload-artifact@v4" diff --git a/src/cattrs/converters.py b/src/cattrs/converters.py index 16e74ed8..2559bca1 100644 --- a/src/cattrs/converters.py +++ b/src/cattrs/converters.py @@ -963,7 +963,6 @@ def _get_dis_func( # logic. union_types = tuple(e for e in union_types if e is not NoneType) - # TODO: technically both disambiguators could support TypedDicts too if not all(has(get_origin(e) or e) for e in union_types): raise StructureHandlerNotFoundError( "Only unions of attrs classes and dataclasses supported " diff --git a/src/cattrs/disambiguators.py b/src/cattrs/disambiguators.py index 83e8c3f1..80288024 100644 --- a/src/cattrs/disambiguators.py +++ b/src/cattrs/disambiguators.py @@ -30,7 +30,7 @@ def is_supported_union(typ: Any) -> bool: - """Whether the type is a union of attrs classes.""" + """Whether the type is a union of attrs classes or dataclasses.""" return is_union_type(typ) and all( e is NoneType or has(get_origin(e) or e) for e in typ.__args__ ) diff --git a/src/cattrs/gen/_generics.py b/src/cattrs/gen/_generics.py index 069c48c8..63d2fb91 100644 --- a/src/cattrs/gen/_generics.py +++ b/src/cattrs/gen/_generics.py @@ -36,14 +36,7 @@ def generate_mapping(cl: type, old_mapping: dict[str, type] = {}) -> dict[str, t origin = get_origin(cl) if origin is not None: - # To handle the cases where classes in the typing module are using - # the GenericAlias structure but aren't a Generic and hence - # end up in this function but do not have an `__parameters__` - # attribute. These classes are interface types, for example - # `typing.Hashable`. - parameters = getattr(get_origin(cl), "__parameters__", None) - if parameters is None: - return dict(old_mapping) + parameters = origin.__parameters__ for p, t in zip(parameters, get_args(cl)): if isinstance(t, TypeVar): diff --git a/src/cattrs/strategies/_subclasses.py b/src/cattrs/strategies/_subclasses.py index 06a92afa..47f3e7de 100644 --- a/src/cattrs/strategies/_subclasses.py +++ b/src/cattrs/strategies/_subclasses.py @@ -84,7 +84,7 @@ def include_subclasses( def _include_subclasses_without_union_strategy( cl, converter: BaseConverter, - parent_subclass_tree: tuple[type], + parent_subclass_tree: tuple[type, ...], overrides: dict[str, AttributeOverride] | None, ): # The iteration approach is required if subclasses are more than one level deep: diff --git a/tests/strategies/test_include_subclasses.py b/tests/strategies/test_include_subclasses.py index 7b6b9861..02746305 100644 --- a/tests/strategies/test_include_subclasses.py +++ b/tests/strategies/test_include_subclasses.py @@ -1,13 +1,12 @@ import typing from copy import deepcopy from functools import partial -from typing import List, Tuple import pytest from attrs import define from cattrs import Converter, override -from cattrs.errors import ClassValidationError +from cattrs.errors import ClassValidationError, StructureHandlerNotFoundError from cattrs.strategies import configure_tagged_union, include_subclasses @@ -148,7 +147,7 @@ def conv_w_subclasses(request): "struct_unstruct", IDS_TO_STRUCT_UNSTRUCT.values(), ids=IDS_TO_STRUCT_UNSTRUCT ) def test_structuring_with_inheritance( - conv_w_subclasses: Tuple[Converter, bool], struct_unstruct + conv_w_subclasses: tuple[Converter, bool], struct_unstruct ) -> None: structured, unstructured = struct_unstruct @@ -219,7 +218,7 @@ def test_circular_reference(conv_w_subclasses): "struct_unstruct", IDS_TO_STRUCT_UNSTRUCT.values(), ids=IDS_TO_STRUCT_UNSTRUCT ) def test_unstructuring_with_inheritance( - conv_w_subclasses: Tuple[Converter, bool], struct_unstruct + conv_w_subclasses: tuple[Converter, bool], struct_unstruct ): structured, unstructured = struct_unstruct converter, included_subclasses_param = conv_w_subclasses @@ -389,5 +388,27 @@ class Derived(A): "_type": "Derived", } ], - List[A], + list[A], ) == [Derived(9, Derived(99, A(999)))] + + +def test_unsupported_class(genconverter: Converter): + """Non-attrs/dataclass classes raise proper errors.""" + + class NewParent: + """Not an attrs class.""" + + a: int + + @define + class NewChild(NewParent): + pass + + @define + class NewChild2(NewParent): + pass + + genconverter.register_structure_hook(NewParent, lambda v, _: NewParent(v)) + + with pytest.raises(StructureHandlerNotFoundError): + include_subclasses(NewParent, genconverter) diff --git a/tests/strategies/test_tagged_unions.py b/tests/strategies/test_tagged_unions.py index abd38fef..ba5ae49b 100644 --- a/tests/strategies/test_tagged_unions.py +++ b/tests/strategies/test_tagged_unions.py @@ -161,7 +161,10 @@ class B: configure_tagged_union(Union[A, B], c, default=A) data = c.unstructure(A(), Union[A, B]) - c.structure(data, Union[A, B]) + assert c.structure(data, Union[A, B]) == A() + + data.pop("_type") + assert c.structure(data, Union[A, B]) == A() def test_nested_sequence_union(): diff --git a/tests/test_converter.py b/tests/test_converter.py index 118d407a..dff77f36 100644 --- a/tests/test_converter.py +++ b/tests/test_converter.py @@ -16,7 +16,7 @@ ) import pytest -from attrs import Factory, define, fields, has, make_class +from attrs import Factory, define, field, fields, has, make_class from hypothesis import HealthCheck, assume, given, settings from hypothesis.strategies import booleans, just, lists, one_of, sampled_from @@ -27,6 +27,7 @@ ForbiddenExtraKeysError, StructureHandlerNotFoundError, ) +from cattrs.fns import raise_error from cattrs.gen import make_dict_structure_fn, override from ._compat import is_py310_plus @@ -423,9 +424,9 @@ def test_type_overrides(cl_and_vals): inst = cl(*vals, **kwargs) unstructured = converter.unstructure(inst) - for field, val in zip(fields(cl), vals): - if field.type is int and field.default is not None and field.default == val: - assert field.name not in unstructured + for attr, val in zip(fields(cl), vals): + if attr.type is int and attr.default is not None and attr.default == val: + assert attr.name not in unstructured def test_calling_back(): @@ -744,6 +745,35 @@ class Test: assert isinstance(c.structure({}, Test), Test) +def test_legacy_structure_fallbacks(converter_cls: Type[BaseConverter]): + """Restoring legacy behavior works.""" + + class Test: + """Unsupported by default.""" + + def __init__(self, a): + self.a = a + + c = converter_cls( + structure_fallback_factory=lambda _: raise_error, detailed_validation=False + ) + + # We can get the hook, but... + hook = c.get_structure_hook(Test) + + # it won't work. + with pytest.raises(StructureHandlerNotFoundError): + hook({}, Test) + + # If a field has a converter, we honor that instead. + @define + class Container: + a: Test = field(converter=Test) + + hook = c.get_structure_hook(Container) + hook({"a": 1}, Container) + + def test_fallback_chaining(converter_cls: Type[BaseConverter]): """Converters can be chained using fallback hooks.""" diff --git a/tests/test_converter_inheritance.py b/tests/test_converter_inheritance.py index 6f4739e3..27c68ade 100644 --- a/tests/test_converter_inheritance.py +++ b/tests/test_converter_inheritance.py @@ -1,5 +1,5 @@ import collections -import typing +from typing import Hashable, Iterable, Reversible import pytest from attrs import define @@ -41,9 +41,7 @@ class B(A): assert converter.structure({"i": 1}, B) == B(2) -@pytest.mark.parametrize( - "typing_cls", [typing.Hashable, typing.Iterable, typing.Reversible] -) +@pytest.mark.parametrize("typing_cls", [Hashable, Iterable, Reversible]) def test_inherit_typing(converter: BaseConverter, typing_cls): """Stuff from typing.* resolves to runtime to collections.abc.*. diff --git a/tests/test_gen_dict.py b/tests/test_gen_dict.py index 6bb61a6b..d9ae4666 100644 --- a/tests/test_gen_dict.py +++ b/tests/test_gen_dict.py @@ -558,6 +558,7 @@ def test_init_false_no_structure_hook(converter: BaseConverter): @define class A: a: int = field(converter=int, init=False) + b: int = field(converter=int, init=False, default=5) converter.register_structure_hook( A, @@ -636,8 +637,8 @@ class A: converter.structure({"a": "a"}, A) -@given(prefer=...) -def test_prefer_converters_from_converter(prefer: bool): +@given(prefer=..., dv=...) +def test_prefer_converters_from_converter(prefer: bool, dv: bool): """ `prefer_attrs_converters` is taken from the converter by default. """ @@ -645,13 +646,17 @@ def test_prefer_converters_from_converter(prefer: bool): @define class A: a: int = field(converter=lambda x: x + 1) + b: int = field(converter=lambda x: x + 1, default=5) converter = BaseConverter(prefer_attrib_converters=prefer) converter.register_structure_hook(int, lambda x, _: x + 1) - converter.register_structure_hook(A, make_dict_structure_fn(A, converter)) + converter.register_structure_hook( + A, make_dict_structure_fn(A, converter, _cattrs_detailed_validation=dv) + ) if prefer: - assert converter.structure({"a": 1}, A).a == 2 + assert converter.structure({"a": 1, "b": 2}, A).a == 2 + assert converter.structure({"a": 1, "b": 2}, A).b == 3 else: assert converter.structure({"a": 1}, A).a == 3 diff --git a/tests/test_v.py b/tests/test_v.py index 4aa97164..d2ba3f75 100644 --- a/tests/test_v.py +++ b/tests/test_v.py @@ -15,6 +15,7 @@ from cattrs import Converter, transform_error from cattrs._compat import Mapping, TypedDict +from cattrs.errors import IterableValidationError from cattrs.gen import make_dict_structure_fn from cattrs.v import format_exception @@ -22,7 +23,7 @@ @fixture def c() -> Converter: """We need only converters with detailed_validation=True.""" - return Converter() + return Converter(detailed_validation=True) def test_attribute_errors(c: Converter) -> None: @@ -190,6 +191,11 @@ class C: "invalid value for type, expected int @ $.b[1][2]", ] + # IterableValidationErrors with subexceptions without notes + exc = IterableValidationError("Test", [TypeError("Test")], list[str]) + + assert transform_error(exc) == ["invalid type (Test) @ $"] + def test_mapping_errors(c: Converter) -> None: try: