diff --git a/pcapkit/protocols/internet/mh.py b/pcapkit/protocols/internet/mh.py index 1f2f8aa1d..729400e1f 100644 --- a/pcapkit/protocols/internet/mh.py +++ b/pcapkit/protocols/internet/mh.py @@ -488,8 +488,7 @@ UpdateNotificationMessage as Schema_UpdateNotificationMessage from pcapkit.protocols.schema.internet.mh import VendorSpecificOption as Schema_VendorSpecificOption from pcapkit.protocols.schema.schema import Schema -from pcapkit.utilities.exceptions import (EnumKeyError, EnumValueError, ProtocolError, - UnsupportedCall) +from pcapkit.utilities.exceptions import EnumValueError, ProtocolError, UnsupportedCall from pcapkit.utilities.warnings import ProtocolWarning, RegistryWarning, warn if TYPE_CHECKING: @@ -576,21 +575,47 @@ class FastBindingAcknowledgmentStatus(EnumLookup, IntEnum): Note: Re-parented onto :class:`~pcapkit.corekit.enum.EnumLookup` per GitHub - issue #930, finishing #877's phase 2. :meth:`get` and - :meth:`_missing_` below are untouched -- #923 already converted - :meth:`get`'s own name-miss to the in-library - :exc:`~pcapkit.utilities.exceptions.EnumKeyError`, which is exactly - the shape the base's own ``get`` uses, so there is nothing to - reconcile. Unlike :meth:`~pcapkit.const.reg.apptype.apptype. - TransportProtocol.get` and :meth:`~pcapkit.protocols.application. - ngap.Criticality.get` in GitHub issue #921, :meth:`get` keeps its - ``@staticmethod`` decorator rather than becoming a delegating - ``classmethod`` -- it never calls ``super().get(...)``, so the - :exc:`RuntimeError` trap a ``staticmethod`` delegating to a - ``classmethod`` base would hit does not apply here, and the - resulting ``mypy`` ``[override]`` complaint about the signature - mismatch (no ``cls``, no ``default``) is silenced rather than - resolved by widening the signature. + issue #930, finishing #877's phase 2. :meth:`_missing_` below is + untouched, since :class:`EnumLookup` does not touch that hook. + + This class carried its own hand-rolled ``get()`` override through + #930, and briefly again through GitHub issue #935's first attempt, + which widened the override to accept ``default`` rather than delete + it outright. The owner's final ruling on #935 went the other way, + verbatim -- asked *"why must we have the two overrides tho? cant + they directly fall back to the base class's?"*, the answer was *"I + prefer (2) directly"*, ``(2)`` naming deletion among the ruling's + own options. Measured before acting on it: the override's own + docstring called it a "Backport support for original codes", but + this class mints no alias -- ``__members__`` and ``list(cls)`` + agree at 6 -- so what the override actually did was resolve an + :class:`int` by direct construction and a name by subscript, + exactly the dual resolution + :meth:`~pcapkit.corekit.enum.EnumLookup.get` already provides for + every other :class:`int`-valued registry in this tree. There was + nothing left to backport. ``get``/``get_all`` now come from the + base alone, the same as the five other re-parents #930 finished + alongside this one -- including :class:`LocalizedRoutingStatus` + and :class:`LMAAddressCode` below, whose own hand-rolled ``get()`` + GitHub issue #880 had already deleted outright, for the same + reason: zero callers depended on anything the base does not + already do. + + A behaviour change comes with the deletion, deliberately: the + override branched on ``isinstance(key, int)`` and routed every + other type -- ``None``, a :class:`float`, ... -- through the + *name* path, so ``get(None)`` and ``get(1.5)`` used to answer with + a quiet :exc:`~pcapkit.utilities.exceptions.EnumKeyError` here + while the base -- branching on ``isinstance(key, str)`` instead -- + answers every other :class:`~pcapkit.corekit.enum.EnumLookup` + subclass, and now this one too, with a loud + :exc:`~pcapkit.utilities.exceptions.EnumValueError`. Nothing in + this tree calls ``get`` with such a key: the 20 call sites this + class and :class:`IPv6AddressPrefixCode` had between them, all in + tests, all passed an :class:`int` or a :class:`str`, so the + divergence was live but unreached -- and its removal is what makes + "all seven behave alike" literally true, rather than true only for + the keys a caller happens to pass today. :rfc:`5568#section-6.2.3` defines these values inline and IANA keeps no registry of them, so the enumeration lives here rather than in @@ -641,61 +666,6 @@ class FastBindingAcknowledgmentStatus(EnumLookup, IntEnum): #: Incorrect interface identifier length [:rfc:`5568#section-6.2.3`] Incorrect_interface_identifier_length = 131 - @staticmethod - def get( # type: ignore[override] # pylint: disable=arguments-differ - key: 'int | str') -> 'FastBindingAcknowledgmentStatus': - """Backport support for original codes. - - Raises quietly on a name miss, matching the base's own - :meth:`~pcapkit.corekit.enum.EnumLookup.get` - (:mod:`pcapkit.corekit.enum`, the ``EnumKeyError`` raised at its - ``str`` branch) rather than diverging from it. This override used - to raise loud instead -- logging once at :data:`logging.CRITICAL` - and setting :data:`sys.tracebacklimit` to ``0`` process-wide -- - until GitHub issue #930 converged it onto house convention, - settled on GitHub issue #933's follow-up ruling, verbatim: *"Oh - wait. I meant, they should follow house convention and not to be - loud."* Re-parenting onto - :class:`~pcapkit.corekit.enum.EnumLookup` is what makes the - convergence reach further than this one method: :meth:`get_all - ` did not exist on this - class before #930 and is now inherited from the base, which calls - this ``get`` internally -- so a name miss reached through - ``get_all`` is quiet too, for the same reason. - - Args: - key: Key to get enum item. - - Returns: - The matching member. - - Raises: - EnumKeyError: If ``key`` names no member. A :exc:`KeyError`, per the - owner's ruling on GitHub issue #923 -- *"Either ``ValueError`` - or ``KeyError``, that's depending on how stdlib's ``Enum`` would - raise on these circumstances"* -- since a stdlib - ``E['nosuch']`` raises :exc:`KeyError` and only the *value* - miss :meth:`_missing_` reports is :exc:`ValueError`-shaped. This - used to raise :exc:`~pcapkit.utilities.exceptions.EnumValueError` - so that the two ways of getting it wrong reported identically; - that is exactly the conversion #923 rejects, and 119 of this - tree's 127 concrete - :class:`~pcapkit.corekit.enum.EnumLookup` subclasses already - answered a name miss with a :exc:`KeyError`. There is no - ``default`` parameter: this enumeration cannot grow, so a - default that could only ever be ignored would be worse than one - that is absent. - - """ - if isinstance(key, int): - return FastBindingAcknowledgmentStatus(key) - try: - return FastBindingAcknowledgmentStatus[key] # type: ignore[misc] - except KeyError: - raise EnumKeyError('%r is not a valid %s' % - (key, FastBindingAcknowledgmentStatus.__name__), - quiet=True) from None - @classmethod def _missing_(cls, value: 'int') -> 'NoReturn': """Lookup function used when value is not found. @@ -722,21 +692,48 @@ class IPv6AddressPrefixCode(EnumLookup, IntEnum): Note: Re-parented onto :class:`~pcapkit.corekit.enum.EnumLookup` per GitHub - issue #930, finishing #877's phase 2. :meth:`get` and - :meth:`_missing_` below are untouched -- #923 already converted - :meth:`get`'s own name-miss to the in-library - :exc:`~pcapkit.utilities.exceptions.EnumKeyError`, which is exactly - the shape the base's own ``get`` uses, so there is nothing to - reconcile. Unlike :meth:`~pcapkit.const.reg.apptype.apptype. - TransportProtocol.get` and :meth:`~pcapkit.protocols.application. - ngap.Criticality.get` in GitHub issue #921, :meth:`get` keeps its - ``@staticmethod`` decorator rather than becoming a delegating - ``classmethod`` -- it never calls ``super().get(...)``, so the - :exc:`RuntimeError` trap a ``staticmethod`` delegating to a - ``classmethod`` base would hit does not apply here, and the - resulting ``mypy`` ``[override]`` complaint about the signature - mismatch (no ``cls``, no ``default``) is silenced rather than - resolved by widening the signature. + issue #930, finishing #877's phase 2. :meth:`_missing_` below is + untouched, since :class:`EnumLookup` does not touch that hook. + + This class carried its own hand-rolled ``get()`` override through + #930, and briefly again through GitHub issue #935's first attempt, + which widened the override to accept ``default`` rather than delete + it outright. The owner's final ruling on #935 went the other way, + verbatim -- asked *"why must we have the two overrides tho? cant + they directly fall back to the base class's?"*, the answer was *"I + prefer (2) directly"*, ``(2)`` naming deletion among the ruling's + own options. Measured before acting on it: the override's own + docstring called it a "Backport support for original codes", but + this class mints no alias -- ``__members__`` and ``list(cls)`` + agree at 4 -- so what the override actually did was resolve an + :class:`int` by direct construction and a name by subscript, + exactly the dual resolution + :meth:`~pcapkit.corekit.enum.EnumLookup.get` already provides for + every other :class:`int`-valued registry in this tree. There was + nothing left to backport. ``get``/``get_all`` now come from the + base alone, the same as the five other re-parents #930 finished + alongside this one -- including + :class:`~pcapkit.protocols.internet.mh.LocalizedRoutingStatus` and + :class:`~pcapkit.protocols.internet.mh.LMAAddressCode` below, whose + own hand-rolled ``get()`` GitHub issue #880 had already deleted + outright, for the same reason: zero callers depended on anything + the base does not already do. + + A behaviour change comes with the deletion, deliberately: the + override branched on ``isinstance(key, int)`` and routed every + other type -- ``None``, a :class:`float`, ... -- through the + *name* path, so ``get(None)`` and ``get(1.5)`` used to answer with + a quiet :exc:`~pcapkit.utilities.exceptions.EnumKeyError` here + while the base -- branching on ``isinstance(key, str)`` instead -- + answers every other :class:`~pcapkit.corekit.enum.EnumLookup` + subclass, and now this one too, with a loud + :exc:`~pcapkit.utilities.exceptions.EnumValueError`. Nothing in + this tree calls ``get`` with such a key: the 20 call sites this + class and :class:`FastBindingAcknowledgmentStatus` had between + them, all in tests, all passed an :class:`int` or a :class:`str`, + so the divergence was live but unreached -- and its removal is + what makes "all seven behave alike" literally true, rather than + true only for the keys a caller happens to pass today. :rfc:`5568#section-6.4.2` defines these values inline and IANA keeps no registry of them, so the enumeration lives here rather than in @@ -778,61 +775,6 @@ class IPv6AddressPrefixCode(EnumLookup, IntEnum): #: of valid leading bits in the prefix [:rfc:`5568#section-6.4.2`] NAR_Prefix = 4 - @staticmethod - def get( # type: ignore[override] # pylint: disable=arguments-differ - key: 'int | str') -> 'IPv6AddressPrefixCode': - """Backport support for original codes. - - Raises quietly on a name miss, matching the base's own - :meth:`~pcapkit.corekit.enum.EnumLookup.get` - (:mod:`pcapkit.corekit.enum`, the ``EnumKeyError`` raised at its - ``str`` branch) rather than diverging from it. This override used - to raise loud instead -- logging once at :data:`logging.CRITICAL` - and setting :data:`sys.tracebacklimit` to ``0`` process-wide -- - until GitHub issue #930 converged it onto house convention, - settled on GitHub issue #933's follow-up ruling, verbatim: *"Oh - wait. I meant, they should follow house convention and not to be - loud."* Re-parenting onto - :class:`~pcapkit.corekit.enum.EnumLookup` is what makes the - convergence reach further than this one method: :meth:`get_all - ` did not exist on this - class before #930 and is now inherited from the base, which calls - this ``get`` internally -- so a name miss reached through - ``get_all`` is quiet too, for the same reason. - - Args: - key: Key to get enum item. - - Returns: - The matching member. - - Raises: - EnumKeyError: If ``key`` names no member. A :exc:`KeyError`, per the - owner's ruling on GitHub issue #923 -- *"Either ``ValueError`` - or ``KeyError``, that's depending on how stdlib's ``Enum`` would - raise on these circumstances"* -- since a stdlib - ``E['nosuch']`` raises :exc:`KeyError` and only the *value* - miss :meth:`_missing_` reports is :exc:`ValueError`-shaped. This - used to raise :exc:`~pcapkit.utilities.exceptions.EnumValueError` - so that the two ways of getting it wrong reported identically; - that is exactly the conversion #923 rejects, and 119 of this - tree's 127 concrete - :class:`~pcapkit.corekit.enum.EnumLookup` subclasses already - answered a name miss with a :exc:`KeyError`. There is no - ``default`` parameter: this enumeration cannot grow, so a - default that could only ever be ignored would be worse than one - that is absent. - - """ - if isinstance(key, int): - return IPv6AddressPrefixCode(key) - try: - return IPv6AddressPrefixCode[key] # type: ignore[misc] - except KeyError: - raise EnumKeyError('%r is not a valid %s' % - (key, IPv6AddressPrefixCode.__name__), - quiet=True) from None - @classmethod def _missing_(cls, value: 'int') -> 'NoReturn': """Lookup function used when value is not found. @@ -891,15 +833,19 @@ class LocalizedRoutingStatus(EnumLookup, IntEnum): naming merely an unassigned byte is simply a more likely way to reach it. See GitHub issue #880. - There is no hand-rolled ``get()`` backport here, unlike - :class:`FastBindingAcknowledgmentStatus` and - :class:`IPv6AddressPrefixCode`: it had zero callers repo-wide -- tests - included -- so GitHub issue #880 deleted it outright rather than - rebuilding it on the immutable contract. GitHub issue #930's - re-parenting above gives this class ``get``/``get_all`` again, but as - the base's own bare lookup rather than a bespoke override -- it still - cannot mint, so an unassigned value raises through ``get`` exactly as - it does through the bare constructor. + There is no hand-rolled ``get()`` backport here -- nor, since GitHub + issue #935, on :class:`FastBindingAcknowledgmentStatus` or + :class:`IPv6AddressPrefixCode` either: it had zero callers repo-wide + -- tests included -- so GitHub issue #880 deleted it outright rather + than rebuilding it on the immutable contract, the same conclusion + #935 reached separately for the other two, on the owner's ruling + there, verbatim: *"I prefer (2) directly"* -- ``(2)`` being deletion + of those two overrides rather than widening them to match the base. + GitHub issue #930's re-parenting above gives this class + ``get``/``get_all`` again, but as the base's own bare lookup rather + than a bespoke override -- it still cannot mint, so an unassigned + value raises through ``get`` exactly as it does through the bare + constructor. """ @@ -966,15 +912,19 @@ class LMAAddressCode(EnumLookup, IntEnum): naming merely an unassigned byte is simply a more likely way to reach it. See GitHub issue #880. - There is no hand-rolled ``get()`` backport here, unlike - :class:`FastBindingAcknowledgmentStatus` and - :class:`IPv6AddressPrefixCode`: it had zero callers repo-wide -- tests - included -- so GitHub issue #880 deleted it outright rather than - rebuilding it on the immutable contract. GitHub issue #930's - re-parenting above gives this class ``get``/``get_all`` again, but as - the base's own bare lookup rather than a bespoke override -- it still - cannot mint, so an unassigned value raises through ``get`` exactly as - it does through the bare constructor. + There is no hand-rolled ``get()`` backport here -- nor, since GitHub + issue #935, on :class:`FastBindingAcknowledgmentStatus` or + :class:`IPv6AddressPrefixCode` either: it had zero callers repo-wide + -- tests included -- so GitHub issue #880 deleted it outright rather + than rebuilding it on the immutable contract, the same conclusion + #935 reached separately for the other two, on the owner's ruling + there, verbatim: *"I prefer (2) directly"* -- ``(2)`` being deletion + of those two overrides rather than widening them to match the base. + GitHub issue #930's re-parenting above gives this class + ``get``/``get_all`` again, but as the base's own bare lookup rather + than a bespoke override -- it still cannot mint, so an unassigned + value raises through ``get`` exactly as it does through the bare + constructor. """ diff --git a/tests/corekit/test_enum_lookup_reparent_930_unit.py b/tests/corekit/test_enum_lookup_reparent_930_unit.py index 30bec0572..34973fcb2 100644 --- a/tests/corekit/test_enum_lookup_reparent_930_unit.py +++ b/tests/corekit/test_enum_lookup_reparent_930_unit.py @@ -13,40 +13,65 @@ non-registry enumerations, with zero still outside the hierarchy. Two of the seven, :class:`FastBindingAcknowledgmentStatus` and -:class:`IPv6AddressPrefixCode`, already defined their own ``get`` -- both as a -:class:`staticmethod`, the exact shape GitHub issue #908 found dangerous once a base +:class:`IPv6AddressPrefixCode`, defined their own ``get`` at #930's own revision -- both as +a :class:`staticmethod`, the exact shape GitHub issue #908 found dangerous once a base ``get`` becomes a :class:`classmethod`: calling ``super().get(...)`` from a ``staticmethod`` raises :exc:`RuntimeError: super(): no arguments`. Neither override -calls ``super()`` at all, so the trap does not fire, and per this issue's own brief they -are left **untouched** -- their ``except KeyError: raise EnumKeyError(...)`` conversions -were put there by GitHub issue #923 and already answer a name miss in exactly the shape -the base now uses. Keeping the plain ``@staticmethod`` does cost something, though: -``mypy``'s ``[override]`` check and ``pylint``'s ``arguments-differ`` both flag the -resulting shape mismatch against the base's ``classmethod`` (``cls, key, default``) -signature, and both are silenced rather than resolved by widening the signature -- -:class:`ReparentedBasesTests` pins, alongside each class's own base-tuple change, -that the decorator itself survived the re-parenting, which is what makes those -suppressions still apply to the right thing. - -Re-parenting also makes both kept overrides reachable through a door that -did not exist before: :meth:`~pcapkit.corekit.enum.EnumLookup.get_all`, -inherited from the base for the first time, calls ``get`` internally. Before -this issue, that mattered because ``get`` itself raised **loud**: both -overrides used to log once at :data:`logging.CRITICAL` and set -:data:`sys.tracebacklimit` to ``0`` process-wide on a name miss, unlike the -base's own quiet raise. GitHub issue #930 converges both onto the base's -quiet shape instead -- a real behaviour change, not merely a re-parent -- -settled on GitHub issue #933's follow-up ruling, verbatim: *"Oh wait. I -meant, they should follow house convention and not to be loud."* -:class:`KeptOverrideQuietnessTests` pins both halves of that: the ``get`` -half, which genuinely changes (loud on the tree before this issue, quiet -here), and the ``get_all`` half, which is new outright (the attribute does -not exist on that tree at all). +called ``super()`` at all, so the trap never fired, and per this issue's own brief they +were left **untouched** there -- their ``except KeyError: raise EnumKeyError(...)`` +conversions were put there by GitHub issue #923 and already answered a name miss in +exactly the shape the base now uses. Keeping the plain ``@staticmethod`` cost something at +that revision, though: re-parenting made both **advertise** the base's two-argument +``get(key, default)`` through inheritance while still only accepting one, so a ``default`` +argument raised :exc:`TypeError` instead of resolving through the base's own fallback, and +``mypy``'s ``[override]`` check plus ``pylint``'s ``arguments-differ`` both flagged the +resulting shape mismatch, silenced with a suppression. GitHub issue #935 first answered +that on the owner's ruling, verbatim: *"I lean on 1"* -- widen both signatures to accept +``default`` and delete the suppression. Asked, on the same issue, *"why must we have the +two overrides tho? cant they directly fall back to the base class's?"*, the owner's final +ruling went further, verbatim: *"I prefer (2) directly"* -- deleting both overrides +outright rather than widening them. + +Measured before acting on that final ruling: neither override ever minted an alias -- +``__members__`` and ``list(cls)`` agree at 6 and 4 -- so what each docstring called +"Backport support for original codes" was the int-or-name dual resolution +:meth:`~pcapkit.corekit.enum.EnumLookup.get` already provides for every other +:class:`int`-valued registry in this tree, and none of the 20 call sites either override +had (all in tests, none in :mod:`pcapkit`) passed a key the base would have resolved +differently. There was nothing left to backport, so ``get``/``get_all`` on both now come +from the base alone, the same as the five classes below that were pure re-parents from the +start. :class:`ReparentedBasesTests` used to pin, alongside each class's own base-tuple +change, that the ``@staticmethod`` decorator survived re-parenting and then the signature +widening; now that the method is deleted rather than converted, there is nothing left to +decorate, and :class:`AllSevenInheritTheBareClassmethodTests` covers these two the same way +it always covered the other five. + +Deleting the overrides is a real behaviour change, deliberately so: each branched on +``isinstance(key, int)`` and routed every other type -- ``None``, a :class:`float`, ... -- +through its own *name* path, so ``get(None)`` and ``get(1.5)`` used to answer with a quiet +:exc:`~pcapkit.utilities.exceptions.EnumKeyError` on these two while the base -- branching +on ``isinstance(key, str)`` instead -- answers every other +:class:`~pcapkit.corekit.enum.EnumLookup` subclass with a loud +:exc:`~pcapkit.utilities.exceptions.EnumValueError`. +:class:`NonCanonicalKeyConvergenceTests` pins the convergence this deletion produces: all +seven now answer such a key alike, for the first time. + +Re-parenting separately made both former overrides reachable through a door that did not +exist before: :meth:`~pcapkit.corekit.enum.EnumLookup.get_all`, inherited from the base for +the first time, calls ``get`` internally. Before this issue, that mattered because ``get`` +itself raised **loud**: both overrides used to log once at :data:`logging.CRITICAL` and set +:data:`sys.tracebacklimit` to ``0`` process-wide on a name miss, unlike the base's own quiet +raise. GitHub issue #930 converged both onto the base's quiet shape instead -- a real +behaviour change, not merely a re-parent -- settled on GitHub issue #933's follow-up ruling, +verbatim: *"Oh wait. I meant, they should follow house convention and not to be loud."* +:class:`InheritedQuietnessTests` (renamed from ``KeptOverrideQuietnessTests`` once GitHub +issue #935 deleted the overrides that name described) pins that the quiet shape survived +the deletion too -- purely inherited now, rather than reconciled by hand on each class. The other five -- :class:`CommandType`, :class:`ConformanceRequirement`, -:class:`ESPStatus`, :class:`LocalizedRoutingStatus` and :class:`LMAAddressCode` -- are -pure re-parents: none defines a ``get`` of its own to reconcile with the base, so each -gains ``get``/``get_all`` for the first time. :class:`CommandType`, +:class:`ESPStatus`, :class:`LocalizedRoutingStatus` and :class:`LMAAddressCode` -- were +pure re-parents from the start: none defines a ``get`` of its own to reconcile with the +base, so each gained ``get``/``get_all`` for the first time in #930. :class:`CommandType`, :class:`LocalizedRoutingStatus` and :class:`LMAAddressCode` do carry their own ``_missing_`` range guards, which are untouched -- :class:`EnumLookup` does not override that hook, so a re-parent cannot change what it does. @@ -97,14 +122,15 @@ base-tuple/MRO change is real for all seven regardless of whether ``get`` itself was already working. -Neither method of :class:`KeptOverrideQuietnessTests` holds, and deliberately -so -- both are pinning the one thing this issue actually changes about the two -kept overrides. ``test_get_is_now_quiet_on_both_classes`` fails against the -reverted tree because ``get`` really was loud there (see that class's own -docstring): this is not a scaffolding failure, it is the behaviour change -itself, caught in the act. ``test_get_all_is_new_and_quiet_too`` fails with -``AttributeError`` instead, since ``get_all`` does not exist on the reverted -tree at all. +Neither of the first two methods of :class:`InheritedQuietnessTests` (called +``KeptOverrideQuietnessTests`` at the time this measurement was taken, before GitHub issue +#935 deleted the overrides that name described) holds against the reverted tree, and +deliberately so -- both pin the one thing this issue actually changes about the two +classes' ``get``. ``test_get_is_now_quiet_on_both_classes`` fails against the reverted tree +because ``get`` really was loud there (see that class's own docstring): this is not a +scaffolding failure, it is the behaviour change itself, caught in the act. +``test_get_all_is_new_and_quiet_too`` fails with ``AttributeError`` instead, since +``get_all`` does not exist on the reverted tree at all. """ from __future__ import annotations @@ -122,8 +148,8 @@ __all__ = [ 'ReparentedBasesTests', 'GetByNameAndValueTests', 'FailedLookupTests', - 'KeptOverrideQuietnessTests', 'NoMintingTests', 'PureReparentClassmethodTests', - 'ZeroRemainOutsideEnumLookupTests', + 'InheritedQuietnessTests', 'NoMintingTests', 'AllSevenInheritTheBareClassmethodTests', + 'NonCanonicalKeyConvergenceTests', 'ZeroRemainOutsideEnumLookupTests', ] @@ -172,10 +198,11 @@ def test_esp_status(self) -> None: self.assertEqual(len(list(ESPStatus)), 6) def test_fast_binding_acknowledgment_status(self) -> None: - """Also pins that its kept ``get`` override is still a - :class:`staticmethod` -- unlike ``TransportProtocol.get`` and - ``Criticality.get`` in GitHub issue #921, it never calls - ``super().get(...)``, so there is no delegation to convert it for.""" + """GitHub issue #935 later deleted its kept ``get`` override outright + (the owner's ruling, verbatim: *"I prefer (2) directly"*), so the + decorator this once pinned no longer exists to pin -- + :class:`AllSevenInheritTheBareClassmethodTests` now covers this class + alongside the other six.""" from aenum import IntEnum from pcapkit.protocols.internet.mh import FastBindingAcknowledgmentStatus @@ -184,13 +211,9 @@ def test_fast_binding_acknowledgment_status(self) -> None: self.assertIn(EnumLookup, FastBindingAcknowledgmentStatus.__mro__) self.assertEqual(len(FastBindingAcknowledgmentStatus.__members__), 6) self.assertEqual(len(list(FastBindingAcknowledgmentStatus)), 6) - self.assertIsInstance( - inspect.getattr_static(FastBindingAcknowledgmentStatus, 'get'), staticmethod) def test_ipv6_address_prefix_code(self) -> None: - """Also pins that its kept ``get`` override is still a - :class:`staticmethod`, for the same reason as - :class:`FastBindingAcknowledgmentStatus`.""" + """Same history as :class:`FastBindingAcknowledgmentStatus`.""" from aenum import IntEnum from pcapkit.protocols.internet.mh import IPv6AddressPrefixCode @@ -199,8 +222,6 @@ def test_ipv6_address_prefix_code(self) -> None: self.assertIn(EnumLookup, IPv6AddressPrefixCode.__mro__) self.assertEqual(len(IPv6AddressPrefixCode.__members__), 4) self.assertEqual(len(list(IPv6AddressPrefixCode)), 4) - self.assertIsInstance( - inspect.getattr_static(IPv6AddressPrefixCode, 'get'), staticmethod) def test_localized_routing_status(self) -> None: from aenum import IntEnum @@ -268,8 +289,11 @@ def test_a_battery_of_lookups_does_not_grow_any_of_the_seven(self) -> None: class GetByNameAndValueTests(unittest.TestCase): - """``get`` resolves by name and by value on each of the seven, whether - the ``get`` reached is the base's own or one of the two kept overrides.""" + """``get`` resolves by name and by value on each of the seven, now + uniformly through the base's own inherited implementation -- GitHub + issue #935 deleted the two hand-rolled overrides that used to answer + this for :class:`FastBindingAcknowledgmentStatus` and + :class:`IPv6AddressPrefixCode`.""" def test_command_type(self) -> None: from pcapkit.const.ftp.command import CommandType @@ -324,8 +348,10 @@ class FailedLookupTests(unittest.TestCase): """What a miss raises on each of the seven -- a name miss is :exc:`KeyError`-derived and a value miss :exc:`ValueError`-derived, matching stdlib :class:`~enum.Enum`'s own shape (GitHub issue #923) on - every one of the seven regardless of whether its ``get`` is the base's - own or a kept :class:`staticmethod` override. + every one of the seven, now uniformly through the base's own ``get`` + since GitHub issue #935 deleted the two hand-rolled overrides that used + to answer this for :class:`FastBindingAcknowledgmentStatus` and + :class:`IPv6AddressPrefixCode`. """ def test_command_type(self) -> None: @@ -362,10 +388,9 @@ def test_esp_status(self) -> None: self.assertIsInstance(value_miss.exception, EnumValueError) def test_fast_binding_acknowledgment_status(self) -> None: - """Its own kept ``get`` raises :exc:`EnumKeyError` directly for a - name miss (GitHub issue #923's conversion, left untouched by this - change) and delegates to :meth:`_missing_` for a value miss, which - always raises :exc:`EnumValueError`.""" + """Since GitHub issue #935 deleted its kept override, this now + resolves through the base's own ``get`` -- the same mechanism + :meth:`test_localized_routing_status` below exercises.""" from pcapkit.protocols.internet.mh import FastBindingAcknowledgmentStatus with self.assertRaises(KeyError) as name_miss: @@ -414,30 +439,43 @@ def test_lma_address_code(self) -> None: self.assertIsInstance(value_miss.exception, EnumValueError) -class KeptOverrideQuietnessTests(unittest.TestCase): - """The now-quiet raise of the two kept ``get`` overrides, and the new - way this re-parenting opens to reach it. - - :meth:`FastBindingAcknowledgmentStatus.get` and - :meth:`IPv6AddressPrefixCode.get` used to raise **loud** on a name - miss -- logging once at :data:`logging.CRITICAL` and setting - :data:`sys.tracebacklimit` to ``0`` process-wide, unlike the base's own - quiet raise at :meth:`~pcapkit.corekit.enum.EnumLookup.get` - (:mod:`pcapkit.corekit.enum`). GitHub issue #930 converges both onto - that quiet shape instead, settled on GitHub issue #933's follow-up - ruling, verbatim: *"Oh wait. I meant, they should follow house - convention and not to be loud."* (An earlier message on the same issue - said the opposite -- plain *"No."* -- and an earlier revision of this - file briefly pinned loud as the settled answer on the strength of that +class InheritedQuietnessTests(unittest.TestCase): + """The quiet raise :class:`FastBindingAcknowledgmentStatus` and + :class:`IPv6AddressPrefixCode` answer a name miss with, and the door + this re-parenting opened to reach it -- both now purely inherited from + the base rather than reconciled by a hand-rolled override. + + Named for what survives rather than for what used to sit here: this + class was ``KeptOverrideQuietnessTests`` while both classes still + carried their own ``get``, first through #930's re-parenting and + briefly again through GitHub issue #935's first attempt, which widened + that override to accept ``default`` rather than delete it. The owner's + final ruling on #935 deleted both outright instead, verbatim: *"I + prefer (2) directly"*. What this class pins did not change with that + deletion -- the quiet raise -- only *how* it is produced: through + :meth:`~pcapkit.corekit.enum.EnumLookup.get` + (:mod:`pcapkit.corekit.enum`) directly now, rather than through an + override that reconciled itself onto the base's shape. + + Before either override existed, ``get`` on both classes raised + **loud** -- logging once at :data:`logging.CRITICAL` and setting + :data:`sys.tracebacklimit` to ``0`` process-wide on a name miss, unlike + the base's own quiet raise. GitHub issue #930 converged both onto that + quiet shape instead, settled on GitHub issue #933's follow-up ruling, + verbatim: *"Oh wait. I meant, they should follow house convention and + not to be loud."* (An earlier message on the same issue said the + opposite -- plain *"No."* -- and an earlier revision of this file + briefly pinned loud as the settled answer on the strength of that message; the follow-up four minutes later superseded it, and what follows is the corrected version.) - Both halves are pinned quiet, and for two different reasons against the - reverted tree. :meth:`test_get_is_now_quiet_on_both_classes` covers - ``get`` itself, which existed and was already loud on the tree before - this issue -- so this test fails against that reverted tree not because - ``get`` is structurally different there, but because its behaviour - genuinely changes: reverted, it is loud; here, it is quiet. + The first two methods are pinned quiet, and for two different reasons + against the tree reverted to before #930. + :meth:`test_get_is_now_quiet_on_both_classes` covers ``get`` itself, + which existed and was already loud on the tree before that issue -- so + this test fails against that reverted tree not because ``get`` is + structurally different there, but because its behaviour genuinely + changes: reverted, it is loud; here, it is quiet. :meth:`test_get_all_is_new_and_quiet_too` covers :meth:`~pcapkit.corekit.enum.EnumLookup.get_all`, which did not exist on either class before #930 at all and is now inherited from the base, @@ -498,33 +536,123 @@ def test_get_all_is_new_and_quiet_too(self) -> None: self.assertFalse(hasattr(sys, 'tracebacklimit')) self.assertEqual(recorder.messages, []) + def test_get_with_unusable_default_is_quiet_too(self) -> None: + """The base's own ``get`` (:mod:`pcapkit.corekit.enum`) has always + accepted ``default``; a name miss whose ``default`` does not itself + resolve falls through to the same quiet + :exc:`~pcapkit.utilities.exceptions.EnumKeyError` a bare miss would + have raised -- not a new, louder path. Before GitHub issue #935 + deleted the two hand-rolled overrides, this same call raised + ``TypeError`` on a second positional argument instead; between + #935's first attempt and its final ruling, the overrides answered + it themselves rather than through this inherited path. + """ + from pcapkit.protocols.internet.mh import (FastBindingAcknowledgmentStatus, + IPv6AddressPrefixCode) + + for cls in (FastBindingAcknowledgmentStatus, IPv6AddressPrefixCode): + with self.subTest(cls=cls.__name__): + if hasattr(sys, 'tracebacklimit'): + del sys.tracebacklimit + with capture(logger) as recorder: + with self.assertRaises(EnumKeyError): + cls.get('NOT_A_REAL_MEMBER', 'ALSO_NOT_A_REAL_MEMBER') + self.assertFalse(hasattr(sys, 'tracebacklimit')) + self.assertEqual(recorder.messages, []) + + # A default that *does* resolve returns it without raising + # at all. + member = next(iter(cls)) + self.assertIs(cls.get('NOT_A_REAL_MEMBER', member), member) + -class PureReparentClassmethodTests(unittest.TestCase): - """The five pure re-parents inherit the base's ``classmethod`` outright, - having defined no ``get`` of their own to begin with -- unlike +class AllSevenInheritTheBareClassmethodTests(unittest.TestCase): + """All seven now inherit the base's ``classmethod`` outright, none + declaring a ``get`` of its own. + + Five were always this way -- pure re-parents, having defined no + ``get`` of their own to begin with. The other two, :class:`FastBindingAcknowledgmentStatus` and - :class:`IPv6AddressPrefixCode`, whose kept ``get`` overrides stay - :class:`staticmethod` and are pinned alongside their base-tuple change in - :class:`ReparentedBasesTests` instead, since a test that only checked the - decorator would pass identically before and after this change and pin - nothing about it. + :class:`IPv6AddressPrefixCode`, joined them only at GitHub issue #935's + final revision: their own kept ``get`` stayed a :class:`staticmethod` + through #930's re-parenting and briefly again through #935's first + attempt (which widened it to accept ``default`` rather than delete it), + and only the owner's final ruling there -- verbatim, *"I prefer (2) + directly"* -- deleted it outright, collapsing the seven-way split this + class used to test as five-plus-two into one uniform case. This class + was named for the five alone before that ruling, and + :class:`ReparentedBasesTests` pinned the other two's surviving + ``@staticmethod`` separately, since a test that only checked the + decorator would have passed identically whether ``get`` were kept or + converted, and pinned nothing about which. """ - def test_the_five_pure_reparents_inherit_the_bare_classmethod(self) -> None: + def test_all_seven_inherit_the_bare_classmethod(self) -> None: from pcapkit.const.ftp.command import CommandType, ConformanceRequirement from pcapkit.protocols.internet.esp import ESPStatus - from pcapkit.protocols.internet.mh import LMAAddressCode, LocalizedRoutingStatus + from pcapkit.protocols.internet.mh import (FastBindingAcknowledgmentStatus, + IPv6AddressPrefixCode, LMAAddressCode, + LocalizedRoutingStatus) for cls in (CommandType, ConformanceRequirement, ESPStatus, - LocalizedRoutingStatus, LMAAddressCode): + LocalizedRoutingStatus, LMAAddressCode, + FastBindingAcknowledgmentStatus, IPv6AddressPrefixCode): with self.subTest(cls=cls.__name__): self.assertIsInstance(inspect.getattr_static(cls, 'get'), classmethod) - # Inherited, not redeclared: the class's own ``__dict__`` carries no - # ``get`` of its own, which is the difference between a pure - # re-parent and a kept-and-converted override. + # Inherited, not redeclared: the class's own ``__dict__`` carries + # no ``get`` of its own -- the fact that used to distinguish a + # pure re-parent from a kept-and-converted override, and now + # holds for all seven alike. self.assertNotIn('get', vars(cls)) +class NonCanonicalKeyConvergenceTests(unittest.TestCase): + """The point of GitHub issue #935's final ruling, made literally true: + ``get(None)`` and ``get(1.5)`` now answer alike on all seven. + + Before the two overrides were deleted, each branched on + ``isinstance(key, int)`` and routed every other type through its own + *name* path -- so a key that is neither an :class:`int` nor a + :class:`str` fell through to a subscript lookup that always misses, + answering with a quiet + :exc:`~pcapkit.utilities.exceptions.EnumKeyError`. The base + (:mod:`pcapkit.corekit.enum`) branches on ``isinstance(key, str)`` + instead, so the same key falls through to + :meth:`~pcapkit.corekit.enum.EnumLookup._validate_value` and the + constructor, reaching :meth:`_missing_` and answering with a loud + :exc:`~pcapkit.utilities.exceptions.EnumValueError`. Deleting the + overrides removes the branch that disagreed, so all seven now answer + both keys the same way -- measured here rather than assumed, since + "all seven behave alike" is exactly the claim GitHub issue #935 set + out to make true, and the two overrides are exactly what kept it from + being true before this. + + Verified before writing this test: no call site in this tree -- tests + included -- ever passed ``get`` a key that is neither an :class:`int` + nor a :class:`str`, so this divergence was live on the two overrides + but never actually reached; its removal changes no behaviour any + caller in this tree observed, only what a caller passing such a key + would see. + """ + + def test_none_and_float_keys_all_raise_enumvalueerror(self) -> None: + from pcapkit.const.ftp.command import CommandType, ConformanceRequirement + from pcapkit.protocols.internet.esp import ESPStatus + from pcapkit.protocols.internet.mh import (FastBindingAcknowledgmentStatus, + IPv6AddressPrefixCode, LMAAddressCode, + LocalizedRoutingStatus) + + for cls in (CommandType, ConformanceRequirement, ESPStatus, + LocalizedRoutingStatus, LMAAddressCode, + FastBindingAcknowledgmentStatus, IPv6AddressPrefixCode): + with self.subTest(cls=cls.__name__, key=None): + with self.assertRaises(EnumValueError): + cls.get(None) + with self.subTest(cls=cls.__name__, key=1.5): + with self.assertRaises(EnumValueError): + cls.get(1.5) + + class ZeroRemainOutsideEnumLookupTests(unittest.TestCase): """The census this issue exists to finish: a runtime walk over every importable :mod:`pcapkit.*` module (vendor templates excluded), filtering diff --git a/tests/protocols/internet/test_mh_unit.py b/tests/protocols/internet/test_mh_unit.py index b59216161..be3cbe59f 100644 --- a/tests/protocols/internet/test_mh_unit.py +++ b/tests/protocols/internet/test_mh_unit.py @@ -1606,6 +1606,96 @@ def handover_initiate(code: int) -> object: self.assertEqual(bytes(MH(next=parsed.next, type=parsed.type, chksum=parsed.chksum, data=parsed)), hi_raw) + def test_mh_get_default_works_uniformly_through_the_inherited_method(self) -> None: + """GitHub issue #935: the two kept ``get`` overrides are deleted, not widened. + + GitHub issue #930 re-parented :class:`FastBindingAcknowledgmentStatus` + and :class:`IPv6AddressPrefixCode` onto + :class:`~pcapkit.corekit.enum.EnumLookup`, which made both + **advertise** the base's two-argument ``get(key, default)`` through + inheritance while their own kept ``@staticmethod`` overrides still + only accepted one -- calling either with a ``default`` raised + ``TypeError: get() takes 1 positional argument but 2 were given``. + GitHub issue #935's first ruling, verbatim *"I lean on 1"*, widened + both signatures to accept ``default`` rather than delete them; asked + next *"why must we have the two overrides tho? cant they directly + fall back to the base class's?"*, the owner's final ruling went + further, verbatim: *"I prefer (2) directly"* -- deleting both + overrides outright. Both classes now inherit ``get`` from the base + exactly as :class:`LMAAddressCode` and :class:`LocalizedRoutingStatus` + -- the two pure re-parents already in this module -- always have, so + ``default`` now works the same way on all four, uniformly, because + there is only one implementation left to call. This test fails with + the ``TypeError`` above against the tree at 382375811, before either + of #935's rulings landed. + """ + import sys + + from pcapkit.protocols.internet.mh import (FastBindingAcknowledgmentStatus, + IPv6AddressPrefixCode, LMAAddressCode, + LocalizedRoutingStatus) + from pcapkit.utilities.exceptions import EnumKeyError, EnumValueError + + def _drop_tracebacklimit() -> None: + # The int-value-miss subTest below reaches the loud + # ``_missing_`` on ``key`` itself, which sets this process-wide + # -- restore it so a later test in the same run does not read + # truncated tracebacks because of what this one did. + if hasattr(sys, 'tracebacklimit'): + del sys.tracebacklimit + + self.addCleanup(_drop_tracebacklimit) + + classes = (FastBindingAcknowledgmentStatus, IPv6AddressPrefixCode, + LMAAddressCode, LocalizedRoutingStatus) + + with self.subTest('a default argument no longer raises TypeError'): + # This is exactly the call GitHub issue #935 reports as failing + # on the tree before either ruling landed: a second positional + # argument to a ``@staticmethod`` that only declared one. + for cls in classes: + member = next(iter(cls)) + try: + cls.get('bogus', member) + except TypeError as error: + self.fail(f'{cls.__name__}.get() still rejects a default argument: {error}') + + with self.subTest('a name miss falls back to a default naming a real member'): + for cls in classes: + with self.subTest(cls=cls.__name__): + member = next(iter(cls)) + self.assertIs(cls.get('bogus', member), member) + self.assertIs(cls.get('bogus', int(member)), member) + + with self.subTest('an int value miss falls back to a default naming a real member'): + for cls in classes: + with self.subTest(cls=cls.__name__): + member = next(iter(cls)) + self.assertIs(cls.get(9999, member), member) + + with self.subTest('a default naming no registered member still raises, not TypeError'): + # The issue's own repro: a default that does not itself resolve + # does not silently swallow the miss -- it falls through to the + # same EnumKeyError ``key`` alone would have raised, uniformly + # on all four now. + for cls in classes: + with self.subTest(cls=cls.__name__): + with self.assertRaises(EnumKeyError) as caught: + cls.get('bogus', 'also-bogus') + self.assertIn('bogus', str(caught.exception)) + + with self.subTest('an unusable default on an int value miss still raises EnumValueError'): + for cls in classes: + with self.subTest(cls=cls.__name__): + with self.assertRaises(EnumValueError): + cls.get(9999, 99999) + + with self.subTest('omitting default still raises EnumKeyError, exactly as before'): + for cls in classes: + with self.subTest(cls=cls.__name__): + with self.assertRaises(EnumKeyError): + cls.get('bogus') + def test_mh_local_enums_raise_and_do_not_alias(self) -> None: """GitHub issue #880: the four RFC-inline helper enums stay immutable.