From 89b56354c57302ca25e2c721c6f37b3a1487fa33 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Thu, 15 Oct 2026 12:44:55 -0400 Subject: [PATCH] docs(tests): second pass over tests/protocols (#1103) Correct tests/protocols prose that no longer matches the code. Prose only: the AST guard finds all seven files identical. - test_layer_placement_unit: Link sets ten names of its own, not seven, and OSPF loses four of them, `_shipped_proto` included. - test_protocol_code_registration_unit: Engine's registry keyword is `engine=`, renamed from `name=` by #557. - test_http_unit: Command.get's case rule is RFC 959 section 5.3, not 4.1; TCP ports 80 and 8080 bind the version dispatcher, not httpv1.HTTP, so the "straight to this class" claim is dropped. - test_hip_unit: all three two_octet_prefix_list_len call sites take two-octet items, so none is a one-octet contrast. - test_ipv4_unit: the ProtocolBase descendant count is stated as a lower bound, and a test_schema_unit.py line is re-measured. - test_dispatch_reachability_unit: drop the stale count of 38 `__proto__` entries; the registry test pins the number. - test_hip_locator_set_length_unit: rewrap an over-long line. Refs #1103 --- tests/protocols/application/test_http_unit.py | 10 ++++------ .../protocols/application/test_layer_placement_unit.py | 8 +++++--- .../internet/test_hip_locator_set_length_unit.py | 6 +++--- tests/protocols/internet/test_hip_unit.py | 6 +++--- tests/protocols/internet/test_ipv4_unit.py | 9 +++++---- tests/protocols/test_dispatch_reachability_unit.py | 2 +- .../protocols/test_protocol_code_registration_unit.py | 5 +++-- 7 files changed, 24 insertions(+), 22 deletions(-) diff --git a/tests/protocols/application/test_http_unit.py b/tests/protocols/application/test_http_unit.py index 12ca34d8d9..722c1fa4dd 100644 --- a/tests/protocols/application/test_http_unit.py +++ b/tests/protocols/application/test_http_unit.py @@ -1016,10 +1016,8 @@ def test_httpv1_read_raises_protocol_error_for_a_malformed_message(self) -> None Asserted on ``httpv1.HTTP`` directly rather than through the proxy because that is the difference between the two candidate fixes: the - contract now holds for every caller, including - ``Transport._decode_next_layer``, which dispatches TCP ports 80 and 8080 - straight to this class and never passes through ``_guess_version`` at - all. + contract now holds for every caller of ``httpv1.HTTP``, not only for + ``_guess_version``'s first arm. ``__cause__`` is asserted because the chain is what keeps the underlying unpacking error reportable, and what keeps @@ -1643,8 +1641,8 @@ def test_method_get_is_now_case_sensitive(self) -> None: """``Method.get`` used to test the upper-cased key, so a mixed-case method resolved to the same registered member regardless of its own casing -- #583, item 1, at the time framed the same way as #582's fix - in :mod:`pcapkit.const.ftp.command` (`Command.get` stays that way; see - its own RFC 959 Section 4.1 rationale). GitHub issue #896 retired that + in :mod:`pcapkit.const.ftp.command` (``Command.get`` stays that way, per + RFC 959 Section 5.3). GitHub issue #896 retired that for ``Method`` specifically: RFC 9110 Section 9.1 makes the HTTP method *token* case-sensitive -- "standardized methods are defined in all-uppercase US-ASCII letters" by convention, not by requirement -- diff --git a/tests/protocols/application/test_layer_placement_unit.py b/tests/protocols/application/test_layer_placement_unit.py index fb40645237..f5abc2cdc0 100644 --- a/tests/protocols/application/test_layer_placement_unit.py +++ b/tests/protocols/application/test_layer_placement_unit.py @@ -197,10 +197,12 @@ def test_dispatch_keys_did_not_move_with_the_modules(self) -> None: self.assertIs(getattr(importlib.import_module(entry.module), entry.name), cls) def test_ospf_leaves_links_registry_behind_and_rarp_keeps_it(self) -> None: - """``Link`` sets seven names beyond ``ProtocolBase``; OSPF loses three of them. + """``Link`` sets ten names of its own; OSPF loses four of them. - ``__proto__``, ``register`` and ``_read_protos`` are ``Link``'s EtherType - machinery. The loss is inert because a ``-1`` lookup misses in either + ``__proto__``, its shipped copy ``_shipped_proto``, ``register`` and + ``_read_protos`` are ``Link``'s EtherType machinery, and OSPF takes all + four from ``ProtocolBase`` instead. The first assertion pins seven of the + ten. The loss is inert because a ``-1`` lookup misses in either registry and falls back to :class:`~pcapkit.protocols.misc.raw.Raw`. """ from pcapkit.protocols.application.ospf import OSPF diff --git a/tests/protocols/internet/test_hip_locator_set_length_unit.py b/tests/protocols/internet/test_hip_locator_set_length_unit.py index a1e4e655ea..816aa96597 100644 --- a/tests/protocols/internet/test_hip_locator_set_length_unit.py +++ b/tests/protocols/internet/test_hip_locator_set_length_unit.py @@ -46,9 +46,9 @@ =================== ============== ====== ========================= Four of the seven were not even multiples of eight. A sweep over n = 1..5 of -plain locators passed by cancellation before the fix and would pass a half-fix too, so it -discriminates nothing; the empty set, the SPI-bearing variant and a mixed set in -both orders are the shapes that do. +plain locators passed by cancellation before the fix and would pass a half-fix +too, so it discriminates nothing; the empty set, the SPI-bearing variant and a +mixed set in both orders are the shapes that do. The reader consumed the same quantity ------------------------------------- diff --git a/tests/protocols/internet/test_hip_unit.py b/tests/protocols/internet/test_hip_unit.py index 760a4bdb1d..5d83ab421b 100644 --- a/tests/protocols/internet/test_hip_unit.py +++ b/tests/protocols/internet/test_hip_unit.py @@ -1999,9 +1999,9 @@ def test_hip_transport_format_list_parameter_parses_the_full_declared_length(sel Each ``TF type`` entry is two octets, not one -- :rfc:`7401` Section 5.2.11 fixes it at "2x number of TF types" and the diagram shows two 16-bit ``TF type`` fields per 32-bit row, matching - :class:`HIPTransportModeParameter`'s ``mode`` field rather than the - one-octet items :func:`two_octet_prefix_list_len`'s other two call - sites use. So this test's four entries are four *two*-octet values + :class:`HIPTransportModeParameter`'s ``mode`` field and the items at + :func:`two_octet_prefix_list_len`'s other two call sites, all three + two-octet. So this test's four entries are four *two*-octet values (``Length = 8``), and with both defects fixed -- :func:`~pcapkit.protocols.schema.internet.hip. transport_format_list_len` returning ``Length`` unchanged, and diff --git a/tests/protocols/internet/test_ipv4_unit.py b/tests/protocols/internet/test_ipv4_unit.py index 283c8dbca7..92950da2c8 100644 --- a/tests/protocols/internet/test_ipv4_unit.py +++ b/tests/protocols/internet/test_ipv4_unit.py @@ -222,7 +222,7 @@ def test_ipv4_make_accepts_a_protocol_instance_as_payload(self) -> None: # defect. Asserting the negative matters because the branch was not # uncovered before the fix -- it was covered by the only class in the # tree that satisfied it, ``DummyProtocol`` in - # tests/protocols/schema/test_schema_unit.py:167, which subclasses + # tests/protocols/schema/test_schema_unit.py:168, which subclasses # ``Protocol`` and so packed happily while every protocol in the # library raised. Without this line the same hole could be reopened # by making a protocol subclass ``Protocol`` rather than by fixing @@ -311,8 +311,9 @@ def test_schema_pack_packs_a_second_real_protocol_as_payload(self) -> None: defect of its own. Every one of them hands the payload field a :class:`~pcapkit.protocols.misc.raw.Raw` or a :class:`~pcapkit.protocols.misc.null.NoPayload`, because those are what - *parsing* yields, so between them they exercise exactly two of the 43 - :class:`~pcapkit.protocols.protocol.ProtocolBase` descendants the widened + *parsing* yields, so between them they exercise exactly two of the more + than forty :class:`~pcapkit.protocols.protocol.ProtocolBase` descendants + the widened check in :meth:`Schema.pack ` has to accept. That is enough coverage to be defeated by a fix that is not one: replacing the @@ -324,7 +325,7 @@ def test_schema_pack_packs_a_second_real_protocol_as_payload(self) -> None: -- special-casing precisely the classes the tests use -- passes all four of them, measured, while leaving the general defect in place for the - other 40 protocols. #536's own reviewer found that by falsification, and + other protocols. #536's own reviewer found that by falsification, and this test is what makes the shortcut fail. So the payload here is a ``UDP``, and the assertions are chosen to be diff --git a/tests/protocols/test_dispatch_reachability_unit.py b/tests/protocols/test_dispatch_reachability_unit.py index dd4cb94a86..fac497e440 100644 --- a/tests/protocols/test_dispatch_reachability_unit.py +++ b/tests/protocols/test_dispatch_reachability_unit.py @@ -9,7 +9,7 @@ rather than just this one. :file:`test_dispatch_registry_unit.py` walks the problem from the other end: it -takes each of the 38 ``__proto__`` entries that *exist* and checks the class it +takes each ``__proto__`` entry that *exists* and checks the class it names can parse a packet. That cannot see a class nobody registered at all, which is exactly how :class:`~pcapkit.protocols.application.ospf.OSPF` shipped reachable from no table (fixed in #436). This module walks it from the class diff --git a/tests/protocols/test_protocol_code_registration_unit.py b/tests/protocols/test_protocol_code_registration_unit.py index a3050960d6..00a6a9e2e5 100644 --- a/tests/protocols/test_protocol_code_registration_unit.py +++ b/tests/protocols/test_protocol_code_registration_unit.py @@ -6,11 +6,12 @@ keyword that registers a subclass into the next-layer dispatch registry (or registries) it names -- e.g. :attr:`Link.__proto__ `. Registration is opt-in, -exactly like the ``name=``/``protocol=``/``fmt=`` keywords part (a) (#547) +exactly like the ``engine=``/``protocol=``/``fmt=`` keywords part (a) (#547) gave :class:`~pcapkit.foundation.engines.engine.Engine`, :class:`~pcapkit.foundation.reassembly.reassembly.Reassembly`, :class:`~pcapkit.foundation.traceflow.traceflow.TraceFlow` and -:class:`~pcapkit.dumpkit.common.Dumper`: omitting ``code`` leaves the +:class:`~pcapkit.dumpkit.common.Dumper` (#557 renamed ``Engine``'s from +``name=``): omitting ``code`` leaves the subclass unregistered, which is exactly what every built-in protocol class does today, since the built-in dispatch tables are populated by literal assignment in each layer module rather than by ``__init_subclass__``.