From b6bd390d0d6db0aba9d44cd7e1e94f09714bd68e Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Sun, 25 Oct 2026 23:19:43 -0400 Subject: [PATCH] docs(tests): drop never-released names (IPv6_GenericExt and helpers) Problem: docs and tests named IPv6_GenericExt, _PeekableStream, _TCPSegment and _can_seek, none of which is in any release tag. The no-re-export rule already forbids an old-path alias, so the mentions and the absence guards for them add nothing. Fix: remove the conventions page section on IPv6_GenericExt and RetiredNameTests; drop _PeekableStream and _TCPSegment from test docstrings and _TCPSegment from the hasattr loop; drop the _can_seek absence assertion and rename that test to test_can_seek_is_exported. Test: the edited test files, the conventions, docs, docstring-contract and tier-guard tests pass. --- .../extension-header-subclassing.rst | 17 ------ .../test_io_advance_lying_pipe_2108_unit.py | 5 +- .../test_io_stream_proxies_1516_unit.py | 8 +-- tests/project/test_conventions_doc_claims.py | 53 ++++--------------- .../test_pcap_tcp_segment_public_1570_unit.py | 11 ++-- 5 files changed, 22 insertions(+), 72 deletions(-) diff --git a/docs/source/contributing/conventions/extension-header-subclassing.rst b/docs/source/contributing/conventions/extension-header-subclassing.rst index bb56611fe2..1228b901d7 100644 --- a/docs/source/contributing/conventions/extension-header-subclassing.rst +++ b/docs/source/contributing/conventions/extension-header-subclassing.rst @@ -137,23 +137,6 @@ RFC ground in the ``#:`` comment beside it. The test walks ``IPv6_Ext.__subclasses__()`` rather than a hard-coded list, so any further header is held to the convention whether or not anyone remembers this page. -The Retired ``IPv6_GenericExt`` Name -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -The class arrived as ``IPv6_GenericExt``, a fallback parser for an unrecognised -extension header (:issue:`891`), and -:issue:`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 ruled that the -``IPv6_GenericExt`` name goes for good: it was an intermediate state, and it was never -released. - -What makes that safe rather than merely decided: ``IPv6_GenericExt`` existed on -``main`` from ``b3551cb63`` to ``93cf940b3``, under four hours on one day, and no release tag falls -between the two commits -- ``git tag --contains`` names the same tags for both -- so it -appears in **no** release and the break has no callers to inconvenience. Do not -reintroduce it as an alias, and do not cite it as a former public name. - ESP's Two Facts ~~~~~~~~~~~~~~~ diff --git a/tests/corekit/test_io_advance_lying_pipe_2108_unit.py b/tests/corekit/test_io_advance_lying_pipe_2108_unit.py index 04d763ea21..71cf1d7905 100644 --- a/tests/corekit/test_io_advance_lying_pipe_2108_unit.py +++ b/tests/corekit/test_io_advance_lying_pipe_2108_unit.py @@ -106,12 +106,11 @@ class TestCanSeekIsPublic(unittest.TestCase): def setUp(self) -> None: reimport_once_per_class(self) - def test_can_seek_is_exported_and_the_private_name_is_gone(self) -> None: - """No alias keeps the old private name importable.""" + def test_can_seek_is_exported(self) -> None: + """``can_seek`` is listed in the module's ``__all__``.""" from pcapkit.corekit import io as corekit_io # pylint: disable=import-outside-toplevel self.assertIn('can_seek', corekit_io.__all__) - self.assertFalse(hasattr(corekit_io, '_can_seek')) def test_can_seek_tells_a_lying_pipe_from_a_seekable_stream(self) -> None: """A pipe claiming to seek does not; a stream without a descriptor is trusted.""" diff --git a/tests/corekit/test_io_stream_proxies_1516_unit.py b/tests/corekit/test_io_stream_proxies_1516_unit.py index 76458fc526..550110a893 100644 --- a/tests/corekit/test_io_stream_proxies_1516_unit.py +++ b/tests/corekit/test_io_stream_proxies_1516_unit.py @@ -4,10 +4,10 @@ :class:`~pcapkit.corekit.io.PeekableStream` gives a seekable stream without ``peek`` one, for :class:`~pcapkit.foundation.extraction.Extractor` (#1506, #1509). :class:`~pcapkit.corekit.io.NamedStream` gives a stream the ``name`` -:func:`pcapfile.savefile.load_savefile` reads, for the PyPCAPFile engine. Both -live beside :class:`~pcapkit.corekit.io.SeekableReader` (formerly the -file-local ``_PeekableStream`` in :mod:`pcapkit.foundation.extraction` and -``_NamedStream`` in :mod:`pcapkit.foundation.engines.pypcapfile`). +:func:`pcapfile.savefile.load_savefile` reads, for the PyPCAPFile engine +(formerly the file-local ``_NamedStream`` in +:mod:`pcapkit.foundation.engines.pypcapfile`). Both live beside +:class:`~pcapkit.corekit.io.SeekableReader`. Every class is imported from :mod:`pcapkit.corekit.io` inside each test, after :func:`~tests._support.reimport_once_per_class`. diff --git a/tests/project/test_conventions_doc_claims.py b/tests/project/test_conventions_doc_claims.py index ae6802c3ed..da23e75440 100644 --- a/tests/project/test_conventions_doc_claims.py +++ b/tests/project/test_conventions_doc_claims.py @@ -3,8 +3,8 @@ The *House Conventions* pages, under :file:`docs/source/contributing/conventions/`, record the settled design rulings, and most of what they record is reasoning -- -which no test can check. Some of it is not: a per-class classification, a retired -name, an exception type. Those are the parts that rot silently, because Sphinx +which no test can check. Some of it is not: a per-class classification, a forbidden +role, an exception type. Those are the parts that rot silently, because Sphinx builds without ``-W`` and without ``nitpicky``, so a page whose every factual claim has gone stale still renders and CI still passes. @@ -31,9 +31,6 @@ 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 new header lands. -* :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 names :exc:`~pcapkit.utilities.exceptions.EnumKeyError`, not the true but unspecific :exc:`KeyError` (#918). @@ -48,13 +45,13 @@ * :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 no ``py:`` objects), converted to plain literals rather - than roles. A forbidden role needs a test or it comes back, exactly as - :class:`RetiredNameTests` guards a retired name. Pins the other half of the same - fix alongside it: the sentinel references #934 part B qualified stay - qualified, since none of ``AbsentType``, ``NoValueType`` or ``ABSENT`` resolves - unqualified outside :file:`docs/source/pcapkit/corekit/sentinels.rst`'s own module - context. Both checks scan every split page rather than one file, since either - could in principle land on any of them. + than roles. A forbidden role needs a test or it comes back. Pins the other + half of the same fix alongside it: the sentinel references #934 part B + qualified stay qualified, since none of ``AbsentType``, ``NoValueType`` or + ``ABSENT`` resolves unqualified outside + :file:`docs/source/pcapkit/corekit/sentinels.rst`'s own module context. Both + checks scan every split page rather than one file, since either could in + principle land on any of them. * :class:`ProcessConventionTests` -- the *process* rulings #918 harvested onto :file:`process.rst`: what the ``all`` extra carries (#910), what a @@ -370,9 +367,7 @@ def test_the_index_preamble_does_not_claim_to_be_a_ruling_page_itself(self) -> ' never needs that phrase, since every true statement about ruling content here names a child page, or says "the pages below" / "here" for the set of them. Paired with a positive check that the standing-instruction phrase is - present, rather than merely that the single-page one is absent, following - :class:`RetiredNameTests`'s two-sided pattern for a retired name elsewhere - in this module. + present, rather than merely that the single-page one is absent. """ text = INDEX.read_text(encoding='utf-8') @@ -626,31 +621,6 @@ def test_the_page_agrees_with_the_test_that_pins_the_code(self) -> 'None': set(IPv6ExtSharedBaseContractTests.STANDALONE_MEMBERS)) -class RetiredNameTests(unittest.TestCase): - """#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. - - Checked over the package source rather than by import, because the failure - this guards against is a *reintroduced alias* -- which would import perfectly - well and satisfy any behavioural assertion. - - """ - offenders = [str(path.relative_to(ROOT)) - for path in sorted((ROOT / 'pcapkit').rglob('*.py')) - if 'IPv6_GenericExt' in path.read_text(encoding='utf-8')] - self.assertEqual(offenders, [], - 'the retired name is back; the ruling on GitHub pull request ' - '#924 is that it was an intermediate state and never released') - - def test_the_shared_base_still_carries_the_name_the_ruling_left(self) -> 'None': - """The other half: the name was renamed, not simply removed.""" - from pcapkit.protocols.internet.ipv6_ext import IPv6_Ext - - self.assertEqual(IPv6_Ext.__name__, 'IPv6_Ext') - - class FailedLookupExceptionTests(unittest.TestCase): """The worked example the page gives for a lookup that does not resolve.""" @@ -693,8 +663,7 @@ class AenumRoleExclusionTests(unittest.TestCase): B converted every such role this page carried to plain double-backtick literals rather than leaving them promising a link that can never exist. A forbidden role needs a test, or a later edit reintroduces one without - noticing -- exactly the failure mode :class:`RetiredNameTests` guards a - retired name against. + noticing. The other half of the same fix is pinned alongside it: the references to ``AbsentType``, ``NoValueType`` and ``ABSENT`` that part B qualified to their diff --git a/tests/toolkit/test_pcap_tcp_segment_public_1570_unit.py b/tests/toolkit/test_pcap_tcp_segment_public_1570_unit.py index bd08a33bbb..81d330b823 100644 --- a/tests/toolkit/test_pcap_tcp_segment_public_1570_unit.py +++ b/tests/toolkit/test_pcap_tcp_segment_public_1570_unit.py @@ -3,11 +3,10 @@ The helper the TCP adapters of :mod:`pcapkit.toolkit.pcap` and :mod:`pcapkit.toolkit.pcapng` share was the private ``pcap._tcp_segment``, which -``pcapng`` imported across modules, and its result type was the private -``pcap._TCPSegment``. Under the rulings on #1519 a shared helper becomes public in -the module both callers already import, as ``test_start_line`` did in #1525, so the -two keep their names without the underscore and stay in ``pcap``, which ``pcapng`` -already imports. The adapters' end-to-end behaviour is covered by +``pcapng`` imported across modules. Under the rulings on #1519 a shared helper +becomes public in the module both callers already import, as ``test_start_line`` +did in #1525, so the helper and its result type are public and stay in ``pcap``, +which ``pcapng`` already imports. The adapters' end-to-end behaviour is covered by ``tests/toolkit/test_pcap_tcp_segment_unit.py``; these tests pin the public surface and that both toolkits read the segment through it. @@ -96,7 +95,7 @@ def test_callers_share_the_public_helper(self) -> None: from pcapkit.toolkit import pcap, pcapng self.assertIs(pcapng.tcp_segment, pcap.tcp_segment) - for module, name in ((pcap, '_tcp_segment'), (pcap, '_TCPSegment'), (pcapng, '_tcp_segment')): + for module, name in ((pcap, '_tcp_segment'), (pcapng, '_tcp_segment')): with self.subTest(module=module.__name__, name=name): self.assertFalse(hasattr(module, name))