From 31a05ae19d8cfe3854c9d06910f06ae8bc48584d Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Fri, 2 Oct 2026 15:00:00 -0400 Subject: [PATCH] docs(tests): state the rulings these tests cite instead of quoting them Part of #987. Twenty-nine quoted maintainer rulings across fourteen test files become statements of what was settled, keeping every pull-request citation. The inventory had to be re-derived rather than taken from the issue, and the count I supplied was wrong in a way worth recording: my scan matched "[^"\n]{4,}", which excludes newlines, so it missed every quotation that wraps across a docstring line. It reported 22 spans in 12 files; the real in-scope set is 29 across 14, and test_enum_lookup_base_unit.py alone holds 5 where I had counted 1. - One provenance correction: test_final_enforcement.py cited the warn-on-reuse ruling to #778, but #778 carries no such comment. It was given on #788, the pull request that implemented it, so the prose now names both. - test_const_enum_get.py is the one file with an edited comment rather than a docstring. Its block is re-wrapped at the same ten lines. Sites deliberately left alone, listed so a later pass does not re-litigate them: the two sanctioned survivors in test_enum_lookup_reparent_930_unit.py; quotations of the project's own code, docs pages and earlier revisions; scare quotes; and five spans traceable to an agent's wording rather than the maintainer's, three of which sit in test_const_enum_no_mint.py and want a second opinion before anyone converts them. Prose only, measured per file at the strictest setting -- every token kept, only strings masked. Thirteen files compare identical; test_const_enum_get.py differs in ten comment tokens and is identical once comments are masked too. The AST with docstrings blanked compares equal in all fourteen, while the raw dump differs. Every differing string token is a docstring, no assertion depends on any changed text, and no file gained a line over 95 characters. --- tests/const/test_const_apptype_split_unit.py | 54 +++++++++---------- tests/const/test_const_enum_get.py | 40 +++++++------- .../test_const_ftp_featcode_case_903_unit.py | 31 +++++------ ..._enum_get_exception_provenance_923_unit.py | 8 +-- tests/corekit/test_enum_lookup_base_unit.py | 32 ++++++----- .../test_enum_lookup_reparent_877_unit.py | 7 +-- ...fields_numbers_port_option_no_mint_unit.py | 10 ++-- tests/corekit/test_sentinels_housing_unit.py | 16 +++--- tests/project/test_bump_version.py | 7 +-- tests/protocols/internet/test_mh_unit.py | 12 ++--- tests/test_final_enforcement.py | 15 +++--- tests/toolkit/test_pyshark_unit.py | 7 ++- .../test_vendor_reg_apptype_generator_unit.py | 25 ++++----- .../test_vendor_snapshot_restore_unit.py | 11 ++-- 14 files changed, 145 insertions(+), 130 deletions(-) diff --git a/tests/const/test_const_apptype_split_unit.py b/tests/const/test_const_apptype_split_unit.py index 9c25137bc5..7003713ea9 100644 --- a/tests/const/test_const_apptype_split_unit.py +++ b/tests/const/test_const_apptype_split_unit.py @@ -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, @@ -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``). @@ -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 diff --git a/tests/const/test_const_enum_get.py b/tests/const/test_const_enum_get.py index d3f8497812..2ef0957926 100644 --- a/tests/const/test_const_enum_get.py +++ b/tests/const/test_const_enum_get.py @@ -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)) @@ -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. diff --git a/tests/const/test_const_ftp_featcode_case_903_unit.py b/tests/const/test_const_ftp_featcode_case_903_unit.py index 9e09be09b4..72cf4492d1 100644 --- a/tests/const/test_const_ftp_featcode_case_903_unit.py +++ b/tests/const/test_const_ftp_featcode_case_903_unit.py @@ -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 @@ -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: diff --git a/tests/corekit/test_enum_get_exception_provenance_923_unit.py b/tests/corekit/test_enum_get_exception_provenance_923_unit.py index bd0a3ecc39..0aa01b1833 100644 --- a/tests/corekit/test_enum_get_exception_provenance_923_unit.py +++ b/tests/corekit/test_enum_get_exception_provenance_923_unit.py @@ -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`. diff --git a/tests/corekit/test_enum_lookup_base_unit.py b/tests/corekit/test_enum_lookup_base_unit.py index 6b778d90ba..175c010e0d 100644 --- a/tests/corekit/test_enum_lookup_base_unit.py +++ b/tests/corekit/test_enum_lookup_base_unit.py @@ -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: @@ -223,7 +226,8 @@ class that cannot be overriding ``get``, since it defines none. self.assertIs(_Str.get(''), _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 @@ -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 @@ -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.""" diff --git a/tests/corekit/test_enum_lookup_reparent_877_unit.py b/tests/corekit/test_enum_lookup_reparent_877_unit.py index 4efd07f2f7..927992a48a 100644 --- a/tests/corekit/test_enum_lookup_reparent_877_unit.py +++ b/tests/corekit/test_enum_lookup_reparent_877_unit.py @@ -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 diff --git a/tests/corekit/test_fields_numbers_port_option_no_mint_unit.py b/tests/corekit/test_fields_numbers_port_option_no_mint_unit.py index 8ecb88d939..a458da4145 100644 --- a/tests/corekit/test_fields_numbers_port_option_no_mint_unit.py +++ b/tests/corekit/test_fields_numbers_port_option_no_mint_unit.py @@ -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 diff --git a/tests/corekit/test_sentinels_housing_unit.py b/tests/corekit/test_sentinels_housing_unit.py index a4c4722f03..019c24253b 100644 --- a/tests/corekit/test_sentinels_housing_unit.py +++ b/tests/corekit/test_sentinels_housing_unit.py @@ -3,9 +3,9 @@ The `__all__` half of #911 landed as #916 and is pinned by :mod:`tests.corekit.test_sentinel_exports_unit`. The owner left the second -question -- where the four sentinels should be *defined* -- open, and settled it -with a one-line ruling once offered the choice: *"Okay one module for all four it -is."* +question -- where the four sentinels should be *defined* -- open, offering +either one module per sentinel or one module for all of them, and settled it with +a one-line ruling once offered the choice: one module for all four. So :mod:`pcapkit.corekit.sentinels` is now the single defining module for all four -- :class:`~pcapkit.corekit.sentinels.NullType`, @@ -255,11 +255,11 @@ def test_each_consumer_module_still_imports_cleanly_on_its_own(self) -> 'None': class SentinelsModuleExportRuleTests(unittest.TestCase): """The canonical module follows its own house rule: objects only, in ``__all__``. - Nothing forces :mod:`pcapkit.corekit.sentinels` to honour the *"only export the - objects"* ruling for itself -- the ruling was stated about the shim locations, - which #916 already fixed -- but shipping a brand new module that violates the - rule its own docstring cites would be a strange way to land it, so this pins - that it does not. + Nothing forces :mod:`pcapkit.corekit.sentinels` to honour the ruling that the + sentinel objects, and not their types, are what gets exported -- the ruling + was stated about the shim locations, which #916 already fixed -- but shipping + a brand new module that violates the rule its own docstring cites would be a + strange way to land it, so this pins that it does not. """ diff --git a/tests/project/test_bump_version.py b/tests/project/test_bump_version.py index aaa3ac45d2..c7fb5647c8 100644 --- a/tests/project/test_bump_version.py +++ b/tests/project/test_bump_version.py @@ -2,9 +2,10 @@ """Tests for :file:`util/bump_version.py`, the version bump the vendor cron runs. The script moves ``__version__`` in :file:`pcapkit/__init__.py` on and, since the -owner asked for it -- *"we have version cited in CITATION.cff, might need to have -the version_bump.py handle that as well"* -- the ``version`` and ``date-released`` -fields of :file:`CITATION.cff` with it. +owner asked for it -- he pointed out that the version is also cited in +:file:`CITATION.cff` and that the bump script might need to handle that as well +(#625; no issue exists for it) -- the ``version`` and ``date-released`` fields +of :file:`CITATION.cff` with it. Why the citation half is tested against a fixture ------------------------------------------------- diff --git a/tests/protocols/internet/test_mh_unit.py b/tests/protocols/internet/test_mh_unit.py index 15eb55c7a2..dc9339742c 100644 --- a/tests/protocols/internet/test_mh_unit.py +++ b/tests/protocols/internet/test_mh_unit.py @@ -1616,12 +1616,12 @@ def test_mh_get_default_works_uniformly_through_the_inherited_method(self) -> No 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, on GitHub pull request #940, *"why must we have the two - overrides tho? cant they directly fall back to the base class's?"*, - the owner's final ruling there went further, verbatim: *"I prefer (2) - directly"* -- deleting both overrides outright. Both classes now + GitHub issue #935's first ruling, the owner's lean toward option 1 of + that thread, widened both signatures to accept ``default`` rather than + delete them. The owner then asked, on GitHub pull request #940, why the + two overrides were needed at all when they could fall back to the base + class's, and his final ruling there went further: he preferred the + second option, 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 diff --git a/tests/test_final_enforcement.py b/tests/test_final_enforcement.py index 4ec28ac815..3809feed7f 100644 --- a/tests/test_final_enforcement.py +++ b/tests/test_final_enforcement.py @@ -118,13 +118,14 @@ class Derived(Sealed): # pylint: disable=unused-variable def test_re_finalising_the_same_info_class_only_warns(self) -> None: """``@info_final @info_final class X(Info)`` warns and carries on. - The ruling on #778, in the maintainer's words: a second application to - the *same* class "should only warn", where a *subclass* of a finalised - class raises. The distinction is that the duplicate is redundant rather - than wrong -- the first application already generated the ``__init__`` - and the ``__builtin__`` set, so there is nothing to refuse and nothing - to redo. What the guard must not do is hand back a half-built class, so - the returned class is exercised here rather than merely identified. + The maintainer's ruling, given on the pull request that implemented #778 + (#788): applying the decorator a second time to the *same* class should + only warn, while a *subclass* of a finalised class should raise. The + distinction is that the duplicate is redundant rather than wrong -- the + first application already generated the ``__init__`` and the + ``__builtin__`` set, so there is nothing to refuse and nothing to redo. + What the guard must not do is hand back a half-built class, so the + returned class is exercised here rather than merely identified. """ from pcapkit.corekit.infoclass import Info, info_final diff --git a/tests/toolkit/test_pyshark_unit.py b/tests/toolkit/test_pyshark_unit.py index 4fe156e125..ff7597d41e 100644 --- a/tests/toolkit/test_pyshark_unit.py +++ b/tests/toolkit/test_pyshark_unit.py @@ -335,8 +335,11 @@ def test_filter_name_fallback_no_longer_upper_cases_onto_a_member(self) -> None: def test_filter_name_fallback_resolves_the_measured_names(self) -> None: """Per the maintainer's ruling on #842, the fallback table covers **every** filter name the sweep measured as unambiguous, not only the - two it started with -- "so that we don't have to come back in future - and update". None of the names below spells a LinkType member, so + two it started with. The owner's reason on #842 for covering every + filter name that differs from the one pcapkit uses was that nobody + should have to come back later to update the table; he also wanted the + values generated if possible, or at least a comment pointing at the + source of truth. None of the names below spells a LinkType member, so each one raised before the expansion. Each pair was measured the same way as the rest of the table: the diff --git a/tests/vendor/test_vendor_reg_apptype_generator_unit.py b/tests/vendor/test_vendor_reg_apptype_generator_unit.py index 335300be13..e783693851 100644 --- a/tests/vendor/test_vendor_reg_apptype_generator_unit.py +++ b/tests/vendor/test_vendor_reg_apptype_generator_unit.py @@ -62,12 +62,12 @@ against a mypy error any more, since an ``auto()``-valued member infers as ``Any``, which is assignable to ``TransportProtocol`` with no cast at all. The owner's final ruling, given on GitHub pull request #874, reinstated the -original shape instead, verbatim: *"undefined direct uses 0. then other real -transport use auto. so we don't have to define a _start_ and the undefined -declaration is explicit."* So ``undefined`` is a direct, explicit ``0`` -again, and ``tcp``/``udp``/``sctp``/``dccp`` continue from it via plain -``auto()`` with no ``_start_`` needed at all -- ``auto()`` picks up the -next value after whatever came immediately before it, explicit literal or +original shape instead: ``undefined`` takes ``0`` directly, and the other real +transports use ``auto()``, so that no ``_start_`` has to be defined and the +declaration of ``undefined`` is explicit. So ``undefined`` is a direct, +explicit ``0`` again, and ``tcp``/``udp``/``sctp``/``dccp`` continue from it +via plain ``auto()`` with no ``_start_`` needed at all -- ``auto()`` picks up +the next value after whatever came immediately before it, explicit literal or not. #770's own account above is therefore accurate again as shipped, and ``undefined`` is once more the one member among the five whose wrapper is load-bearing against a mypy error, exactly as it always was -- see @@ -226,12 +226,13 @@ def test_undefined_member_is_cast_rather_than_a_bare_literal(self) -> None: a mypy error any more (measured at the time: unwrapping ``undefined`` alone stayed mypy-clean, identically to unwrapping ``tcp`` instead). The owner's final ruling, given on GitHub pull request #874, - reinstated the original shape, verbatim: *"undefined direct uses 0. - then other real transport use auto. so we don't have to define a - _start_ and the undefined declaration is explicit."* So ``undefined`` - is a direct, explicit ``cast('TransportProtocol', 0)`` again, exactly - as #770 first shaped it, and this test's own check (and #770's - asymmetry) are both back to describing the tree as it actually ships. + reinstated the original shape: ``undefined`` takes ``0`` directly, and + the other real transports use ``auto()``, so that no ``_start_`` has to + be defined and the declaration of ``undefined`` is explicit. So + ``undefined`` is a direct, explicit ``cast('TransportProtocol', 0)`` + again, exactly as #770 first shaped it, and this test's own check (and + #770's asymmetry) are both back to describing the tree as it actually + ships. Measured directly, to prove the load-bearing claim rather than assert it: with ``undefined`` unwrapped to a bare ``0`` (everything diff --git a/tests/vendor/test_vendor_snapshot_restore_unit.py b/tests/vendor/test_vendor_snapshot_restore_unit.py index d13fc270b8..df59c22b2e 100644 --- a/tests/vendor/test_vendor_snapshot_restore_unit.py +++ b/tests/vendor/test_vendor_snapshot_restore_unit.py @@ -4,11 +4,12 @@ GitHub issue #872. Rounds 4-8 made :meth:`~pcapkit.vendor.default.Vendor.__init__`'s own write atomic -- a temp-file-then-:func:`os.replace` at the point of the write, with permission -matching for the replacement. The owner's ruling on GitHub pull request -#873, verbatim: *"I prefer we use contextlib over manually manage the temp -file deletion based on pure best intent (try-finally) and for atomic -writing, an easier path is simply keep a copy before running the sub-vendor -and revert if anything failed."* +matching for the replacement. The owner's ruling on GitHub pull request #873, +answering two inline questions: he preferred :mod:`contextlib` to managing the +temp file's deletion by hand on the strength of best intent alone (a +try-finally), and for atomic writing he saw an easier path in simply keeping a +copy of the file before running the sub-vendor and reverting if anything +failed. That supersedes the atomic write entirely rather than adjusting it. ``Vendor._write_atomic`` no longer exists;