From 726dd364caa43018127cd0b1d26d31065970a463 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Wed, 30 Sep 2026 09:18:49 -0400 Subject: [PATCH 1/2] docs(contributing): record the get-override rulings on the conventions page (#918) Part 1 of #918: the rulings that settled what a ``get`` override owes ``EnumLookup.get`` lived only in issue and PR comments, where nothing keeps them findable. Part 2 split ``conventions.rst`` into a page per anchor so they have somewhere to land; this writes them onto the registry-protocol page. - Add *What a ``get`` Override May and May Not Do*, carrying the three rulings (#933 on ``quiet=True``, #935 on honouring an advertised signature rather than suppressing it, #940 on deleting an override that only reimplements the base) plus the ``@classmethod`` requirement, which is a language constraint rather than a ruling. Each is recorded with the reasoning, not the outcome: the code already encodes the outcome, and the reasoning is what is expensive to rediscover. - Paraphrase the rulings rather than block-quoting the maintainer, per his request on #918, and pin the page's *claim* in the tests instead of his wording. Quoting him verbatim had made an off-hand reply load-bearing in CI: an assertion required the literal ``I prefer (2) directly.`` to appear on a docs page. The quotes that predate this change are tracked in #949. - State the ``@classmethod`` constraint from measurement, on every supported version. Zero-argument ``super()`` binds the enclosing function's **first positional parameter**, whatever its name, so in ``@staticmethod def get(key, ...)`` it binds the lookup key and raises ``TypeError``. CPython words that error differently either side of 3.13 -- ``obj must be an instance or subtype of type`` on 3.10-3.12, ``obj (instance of str) is not an instance or subtype of type (Cls)`` on 3.13+ -- so the page shows both and the test asserts only ``instance or subtype of type``, which is common to them. Pinning either full sentence passes on two of the five required Compat legs and fails the other three, invisibly, since this venv is 3.14. - Record the two corollaries that make the obvious summary wrong in both directions: ``RuntimeError: super(): no arguments`` needs a function with no parameters at all, which no real override has; and an instance first argument makes the delegation **succeed silently**, so a ``@staticmethod`` override cannot be relied on to fail loudly. - Correct the audit row #940 falsified. It claimed **two** of the ``mh.py``/``ngap.py`` group define a ``get`` of their own; #940 deleted both, so the count is now none. That row rendered fine and failed nothing while naming methods that no longer exist. - Extend ``tests/project/test_conventions_doc_claims.py`` with ``GetOverrideContractTests``: four tests, ten subtests. The ``@classmethod`` test **executes** all three ``super()`` outcomes rather than grepping the page for an error string -- a prose-only assertion is what let a wrong exception stand. The ``sys.tracebacklimit`` test runs its behavioural check unconditionally instead of skipping it when the attribute is already set, which had made it degrade to a doc-text check that still reported pass. tests/project: 210 passed, 1 skipped, 572 subtests. Each new assertion was shown to fail with its claim removed; the tracebacklimit check verified to run, fail when the base raises loudly, and restore prior state with ``sys.tracebacklimit`` pre-set; and the ``TypeError`` wording measured on 3.10 through 3.14. --- .../conventions/registry-protocol.rst | 134 +++++++++- tests/project/test_conventions_doc_claims.py | 242 ++++++++++++++++++ 2 files changed, 368 insertions(+), 8 deletions(-) diff --git a/docs/source/contributing/conventions/registry-protocol.rst b/docs/source/contributing/conventions/registry-protocol.rst index fad5742f6..a8c3b5ee0 100644 --- a/docs/source/contributing/conventions/registry-protocol.rst +++ b/docs/source/contributing/conventions/registry-protocol.rst @@ -165,6 +165,123 @@ exists for. So a ``get`` override that catches a name miss as control flow is following the convention; one that catches a *value* miss that way is silencing a logged error, and needs a reason. +What a ``get`` Override May and May Not Do +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Three rulings settled what an override owes +:meth:`~pcapkit.corekit.enum.EnumLookup.get`, and the last of them deleted two +overrides outright. They are collected here because each was reached by the same +argument: the base's contract is house-wide, so an override that diverges from it is +a defect rather than a local judgement about its own callers. The first item below is +not a ruling but a language constraint, recorded with them because the three rulings +all presuppose it. + +**An override that delegates is a** ``@classmethod``, and this one is forced by +Python rather than decided. Zero-argument :func:`super` binds the enclosing +function's **first positional parameter** as its instance, whatever that parameter +is named -- so inside ``@staticmethod def get(key, default=NO_DEFAULT)`` it binds +``key``, the lookup key, and ``return super().get(key)`` fails on the key rather +than reaching the base. On Python 3.13 and newer:: + + TypeError: super(type, obj): obj (instance of str) is not an instance or + subtype of type (FastBindingAcknowledgmentStatus). + +and on 3.10 through 3.12, where CPython words it differently and names neither the +instance nor the type:: + + TypeError: super(type, obj): obj must be an instance or subtype of type + +Both forms share ``instance or subtype of type``, which is the only part of the +message anything here relies on -- pinning either full sentence would pass on two of +the five supported versions and fail on the other three. + +Two corollaries worth stating, because the obvious summary of this is wrong in both +directions. ``RuntimeError: super(): no arguments`` is a *different* failure, raised +only when the enclosing function takes no parameters at all -- no real ``get`` +override qualifies, since every one takes ``key``. And the ``TypeError`` is not +guaranteed either: pass a first argument that *is* an instance of the class and the +delegation **silently succeeds**, so a ``@staticmethod`` override cannot even be +relied on to fail loudly. Measured, all three cases, rather than reasoned about. +:meth:`~pcapkit.corekit.enum.EnumLookup.get` is itself a ``@classmethod``. The +precedent is `#913 `__, whose +``FEATCode.get`` is ``@classmethod def get(cls, key, default=NO_DEFAULT)`` ending in +``return super().get(key, default)``; +`#908 `__ followed it, which is +what turned ``Method.get`` into a classmethod. + +Callers cannot see the switch -- ``Method.get('X')`` binds identically either way -- +so there is no compatibility argument for keeping the ``@staticmethod``. Two +``@staticmethod`` overrides do survive and are still correct: +:class:`~pcapkit.const.ftp.command.Command`'s and +:class:`~pcapkit.const.pcapng.option_type.OptionType`'s never call ``super()`` at +all, so neither meets the condition. + +**Raise the way the base raises, which means** ``quiet=True``. +`#933 `__ asked whether two +overrides raising :exc:`~pcapkit.utilities.exceptions.EnumKeyError` **without** +``quiet=True`` should adopt the base's. The owner first declined, then reversed +himself: they should follow the house convention and not be loud. + +Both answers are on the issue deliberately, and the reversal is the ruling. What it +settles is not the one keyword -- it is the tie-breaker. A loud +:class:`~pcapkit.utilities.exceptions.BaseError` sets :data:`sys.tracebacklimit` to +``0`` **process-wide**, the +`#362 `__ hazard, so loudness is +paid for by the whole library rather than by the override's own callers. The argument +against changing them was that ``quiet=True`` exists on the base for a name miss +inside a *successful* call at ``Method.get`` and these two had no such caller; +uniformity beat it, because a per-class judgement about present callers cannot price +a process-wide effect. + +**A signature the base advertises has to be honoured, not suppressed.** Re-parenting +onto :class:`~pcapkit.corekit.enum.EnumLookup` gave those same two classes an +inherited two-argument ``get(key, default)`` that their one-argument overrides then +refused:: + + FastBindingAcknowledgmentStatus.get('bogus', 'Handover_Accepted') + TypeError: get() takes 1 positional argument but 2 were given + +A ``# type: ignore[override] # pylint: disable=arguments-differ`` pair hid the +mismatch from ``mypy`` and ``pylint``, and both docstrings disclosed it in prose +instead. Put to the owner on +`#935 `__ as one of three options +-- widen and delegate, refuse ``default`` explicitly with an in-library error, or +leave the disclosure as the settled answer -- he took the first. So +**a suppression plus a docstring is not an answer to a contract the class advertises +and breaks.** The check that the suppression was load-bearing is the one to repeat +before believing any such pair: stripping these two yielded +``Signature of "get" incompatible with supertype "EnumLookup" [override]`` at both +lines, with ``--warn-unused-ignores`` reporting neither as unused. + +**And an override that only reimplements the base is deleted, not repaired.** Widening +those two signatures made them faithful copies of the base. Rather than merge them, +the owner asked on `#940 `__ why the +two overrides needed to exist at all, if they could simply fall back to the base's. + +They could. Nine cases per class -- name hit, name miss, value hit, value miss and +every ``default`` combination -- differed from ``EnumLookup.get.__func__(cls, ...)`` +in **zero** of them, and neither class carried an alias (``__members__`` 6 and 4, +``list(cls)`` 6 and 4) for the *"Backport support for original codes"* in their +docstrings to refer to. Offered the choice between merging the widened copies and +deleting them in a follow-up, he ruled for deleting them outright. Both ``get`` +methods and +both suppressions went with it, and :mod:`pcapkit.protocols.internet.mh` now defines +no ``get`` at all. + +The one input where the copies **did** differ is why this matters beyond line count, +and it is the trap for whoever writes the next override: the base branches on +``isinstance(key, str)`` and treats everything else as a *value*, while those two +branched on ``isinstance(key, int)`` and fell through to the *name* path for anything +else. ``get(None)`` therefore raised a quiet +:exc:`~pcapkit.utilities.exceptions.EnumKeyError` on two classes and a loud +:exc:`~pcapkit.utilities.exceptions.EnumValueError` on the other five, and no prose +anywhere said so. Re-implementing the dispatch is how an override acquires a +divergence nobody wrote down; delegating to it is how it does not. + +Taken with the ``Criticality.get`` deletion below -- an override emptied by #923 +rather than by redundancy -- the rule generalises: **an override justifies itself by +what it adds to the base, and goes when the answer is nothing.** + Case Sensitivity Is RFC-Directed ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -344,15 +461,16 @@ That leaves the classes with something to decide: :mod:`~pcapkit.protocols.application.ngap` helper enumerations - IANA Mobility Header registries, 3GPP TS 38.413 - Their values are numeric codes, so the criterion is vacuous exactly as for the - :class:`int` tier above. **Two** of them define a ``get`` of their own -- - ``FastBindingAcknowledgmentStatus`` and ``IPv6AddressPrefixCode`` -- for + :class:`int` tier above. **None** of them defines a ``get`` of its own any + more. Two did when this audit was taken -- + ``FastBindingAcknowledgmentStatus`` and ``IPv6AddressPrefixCode``, for signature reasons (no ``default``, and an :class:`int`/:class:`str` dispatch) - rather than for case: each does an exact ``Cls[key]``, and since #923 each - answers a name miss with - :exc:`~pcapkit.utilities.exceptions.EnumKeyError` rather than - :exc:`~pcapkit.utilities.exceptions.EnumValueError`. ``LMAAddressCode`` and - ``LocalizedRoutingStatus`` carry no ``get`` at all, so they have no string - lookup to fold. ``Criticality`` had one when this audit was taken and no + rather than for case -- and + `#940 `__ deleted both as + redundant, per the ruling in the section above; each now inherits ``get`` + from :class:`~pcapkit.corekit.enum.EnumLookup` unchanged. ``LMAAddressCode`` + and ``LocalizedRoutingStatus`` never carried a ``get`` at all, so they had no + string lookup to fold. ``Criticality`` had one when this audit was taken and no longer does: #921 re-parented it onto :class:`~pcapkit.corekit.enum.EnumLookup` and #923 retired the exception conversion that was the override's only remaining job, so it now inherits diff --git a/tests/project/test_conventions_doc_claims.py b/tests/project/test_conventions_doc_claims.py index 516bad25c..4da6208b9 100644 --- a/tests/project/test_conventions_doc_claims.py +++ b/tests/project/test_conventions_doc_claims.py @@ -36,6 +36,14 @@ * :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`. +* :class:`GetOverrideContractTests` -- what a ``get`` override owes the base: + ``@classmethod`` for delegation, which Python forces rather than anyone ruling + (#913's precedent, followed by #908), plus the three rulings part 1 harvested -- + ``quiet=True`` on the raise (#933), no suppression standing in for an honoured + signature (#935), and deletion rather than repair when the override only + reimplements the base (#940). Includes the guard for the audit row #940 falsified, + which rendered fine and failed nothing while claiming two overrides that no longer + exist. * :class:`AenumRoleExclusionTests` -- GitHub issue #934 part C's ruling that ``aenum`` cannot be cross-referenced at all (``conf.py`` excludes it: its ``objects.inv`` carries zero ``py:`` objects), converted to plain literals rather @@ -58,6 +66,7 @@ import enum import importlib +import inspect import pathlib import pkgutil import re @@ -693,5 +702,238 @@ def test_the_qualified_sentinel_targets_stay_qualified(self) -> 'None': f'the split conventions pages, expected {count}') +class GetOverrideContractTests(unittest.TestCase): + """What a ``get`` override owes the base, as recorded onto *Where the registry + protocol lives* by GitHub issue #918 part 1. + + Three of the four items are quoted from the owner on their own threads -- #933 on + ``quiet=True``, #935 on an advertised signature that is refused, #940 on deleting + an override that only reimplements the base. The fourth, the ``@classmethod`` + requirement for delegation, is **not** a ruling and is not quoted as one: it is + forced by the language, since zero-argument :func:`super` inside a + ``@staticmethod`` has nothing to bind, and #913 set the shape that #908 then + followed. Each states something the tree can be asked about. The page's + half is what rots: the same four claims are already pinned in ``tests/corekit`` + and ``tests/protocols`` against the *code*, so a later change that moves the code + fails there, while a page still describing the old shape fails nothing. This + class is the other direction, and it is how the stale audit row this change + corrected (*"Two of them define a ``get`` of their own"*, false since #940) + would have been caught. + + """ + + @staticmethod + def _flat() -> 'str': + """:meth:`_page`'s ``registry-protocol`` text, whitespace-normalised. + + Runs of whitespace are collapsed so a claim can be matched across the line + wraps reStructuredText puts in mid-sentence -- the same normalisation + :class:`FailedLookupExceptionTests` uses. + + """ + return ' '.join(_page('registry-protocol').split()) + + def test_a_delegating_override_is_a_classmethod(self) -> 'None': + """A delegating override is a ``@classmethod``, because the language says so. + + Not a ruling: zero-argument :func:`super` inside a ``@staticmethod`` has no + first argument to bind, so a delegating ``@staticmethod`` cannot work at all. + #913 set the shape and #908 followed it, producing ``Method.get``. The two + surviving ``@staticmethod`` overrides are the stated exception -- neither + calls ``super()``, so neither meets the condition. Read through :func:`vars` + rather than by attribute access, since both descriptor kinds answer + ``Cls.get('X')`` identically, which is the page's own point about callers not + seeing the switch. + + """ + from pcapkit.const.ftp.command import Command + from pcapkit.const.http.method import Method + from pcapkit.const.pcapng.option_type import OptionType + + self.assertIsInstance(vars(Method)['get'], classmethod, + 'a delegating override has to be a classmethod -- ' + 'zero-argument super() inside a staticmethod raises ' + 'RuntimeError; #913 set the shape, #908 followed it') + for klass in (Command, OptionType): + with self.subTest(klass=klass.__name__): + self.assertIsInstance(vars(klass)['get'], staticmethod) + self.assertNotIn('super()', inspect.getsource(vars(klass)['get'].__func__), + f'{klass.__name__}.get now delegates, so the page is ' + 'wrong to list it as a surviving staticmethod') + + # Execute the three outcomes rather than assert the page names them. The + # first draft of this test only grepped for the error string, and the + # string it grepped for was the wrong one -- the page claimed + # `RuntimeError: super(): no arguments` for a shape that actually raises + # `TypeError`, and a prose-only assertion could not tell. + class _Base: + @classmethod + def get(cls, key, default=None): # noqa: D102 + return f'base:{key}' + + class _WithParam(_Base): + @staticmethod + def get(key, default=None): # noqa: D102 + return super().get(key) # type: ignore[misc] + + class _NoParams(_Base): + @staticmethod + def get(): # type: ignore[override] # noqa: D102 + return super().get('x') # type: ignore[misc] + + with self.assertRaises(TypeError) as caught: + _WithParam.get('BASELINE-CONTROL') + # Substring chosen to survive CPython's own rewording: 3.10-3.12 say + # "obj must be an instance or subtype of type" while 3.13+ say "obj + # (instance of str) is not an instance or subtype of type (Cls)". Only + # `instance or subtype of type` is common to both, and pinning either + # full sentence would fail three of the five required Compat legs -- + # invisible locally, since this venv is 3.14. + self.assertIn('instance or subtype of type', str(caught.exception), + 'zero-argument super() in a staticmethod binds the first ' + 'positional parameter -- the lookup key -- as its instance') + + with self.assertRaises(RuntimeError) as caught_runtime: + _NoParams.get() + self.assertIn('super(): no arguments', str(caught_runtime.exception), + 'the no-parameter case is the one that raises RuntimeError; ' + 'the page must not attribute it to a get(key) override') + + # And the case that makes a staticmethod override actively unsafe rather + # than merely broken: an instance first argument delegates silently. + self.assertEqual(_WithParam.get(_WithParam()).split(':')[0], 'base', + 'a staticmethod override cannot be relied on to fail ' + 'loudly, which is why the page says so') + + flat = self._flat() + self.assertIn('instance or subtype of type', flat, + 'the page no longer shows what a staticmethod delegation ' + 'actually raises') + self.assertIn('must be an instance or subtype of type', flat, + 'the page no longer records the 3.10-3.12 wording, so a ' + 'reader on those versions cannot match what they see') + self.assertIn('silently succeeds', flat, + 'the page no longer records that the TypeError is not ' + 'guaranteed') + self.assertIn('forced by Python rather than decided', flat, + 'the page no longer says the classmethod requirement is a ' + 'language constraint rather than a ruling') + + def test_the_base_raises_a_name_miss_quietly(self) -> 'None': + """#933: the owner ruled that overrides follow the base and stay quiet. + + Measured rather than read off the source, because the cost the ruling turns + on is the side effect: a loud :class:`BaseError` sets + :data:`sys.tracebacklimit` to ``0`` for the whole process (#362), and a + quiet one leaves it alone. + + """ + import sys + + from pcapkit.const.ftp.command import FEATCode + from pcapkit.utilities.exceptions import EnumKeyError + + had = hasattr(sys, 'tracebacklimit') + before = getattr(sys, 'tracebacklimit', None) + + def _restore() -> 'None': + if had: + sys.tracebacklimit = before # type: ignore[assignment] + elif hasattr(sys, 'tracebacklimit'): + del sys.tracebacklimit + + self.addCleanup(_restore) + + # Run the behavioural check unconditionally. An earlier version guarded it + # with `if not had:`, which meant a test that had already set + # sys.tracebacklimit turned this into a prose-only check that still + # reported pass -- silent degradation under test-order pollution rather + # than a failure. Clearing it first is safe because _restore puts whatever + # was there back. + if had: + del sys.tracebacklimit + with self.assertRaises(EnumKeyError): + FEATCode.get('ZZ-NOT-REAL') + self.assertFalse(hasattr(sys, 'tracebacklimit'), + 'a name miss set sys.tracebacklimit process-wide, so the ' + "base's raise is no longer quiet -- GitHub issue #933 " + '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") + self.assertIn('``quiet=True``', flat) + + def test_the_two_redundant_overrides_are_gone(self) -> 'None': + """#940: the owner ruled for deleting the redundant overrides, not widening them. + + Three things at once, because the ruling is only settled if all three hold: + neither class defines ``get``, the module defines none at all, and the + ``[override]``/``arguments-differ`` pair the old signatures needed went with + them. Scoped to that exact pair rather than to ``arguments-differ`` alone, + which mh.py still carries four times for ``read`` and ``__post_init__`` -- + unrelated to any ``get``, and measured before asserting on it. + + """ + from pcapkit.protocols.internet import mh + from pcapkit.protocols.internet.mh import (FastBindingAcknowledgmentStatus, + IPv6AddressPrefixCode) + + for klass in (FastBindingAcknowledgmentStatus, IPv6AddressPrefixCode): + with self.subTest(klass=klass.__name__): + self.assertNotIn('get', vars(klass), + f'{klass.__name__} defines get again; GitHub pull ' + 'request #940 deleted it as redundant with ' + 'EnumLookup.get') + + source = pathlib.Path(mh.__file__).read_text(encoding='utf-8') + self.assertNotIn('def get(', source, + 'pcapkit/protocols/internet/mh.py defines a get override ' + 'again, which the page says it does not') + self.assertNotIn('type: ignore[override] # pylint: disable=arguments-differ', + source, + 'the signature-mismatch suppression pair is back in mh.py; ' + '#935 ruled a suppression is not an answer to a refused ' + 'signature') + + flat = self._flat() + self.assertIn('deleting them outright', flat, + 'the page no longer records that #940 ruled for deletion ' + 'rather than widening') + self.assertIn('he took the first', flat, + 'the page no longer records which of #935\'s three options ' + 'was taken') + self.assertNotIn('**Two** of them define a ``get`` of their own', flat, + 'the audit row claims two mh helpers still override get, ' + 'which #940 made false') + + def test_a_non_string_key_raises_a_value_miss_on_all_seven(self) -> 'None': + """The divergence #940's deletion closed, which is the page's worked reason. + + The base branches on ``isinstance(key, str)`` and treats everything else as + a value, so ``get(None)`` is a *value* miss. The deleted overrides branched + on :class:`int` and fell through to the name path, making it a ``KeyError`` + on two of the seven and a ``ValueError`` on the other five. + + """ + from pcapkit.protocols.internet.mh import (FastBindingAcknowledgmentStatus, + IPv6AddressPrefixCode, + LMAAddressCode) + from pcapkit.utilities.exceptions import EnumValueError + + for klass in (FastBindingAcknowledgmentStatus, IPv6AddressPrefixCode, + LMAAddressCode): + for key in (None, 1.5): + with self.subTest(klass=klass.__name__, key=key): + with self.assertRaises(EnumValueError): + klass.get(key) # type: ignore[arg-type] + + flat = self._flat() + self.assertIn('``isinstance(key, str)`` and treats everything else as a ' + '*value*', flat, + "the page no longer records the base's own key dispatch, which " + "is the reason #940's deletion converged the seven") + + if __name__ == '__main__': unittest.main() From e4340e8a654e343c4356c9cb43f1eff5a3b71686 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Wed, 30 Sep 2026 10:03:56 -0400 Subject: [PATCH 2/2] docs(contributing): add the process conventions page (#918) Three settled rulings fit none of the four code-convention pages, because they govern the repository rather than the library. The owner ruled on #918 that they get a fifth page rather than staying in their threads. - Add ``docs/source/contributing/conventions/process.rst`` (``.. _process:``), covering what the ``all`` extra carries (#910), what a changelog entry is, and what the issue and pull request labels mean. Paraphrased throughout rather than quoting the owner, on his instruction on the same issue. - Record the changelog grouping as ruled: a section per top-level module with ``Added``/``Changed``/``Fixed`` nested inside each. The restructure belongs to #657, which owns the file and merges last. The one case the rule does not settle, an entry spanning modules, is flagged rather than decided. - Describe the file's **current** shape from measurement. An earlier draft called it "three flat kind-runs", carried from a comment nobody had checked; the 80 entries actually carry the three kind labels in **26** runs. - Document the label scheme the owner asked for alongside ``breaking``, and name **both** automated paths: dependabot, and the issue templates, which apply a label from front matter before anyone reads the issue. An earlier draft called dependabot the single exception. - Correct two ``breaking`` claims measurement contradicts. It **is** applied below #350 -- carriers ``#3``-``#28``, nothing between ``#28`` and ``#350``, issues from ``#775``. And no ruling defines the label, so the page states the behavioural test rather than attributing a phrasing to anyone. - Correct the audit table's population to **six** -- the ``mh``/``ngap`` helpers ``EnumLookup`` subclasses those modules define, ``ProcedureCode``/``ProtocolIE`` being re-exports. The page had carried three figures for one population. Tests, and the reason each exists rather than the shape of it: - ``ProcessConventionTests`` pins the page's claims against the tree. Two were rewritten because they could pass for the wrong reason: one read only ``dependabot.yml`` and never ``ISSUE_TEMPLATE/``, and one asserted the bare string ``26``, which the page's own ``wc -l`` output satisfied. - ``test_a_non_string_key_raises_a_value_miss_on_every_helper`` **derives** the population instead of listing it. Its predecessor was named ``..._on_all_seven`` and exercised three. - ``test_every_issue_link_number_matches_its_own_url`` closes the largest class of unpinned claim: a cross-review corrupted ~30 link numbers and not one was caught, because nothing compared the displayed ``#NNN`` against the number in its own URL. Invisible to a reader, unwarnable by Sphinx. - ``_optional_dependencies`` replaces ``tomllib``, which is 3.11+ and broke the ``Python 3.10`` leg while passing locally on 3.14 -- ``tomli`` is not a dependency either, so there was nothing to fall back to. It now strips comments and tracks bracket **depth**: a version matching to the first ``]`` mis-parsed **3 of the 14** extras silently, emptying ``vendor`` on the ``]`` inside ``"requests[socks]"``, truncating ``dev``, and **inventing a requirement called** ``9 skipped`` **out of comment prose**. No shipped assertion read those keys, but ``dev`` and ``vendor`` are both described in the page's prose, so the next assertion to check either would have got wrong data with nothing raised. ``test_the_extras_reader_agrees_with_tomllib`` now compares the two key for key wherever a real parser exists, and skips on 3.10 rather than pretending to. - The page states plainly what cannot be pinned here: any claim whose ground truth is a GitHub query, since CI has no network. The command beside the figure is the pin, run by a reader rather than by CI. tests/project: 221 passed, 1 skipped, 640 subtests. The extras reader verified byte-identical under python3.10 and 3.14, and every new assertion shown to fail with its claim removed or its defect reintroduced. --- .../source/contributing/conventions/index.rst | 5 +- .../contributing/conventions/process.rst | 299 ++++++++ .../conventions/registry-protocol.rst | 7 +- tests/project/test_conventions_doc_claims.py | 650 +++++++++++++++++- 4 files changed, 933 insertions(+), 28 deletions(-) create mode 100644 docs/source/contributing/conventions/process.rst diff --git a/docs/source/contributing/conventions/index.rst b/docs/source/contributing/conventions/index.rst index c2c76f035..7a7a22cf9 100644 --- a/docs/source/contributing/conventions/index.rst +++ b/docs/source/contributing/conventions/index.rst @@ -12,7 +12,9 @@ House Conventions questions have mostly arisen, and the page was titled *Registry Conventions* until `#918 `__ widened it, then split it. :ref:`extension-header-subclassing` is the first ruling - here that governs a protocol class hierarchy rather than a registry. + here that governs a protocol class hierarchy rather than a registry, and + :ref:`process` the first that governs the repository rather than any of its + code. **A ruling that stays in its thread is a ruling that gets rediscovered.** So when a question is answered in a way the code cannot express on its own -- a @@ -29,3 +31,4 @@ House Conventions sentinel-convention registry-protocol extension-header-subclassing + process diff --git a/docs/source/contributing/conventions/process.rst b/docs/source/contributing/conventions/process.rst new file mode 100644 index 000000000..eb5f4769d --- /dev/null +++ b/docs/source/contributing/conventions/process.rst @@ -0,0 +1,299 @@ +.. _process: + +How the repository itself is run +-------------------------------- + +The four pages before this one are about writing library code. The rulings here are +about running the repository -- what an install carries, what a changelog entry is, +and what the issue and pull request labels mean. None of them is derivable from a +module, which is why they are recorded rather than left to be rediscovered. + +The page exists because those three settled questions fit none of the code-convention +pages, and the owner ruled on +`#918 `__ that they get a page of +their own rather than being left in their threads. Each ruling below is **paraphrased +rather than quoted**, also on his instruction there; the issue named beside it is +where the original wording is. + +What the ``all`` extra carries +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +``all`` means **core addons only** -- the things that let the library itself run at +full functionality -- rather than everything a user might conceivably want. The owner +settled that on `#910 `__ and named +the three that qualify to date: the CLI addon, the crypto addon, and ``pycrate``. + +On the tree, in :file:`pyproject.toml`: + +.. code-block:: toml + + cli = [ "emoji" ] + crypto = [ "cryptography>=3.4" ] + NGAP = [ "pycrate" ] + + all = [ "emoji", "cryptography>=3.4", "pycrate" ] + +So ``all`` is exactly the union of those three extras, written out as literals rather +than referenced -- eight requirements before the ruling, three after it. + +Two groups stay out, and the reasons are **different** rather than two versions of +one reason. Keeping them apart is what stops the list drifting the way it already did +once: + +* **The third-party capture engines are excluded by kind.** ``DPKT``, ``Scapy``, + ``PyShark``, ``PyPCAPFile``, ``PyPCAP`` and ``PCAP_CT`` are not core addons, so + they are on demand -- one extra at a time, ``pip install pypcapkit[Scapy]`` and so + on. This holds however easy they are to install, and it is what the ruling + changed: four of them used to be in ``all``. +* **``PyPCAP`` and ``PCAP_CT`` were already excluded for installability**, which the + ruling leaves untouched. ``pypcap`` is an sdist-only C extension needing a + compiler and the libpcap development files; ``pcap-ct`` and ``libpcap`` are + published only as pre-releases, and ``all`` should not be how somebody acquires a + beta they did not ask for. + +``vendor`` is the crawler dependency set and is not for end users, so it is out on the +same audience grounds. Narrowing ``all`` costs a user nothing at run time: +:meth:`Extractor.run ` warns and falls +back to the default engine rather than raising when a requested engine is absent. + +.. note:: + + The ``dev`` extra exists **because** of this narrowing, and is not a second + catch-all. pylint, mypy and autodoc all resolve imports against what is installed, + so the four engines leaving ``all`` would have made them newly unresolvable to the + toolchain. ``dev`` is defined by what the toolchain must be able to *see*, and the + workflows that need full resolution install ``.[all,dev]``. Do not add + ``pypcap``/``pcap-ct`` to it to silence a lint finding; :file:`.github/workflows/lint.yml` + carries a tracked ``import-error`` count that is deliberate rather than accidental. + +What a changelog entry is +~~~~~~~~~~~~~~~~~~~~~~~~~ + +An entry is **not one line per commit**. Group the changes by topic, and give concise +detail of what actually changed in that version bump. Ruled on +`#918 `__. + +Two different things get confused here, so they are named apart: + +* **A pull request's commits.** `#657 `__ + is the shared changelog for the 1.5.0 cycle, and it carries roughly one commit per + code pull request, deliberately unsquashed so that what is and is not accounted for + stays readable in its log. That is a property of *the pull request*, and it is not + what the ruling is about. +* **A changelog file's entries.** :file:`docs/source/changelog/1.5.0.rst` groups its + entries under ``* **Added**``, ``* **Changed**`` and ``* **Fixed**`` today, and a single + entry routinely cites several issues at once -- *"the Mobility Header registry, + completed (#383, #437)"* is one bullet, not two. That is a property of *the file*, + and it is the axis the ruling governs. + +Both are measurable rather than matters of memory, which is the point of writing the +commands down instead of a figure that will be stale by the next merge: + +.. code-block:: shell + + # entries in the file, and the kind headings they group under + grep -cE '^\* \*\*' docs/source/changelog/1.5.0.rst + grep -oE '^\* \*\*[A-Za-z]+\*\*' docs/source/changelog/1.5.0.rst | sort | uniq -c + + # commits on the pull request -- a different number, about a different thing + gh pr view 657 -R JarryShaw/PyPCAPKit --json commits -q '.commits|length' + +The grouping scheme was settled after that, on #918: **a section per top-level +module, with** ``Added``/``Changed``/``Fixed`` **nested inside each** -- module +granularity, not per-file and not per-subpackage. The file today is neither shape: its +80 entries carry the three kind labels in **26** separate runs, roughly blocked for the +first 40% of the file and thoroughly interleaved after it -- so the restructure is a +regrouping of scattered entries rather than a transposition of three tidy blocks:: + + $ grep -oE '^\* \*\*[A-Za-z]+\*\*' docs/source/changelog/1.5.0.rst \ + | uniq -c | wc -l + 26 + +The target is one section per top-level module: + +.. code-block:: shell + + ls -d pcapkit/*/ | sed 's|pcapkit/||;s|/||' # const corekit dumpkit foundation + # interface protocols toolkit + # utilities vendor + +.. note:: + + One case the rule does not settle by itself: an entry whose change spans modules -- + the reassembly and extraction ones touch :mod:`pcapkit.foundation` and + :mod:`pcapkit.protocols` together. The intent is to file each under the module the + change is *about* and name the others in the text, rather than duplicating the + entry, but that has not been ruled on. Raised on #918. + + The restructure itself belongs to #657, which owns the file and merges last; doing + it earlier would conflict with every open change that touches an entry. + +How the issue and pull request labels work +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The owner asked on `#918 `__ for +this to be written down alongside ``breaking``, since ``breaking``'s meaning only +makes sense against the scheme it sits in. + +**Almost every label is applied by hand, by the owner** -- so a label is generally a +statement someone made rather than a value derived from the change. That matters most +for the ``review:`` family below: such a label is not evidence of the state it names, +it is a record that the owner asserted it. + +Two paths are automated, and both are worth knowing about because a label they set has +had no human judgement behind it: + +* **dependabot** puts ``dependencies`` and ``python`` on its own pull requests, and + only those two: :file:`.github/dependabot.yml` configures a single ecosystem, + ``pip``, so it never opens a workflow bump here. ``github_actions`` exists as a + label but is **hand-applied like the rest** -- naming it as dependabot's would tell + a reader the opposite of this section's point. +* **The issue templates** apply an issue-kind label from their front matter, before + anyone reads the issue -- :file:`.github/ISSUE_TEMPLATE/bug_report.md` carries + ``labels: bug`` and :file:`.github/ISSUE_TEMPLATE/feature_request.md` carries + ``labels: enhancement``. So ``bug`` and ``enhancement`` on a template-opened issue + are defaults rather than assessments. + +Nothing else automates a label. :file:`.github/release.yml` only *reads* existing +labels to bucket release notes, and the ``PCAPKIT_CONDA_LABEL`` in the release +workflows is a conda channel label, unrelated to these. + +The ones that carry meaning here fall into five groups, which stack rather than +compete: a pull request normally carries one from the first group and as many of the +rest as apply. **The five groups are not the whole label set** -- the repository also +has GitHub's own defaults, of which ``wontfix``, ``invalid``, ``help wanted`` and +``duplicate`` are all in live use -- only ``good first issue`` has never been +applied. They are documented by +GitHub rather than here, and are counted rather than listed so this page does not go +stale every time one is added:: + + $ gh label list -R JarryShaw/PyPCAPKit --limit 100 --json name -q '.[].name' | wc -l + 29 + +One of those defaults carries a local ruling worth knowing: an issue closed as +unnecessary takes ``invalid`` (or the nearest applicable) rather than ``bug``, since +the issue was not a defect. `#275 +`__ is where it was applied -- +``bug`` removed and ``invalid`` added in the same second -- and `#707 +`__ is the worked example, closed +as invalid because it was filed against ``main`` rather than against the pull +request's diff. + +**Type -- what kind of change it is.** Each corresponds to the subject prefix of the +commit, so the label and the message agree by construction: + +.. list-table:: + :header-rows: 1 + :widths: 22 78 + + * - Label + - Applies to + * - ``feat`` + - a new capability + * - ``fix`` + - a defect repaired + * - ``refactor`` + - restructuring for its own sake -- neither a fix nor a new capability + * - ``perf`` + - a performance improvement + * - ``docs`` + - documentation only + * - ``test`` + - tests added or corrected + * - ``ci`` + - CI or workflow configuration + * - ``chore`` + - tooling and repository hygiene, with no library behaviour change + * - ``release`` + - version bumps and distribution rollups + * - ``const`` + - regenerated IANA or vendor constant tables, members keeping their numeric + values + +**Issue kind**, for issues rather than pull requests: ``design`` marks a pattern being +decided rather than a defect or a request, which is the label most of the rulings on +these pages were settled under; alongside ``bug``, ``enhancement`` and ``question``. + +**State -- what is happening to it now.** An open issue is meant to carry one of +these, so that its status is readable without opening it: + +* ``wip`` -- in flight: a covering pull request is open, or an agent is on it. +* ``blocked`` -- deferred behind other work or a decision, with the last comment + saying what unblocks it. The condition is meant to be *checkable* rather than + remembered -- a command someone else can run and get an answer from. +* ``needs: decision`` -- waiting on the owner, and on nothing else. + +Two things the board shows rather than the rule: ``wip`` and ``needs: decision`` +legitimately **co-occur**, when the bulk of an issue is being worked and one +sub-question is held for the owner -- #918 itself was labelled that way while this +page was being written. And an open issue with no state label at all is a gap rather +than a category, which is worth checking for rather than assuming away: + +.. code-block:: shell + + gh issue list -R JarryShaw/PyPCAPKit --state open --limit 100 \ + --json number,labels -q '.[]|"#\(.number) \(.labels|map(.name)|join(","))"' + +**Review -- the cross-review verdict, at the current head.** Separate from CI, which +has its own status of its own: ``review: pending`` means no verdict for this head, +either never reviewed or the head moved since; ``review: good-to-go`` and +``review: needs-changes`` are the two verdicts. Because they are keyed on the head +rather than on the pull request, a new push invalidates the label -- a verdict that +outlives the commit it was given on is worse than none. + +**Scope.** ``dependencies`` and ``python`` are dependabot's, per above. +``github_actions`` is the same kind of label -- it scopes a change to the workflows -- +but nothing applies it automatically, because dependabot is not configured for that +ecosystem here. + +What ``breaking`` marks +~~~~~~~~~~~~~~~~~~~~~~~ + +``breaking`` is **additive** -- it goes on alongside the type label, never instead of +it. Its own description in the label set says so, and defines it as breaking +public-facing behaviour or API. + +So the question it answers is not "how big is this change" but **"can a caller +observe the difference without changing their code"**. On the tree, the changes +carrying it are that kind: + +* an exception type a caller catches -- ``#811`` raising + :exc:`~pcapkit.utilities.exceptions.ProtocolError` where a bare + :exc:`struct.error` used to escape, and ``#783`` raising one where a single-bit + lookup used to return a member; +* a public attribute's meaning -- ``#635`` swapping ``Frame.len`` and + ``Frame.cap_len`` between the PCAP and PCAP-NG readers; +* a signature or a name a caller writes -- ``#815`` retyping ``AppType.proto`` and + giving ``register_apptype`` varargs, ``#788`` enforcing ``@final`` at runtime; +* a path a caller or a script depends on -- ``#350`` naming the examples directories + apart. + +.. warning:: + + **A pull request's prose and its label can disagree, and the label is not + automatically right.** Both directions have happened here. ``#783`` and ``#811`` + carry the label while their changelog bullets never said so, which a review round + on #657 caught and corrected. ``#848`` carries it too, and its own body argues at + length that the change is *not* breaking -- a review round checked that argument + and found it right on the facts, so there the label is the half that overstates. + So when the two conflict, settle it on what a caller can observe, and fix + whichever of the two is wrong rather than letting the pair stand. + +.. note:: + + **The label is not applied uniformly across the repository's history, and a census + that assumes it is will be wrong.** It is dense on pull requests from ``#350`` + upward and effectively absent below: the only earlier carriers are seven + pre-``0.15`` pull requests, ``#3`` to ``#28``, with nothing at all between ``#28`` + and ``#350``. Three of those seven are distribution rollups (``#25``, ``#26``, + ``#28``, each also carrying ``release``); the other four are early + ``refactor``/``feat`` work from before the project stabilised. On issues it is sparser still, appearing only from + ``#775`` up. So ``breaking``'s absence on an old pull request is weak evidence at + best. The current figures, rather than these: + + .. code-block:: shell + + gh pr list -R JarryShaw/PyPCAPKit --state all --label breaking --limit 200 \ + --json number -q '[.[].number]|sort|@json' + gh issue list -R JarryShaw/PyPCAPKit --state all --label breaking --limit 100 \ + --json number -q '[.[].number]|sort|@json' diff --git a/docs/source/contributing/conventions/registry-protocol.rst b/docs/source/contributing/conventions/registry-protocol.rst index a8c3b5ee0..19fabc572 100644 --- a/docs/source/contributing/conventions/registry-protocol.rst +++ b/docs/source/contributing/conventions/registry-protocol.rst @@ -273,8 +273,9 @@ and it is the trap for whoever writes the next override: the base branches on ``isinstance(key, str)`` and treats everything else as a *value*, while those two branched on ``isinstance(key, int)`` and fell through to the *name* path for anything else. ``get(None)`` therefore raised a quiet -:exc:`~pcapkit.utilities.exceptions.EnumKeyError` on two classes and a loud -:exc:`~pcapkit.utilities.exceptions.EnumValueError` on the other five, and no prose +:exc:`~pcapkit.utilities.exceptions.EnumKeyError` on those two and a loud +:exc:`~pcapkit.utilities.exceptions.EnumValueError` on the other four of the six +locally-defined helpers in those two modules, and no prose anywhere said so. Re-implementing the dispatch is how an override acquires a divergence nobody wrote down; delegating to it is how it does not. @@ -457,7 +458,7 @@ That leaves the classes with something to decide: the CSV columns the crawler reads are lower case in every row (``s`` 26, ``a`` 18, ``s/p`` 3, blank 1; ``o`` 28, ``m`` 27, ``h`` 7, ``m [1]`` 2). - **open** -- see below - * - The 5 :mod:`~pcapkit.protocols.internet.mh` and + * - The 6 :mod:`~pcapkit.protocols.internet.mh` and :mod:`~pcapkit.protocols.application.ngap` helper enumerations - IANA Mobility Header registries, 3GPP TS 38.413 - Their values are numeric codes, so the criterion is vacuous exactly as for the diff --git a/tests/project/test_conventions_doc_claims.py b/tests/project/test_conventions_doc_claims.py index 4da6208b9..ab1792036 100644 --- a/tests/project/test_conventions_doc_claims.py +++ b/tests/project/test_conventions_doc_claims.py @@ -14,8 +14,8 @@ preamble and the toctree. This file pins the checkable claims, plus the split's own structure: -* :class:`ConventionAnchorTests` -- the four ``.. _label:`` anchors, one now per - file. Before the split all four lived on one page, and +* :class:`ConventionAnchorTests` -- every ``.. _label:`` anchor, one now per + file. Before the split the original four lived on one page, and ``tests/corekit/test_sentinel_exports_unit.py`` sliced the file *between* two of them -- which is exactly what GitHub issue #930 named as the split's concrete blocker, since separating those two anchors into different files made that slice @@ -55,6 +55,13 @@ context. Both checks scan every split page rather than one file, since either could in principle land on any of them. +* :class:`ProcessConventionTests` -- the three *process* rulings #918 harvested onto + the fifth page, :file:`process.rst`: what the ``all`` extra carries (#910), what a + changelog entry is, and what the issue and pull request labels mean. Grounded in + :file:`pyproject.toml`, :file:`docs/source/changelog/1.5.0.rst` and + :file:`.github/PULL_REQUEST_TEMPLATE.md` rather than in the owner's phrasing, which + that page is required to paraphrase rather than quote. + Deliberately **not** checked here: whether the page's cross-references resolve. That is a property of the built inventory rather than of the source, for the reason :file:`tests/project/test_documentation_claims.py` gives at length, and the honest check @@ -90,12 +97,18 @@ #: Every ``.. _label:`` anchor, in the narrative order the pre-split page carried #: them in -- which is also the order the index's toctree lists the files in. The -#: first three predate #918; ``extension-header-subclassing`` arrived with it. +#: first three predate #918; ``extension-header-subclassing`` arrived with it, and +#: ``process`` came last, on the owner's ruling that the three settled *process* +#: rulings -- the ``all`` extra, what a changelog entry is, and what the labels +#: mean -- get a page of their own rather than being wedged onto a code-convention +#: page. Its anchor is the bare file stem like the other four, which is what +#: :data:`PAGES` below depends on. ANCHORS = ( 'mint-criterion', 'sentinel-convention', 'registry-protocol', 'extension-header-subclassing', + 'process', ) #: Each anchor's own file, one-to-one since the split -- there is no longer a single @@ -145,7 +158,7 @@ def _every_page() -> 'str': """Every split page's text, concatenated, index included. For the checks that used to scan the single-page file end to end -- a forbidden - role, a qualified reference -- and still need to scan across all four sections + role, a qualified reference -- and still need to scan across every section plus the preamble, since either could in principle land on any of them. """ @@ -258,7 +271,7 @@ class ConventionAnchorTests(unittest.TestCase): """The labels other files cross-reference, one now per file.""" def test_every_page_exists(self) -> 'None': - """The split's own four files, plus the index, are all on disk.""" + """Every page :data:`ANCHORS` names, plus the index, is on disk.""" for path in [INDEX] + [PAGES[anchor] for anchor in ANCHORS]: with self.subTest(page=path.name): self.assertTrue(path.is_file(), f'{path} does not exist') @@ -293,7 +306,7 @@ def test_no_page_carries_a_different_page_s_anchor(self) -> 'None': :meth:`str.index` rather than a wrong answer. #918 part 2 retired the slice instead of working around it -- ``_sentinel_section`` now reads :file:`sentinel-convention.rst` whole. What used to be a hard constraint on - the single page is now a fact about the four files, pinned here so a later + the single page is now a fact about the separate files, pinned here so a later merge back into one page, or a bad copy-paste across two of them, does not silently resurrect it. @@ -315,7 +328,7 @@ def test_the_index_toctree_lists_every_page_in_order(self) -> 'None': inventory does not care whether a page is reachable from a toctree -- but it produces a distinct "document isn't included in any toctree" warning. Order matches :data:`ANCHORS`, the narrative order the single page used to carry - the four sections in. + its sections in, with pages added since appended in the order they arrived. """ entries = _toctree_entries(INDEX.read_text(encoding='utf-8')) @@ -349,7 +362,7 @@ def test_the_index_preamble_does_not_claim_to_be_a_ruling_page_itself(self) -> ' :data:`INDEX` unedited, and its prose was written when the whole thing was one page: *"This page records design rulings"*, and a future one *"is written onto this page"*. Both went false the moment the index stopped - carrying any ruling of its own -- the four children do -- and the second + carrying any ruling of its own -- its children do -- and the second is worse than stale, because it is the standing instruction #918 part 3 exists to keep alive, now telling a contributor to write onto the wrong file. A cross-review caught this on the first PR revision. @@ -736,8 +749,10 @@ def _flat() -> 'str': def test_a_delegating_override_is_a_classmethod(self) -> 'None': """A delegating override is a ``@classmethod``, because the language says so. - Not a ruling: zero-argument :func:`super` inside a ``@staticmethod`` has no - first argument to bind, so a delegating ``@staticmethod`` cannot work at all. + Not a ruling: zero-argument :func:`super` binds the enclosing function's + first positional parameter, so in ``get(key, ...)`` it binds the lookup key + and raises :exc:`TypeError` -- ``RuntimeError: super(): no arguments`` needs a + function with no parameters at all, which no real override has. #913 set the shape and #908 followed it, producing ``Method.get``. The two surviving ``@staticmethod`` overrides are the stated exception -- neither calls ``super()``, so neither meets the condition. Read through :func:`vars` @@ -752,8 +767,9 @@ def test_a_delegating_override_is_a_classmethod(self) -> 'None': self.assertIsInstance(vars(Method)['get'], classmethod, 'a delegating override has to be a classmethod -- ' - 'zero-argument super() inside a staticmethod raises ' - 'RuntimeError; #913 set the shape, #908 followed it') + 'zero-argument super() in a staticmethod binds the ' + 'first positional parameter, so get(key) raises ' + 'TypeError; #913 set the shape, #908 followed it') for klass in (Command, OptionType): with self.subTest(klass=klass.__name__): self.assertIsInstance(vars(klass)['get'], staticmethod) @@ -806,9 +822,21 @@ def get(): # type: ignore[override] # noqa: D102 'loudly, which is why the page says so') flat = self._flat() - self.assertIn('instance or subtype of type', flat, - 'the page no longer shows what a staticmethod delegation ' - 'actually raises') + # Each wording pinned by a needle unique to it. The shared substring + # ``instance or subtype of type`` occurs **three** times on the page -- both + # quoted errors plus the sentence explaining what they share -- so asserting it + # pinned neither: corrupting the 3.13+ quote to something false still passed. + # + # **Fourth instance of one defect shape**, after ``labels: bug`` (a bare token + # matched elsewhere), the bare ``26`` (matched by the page's own ``wc -l`` + # output) and ``quiet=True`` (four occurrences). The rule, stated once here + # because it kept being rediscovered: **an assertIn whose needle appears more + # than once on the page pins nothing** -- a later occurrence satisfies it and + # the claim it guards can be inverted freely. Count occurrences before + # asserting, and anchor to something unique. + self.assertIn('is not an instance or subtype of type', flat, + 'the page no longer shows the 3.13+ wording of what a ' + 'staticmethod delegation raises') self.assertIn('must be an instance or subtype of type', flat, 'the page no longer records the 3.10-3.12 wording, so a ' 'reader on those versions cannot match what they see') @@ -862,7 +890,15 @@ def _restore() -> 'None': flat = self._flat() self.assertIn('not be loud', flat, "the page no longer quotes #933's reversal") - self.assertIn('``quiet=True``', flat) + # 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. + # Third instance of this shape: the `labels: bug` assertion and the bare `26` + # both passed the same way, so the rule is now explicit -- never assert a token + # that appears more than once on the page it is meant to pin. + self.assertIn('Raise the way the base raises, which means** ``quiet=True``', flat, + 'the page no longer states the ruling as its headline, so a ' + 'later mention of quiet=True is doing the work of pinning it') def test_the_two_redundant_overrides_are_gone(self) -> 'None': """#940: the owner ruled for deleting the redundant overrides, not widening them. @@ -907,22 +943,50 @@ def test_the_two_redundant_overrides_are_gone(self) -> 'None': 'the audit row claims two mh helpers still override get, ' 'which #940 made false') - def test_a_non_string_key_raises_a_value_miss_on_all_seven(self) -> 'None': + def test_a_non_string_key_raises_a_value_miss_on_every_helper(self) -> 'None': """The divergence #940's deletion closed, which is the page's worked reason. The base branches on ``isinstance(key, str)`` and treats everything else as a value, so ``get(None)`` is a *value* miss. The deleted overrides branched on :class:`int` and fell through to the name path, making it a ``KeyError`` - on two of the seven and a ``ValueError`` on the other five. + on two of the six and a ``ValueError`` on the other four -- the same figures + the page carries, and the ones this method derives below rather than trusts. """ - from pcapkit.protocols.internet.mh import (FastBindingAcknowledgmentStatus, - IPv6AddressPrefixCode, - LMAAddressCode) from pcapkit.utilities.exceptions import EnumValueError - for klass in (FastBindingAcknowledgmentStatus, IPv6AddressPrefixCode, - LMAAddressCode): + # Derived, not listed: the population is every EnumLookup subclass those two + # modules *define* -- re-exports from pcapkit.const.ngap.* are not helpers of + # theirs. An earlier version hard-coded three while its own name said seven + # and the page said five; the real answer is six, so it is measured here and + # asserted, rather than any of the three being trusted. + import inspect + + from pcapkit.corekit.enum import EnumLookup + import pcapkit.protocols.application.ngap as ngap_mod + import pcapkit.protocols.internet.mh as mh_mod + + helpers = tuple( + obj for mod in (mh_mod, ngap_mod) for obj in vars(mod).values() + if inspect.isclass(obj) and issubclass(obj, EnumLookup) + and obj is not EnumLookup and obj.__module__ == mod.__name__) + self.assertEqual( + len(helpers), 6, + 'the mh/ngap helper population changed, so the page\'s count of them is ' + f'stale: {sorted(k.__name__ for k in helpers)}') + self.assertEqual( + {k.__name__ for k in helpers}, + {'FastBindingAcknowledgmentStatus', 'IPv6AddressPrefixCode', + 'LMAAddressCode', 'LocalizedRoutingStatus', 'Criticality', 'PDUKind'}) + + # The page's own figure, which measurement alone does not pin. Round 6 found + # this row reading "The 5" against the measured six; the correction landed + # unguarded, so it could have regressed exactly as it arrived. + self.assertIn(f'The {len(helpers)} :mod:', _page('registry-protocol'), + 'the audit table no longer states the measured helper count, ' + 'which is the figure that was already wrong once') + + for klass in helpers: for key in (None, 1.5): with self.subTest(klass=klass.__name__, key=key): with self.assertRaises(EnumValueError): @@ -932,7 +996,545 @@ def test_a_non_string_key_raises_a_value_miss_on_all_seven(self) -> 'None': self.assertIn('``isinstance(key, str)`` and treats everything else as a ' '*value*', flat, "the page no longer records the base's own key dispatch, which " - "is the reason #940's deletion converged the seven") + "is the reason #940's deletion converged the six") + + +def _optional_dependencies() -> 'dict[str, list[str]]': + """``pyproject.toml``'s ``[project.optional-dependencies]``, without a TOML library. + + :mod:`tomllib` is 3.11+ and this repository supports 3.10 -- a first version used it + and failed the ``Python 3.10`` leg with ``ModuleNotFoundError`` while passing locally + on 3.14. ``tomli``, the usual backport, is not a dependency here either + (``grep -n tomli pyproject.toml`` finds nothing), so there is nothing to fall back to + and adding one for a docs-claims test is not worth it. + + **Verified against** :mod:`tomllib` **rather than asserted.** A second version + matched arrays with a non-greedy bracket pattern, which stopped at the first + literal ``]`` -- which mis-parsed **3 of the 14** extras, silently: + + * ``vendor`` came back **empty**, because the ``]`` inside ``"requests[socks]"`` + closed the match early. + * ``dev`` silently dropped its last two entries for the same reason. + * ``test`` was truncated at a ``]`` inside a *comment* and then picked up a quoted + example from the prose, **inventing a dependency called** ``9 skipped``. + + No shipped assertion read those three keys, so no test gave a wrong verdict -- but + ``dev`` and ``vendor`` are both described in the page's own prose, so the next + assertion to check either would have got wrong data with nothing raised. That is + precisely the "renders fine and fails nothing" failure this suite exists to close, + which is why it is fixed rather than documented around. + + So: strip ``#`` comments, then track bracket **depth** rather than matching to the + first ``]``. Nested brackets inside requirement strings and comments containing ``]`` + are both handled; anything else in TOML is not attempted, and + :meth:`ProcessConventionTests.test_the_extras_reader_agrees_with_tomllib` pins the + agreement wherever a real parser is available. + + """ + text = (ROOT / 'pyproject.toml').read_text(encoding='utf-8') + block = re.search(r'^\[project\.optional-dependencies\]\n(.*?)^\[', + text, re.MULTILINE | re.DOTALL) + if block is None: # pragma: no cover + raise AssertionError( + 'pyproject.toml has no [project.optional-dependencies] section, so the ' + "page's extras claims have nothing to be checked against") + + # Comments first: a comment may contain ``]`` or a quoted string, and both fooled + # the previous version. A ``#`` inside a requirement string is not a comment, so + # quoted spans are skipped rather than blindly cut at the first ``#``. + stripped = [] + for line in block.group(1).splitlines(): + out, quote = [], None + for char in line: + if quote: + out.append(char) + if char == quote: + quote = None + elif char in '"\'': + quote = char + out.append(char) + elif char == '#': + break + else: + out.append(char) + stripped.append(''.join(out)) + body = '\n'.join(stripped) + + extras = {} + for match in re.finditer(r'^([A-Za-z_][A-Za-z0-9_-]*)\s*=\s*\[', body, re.MULTILINE): + name = match.group(1) + depth, index, quote = 1, match.end(), None + while index < len(body) and depth: + char = body[index] + if quote: + if char == quote: + quote = None + elif char in '"\'': + quote = char + elif char == '[': + depth += 1 + elif char == ']': + depth -= 1 + index += 1 + extras[name] = re.findall(r'"([^"]*)"', body[match.end():index - 1]) + return extras + + +class ProcessConventionTests(unittest.TestCase): + """The three process rulings :file:`process.rst` carries, against the tree. + + Each check pins the *claim the page makes* next to the *fact behind it*, and + 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. + + """ + + #: The page whose claims this class checks. + ANCHOR = 'process' + + #: The changelog entry file the changelog ruling is about. Named rather than + #: discovered: this is the 1.5.0 cycle's file, which is the one GitHub pull + #: request #657 accumulates into and the one the page cites. + CHANGELOG = ROOT / 'docs' / 'source' / 'changelog' / '1.5.0.rst' + + #: ``.github/PULL_REQUEST_TEMPLATE.md``'s commit-type tickbox list is the tree's + #: own manifest of the type labels, so the page is checked against that rather + #: than against a list retyped here -- which would only pin this file's memory of + #: it. + TEMPLATE = ROOT / '.github' / 'PULL_REQUEST_TEMPLATE.md' + + def setUp(self) -> 'None': + # Whitespace-normalised, because the page wraps at 88 columns and every + # sentence a claim is read out of is routinely split across lines. + self.flat = ' '.join(_page(self.ANCHOR).split()) + + def test_the_extras_reader_agrees_with_tomllib(self) -> 'None': + """:func:`_optional_dependencies` matches a real TOML parser, key for key. + + The reader exists because :mod:`tomllib` is 3.11+ and this repository supports + 3.10, so the assertions above cannot use it. That makes the reader itself an + unverified dependency of every extras claim on the page -- and its second + version mis-parsed **3 of the 14** extras silently, inventing a requirement + called ``9 skipped`` out of comment prose. + + So wherever a real parser *is* available -- which is every interpreter from 3.11 + up, including the one this suite usually runs on -- the two are compared + directly. On 3.10 there is nothing to compare against and the test skips, which + is honest: the reader is then unverified on the one version it was written for, + and the CI matrix covers the other four. + + """ + try: + import tomllib + except ModuleNotFoundError: # pragma: no cover + self.skipTest('tomllib is 3.11+; no parser available to compare against') + + oracle = tomllib.loads( + (ROOT / 'pyproject.toml').read_text(encoding='utf-8') + )['project']['optional-dependencies'] + mine = _optional_dependencies() + + self.assertEqual( + set(mine), set(oracle), + 'the extras reader found a different set of extras than tomllib does') + for name in sorted(oracle): + with self.subTest(extra=name): + self.assertEqual( + mine[name], oracle[name], + f'the extras reader mis-parses {name!r}, so every page claim ' + 'resting on it is unverified -- nested brackets inside a ' + 'requirement string and a comment containing "]" are the two ' + 'shapes that broke it before') + + def test_the_all_extra_is_exactly_the_three_core_addon_extras(self) -> 'None': + """``all`` carries core addons only, and the page's listing says what it is. + + Two halves, because either can rot without the other. The tree half asks + :file:`pyproject.toml` whether ``all`` is still the union of ``cli``, + ``crypto`` and ``NGAP`` and nothing else -- the shape GitHub issue #910's + ruling produced, eight requirements down to three. The page half asks whether + the ``toml`` block on the page still shows that same list, since a page that + prints a stale ``all =`` line is worse than one that prints none. + + """ + extras = _optional_dependencies() + + core = [requirement for extra in ('cli', 'crypto', 'NGAP') + for requirement in extras[extra]] + self.assertEqual(sorted(extras['all']), sorted(core), + f"all is {extras['all']!r}, which is no longer the union of " + f'cli/crypto/NGAP {core!r} -- #910 narrowed it to the core ' + 'addons, so a change here is a change to that ruling') + + # The page prints the literal list, so the literal list is what is checked. + listing = 'all = [ ' + ', '.join(f'"{req}"' for req in extras['all']) + ' ]' + self.assertIn(listing, _page(self.ANCHOR), + f'process.rst no longer shows {listing!r}; its toml block has ' + "drifted from pyproject.toml's own all extra") + + def test_the_page_keeps_the_engine_and_installability_exclusions_apart(self) -> 'None': + """The two exclusion reasons are the part a reader collapses into one. + + #910 excluded the four third-party engines **by kind** -- they are not core + addons -- where ``PyPCAP`` and ``PCAP_CT`` were already out for + *installability*, which the ruling did not touch. Reading those as one reason + is what let the list drift the first time, so the page has to state both. The + tree half checks the six engine extras still exist to be excluded from, since + a prose distinction about extras that no longer exist is not a distinction. + + """ + extras = _optional_dependencies() + + for extra in ('DPKT', 'Scapy', 'PyShark', 'PyPCAPFile', 'PyPCAP', 'PCAP_CT'): + with self.subTest(extra=extra): + self.assertIn(extra, extras, + f'the {extra} extra is gone, so the page names an ' + 'extra a user cannot install') + # Compare *requirements*, not the extra's name against them. The + # previous form asked whether 'DPKT' was an element of + # ['emoji', 'cryptography>=3.4', 'pycrate'] -- element equality against + # a requirement string, so it could never be true and the regression its + # own message names was unreachable. Measured: widening `all` back to + # the pre-#910 engine set left this green while the sibling assertEqual + # caught it. + self.assertFalse( + set(extras[extra]) & set(extras['all']), + f"{extra}'s requirements are back inside all, which #910 excluded: " + f"{sorted(set(extras[extra]) & set(extras['all']))}") + + self.assertIn('excluded by kind', self.flat, + 'the page no longer says the engines are excluded by kind, ' + 'which is the half of #910 that changed the list') + self.assertIn('installability', self.flat, + 'the page no longer separates the installability exclusion ' + 'the ruling left untouched from the by-kind one it introduced') + + def test_the_page_denies_one_changelog_entry_per_commit(self) -> 'None': + """The ruling's substance, in the page's words rather than the owner's. + + The claim is narrow on purpose: entries are grouped by topic and are **not** + one per commit. The grouping scheme itself was open when this test was first + written and the page then carried a disclaimer saying so; it was ruled shortly + afterwards -- a section per top-level module, with the kind headings nested + inside -- so the page records the scheme and this checks for it. What is still + unruled is narrower: which module an entry spanning several belongs under. + + """ + self.assertIn('not one line per commit', self.flat, + 'the page no longer denies one entry per commit, which is the ' + 'whole of what the changelog ruling settled') + self.assertIn('a section per top-level', self.flat, + 'the page no longer records the module grouping the changelog ' + 'was ruled into') + for module in ('const', 'corekit', 'foundation', 'protocols', 'vendor'): + with self.subTest(module=module): + self.assertTrue((ROOT / 'pcapkit' / module).is_dir(), + f'the page names {module} as a top-level module the ' + 'changelog groups by, but no such package exists') + self.assertIn('spans modules', self.flat, + 'the page no longer flags that an entry touching several ' + 'modules has no ruled home -- without it the scheme reads as ' + 'more complete than it is') + + def test_the_changelog_file_is_not_shaped_one_entry_per_commit(self) -> 'None': + """The tree half: the file already groups, and already merges issues. + + Structural rather than counted. An entry count would be pinned to whatever + GitHub pull request #657 had accumulated on the day, and would fail on its + next merge for no reason a reader could act on -- so what is checked is that + the kind headings are all present and that at least one entry cites two or + more issues, which is the property that makes "one entry per commit" false of + the file rather than merely discouraged. + + """ + text = self.CHANGELOG.read_text(encoding='utf-8') + + for kind in ('Added', 'Changed', 'Fixed'): + with self.subTest(kind=kind): + self.assertIn(f'* **{kind}** --', text, + f'{self.CHANGELOG.name} no longer groups entries under ' + f'**{kind}**, which the page describes as its grouping') + + multi = [entry for entry in re.findall(r'(?ms)^\* \*\*.*?(?=^\* \*\*|\Z)', text) + if len(re.findall(r'#\d+', entry)) > 1] + self.assertTrue(multi, + f'no entry in {self.CHANGELOG.name} cites more than one ' + 'issue, so the file is now consistent with one entry per ' + 'commit and the page describes something else') + + def test_the_page_names_every_commit_type_the_template_ticks(self) -> 'None': + """The type labels come from the commit prefixes, so check the manifest. + + :file:`.github/PULL_REQUEST_TEMPLATE.md` is where a contributor actually meets + the list, so it is the ground truth rather than a list retyped into this file. + Note the label set is a **superset**: ``release`` and ``const`` are type-ish + labels with no tickbox, so this is a one-way check by design. + + """ + types = re.findall(r'^- \[ \] `([a-z]+)` ', self.TEMPLATE.read_text(encoding='utf-8'), + re.MULTILINE) + self.assertGreaterEqual(len(types), 8, + f'only {types!r} parsed out of the pull request ' + 'template; the check below would pass vacuously') + for commit_type in types: + with self.subTest(commit_type=commit_type): + self.assertIn(f'``{commit_type}``', self.flat, + f'process.rst does not name the {commit_type} type ' + 'label, which the pull request template asks every ' + 'contributor to tick') + + def test_the_page_pins_its_own_measured_numbers(self) -> 'None': + """Every figure the page states that has local ground truth. + + A cross-review corrupted **46** claims on these two pages simultaneously and + the suite stayed green, which is the honest measure of how much of the prose + was decorative. This closes the subset that has local ground truth: a figure + derived from a file in this repository, a path the page cites, a module it + names. + + **What stays unpinned, and why, so the gap is stated rather than implied.** + Every claim whose ground truth is a GitHub query -- the label count, which + default labels are in live use, the ``breaking`` census, the issue and pull + request numbers -- cannot be checked here, because this repository's CI has no + network. Those are why the page gives the *command* alongside the figure: the + command is the pin, run by a reader rather than by CI. Inverting the page's + live-use claim about GitHub's default labels still passes this suite, measured, + and no offline test can change that. + + """ + import inspect + import re as _re + + changelog = (ROOT / 'docs' / 'source' / 'changelog' / '1.5.0.rst') \ + .read_text(encoding='utf-8') + entries = _re.findall(r'^\* \*\*[A-Za-z]+\*\*', changelog, _re.MULTILINE) + + self.assertIn(f'{len(entries)} entries', self.flat, + f'the page no longer states the entry count, measured at ' + f'{len(entries)}') + + # The module list the page prints as the by-module target. The tree half was + # already checked; this is the page half, which a corruption inserting a + # non-existent module survived. + packages = sorted(d.name for d in (ROOT / 'pcapkit').iterdir() + if d.is_dir() and not d.name.startswith('__')) + for name in packages: + with self.subTest(module=name): + self.assertIn(name, self.flat, + f'pcapkit/{name}/ exists but the page does not name it ' + 'among the modules the changelog groups by') + # Only the ``ls -d`` comment block, not the whole page: elsewhere the page + # legitimately writes the *distribution* name (``pip install + # pypcapkit[Scapy]``), which is not a module and which a looser sweep flagged. + listing = _re.search(r'ls -d pcapkit/\*/.*?(?=\n\n\S)', + _page(self.ANCHOR), _re.DOTALL) + self.assertIsNotNone( + listing, 'the page no longer prints the module listing command, so the ' + 'by-module target names no modules at all') + named = set(_re.findall(r'\b([a-z]{4,12})\b', listing.group(0))) - { + 'pcapkit', 'sed'} + self.assertFalse( + named - set(packages), + f'the module listing names something that is not a package under ' + f'pcapkit/: {sorted(named - set(packages))}') + + # Every workflow and config file the page cites by path. + for cited in ('.github/workflows/lint.yml', '.github/release.yml', + '.github/ISSUE_TEMPLATE/bug_report.md', + '.github/ISSUE_TEMPLATE/feature_request.md', + '.github/dependabot.yml', 'pyproject.toml'): + if cited.rsplit('/', 1)[-1] in self.flat: + with self.subTest(path=cited): + self.assertTrue((ROOT / cited).is_file(), + f'the page cites {cited}, which does not exist') + + def test_the_page_says_the_labels_are_set_by_hand(self) -> 'None': + """The one statement the page was asked for outright. + + The owner asked on GitHub issue #918 for the labelling scheme to be explained + here, and the thing repeatedly got wrong is agency: several agents have + independently reported these labels as automation and acted on that. Nothing + in the tree sets them -- :file:`.github/dependabot.yml` configures no + ``labels:`` key, so even dependabot's are its own default rather than this + repository's instruction -- and a reader who believes otherwise treats a + ``review:`` label as evidence instead of as an assertion someone made. + + """ + self.assertNotIn('labels', (ROOT / '.github' / 'dependabot.yml') + .read_text(encoding='utf-8'), + 'dependabot.yml now configures labels, so the page is wrong ' + 'to say dependabot uses its own defaults') + self.assertIn('applied by hand', self.flat, + 'process.rst no longer states that the labels are applied by ' + 'hand, which is the correction it was asked to carry') + + # The issue templates are the second automated path, and the first version of + # this test could not see it: it read only dependabot.yml, so it passed while + # the page claimed dependabot was the *single* exception. Every template that + # sets a label in its front matter has to be named on the page, or the page + # understates how many labels arrive without judgement behind them. + templated = {} + for template in sorted((ROOT / '.github' / 'ISSUE_TEMPLATE').glob('*.md')): + for line in template.read_text(encoding='utf-8').splitlines(): + if line.startswith('labels:'): + templated[template.name] = line.split(':', 1)[1].strip() + + # The dependabot attribution, which had no pin at all -- which is how the page + # came to name ``github_actions`` as dependabot-applied when + # .github/dependabot.yml configures only ``pip``, so dependabot can never open a + # workflow bump here. Saying a hand-applied label is a machine default is this + # section's own point stated backwards, and the two places the page mentioned it + # disagreed with each other about ``python``. + dependabot = (ROOT / '.github' / 'dependabot.yml').read_text(encoding='utf-8') + ecosystems = re.findall(r'package-ecosystem:\s*"([^"]+)"', dependabot) + + self.assertEqual( + 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") + + self.assertTrue(templated, + 'no issue template sets a label any more, so the page is now ' + 'wrong in the other direction -- it names a path that is gone') + for name, label in sorted(templated.items()): + with self.subTest(template=name): + self.assertIn(name, self.flat, + f'{name} applies a label from its front matter, but the ' + 'page does not name it among the automated paths') + # Anchored to the template's own sentence, not a bare substring. + # `assertIn('bug', flat)` passed against a page saying `labels: + # defect`, because the word `bug` occurs elsewhere ("alongside ``bug``, + # ``enhancement``") -- the same coincidence trap the changelog-runs + # test already guards against. + self.assertIn(f'``labels: {label}``', self.flat, + f'{name} applies {label!r} automatically, but the page ' + 'does not state that label next to the template that ' + 'applies it') + + def test_every_issue_link_number_matches_its_own_url(self) -> 'None': + """A ``#NNN`` label and the issue or pull number in its own URL must agree. + + Purely local, needing no network, and it closes the largest single class of + unpinned claim on these pages: a cross-review corrupted roughly thirty link + numbers across both files and not one was caught, because nothing compared the + displayed number against the target. A mismatch is invisible to a reader -- + the text says #918 and the link goes to #919 -- and Sphinx cannot warn, since + both halves are well-formed. + + Scans every page rather than one, since a link can land on any of them. + + """ + pattern = re.compile( + r'`#(\d+)\s*`__') + found = pattern.findall(_every_page()) + + # A floor, because `assertEqual([], [])` is what an emptied page produces. This + # check disables itself silently on any link-style change -- a single-underscore + # named reference, or a move to an `:issue:` role, takes the regex to zero + # matches while the docstring goes on claiming it closes the largest class of + # unpinned claim. The module guards this shape five other times; this one had + # been left out. + self.assertGreater( + len(found), 40, + f'only {len(found)} issue links matched the pinned form, so this check is ' + 'no longer examining the pages -- the link style changed and the test went ' + 'quiet rather than red') + + mismatched = [(shown, target) for shown, target in found if shown != target] + + self.assertEqual( + mismatched, [], + 'a link shows one issue number and points at another, which no reader ' + f'can see and no build can warn about: {mismatched}') + + def test_the_page_describes_the_changelog_kind_runs_as_they_are(self) -> 'None': + """The current shape of ``1.5.0.rst``, counted rather than eyeballed. + + The page's first draft said the file was "three flat kind-runs", which came + from a comment on #918 that nobody measured. It is not: the entries carry the + three kind labels in far more runs than that, blocked near the top and + interleaved lower down. The distinction matters because it is the difference + between transposing three blocks and regrouping scattered entries, which is + what the by-module restructure on #657 actually has to do. + + Pinned two ways, and the difference matters. The **floor** is what makes + "three flat runs" unsayable regardless of how the file grows. The **exact** + figure is required in the page's prose as well, deliberately: a page that + states a measured number has to restate it when the measurement moves, and + #657's restructure will move it. So a red test here after a changelog merge is + the intended signal to re-measure the sentence, not a brittleness to route + around. + + """ + import itertools + + kinds = re.findall(r'^\* \*\*([A-Za-z]+)\*\*', + (ROOT / 'docs' / 'source' / 'changelog' / '1.5.0.rst') + .read_text(encoding='utf-8'), re.MULTILINE) + runs = [kind for kind, _ in itertools.groupby(kinds)] + + self.assertGreater( + len(runs), len(set(kinds)), + 'the changelog entries are now grouped one run per kind, so the page is ' + 'wrong to describe them as interleaved -- and the by-module restructure ' + 'is a smaller job than it says') + self.assertGreater( + len(runs), 10, + f'only {len(runs)} kind-runs, so the page overstates how scattered the ' + 'entries are') + # Assert the sentence, not the bare number: the first version of this check + # asserted '26' alone, which the ``wc -l`` output in the page's own code block + # satisfied -- so it passed with "three flat runs" restored in the prose. A + # figure that appears twice on a page cannot pin the claim that uses it. + self.assertIn(f'**{len(runs)}** separate runs', self.flat, + 'the page no longer states the measured run count in its prose, ' + 'which is the whole correction to "three flat kind-runs"') + self.assertNotIn('three flat', self.flat, + 'the page has gone back to calling the file three flat ' + 'kind-runs, which measurement contradicts') + + def test_the_page_keeps_breaking_additive_and_its_coverage_honest(self) -> 'None': + """``breaking`` stacks on a type label, and its history is uneven. + + Both halves are prose claims with no local ground truth -- the label census is + a GitHub query, which is why the page gives the command rather than a figure. + What is checkable here is that the page has not quietly dropped either point: + that the label is additive rather than a category of its own, and that its + absence on an old item is weak evidence. The exception type the page cites as + its worked example *is* local, so that much is verified rather than described. + + """ + from pcapkit.utilities.exceptions import BaseError, ProtocolError + + # Not `issubclass(ProtocolError, Exception)`, which every exception class + # satisfies and which therefore verified nothing. The page's claim is that + # #811 made a *library* error escape where a bare struct.error used to, so + # what is checkable locally is that the type is the library's own. + self.assertEqual(ProtocolError.__module__, 'pcapkit.utilities.exceptions', + 'the page cites ProtocolError as an in-library exception ' + 'type, but it no longer comes from ' + 'pcapkit.utilities.exceptions') + self.assertTrue(issubclass(ProtocolError, BaseError), + 'ProtocolError no longer descends from the library base, so ' + "the page's worked example no longer illustrates what it says") + + self.assertIn('additive', self.flat, + 'the page no longer says breaking is additive; its own label ' + 'description is explicit that it goes alongside the type label') + self.assertIn('not applied uniformly', self.flat, + 'the page no longer warns that breaking is applied unevenly ' + 'across history, which is what makes an absence weak evidence') if __name__ == '__main__':