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

Author: Tinche

.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"

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 "

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__
     )

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):

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:

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)

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():

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."""
 

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.*.
 

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,22 +637,26 @@ 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.
     """
 
     @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
 

tests/test_v.py

@@ -15,14 +15,15 @@
 
 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
 
 
 @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: