diff --git a/docs/source/contributing/conventions/extension-header-subclassing.rst b/docs/source/contributing/conventions/extension-header-subclassing.rst index 766f4b192..33f235800 100644 --- a/docs/source/contributing/conventions/extension-header-subclassing.rst +++ b/docs/source/contributing/conventions/extension-header-subclassing.rst @@ -5,14 +5,11 @@ Which bases an IPv6 extension header names Every IPv6 extension header in this package subclasses :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext`. Some name a **second** base -as well, and which ones do is a ruling rather than an accident. The owner's words, -on `#924 `__: - - I think on subclassing, we might wanna keep this convention: if the IPv6 - extension header is only usable as an extension header, then it only inherit - from ``IPv6_Ext``, like ``IPv6_Frag``; but if it is useable as a standalone - protocol itself, then it herit from both ``IPv6_Ext`` and ``Internet`` (or - ``IPsec``), like ``ESP``. +as well, and which ones do is a ruling rather than an accident. The owner ruled on +`#924 `__ that a header usable *only* +as an extension header inherits ``IPv6_Ext`` and nothing else -- ``IPv6_Frag`` being +the example -- while one that is usable as a standalone protocol in its own right +inherits both ``IPv6_Ext`` and ``Internet`` (or ``IPsec``), as ``ESP`` does. The family as it stands: @@ -40,7 +37,7 @@ The family as it stands: :class:`~pcapkit.protocols.internet.ipsec.IPsec` is itself an :class:`~pcapkit.protocols.internet.internet.Internet` subclass, which is the -parenthetical *"(or* ``IPsec``\ *)"* in the ruling: naming it satisfies the +parenthetical ``IPsec`` alternative in the ruling: naming it satisfies the convention, and it is the right second base for a header whose standalone form is an IPsec one. @@ -74,11 +71,10 @@ whether the same header is also a protocol in its own right. The operative test is what the RFCs say ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -So the census is read out of the specifications, on the owner's instruction: - - So my suggestion is to read through the RFCs to figure out any of the defined - IPv6 extension headers are extension header only or standalone protocol as well. - Then we can decide if they should inherit only ``IPv6_Ext`` or additional bases. +So the census is read out of the specifications, on the owner's instruction: work +through the RFCs to establish, for each defined IPv6 extension header, whether it is +extension-header-only or a standalone protocol as well, and decide from that whether it +inherits ``IPv6_Ext`` alone or names additional bases. And the limb that decides is **whether a primary source shows the header carried directly as an IPv4 payload**: @@ -139,9 +135,9 @@ The class arrived as ``IPv6_GenericExt``, a fallback parser for an unrecognised extension header (`#891 `__), and `#917 `__ merged that role with the shared-base role into one class under the shorter name. No compatibility alias -was left behind, and that was deliberate. The owner's ruling: - - No more ``IPv6_GenericExt`` name. Its an intermediate state and never released. +was left behind, and that was deliberate. The owner ruled that the +``IPv6_GenericExt`` name goes for good: it was an intermediate state, and it was never +released. The reasoning is what makes it safe rather than merely decided: the old name existed on ``main`` from ``b3551cb63`` to ``93cf940b3`` -- under four hours on one day, and diff --git a/docs/source/contributing/conventions/index.rst b/docs/source/contributing/conventions/index.rst index 7a7a22cf9..119c63e00 100644 --- a/docs/source/contributing/conventions/index.rst +++ b/docs/source/contributing/conventions/index.rst @@ -20,9 +20,10 @@ House Conventions when a question is answered in a way the code cannot express on its own -- a classification, a naming rule, a deliberate asymmetry -- it is written onto the page that covers it in the same change that implements it, rather than - left in the issue for the next contributor to find. The owner's standing - ask, on `#918 `__: *"And - any future conventions to be settled - document them as well."* + left in the issue for the next contributor to find. That is the owner's + standing ask on + `#918 `__: every + convention settled from here on gets documented here as well. .. toctree:: :maxdepth: 1 diff --git a/docs/source/contributing/conventions/mint-criterion.rst b/docs/source/contributing/conventions/mint-criterion.rst index 1cfc10cd0..b6a2917dd 100644 --- a/docs/source/contributing/conventions/mint-criterion.rst +++ b/docs/source/contributing/conventions/mint-criterion.rst @@ -20,14 +20,13 @@ a given range gets is a **design decision, not a style preference**: The criterion ~~~~~~~~~~~~~ -The test, in the maintainer's words: +The test, paraphrased from the maintainer's ruling: does the upstream registry treat +the label as the final, concrete assigned name (**mint**), or only as a notation for a +human reading the table (**unmint**)? - Is this considered as the final concrete assigned name (**mint**), or just a - notation for the readers (**unmint**)? - -Settled on `#847 `__ and confirmed -as "a core concept of the ruling" on -`#775 `__. +Settled on `#847 `__ and reaffirmed +on `#775 `__ as a core concept of +the ruling. So the question to ask of a range is **what the upstream registry actually did**, not what the generated code happens to look like: @@ -74,9 +73,9 @@ Why the company names mint The ethertype case looks like an exception to the rule and is not. The maintainer's reasoning, settled on `#775 `__ after -being raised on `#847 `__: - - Proprietary protocols won't have public names so company names serve this purpose. +being raised on `#847 `__: a +proprietary protocol will never have a public name, so the company name is what serves +that purpose in its place. So the company name is not a note *about* the code -- it is the best name that will ever exist *for* it, which makes it the final concrete assigned name under the test above. diff --git a/docs/source/contributing/conventions/registry-protocol.rst b/docs/source/contributing/conventions/registry-protocol.rst index 96a0d5a85..225ff583c 100644 --- a/docs/source/contributing/conventions/registry-protocol.rst +++ b/docs/source/contributing/conventions/registry-protocol.rst @@ -34,15 +34,17 @@ parent, and the line between them is whether the enumeration may *grow*: ``_extend``, ``_unregistered_member`` =================================================== ============================================================== -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."* So a **closed** set inherits :class:`~pcapkit.corekit.enum.EnumLookup` +The owner ruled on #877 that a registry may subclass a bare base enum out of +:mod:`pcapkit.corekit.enum`, with :class:`~pcapkit.corekit.enum.EnumRegistry` +subclassing that base in turn for use by the mutable ones. So a **closed** set +inherits :class:`~pcapkit.corekit.enum.EnumLookup` directly and is never handed a ``register`` it would have to refuse; an **open** registry inherits :class:`~pcapkit.corekit.enum.EnumRegistry` exactly as before. What settled the split is the owner's own second thought about carrying ``register`` -on the base: *"if it carries ``register``, then why not ``register_alias``. We might -be creating a bad ruling."* Following that through leaves +on the base: if the base carries ``register``, there is no principled reason for it +not to carry ``register_alias`` as well, and the ruling would be the worse for it. +Following that through leaves :class:`~pcapkit.corekit.enum.EnumRegistry` holding only three methods, too thin to justify a second class -- so the two tiers collapse into one, which is the opposite of what was ruled. @@ -55,8 +57,8 @@ it was -- ``LinkType -> EnumRegistry -> EnumLookup -> IntEnum -> int`` -- so itself and broken ``int``, ``str`` and flag registries at once. :meth:`~pcapkit.corekit.enum.EnumLookup._validate_value` is what the base carries -*instead* of ``register``, and it answers the owner's other requirement: *"there must -be some sort of range validation logic for the inherited classes to hook in."* The +*instead* of ``register``, and it answers the owner's other requirement: the base has +to offer range-validation logic for its inheriting classes to hook into. The base implementation accepts everything; an override states a range, in the shape the generated registries currently spell by hand in ``_missing_``: @@ -118,13 +120,11 @@ Three things about it are easy to get wrong: What a Failed Lookup Raises ~~~~~~~~~~~~~~~~~~~~~~~~~~~ -Two rules govern it, and they pull in opposite directions on purpose. The owner's -ruling, verbatim, on -`#923 `__: - - 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. +Two rules govern it, and they pull in opposite directions on purpose. The owner ruled +on `#923 `__ that the choice between +:exc:`ValueError` and :exc:`KeyError` follows whichever stdlib's :class:`~enum.Enum` +would raise in the same circumstance, and that whichever it is comes from +:mod:`pcapkit.utilities.exceptions` rather than from builtins. So the **provenance** is in-library and the **shape** is stdlib's: @@ -148,8 +148,8 @@ one into the other is exactly what #923 retired, and it was retired in three pla at once: ``TransportProtocol.get`` and ``Criticality.get`` had each turned the base's :exc:`KeyError` into a :exc:`ValueError`, and ``FastBindingAcknowledgmentStatus.get`` raised -:exc:`~pcapkit.utilities.exceptions.EnumValueError` for a name miss so that "the -two ways of getting it wrong reported identically". +:exc:`~pcapkit.utilities.exceptions.EnumValueError` for a name miss, so that the two +ways of getting it wrong reported identically. One asymmetry between the two is deliberate and is **not** visible from the exception class: the **name** miss is raised quietly @@ -286,17 +286,16 @@ what it adds to the base, and goes when the answer is nothing.** Case Sensitivity Is RFC-Directed ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -The rule, in the owner's own wording on -`#877 `__: - - if RFC states the values are case-insensitive, then our enum should also treat them - that way. otherwise, we should treat them case sensitive. +The rule, as the owner ruled it on +`#877 `__: where the RFC states the +values are case-insensitive, the enumeration treats them that way too; otherwise it +treats them as case-sensitive. And the reason a registry's spelling is never quietly normalised, from the same thread: -*"enum should honour and keep their original writings as in the registrars. case -in-sensitivity only applies to certain selected ones, where logically it makes sense -(like ``TransportProtocol``) and/or RFC documentation itself recognises them as -case-insensitive (like, maybe, FTP/HTTP commands)."* +an enumeration honours and keeps the original writing the registrar used, and +case-insensitivity applies only to the selected registries where it makes logical sense +-- ``TransportProtocol`` being one -- or where the RFC documentation itself recognises +the values as case-insensitive, FTP and HTTP commands being the likely candidates. So :meth:`~pcapkit.corekit.enum.EnumLookup.get` is **case-sensitive**, and that is the default every enumeration gets. Case-insensitivity is a per-class ``get`` override that @@ -330,11 +329,9 @@ The ruling above leaves one question open, and `#903 `__ settled it: does a specification have to state a **comparison rule** for a registry to be treated case-insensitively, or does it also count when the authorities merely **disagree -about spelling**? The owner's answer, 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. +about spelling**? The owner ruled for the lenient reading, with ``TransportProtocol`` +as his own example: upper and lower casings are used throughout the RFCs and IANA's own +data, and that mixed usage is itself an indication of case-insensitivity. So the test a new registry has to pass has **two limbs**, and satisfying either one justifies case-insensitivity: @@ -358,8 +355,8 @@ The Audit, per Class ~~~~~~~~~~~~~~~~~~~~ `#903 `__'s sweep, so that a -registry added later has something to check itself against. The owner's scope for it, -verbatim: *"we should audit all registries and then decide if case (in)sensitive."* +registry added later has something to check itself against. The owner set its scope: +audit every registry first, and decide case-sensitivity per registry from that. The population it covers, with the counting convention spelled out because the figures move: **127** :class:`~pcapkit.corekit.enum.EnumRegistry` subclasses, every diff --git a/docs/source/contributing/conventions/sentinel-convention.rst b/docs/source/contributing/conventions/sentinel-convention.rst index 84f822999..92d7bdb49 100644 --- a/docs/source/contributing/conventions/sentinel-convention.rst +++ b/docs/source/contributing/conventions/sentinel-convention.rst @@ -6,14 +6,14 @@ Naming a sentinel A *sentinel* here is a module-level singleton whose only job is to be recognised by identity -- ``value is SENTINEL`` -- so that it can never be confused with a value a caller might legitimately pass. The house rule, from the maintainer, covers the type: - - Keep the sentinel object's type class naming as ``Type``. +the sentinel object's type class is named ``Type``. That is, the class takes the instance's name in CamelCase with ``Type`` appended. It says nothing about the **object**'s own name, which is what let three casings diverge -with no rule naming any of them wrong. GitHub issue #937 closed that gap, verbatim: -*"take SCREAMING_SNAKE and accept the breaking change (no backport needed)."* So the -object is named in SCREAMING_SNAKE, and the type-naming rule above derives from it +with no rule naming any of them wrong. GitHub issue #937 closed that gap: the owner +ruled for SCREAMING_SNAKE and accepted the resulting breaking change outright, with no +backport. So the object is named in SCREAMING_SNAKE, and the type-naming rule above +derives from it mechanically -- title-case each underscore-separated word and append ``Type``, no per-sentinel exception needed. The four in the tree follow it: @@ -40,9 +40,9 @@ per-sentinel exception needed. The four in the tree follow it: All four used to live beside the one class that used them -- :mod:`pcapkit.corekit.module`, :mod:`pcapkit.corekit.fields.field`, :mod:`pcapkit.corekit.enum` and :mod:`pcapkit.protocols.protocol` respectively. -GitHub issue #911's housing ruling, verbatim -- *"Okay one module for all four it -is."* -- moved the four definitions into the single shared module the table now -names; each original module keeps a re-export so every existing +GitHub issue #911's housing ruling -- one module for all four -- moved the four +definitions into the single shared module the table now names; each original module +keeps a re-export so every existing ``from import `` keeps working, including the ``if TYPE_CHECKING:``-only imports of the types. @@ -59,9 +59,9 @@ every caller for no further gain. The rename also dropped the **leading underscore** ``_Absent``/``_AbsentType`` used to carry. ``ABSENT`` is private -- it is read in ``_declared_keywords`` and discarded there, never leaving :mod:`pcapkit.protocols.protocol` -- and the underscore used to be -the mechanical signal of that. The maintainer's ruling on #937, verbatim: *"we can -change* ``_ABSENT`` *to* ``ABSENT`` *just document it as private type/class in the -documentation and not for public use is enough."* So privacy is documentation-only from +the mechanical signal of that. The owner ruled on #937 that dropping it is fine, so +long as the documentation states that the type and class are private and not for public +use -- that alone is enough. So privacy is documentation-only from here on, carried by this paragraph and by :class:`~pcapkit.corekit.sentinels.AbsentType`'s own docstring (:file:`pcapkit/corekit/sentinels.py`, line 439), which still says so: @@ -78,9 +78,9 @@ filtered on capitalised names did not see ``_Absent`` -- and that history does n change now that nothing in the name itself marks it out. When adding a sentinel, add it here whether or not it is public. -What reaches users is the **object only**. The maintainer's ruling: *"we should ONLY -export the objects (like* ``NULL`` *) to users"* -- so a public sentinel names its -instance in its module's ``__all__`` and leaves the type out of it (GitHub issue #911). +What reaches users is the **object only**. The owner ruled on GitHub issue #911 that +the objects alone -- ``NULL`` and its siblings -- are exported to users, so a public +sentinel names its instance in its module's ``__all__`` and leaves the type out of it. The type stays importable by its dotted path, for an annotation or an ``is`` guard; it is ``import *`` that no longer offers it. A private sentinel such as ``ABSENT`` is in neither, which is what private means here -- dropping its leading underscore did not diff --git a/pyproject.toml b/pyproject.toml index 03e337237..ebd04f509 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -112,8 +112,8 @@ cli = [ "emoji" ] # for ESP payload decryption, c.f. pcapkit.protocols.internet.esp crypto = [ "cryptography>=3.4" ] # for NGAP decoding, c.f. pcapkit.protocols.application.ngap. Counted among -# ``all``'s *core addons* by the owner's ruling on #910 -- "everything that -# makes the library itself work at full functionality" -- even though the +# ``all``'s *core addons* by the owner's ruling on #910 -- everything needed for +# the library itself to work at full functionality -- even though the # ``pycrate`` this needs is not a free addition: # # * Size. ``pycrate`` ships every specification it has ever compiled in one @@ -221,15 +221,15 @@ PyPCAPFile = [ "pypcapfile; python_version < '3.12'" ] # moved to ``dev`` instead. Either way, this extra's own requirements are what # it is correct against, not whatever ``all`` happens to carry. vendor = [ "requests[socks]", "beautifulsoup4[html5lib]", "pycrate" ] -# #910: narrowed to *core addons* only -- the owner's own words are "everything -# that makes the library itself work at full functionality", not "everything an -# end user might ever want". An earlier ruling on the same issue read the -# latter and added ``pycrate`` on that basis; the ruling that stuck narrowed it -# again, this time by kind: ``cli``, ``crypto`` and ``pycrate`` (NGAP) are core -# addons and stay, and the four 3rd-party capture engines that used to be here -# -- ``dpkt``, ``scapy``, ``pyshark``, ``pypcapfile`` -- are "on demand for each -# one" like ``PyPCAP`` and ``PCAP_CT`` always were, and move out to their own -# extras below. Install one explicitly: ``pip install pypcapkit[DPKT]`` / +# #910: narrowed to *core addons* only -- the owner's ruling scopes this to +# everything needed for the library itself to work at full functionality, rather +# than everything an end user might conceivably want. An earlier ruling on the +# same issue read the latter and added ``pycrate`` on that basis; the ruling that +# stuck narrowed it again, this time by kind: ``cli``, ``crypto`` and ``pycrate`` +# (NGAP) are core addons and stay, and the four 3rd-party capture engines that +# used to be here -- ``dpkt``, ``scapy``, ``pyshark``, ``pypcapfile`` -- are +# installed one at a time on demand, as ``PyPCAP`` and ``PCAP_CT`` always were, +# and move out to their own extras below. Install one explicitly: ``pip install pypcapkit[DPKT]`` / # ``[Scapy]`` / ``[PyShark]`` / ``[PyPCAPFile]``. # # ``pypcap`` and ``pcap-ct``/``libpcap`` stay out for the reason they always diff --git a/tests/corekit/test_sentinel_exports_unit.py b/tests/corekit/test_sentinel_exports_unit.py index db38db10f..d48c7a012 100644 --- a/tests/corekit/test_sentinel_exports_unit.py +++ b/tests/corekit/test_sentinel_exports_unit.py @@ -124,6 +124,8 @@ #: below used to name a different module here (``pcapkit.corekit.module``, #: ``pcapkit.corekit.fields.field``, ``pcapkit.corekit.enum`` and #: ``pcapkit.protocols.protocol`` respectively); all four now report this one. +import pcapkit.corekit.sentinels as sentinels + CANONICAL_MODULE = 'pcapkit.corekit.sentinels' #: Every sentinel in the tree that follows the house ``Type`` convention, @@ -375,11 +377,44 @@ def test_conventions_doc_lists_every_sentinel_in_the_tree(self) -> 'None': self.assertNotIn('The three sentinels deliberately differ', section) def test_conventions_doc_records_that_only_the_object_is_exported(self) -> 'None': - """The ruling this change implements belongs in the doc that states the rule.""" + """The ruling this change implements belongs in the doc that states the rule. + + Pinned as the *claim* rather than as wording. The first version asserted the + literal ``'ONLY'`` -- the maintainer's own capitalisation, lifted from a block + quote of his #911 reply -- so paraphrasing that quote under GitHub issue #949 + reddened this test without anything about the rule having changed. Asserting + someone's emphatic casing is exactly the defect #949 exists to remove, and a + test outside :mod:`tests.project` pinning a convention page's prose is how it + went unnoticed there. + + So the substance is checked against the tree, which is where the ruling + actually takes effect, and the page is only required to name the export + boundary (``__all__``, an identifier rather than prose) and the issue that + settled it. + + """ section = _sentinel_section() - self.assertIn('ONLY', section) self.assertIn('#911', section) + self.assertIn('__all__', section, + "the page no longer names ``__all__``, so it no longer says " + 'where the export boundary is -- #911 ruled that the instance ' + 'is exported and the type is not, and a reader cannot act on ' + 'that without being told which list decides it') + + # The ruling itself, against the module it governs rather than against prose: + # every public sentinel's instance is exported and its type is not. + exported = set(sentinels.__all__) + for _, instance_name, type_name in PUBLIC_SENTINELS: + with self.subTest(sentinel=instance_name): + self.assertIn(instance_name, exported, + f'{instance_name} is a public sentinel but is not in ' + f'{CANONICAL_MODULE}.__all__, so #911\'s ruling that ' + 'the objects are what reach users no longer holds') + self.assertNotIn(type_name, exported, + f'{type_name} is in {CANONICAL_MODULE}.__all__, but ' + '#911 ruled the type stays out of it and remains ' + 'reachable only by its dotted path') def test_conventions_doc_carves_out_the_vendored_bare_object(self) -> 'None': """"Why a class and not ``object()``" read as a blanket rule with no exception.""" diff --git a/tests/project/test_conventions_doc_claims.py b/tests/project/test_conventions_doc_claims.py index ab1792036..cdc878b9b 100644 --- a/tests/project/test_conventions_doc_claims.py +++ b/tests/project/test_conventions_doc_claims.py @@ -31,8 +31,9 @@ against the declarations themselves. ``tests/protocols/internet/test_ipv6_ext_unit.py`` already pins the *code* to the ruling; nothing pinned the *page* to the code, which is the half that goes stale when a ninth header lands. -* :class:`RetiredNameTests` -- *"No more ``IPv6_GenericExt`` name."* A ruling that a name - must not exist is exactly the kind a later change reintroduces without noticing. +* :class:`RetiredNameTests` -- the #924 ruling that the ``IPv6_GenericExt`` name goes for + good. A ruling that a name must not exist is exactly the kind a later change + reintroduces without noticing. * :class:`FailedLookupExceptionTests` -- the worked example the page gives for a name miss, which named :exc:`KeyError` until #918 and now names :exc:`~pcapkit.utilities.exceptions.EnumKeyError`. @@ -465,6 +466,106 @@ def test_the_page_states_the_measured_phase_two_progress(self) -> 'None': self.assertEqual(done, len(self.non_registry) - len(self.outside), 'the page\'s re-parented count is stale') + def test_the_audit_population_figures_are_the_measured_ones(self) -> 'None': + """*The Audit, per Class*'s own population, against the same runtime walk. + + Six figures the section opens with -- the registry count, that every one of + them is under :mod:`pcapkit.const`, the file count, the :class:`int`-valued + and flag splits, the ``aenum.StrEnum`` count and the grand total -- were + prose-only until GitHub issue #949 wired them to the walk. The section says + itself that *the figures move*, which is precisely the case that needs a + measurement rather than a reader's diligence: #930 moved the non-registry + figures once already, and only those had a test. + + Each figure is read out of the page by its own regex and compared, so a stale + one fails naming both numbers. Wording is not pinned; the regexes are + deliberately loose about the prose between the figures and would survive a + rephrasing that kept the claims. + + """ + from pcapkit.corekit.enum import EnumRegistry + + registries = {cls: name for cls, name in self.enumerations.items() + if issubclass(cls, EnumRegistry) and cls is not EnumRegistry} + # ``issubclass(cls, int)`` rather than a check on ``_member_type_``: the flag + # registries are ``IntFlag`` subclasses and have to count inside the int tier, + # which is what the page's parenthetical "(of which N are flag registries)" + # says -- a disjoint reading would make the two figures fail to add up. + int_valued = {c: n for c, n in registries.items() if issubclass(c, int)} + str_valued = {c: n for c, n in registries.items() if issubclass(c, str)} + flags = {c: n for c, n in registries.items() + if issubclass(c, (enum.Flag, aenum.Flag))} + files = {cls.__module__ for cls in registries} + + # Guards the rest from passing on a collapsed walk, as + # ``test_the_walk_found_something_to_count`` does for the counts above. + self.assertGreater(len(registries), 100, + 'the registry half of the walk collapsed; the figures below ' + 'would pass vacuously') + neither = sorted(name for cls, name in registries.items() + if not issubclass(cls, (int, str))) + self.assertEqual(len(int_valued) + len(str_valued), len(registries), + 'a registry is neither int- nor str-valued, so the page\'s ' + f'two-tier split no longer partitions the population: {neither}') + + for pattern, measured, what in ( + (r'\*\*(\d+)\*\* :class:`~pcapkit\.corekit\.enum\.EnumRegistry` subclasses', + len(registries), 'EnumRegistry subclasses'), + (r'across (\d+) files', len(files), 'files holding a registry'), + (r'(\d+) :class:`int`\\?-valued', len(int_valued), 'int-valued registries'), + (r'of which (\d+) are flag registries', len(flags), 'flag registries'), + (r'(\d+) ``aenum\.StrEnum``\\?-valued', len(str_valued), + 'str-valued registries'), + (r'\*\*(\d+)\*\* non-registry enumerations', len(self.non_registry), + 'non-registry enumerations'), + (r'(\d+) enumerations in total', len(self.enumerations), + 'enumerations in total'), + ): + with self.subTest(figure=what): + stated = re.search(pattern, self.note) + self.assertIsNotNone( + stated, f'the page no longer states how many {what} there are in ' + 'the shape this test reads; re-derive the figure rather ' + 'than deleting the check') + assert stated is not None # for type checkers; asserted above + self.assertEqual(int(stated.group(1)), measured, + f'the page says {stated.group(1)} {what}, the tree has ' + f'{measured}') + + # "every one of them under pcapkit.const" is a claim about the *whole* + # population, not a count, so a figure comparison cannot reach it. + self.assertIn('every one of them under :mod:`pcapkit.const`', self.note, + 'the page no longer claims every registry lives under ' + 'pcapkit.const, so this check is pinning a claim it has dropped') + stray = sorted(name for cls, name in registries.items() + if not cls.__module__.startswith('pcapkit.const')) + self.assertEqual(stray, [], + 'a registry now lives outside pcapkit.const, which the page ' + f'says none do: {stray}') + + def test_the_case_differing_hip_parameters_both_resolve(self) -> 'None': + """The page's worked reason for never renaming a member to make a lookup work. + + ``R1_Counter`` and ``R1_COUNTER`` are two IANA-registered HIP parameters + differing only in case, and the page names both values. Executed rather than + read off the page: the claim that matters is that a case-sensitive ``get`` + keeps both *resolvable*, which a substring match cannot establish. + + """ + from pcapkit.const.hip.parameter import Parameter + + for name, value in (('R1_Counter', 128), ('R1_COUNTER', 129)): + with self.subTest(member=name): + self.assertEqual(Parameter.get(name).value, value, + f'{name} no longer resolves to {value}, so the page\'s ' + 'worked example for case-sensitivity is stale') + self.assertIn(f'``{name} = {value}``', self.note, + f'the page no longer states {name} = {value}, the ' + 'collision that makes renaming a member unacceptable') + self.assertNotEqual(Parameter['R1_Counter'], Parameter['R1_COUNTER'], + 'the two HIP parameters have collapsed into one member, so ' + 'the page\'s example of a case-significant registry is gone') + class ExtensionHeaderClassificationTests(unittest.TestCase): """The page's bases-per-header table against the declarations themselves.""" @@ -581,8 +682,7 @@ def test_the_page_agrees_with_the_test_that_pins_the_code(self) -> 'None': class RetiredNameTests(unittest.TestCase): - """*"No more* ``IPv6_GenericExt`` *name. Its an intermediate state and never - released."*""" + """#924's ruling that ``IPv6_GenericExt`` goes: an unreleased intermediate name.""" def test_the_retired_base_name_is_absent_from_the_package(self) -> 'None': """A ruling that a name must not exist needs a test, or it comes back. @@ -765,6 +865,25 @@ def test_a_delegating_override_is_a_classmethod(self) -> 'None': from pcapkit.const.http.method import Method from pcapkit.const.pcapng.option_type import OptionType + # Membership before indexing, for all three. `vars(klass)['get']` raises a bare + # `KeyError: 'get'` when a class folds its own override into the inherited base + # -- which is exactly what GitHub pull request #940 did to the mh/ngap helpers, + # so it is a live failure mode rather than a hypothetical one. The KeyError + # fails the test either way, so nothing regressed silently; what it does not do + # is say *which* class stopped defining `get`, or that the page is now wrong to + # list it. The sibling at `test_the_two_redundant_overrides_are_gone` already + # does this the right way round, with `assertNotIn('get', vars(klass))`. + # Deliberately *not* under `subTest`: a subTest records its failure and lets + # the method run on, so the bare `KeyError` this guard exists to pre-empt + # would still be raised by the indexing below and reported alongside it. A + # plain assertion aborts here, which is the whole point. + for klass in (Method, Command, OptionType): + self.assertIn('get', vars(klass), + f'{klass.__name__} no longer defines a get of its own, so ' + 'the page is wrong to name it among the overrides -- an ' + 'override folded into the inherited base is what #940 did ' + 'to the mh/ngap helpers') + self.assertIsInstance(vars(Method)['get'], classmethod, 'a delegating override has to be a classmethod -- ' 'zero-argument super() in a staticmethod binds the ' @@ -888,8 +1007,14 @@ def _restore() -> 'None': 'ruled overrides follow the base here, not the reverse') flat = self._flat() - self.assertIn('not be loud', flat, - "the page no longer quotes #933's reversal") + # The *claim*, not his wording. This asserted `'not be loud'` until GitHub issue + # #949, which is a fragment of the sentence he typed on the issue rather than + # anything the ruling turns on; the substance is that the first answer on #933 is + # not the ruling and the second one is. Occurs once on the page, checked. + self.assertIn('first declined, then reversed', flat, + "the page no longer records that #933's ruling is the owner's " + 'reversal rather than his first answer, which is the whole reason ' + 'both answers are on the issue') # Anchored to the headline sentence, not the bare token. `quiet=True` occurs # four times on the page, so `assertIn('``quiet=True``')` was satisfied by a # later mention -- inverting the ruling itself to `quiet=False` failed nothing. @@ -1087,10 +1212,10 @@ class ProcessConventionTests(unittest.TestCase): deliberately pins neither against the owner's own phrasing. Quoting him is what the page is forbidden to do here -- his instruction on GitHub issue #918 was to paraphrase -- and asserting a quoted sentence is separately a trap this module has - already been bitten by: a test elsewhere pinned the literal - ``'I prefer (2) directly.'`` onto a page, which turned an off-hand reply into a - build dependency. Attribution lives in the issue number the page cites; the tests - check substance. + already been bitten by: tests elsewhere in this file pinned two off-hand replies of + his as literal strings, which made a sentence typed into a GitHub thread a CI build + dependency. GitHub issue #949 removed the last of those. Attribution lives in the + issue number the page cites; the tests check substance. """ @@ -1397,11 +1522,57 @@ def test_the_page_says_the_labels_are_set_by_hand(self) -> 'None': ecosystems, ['pip'], 'dependabot now watches a different set of ecosystems, so the page\'s claim ' f'about which labels it applies needs re-deriving: {ecosystems}') - self.assertNotIn( - '``dependencies`` and ``github_actions``', self.flat, - 'the page attributes github_actions to dependabot again -- it cannot apply ' - 'that label, since no github-actions ecosystem is configured, and calling a ' - "hand-applied label automated inverts this section's point") + # Derived rather than pinned to one phrasing. The previous form asserted the + # single literal ``dependencies`` and ``github_actions``, so a re-attribution + # worded any other way -- "dependabot applies ``github_actions``", a comma for + # the "and", the two labels in the other order -- slipped straight past it, which + # is the weakness GitHub issue #949 asked to be looked at. Every sentence pairing + # the tool with the label is checked instead, so the phrasing no longer matters. + # + # Proximity is not the test, and a first attempt at #949 that used it was wrong: + # the section states **twice**, deliberately, that ``github_actions`` is *not* + # dependabot's, and both of those sentences name the tool and the label together. + # Distinguishing an attribution from a denial by looking for a negation nearby + # also failed -- the attributing bullet's own sentence ends "so it never opens a + # workflow bump here", so the negation is present in the sentence that makes the + # claim as well as in the two that deny it. + # + # So the page's attribution is *parsed* instead: the one clause that says what + # dependabot puts on its pull requests is located, the labels named inside it are + # read out, and the set is compared against what the configured ecosystems could + # actually produce. Set comparison is what makes it phrasing-independent -- a + # comma for the "and", the labels in the other order, a third label added, all + # compare the same -- and locating the clause is asserted rather than assumed, so + # a rewording that this can no longer read fails loudly instead of passing. + ECOSYSTEM_LABELS = {'pip': {'dependencies', 'python'}, + 'github-actions': {'dependencies', 'github_actions'}} + expected = set().union(*(ECOSYSTEM_LABELS[eco] for eco in ecosystems)) + + # `findall`, not `search`: a first-match-only read checks one attributing + # clause and lets a second, contradicting one through. That is the same + # walk-past-the-check defect this whole rewrite exists to remove, so every + # clause matching the shape is required to name the same set. + clauses = re.findall(r'\*\*dependabot\*\* puts (.+?) on its own pull requests', + self.flat) + self.assertTrue( + clauses, + 'the page no longer states which labels dependabot puts on its own pull ' + 'requests in the shape this test reads, so the attribution is unpinned -- ' + 're-derive it rather than dropping the check, because naming a hand-applied ' + "label as dependabot's is this section's own point stated backwards") + for claimed in clauses: + self.assertEqual( + set(re.findall(r'``([^`]+)``', claimed)), expected, + 'the page attributes a different set of labels to dependabot than the ' + f'configured ecosystems {ecosystems} can produce. Claimed: ' + f'{claimed!r}') + + # And the positive half, which the set comparison above cannot reach: the page + # has to say outright that ``github_actions`` is hand-applied. Without this, a + # page that simply stopped mentioning the label would satisfy everything above. + self.assertIn('hand-applied', self.flat, + 'the page no longer says github_actions is hand-applied, so a ' + 'reader is left to assume the label arrives automatically') self.assertTrue(templated, 'no issue template sets a label any more, so the page is now '