Skip to content

test: enumerate the 38 __proto__ dispatch-registry entries - #504

Merged
JarryShaw merged 4 commits into
mainfrom
test/496-proto-dispatch-harness
Sep 19, 2026
Merged

JarryShaw merged 4 commits into
mainfrom
test/496-proto-dispatch-harness

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

Closes #496. Test-only: examples/generators/dispatch.py and tests/protocols/test_dispatch_registry_unit.py, 1,199 insertions, no library code touched.

The gap

pcapkit/foundation/registry/protocols.py defines the registries that decide which Protocol class parses the next layer. 38 entries across seven tables — Link.__proto__ (7), Internet.__proto__ (16), TCP.__proto__ (4), UDP.__proto__ (3), SCTP.__proto__ (2), Frame.__proto__ (3), PCAPNG.__proto__ (3) — and none of them had an enumerating harness.

Existing coverage checked that an entry resolves to the right class object, which is satisfied by a table whose target could not parse a packet if it tried. tests/protocols/test_dispatch_bindings_unit.py exists because this class of bug shipped — its docstring records that OSPF "was reachable from no table at all and could not have parsed a packet if it had been" — but it hand-picks 11 cases rather than enumerating.

This is the __proto__ equivalent of what tests/protocols/test_option_roundtrip_unit.py and examples/generators/options.py already give the option registries, and it follows their structure.

Two things found while building it, both worth reading

1. The first draft's expectation was self-referential. cases() built each Case with target=_resolve(entry) straight from registry.items(), so the expected target was read out of the very table under test. It proved "dispatch honours whatever the table says", not "the table says the right thing". I breakage-tested retargeting AH from ah.AH to raw.Raw with the registry size unchanged and got 5 passed, 45 subtests — not caught.

Fixed with PINNED_TARGETS, a hand-written label -> (module, name) table independent of the registry. Case now carries target (pinned) and registered (whatever the live table resolves to) as separate fields, and a mismatch between them is itself a failure.

2. Even a pinned target was not enough, for a genuinely subtle reason. case.target in frame.protochain still missed the retarget, because ProtoChain.__contains__/index() match on alias strings and .id() names rather than class identity — and Raw adopts the dispatching protocol's own name (pcapkit/protocols/misc/raw.py: alias = self._info.protocol.name). So an entry keyed on TransType.AH but retargeted to Raw still renders as '...:AH' and satisfies a string check, with no AH instance anywhere in the chain.

Fixed by asserting strict class identity against frame.protochain.protocols — the tuple of actual type(instance) per layer. Worth knowing beyond this PR: a protochain string containing a protocol's name is not evidence that protocol parsed anything.

Both breakage shapes, run by me against the final code

Deletion — AH line removed from Internet.__proto__:

FAILED ...::test_cases_cover_every_table_named_in_the_issue
AssertionError: 37 != 38 : expected exactly 38 entries across the seven __proto__ tables
1 failed, 6 passed, 81 subtests passed

Retargeting — AH → Raw, registry size unchanged:

SUBFAILED(case='internet/AH') ...::test_dispatch_reaches_target_or_is_a_recorded_degrade
SUBFAILED(case='internet/AH') ...::test_registry_currently_matches_pinned_target
AssertionError: <class 'pcapkit.protocols.misc.raw.Raw'> is not <class 'pcapkit.protocols.internet.ah.AH'> :
  internet/AH: the registry currently resolves to Raw, but PINNED_TARGETS expects AH. Either the
  registry regressed, or PINNED_TARGETS is stale and needs updating to match a deliberate change.
2 failed, 7 passed, 81 subtests passed

Caught by two independent tests, both naming internet/AH, with a message that distinguishes the two possible causes. Restored after each: 7 passed, 83 subtests passed, clean tree.

Coverage

All 38 of 38 entries, including the ones that previously had only an identity check and no parse test: AH (51), HIP (139), Mobility_Header (135), SCTP (132), RARP (0x8035), and the raw-IP Frame.__proto__ pair (228/229). The PCAPNG.__proto__ IPV4/IPV6 pair — which the audit flagged as unverified because hand-rolling a minimal SHB+IDB+EPB stream looked riskier than the check was worth — is now confirmed working, built through pcapkit's own PCAPNG/Context construction API rather than by hand.

34 of 38 reach their target cleanly. 4 are recorded in KNOWN_DEGRADED rather than fixed — this module reports defects, it does not carry workarounds:

KNOWN_DEGRADED is a ratchet, not a skip-list: a case in it must degrade in the recorded way, the test tells you to delete the entry once the defect is fixed, a separate test rejects entries naming cases that no longer exist, and another keeps degraded cases a small minority.

A trap for whoever extends this

ARP/RARP/InARP/DRARP's reported name comes from the wire oper field (pcapkit/protocols/link/arp.py:176-190), not from the dispatching ethertype, so an ethertype-0x8035 frame with oper=1 legitimately reports Ethernet:ARP. The RARP case uses oper in {3,4} for that reason; anything else exercises the wrong alias while appearing to pass.

Verification

test_dispatch_registry_unit.py: 7 tests, 83 subtests, 0 failures. Full tests/protocols/ blast radius: 470 passed, 1341 subtests, 0 failures.

Three-dot origin/main...HEAD confirms only the two files. (The two-dot form additionally shows pcapkit/__init__.py — that is main's own 1.5.0b3 bump appearing as a reversal, not a change here; git merge-tree against main produces a tree whose __version__ is 1.5.0b3.)

This module constructs its own octets and reads nothing under examples/captures/, so it belongs to the unit tier and runs on a fresh checkout with nothing generated.

- Add examples/generators/dispatch.py, a FAMILIES-style enumerator over the
  seven __proto__ tables (Link, Internet, TCP, UDP, SCTP, pcap Frame, PCAPNG)
  mirroring examples/generators/options.py: it walks each registry directly,
  builds one minimal envelope per registered code, decodes it through
  pcapkit.extract(), and checks the resulting ProtoChain against a pinned
  expectation.
- The expected target is recorded independently in PINNED_TARGETS rather
  than read out of the registry under test (target=_resolve(entry)), which
  was self-referential: a mis-pointed entry -- registry size unchanged,
  class wrong -- passed silently, since dispatch always "reached" whatever
  the corrupted table said. PINNED_TARGETS fixes that, and a case with no
  pinned entry now fails test_every_case_has_a_pinned_target.
- The reached check also had to move off ProtoChain's own `in`/`index()`,
  which matches on alias string as well as class identity: Raw itself
  constructs with alias= set to the dispatching code's own name
  (pcapkit/protocols/misc/raw.py:144), so retargeting a registry entry at
  Raw still rendered under the original code's name and satisfied a
  string-based check. probe() now checks strict class identity via
  frame.protochain.protocols instead.
- Add tests/protocols/test_dispatch_registry_unit.py: fails when a registry
  gains a code with no case or no pinned target, fails when the live
  registry stops matching PINNED_TARGETS (test_registry_currently_matches_pinned_target),
  and otherwise asserts every case dispatches to its pinned target or
  degrades exactly as recorded in KNOWN_DEGRADED (IPX construction defect
  #492, x2; NGAP needing a well-formed PER payload, x2).
- Covers all 38 entries, including the ones test_dispatch_bindings_unit.py
  did not: Internet AH/HIP/Mobility_Header/SCTP, Link RARP, pcap Frame
  IPV4/IPV6, and PCAPNG's matching IPV4/IPV6 pair (previously unverified).

Build/test: tests/protocols/ -- 470 passed, 1341 subtests, 0 failures.
@JarryShaw
JarryShaw force-pushed the test/496-proto-dispatch-harness branch from 0d27064 to 9ff582d Compare September 19, 2026 04:03
@JarryShaw

Copy link
Copy Markdown
Owner Author

Diagnosis of the Python 3.15 failure — it was never version-specific

The failing job's log (reachable via gh api .../actions/jobs/<id>/logs --allow-escape-sequences, which works while the run-level log is still gated) named it precisely:

SUBFAILED(case='sctp/PayloadProtocolIdentifier_3GPP_NG_Application_Protocol')
AssertionError: 'malformed NGAP-PDU' not found in
  'NGAP: decoding needs the optional "pycrate" dependency, which is not installed;
   pip install pypcapkit[NGAP]'
  : still degrades, but not in the recorded way

Both SCTP NGAP entries in KNOWN_DEGRADED pinned their degradation reason to the string 'malformed NGAP-PDU'. That reason is environment-dependent. Where pycrate is installed the PER decoder runs and reports a malformed PDU; where it is not, NGAP reports the missing dependency instead. Both are the same placeholder-payload limitation, not two different defects — and the entry accepted only the first.

.github/workflows/unit-tests.yml:82 installs .[test], which deliberately excludes [NGAP] (pyproject.toml:114-122 keeps pycrate out of all on size and licence grounds). So no CI leg has it. Python 3.15 merely reported first; 3.10–3.14 were still queued and would have failed identically.

The generator's own comment asserted the opposite — that the failure occurs "the same way it would on any checkout regardless of whether the optional pycrate dependency is installed". That was factually wrong and is corrected, since it is the assumption that produced the failure.

The fix

Degrade.fragment now accepts a tuple, meaning any of its substrings is an acceptable reason, and both NGAP entries record both reasons. The assertion's message tells a future reader to add a reason rather than widen the entry to match anything, so this cannot decay into a blanket pass.

No library code touched; still test-only.

Verification — reproducing CI's condition locally

pycrate installs as pycrate_asn1dir/pycrate_asn1rt/pycrate_core; there is no top-level pycrate module, so a bare import pycrate is not a valid presence check. I blocked the real module names with a sys.meta_path finder and confirmed NGAP then emits CI's exact string:

NGAP: decoding needs the optional "pycrate" dependency, which is not installed; pip install pypcapkit[NGAP]
condition before the fix after
pycrate blocked (CI's condition) 2 failed, 7 passed, 81 subtests 7 passed, 83 subtests
pycrate present (local) 7 passed, 83 subtests 7 passed, 83 subtests

Both SUBFAILED lines in the "before" run name the two SCTP cases, matching CI's failure exactly.

The full unit tier — the same command CI runs, pytest -q --ignore=tests/integration --ignore-glob='*_runtime.py' --ignore-glob='*_regression.py' — passes in this branch's worktree on 3.14: 924 passed, 5 skipped, 1867 subtests, 0 failed.

Note on what this was not

This is the harness having pinned an environment-dependent detail too narrowly. It is not a library defect, and the two cases still degrade exactly as recorded — they simply have two legitimate ways of saying so.

#503 added the missing Socket(0x0000) member, so IPX now constructs and
parses. Both entries recorded it as degrading, and this harness asserts a
recorded degrade still degrades -- so leaving them in fails, by design:

  AssertionError: True != False : internet/IPX_in_IP was recorded as
  degrading (...#492) but now reaches its target
  ('Ethernet:IPv4:IPX:Unknown'). If the defect is fixed, delete its
  KNOWN_DEGRADED entry.

That is the ratchet working. Removed from both the test copy and the
generator copy.

Verified the cases genuinely reach IPX rather than merely stopping to
fail:

  link/Novell_Inc_0x8137: reached=True chain='Ethernet:IPX:Unknown'
  internet/IPX_in_IP:     reached=True chain='Ethernet:IPv4:IPX:Unknown'

7 passed, 83 subtests, both with pycrate present and with it blocked
(CI's condition, since .[test] excludes [NGAP]).

KNOWN_DEGRADED now holds only the two SCTP NGAP cases, which are a
placeholder-payload limitation rather than a library defect.
@JarryShaw

Copy link
Copy Markdown
Owner Author

Pushed 52bc0603c — the two IPX KNOWN_DEGRADED entries are gone, because #503 fixed them

Merging main into this branch brought in #503, which added the missing Socket(0x0000) member. Both IPX entries recorded those cases as degrading, and this harness asserts a recorded degrade still degrades — so leaving them in fails, by design:

AssertionError: True != False : internet/IPX_in_IP was recorded as degrading
  (same defect as link/Novell_Inc_0x8137 -- both dispatch to
   pcapkit.protocols.internet.ipx.IPX (#492))
  but now reaches its target ('Ethernet:IPv4:IPX:Unknown').
  If the defect is fixed, delete its KNOWN_DEGRADED entry.

That is the ratchet doing its job rather than a problem: the entry could not silently rot into a permanent exemption. Removed from both the test copy and the generator copy.

Verified the cases genuinely reach IPX rather than merely stopping to fail — worth distinguishing, since "no longer errors" and "parses correctly" are different claims:

link/Novell_Inc_0x8137: reached=True chain='Ethernet:IPX:Unknown'
internet/IPX_in_IP:     reached=True chain='Ethernet:IPv4:IPX:Unknown'

So this PR now also serves as an independent check on #503: IPX dispatch works end to end through a harness that knew nothing about that fix.

Tests, in both environments: 7 passed, 83 subtests with pycrate present, and 7 passed, 83 subtests with it blocked — the latter reproducing CI's condition, since .[test] excludes [NGAP].

KNOWN_DEGRADED now holds only the two SCTP NGAP cases, which are a placeholder-payload limitation of the probe rather than a library defect.

Pushed as a follow-up commit rather than an amend, since the merge commit 81893a7e6 sits in between and amending would have rewritten it.

@JarryShaw
JarryShaw merged commit a70710e into main Sep 19, 2026
24 checks passed
@JarryShaw
JarryShaw deleted the test/496-proto-dispatch-harness branch September 19, 2026 14:39
@JarryShaw JarryShaw added the test Pull requests that add or correct tests (test: subject prefix) label Sep 22, 2026
@JarryShaw JarryShaw added this to the 1.5 milestone Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test Pull requests that add or correct tests (test: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

No enumerating harness for the 38 __proto__ dispatch-registry entries (the OSPF regression's blind spot)

1 participant