Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 27 additions & 27 deletions tests/const/test_const_apptype_split_unit.py
Original file line number Diff line number Diff line change
Expand Up @@ -652,14 +652,14 @@ def test_a_bare_int_composite_is_refused_as_a_whole(self) -> None:
picking the lowest set bit (``tcp``, iterating **LSB-first**) dispatched
every composite containing it into the TCP registry whatever else it
named. The owner's ruling on this PR (#836) retires that decoding
instead of refining it: "since it's no longer a Flag, `|` joined values
are no longer parsed and accepted, we will treat it as a whole, instead
of splitting." A bare-int composite is therefore refused exactly like
any other value that names no registry -- a stray bit, ``undefined``, or
a number with nothing to do with any transport -- through the one plain
:exc:`ValueError` :meth:`AppType._dispatch` already gives those. There is
no longer a "too many transports" refusal distinct from a "no
transport" one.
instead of refining it: with ``TransportProtocol`` no longer a flag,
``|``-joined values are no longer parsed and accepted, and the value is
treated as a whole rather than split. A bare-int composite is therefore
refused exactly like any other value that names no registry -- a stray
bit, ``undefined``, or a number with nothing to do with any transport
-- through the one plain :exc:`ValueError` :meth:`AppType._dispatch`
already gives those. There is no longer a "too many transports" refusal
distinct from a "no transport" one.

GitHub issue #860 step 2 PR 2 moved ``TransportProtocol`` off its old
power-of-two values onto sequential ones instead (``undefined=0,
Expand Down Expand Up @@ -1408,16 +1408,16 @@ def test_get_refuses_an_unrecognised_name_rather_than_minting_it(self) -> None:
``max_val + 1`` -- right after ``dccp``'s 8, so ``.get('bogus')``
minted 9 -- rather than stock ``ad4805f5f``'s ``max_val * 2``
doubling, which mints 16 for that same call; the difference between
the two schemes is explained below. The maintainer's ruling refuses
minting outright either way: "Do not allow extension of
TransportProtocol at all." There is no bound left to walk and
nothing left to mint, so the refusal is the one plain
:class:`ValueError` every unrecognised name gets, whether or not it
happens to spell a composite like ``'tcp|udp'``. A later round of
this PR briefly gave the composite case its own, more specific
message; the owner's ruling retired that split too -- "since it's
no longer a Flag, `|` joined values are no longer parsed and
accepted, we will treat it as a whole, instead of splitting" -- so
the two schemes is explained below. The maintainer's ruling, given in
an inline review comment on this PR, refuses minting outright either
way: ``TransportProtocol`` is not to be extended at all. There is no
bound left to walk and nothing left to mint, so the refusal is the one
plain :class:`ValueError` every unrecognised name gets, whether or not
it happens to spell a composite like ``'tcp|udp'``. A later round of
this PR briefly gave the composite case its own, more specific message;
the owner's ruling retired that split too -- with ``TransportProtocol``
no longer a flag, ``|``-joined values are no longer parsed and
accepted, and the value is treated as a whole rather than split -- so
``'|'`` is not treated specially any more, here or in
:meth:`AppType._dispatch` (see
``test_a_bare_int_composite_is_refused_as_a_whole``).
Expand Down Expand Up @@ -1480,15 +1480,15 @@ def test_get_refuses_a_composite_spelled_string(self) -> None:
readily as a genuinely OR-ed value, the mirror image of the
minted-member defect above -- and that branch is gone now too (see
``test_a_bare_int_composite_is_refused_as_a_whole``).
:func:`~pcapkit.foundation.registry.protocols.register_apptype`
refuses the identical string the same generic way it refuses any
other unrecognised one, and the owner's ruling on this PR -- "since
it's no longer a Flag, `|` joined values are no longer parsed and
accepted, we will treat it as a whole, instead of splitting" --
settles :meth:`TransportProtocol.get` onto that same answer: ``'|'``
is not special, it is simply not the name of a declared member. A
review round of this PR briefly carved the composite case out with
its own diagnostic message; the owner's ruling retired that too.
:func:`~pcapkit.foundation.registry.protocols.register_apptype` refuses
the identical string the same generic way it refuses any other
unrecognised one, and the owner's ruling on this PR -- with
``TransportProtocol`` no longer a flag, ``|``-joined values are no
longer parsed and accepted, and the value is treated as a whole rather
than split -- settles :meth:`TransportProtocol.get` onto that same
answer: ``'|'`` is not special, it is simply not the name of a declared
member. A review round of this PR briefly carved the composite case out
with its own diagnostic message; the owner's ruling retired that too.
"""
from pcapkit.const.reg.apptype import TransportProtocol

Expand Down
40 changes: 20 additions & 20 deletions tests/const/test_const_enum_get.py
Original file line number Diff line number Diff line change
Expand Up @@ -218,16 +218,16 @@ def test_the_reported_case_returns_the_default(self) -> None:
self.assertIs(Hardware.get(99999, 0), Hardware(0))
self.assertIs(Hardware.get(99999, 1), Hardware.Ethernet)

# GitHub issue #864 changes what an *unresolvable* default does,
# though: #584 passed a ``str`` where the signature says ``int``,
# and ``'X'`` is not a registered value either way -- before #864
# that failed with ``cls('X')``'s own error, naming ``'X'``; #864's
# ruling ("get should not mint unless it falls through the
# _missing_'s minted ranges") replaces ``cls(default)`` with a plain
# ``_value2member_map_`` lookup for exactly this reason, so an
# unresolvable default no longer gets an attempt of its own to fail
# from -- it now falls through to the *original* key lookup's own
# error instead, same as if no default had been supplied at all.
# GitHub issue #864 changes what an *unresolvable* default does, though:
# #584 passed a ``str`` where the signature says ``int``, and ``'X'`` is
# not a registered value either way -- before #864 that failed with
# ``cls('X')``'s own error, naming ``'X'``; #864's ruling (only
# ``register`` can mint, and ``get`` may mint only where the lookup falls
# through to the ranges ``_missing_`` mints) replaces ``cls(default)``
# with a plain ``_value2member_map_`` lookup for exactly this reason, so
# an unresolvable default no longer gets an attempt of its own to fail
# from -- it now falls through to the *original* key lookup's own error
# instead, same as if no default had been supplied at all.
with self.assertRaises(ValueError) as caught:
Hardware.get(99999, 'X')
self.assertIn('99999', str(caught.exception))
Expand Down Expand Up @@ -256,16 +256,16 @@ def test_omitting_the_default_still_raises_for_the_original_key(self) -> None:

Before GitHub issue #864, that distinction was *also* observable
through ``get()`` itself: a supplied-but-unresolvable ``-1`` failed
with its own name (``cls(-1)`` raising directly), differently from
the omitted case's original-key error. #864's ruling ("get should
not mint unless it falls through the _missing_'s minted ranges")
removes that particular observation for an *unregistered* default
specifically: ``default`` no longer reaches ``cls(default)`` at
all, and ``-1`` is not a registered value on either registry here
(both domains start at ``0``), so supplying it now converges on
exactly the *same* original-key error as omitting it outright,
rather than a distinguishable one of its own. The test's own title
is, if anything, more true after #864 than before: *omitting* the
with its own name (``cls(-1)`` raising directly), differently from the
omitted case's original-key error. #864's ruling (only ``register`` can
mint, and ``get`` may mint only where the lookup falls through to the
ranges ``_missing_`` mints) removes that particular observation for an
*unregistered* default specifically: ``default`` no longer reaches
``cls(default)`` at all, and ``-1`` is not a registered value on either
registry here (both domains start at ``0``), so supplying it now
converges on exactly the *same* original-key error as omitting it
outright, rather than a distinguishable one of its own. The test's own
title is, if anything, more true after #864 than before: *omitting* the
default raises for the original key, and now so does supplying an
unregistered one -- pinned below for both.

Expand Down
31 changes: 16 additions & 15 deletions tests/const/test_const_ftp_featcode_case_903_unit.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@
all-lower-case one (5 distinct -- ``base``, ``feat``, ``hist``, ``nat6``,
``secu``), 1 is blank, and **none** is mixed. So the two authorities
disagree about the casing of the same field, which is exactly the shape the
owner's ruling on this issue calls case-insensitive, verbatim: *"I say
lenient. TransportProtocol for example should be case-insensitive. Upper or
lower cases are being used everywhere in RFC and IANA themselves so that's an
indication of case insensitivity."*
owner's ruling on this issue treats as case-insensitive. He chose the lenient
reading of the criterion, with ``TransportProtocol`` as his example: when the
RFC and IANA themselves use upper and lower case for the same field, that mix
is itself an indication of case insensitivity.

**Pre-change behaviour, measured before the fix** (throwaway process, no
probe that could mint; ``_member_map_`` and ``_value2member_map_`` both 15
Expand All @@ -59,20 +59,21 @@
FEATCode.get('auth' ) -> KeyError: 'auth'
FEATCode.get('Auth' ) -> KeyError: 'Auth'

**What the fix deliberately does not do.** It does not fold the stored
members, and it does not rename one. Every member keeps the registrar's own
casing, per the ruling on the *House Conventions* page -- *"enum
should honour and keep their original writings as in the registrars"* -- so
``FEATCode.get('BASE').name`` is still ``'base'`` and still says
*placeholder*. Nor does it fold the *value* a lookup resolves to, which is
what would have made the fold lossy. Only the inbound key is folded, and only
after an exact name-or-value match has already missed, so
**What the fix deliberately does not do.** It does not fold the stored members,
and it does not rename one. Every member keeps the registrar's own casing, per
the ruling on #877 that the *House Conventions* page records -- enumerations
keep the registrars' own writing, and case-insensitivity is kept for the
selected registries where it makes logical sense and/or the RFC itself treats
the values as case-insensitive -- so ``FEATCode.get('BASE').name`` is still
``'base'`` and still says *placeholder*. Nor does it fold the *value* a lookup
resolves to, which is what would have made the fold lossy. Only the inbound key
is folded, and only after an exact name-or-value match has already missed, so
:meth:`~pcapkit.corekit.enum.EnumLookup.get`'s own precedence (name before
value) and its non-minting ``str`` path both survive untouched. RFC 5797's
uniqueness rule is what makes the fold unambiguous rather than merely
convenient: a registered ``BASE`` cannot coexist with the placeholder
``base``, so there is no second member for the fold to hide -- pinned below
by :meth:`FEATCodeCaseFoldSafetyTests.test_no_two_members_collide_when_folded`.
convenient: a registered ``BASE`` cannot coexist with the placeholder ``base``,
so there is no second member for the fold to hide -- pinned below by
:meth:`FEATCodeCaseFoldSafetyTests.test_no_two_members_collide_when_folded`.

**Contrast, pinned so the default is not quietly widened.** The audit's other
``str``-valued registries stay case-sensitive and each has a reason:
Expand Down
8 changes: 4 additions & 4 deletions tests/corekit/test_enum_get_exception_provenance_923_unit.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
"""GitHub issue #923: :meth:`~pcapkit.corekit.enum.EnumLookup.get` raises from
:mod:`pcapkit.utilities.exceptions`, in stdlib :class:`~enum.Enum`'s shape.

The owner's ruling, verbatim: *"Either ``ValueError`` or ``KeyError``, that's
depending on how stdlib's ``Enum`` would raise on these circumstances. And we
should raise one from ``pcapkit.utilities.exceptions`` rather builtin
exceptions."*
The owner's ruling, asked for on #921 and recorded on #923: raise whichever of
:exc:`ValueError` and :exc:`KeyError` stdlib's :class:`~enum.Enum` would raise
in the same circumstance, and raise it from :mod:`pcapkit.utilities.exceptions`
rather than as a builtin exception.

Measured on Python 3.14.7, that fixes the shape rather than leaving it open:
``E['nosuch']`` raises :exc:`KeyError` and ``E(999)`` raises :exc:`ValueError`.
Expand Down
32 changes: 19 additions & 13 deletions tests/corekit/test_enum_lookup_base_unit.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
# -*- coding: utf-8 -*-
"""Phase 1 of GitHub issue #877: the bare lookup base under :class:`EnumRegistry`.

The owner's ruling, verbatim: *"they may subclass a bare base enum from
pcapkit.corekit.enum - where EnumRegistry subclasses it for using in the other
mutable ones."* :class:`~pcapkit.corekit.enum.EnumLookup` is that base, and it
carries :meth:`~pcapkit.corekit.enum.EnumLookup.get`,
The owner's ruling on #877: the helper enumerations are to be immutable unless
the RFC or IANA says otherwise, so they may subclass a bare base enumeration
defined in :mod:`pcapkit.corekit.enum`, which :class:`EnumRegistry` itself then
subclasses for the mutable ones. :class:`~pcapkit.corekit.enum.EnumLookup` is
that base, and it carries :meth:`~pcapkit.corekit.enum.EnumLookup.get`,
:meth:`~pcapkit.corekit.enum.EnumLookup.get_all` and the overridable
:meth:`~pcapkit.corekit.enum.EnumLookup._validate_value` hook -- not ``register``,
because the owner's own second thought settled that: *"if it carries ``register``,
then why not ``register_alias``. We might be creating a bad ruling."*
because the owner's own second thought on the same thread settled that: a base
that carried ``register`` would raise the question of why it should not carry
``register_alias`` too, and he thought a bad ruling might be created that way.
He then went with the recommendation to keep both on :class:`EnumRegistry`.

What this module pins, and why each part is worth pinning:

Expand Down Expand Up @@ -223,7 +226,8 @@ class that cannot be overriding ``get``, since it defines none.
self.assertIs(_Str.get('<angled>'), _Str.angled)

def test_get_is_case_sensitive(self) -> 'None':
"""The ruled default: *"otherwise, we should treat them case sensitive."*
"""The ruled default: case-sensitive, unless the RFC states that the values
are case-insensitive (ruled on #877).

Case-insensitivity is a per-class override needing an RFC behind it, so
the base must not fold case itself. Auditing the existing overrides
Expand Down Expand Up @@ -280,11 +284,12 @@ def test_lookups_do_not_grow_a_closed_set(self) -> 'None':
def test_flag_composites_remain_the_ruled_exception(self) -> 'None':
"""A ``Flag`` still caches composites, and that is sanctioned.

The owner's ruling on this issue, verbatim: *"Flag subclasses is the one
only exception where we're expecting values to grow and fill due to
combinations."* So ``_Flag.get(99)`` composing ``first|second|96`` into
``_value2member_map_`` is :class:`~aenum.Flag`'s own machinery behaving as
expected, and the base does not -- and must not -- suppress it.
The owner's ruling on this issue is that :class:`~aenum.Flag` subclasses
are the one exception to enumerations staying immutable: their values are
expected to grow and fill in through combinations. So ``_Flag.get(99)``
composing ``first|second|96`` into ``_value2member_map_`` is
:class:`~aenum.Flag`'s own machinery behaving as expected, and the base
does not -- and must not -- suppress it.

The distinction worth pinning is *which* table moves: no real member is
added, so ``_member_names_`` and ``__members__`` are untouched while the
Expand Down Expand Up @@ -332,7 +337,8 @@ def test_neither_tier_is_an_enumeration(self) -> 'None':


class ValidateValueTests(unittest.TestCase):
"""The hook the owner asked for: *"some sort of range validation logic."*"""
"""The hook the owner asked for on #877: some range validation that
inheriting classes can plug into."""

def test_the_default_hook_accepts_everything(self) -> 'None':
"""A base cannot know any subclass's range, so it forbids nothing."""
Expand Down
7 changes: 4 additions & 3 deletions tests/corekit/test_enum_lookup_reparent_877_unit.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
"""Phase 2 of GitHub issue #877, the unblocked half: re-parenting 11 helper
enumerations across 8 files onto :class:`~pcapkit.corekit.enum.EnumLookup`.

The owner's ruling, verbatim: *"I still prefer to reparent all enums until a
in house base class so that they can share common contracts."* Phase 1
(#906) split :class:`~pcapkit.corekit.enum.EnumLookup` out of
The owner's last word on #877 was to re-parent every non-registry enumeration
onto an in-house base class so that they all share the same contracts; it
overrode the recommendation the thread had reached before it. Phase 1 (#906)
split :class:`~pcapkit.corekit.enum.EnumLookup` out of
:class:`~pcapkit.corekit.enum.EnumRegistry` for exactly this; this module
pins that the split half of the tree that is not blocked by #913 or #904
actually took the base -- :class:`TransportProtocol
Expand Down
10 changes: 5 additions & 5 deletions tests/corekit/test_fields_numbers_port_option_no_mint_unit.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@
directly and skipping the registry's own ``__new__`` (and the
``cls.__registry__.add(...)`` / ``cls.__members_ns__[...]`` line inside it)
entirely, so nothing is ever added to any of its lookup tables. That is the
owner's ruling on this issue's return type: *"we should even apply to all
other Enum's legit but unbounded values -- so that we dont create registered
enums out of unrecognised/unregistered values, unless user/caller explicitly
created them"* -- tracked more broadly as #775, and applied here only to
these four call sites.
owner's ruling on this issue's return type: it is to apply to every other
enumeration's legitimate but unbounded values as well, so that no registered
enumeration member is created out of an unrecognised or unregistered value,
unless the user or caller explicitly created it. That is tracked more broadly
as #775, and applied here only to these four call sites.

This mirrors :mod:`tests.corekit.test_fields_numbers_unassigned_enum`'s shape:
the field is tested directly, in isolation from extraction, and the "still
Expand Down
Loading
Loading