Skip to content

test(const): the registry-protocol census calls a shipped shape synthetic, and EXCLUDED_STILL_BESPOKE no longer means what it says #892

Closed

Description

@JarryShaw

Describe the bug

Two related inaccuracies in tests/const/test_const_registry_protocol.py, both pre-existing and both about the same thing — the file's census vocabulary has fallen behind #860's conversions.

1. A docstring asserts something false. test_default_still_resolves_when_registered_str at :1536-1539 says its case is "synthetic, since no shipped aenum.StrEnum registry mixes in the base yet (see EXCLUDED_STILL_BESPOKE)". Measured on main by walking pcapkit.const and filtering for classes that subclass both EnumRegistry and str with __module__ equal to their own module:

shipped StrEnum registries mixing in EnumRegistry: 10
  pcapkit.const.ftp.command.Command
  pcapkit.const.ftp.command.FEATCode
  pcapkit.const.http.method.Method
  pcapkit.const.pcapng.option_type.OptionType
  pcapkit.const.reg.apptype.apptype.AppType
  pcapkit.const.reg.apptype.dccp.DCCP
  pcapkit.const.reg.apptype.sctp.SCTP
  pcapkit.const.reg.apptype.tcp.TCP
  pcapkit.const.reg.apptype.udp.UDP
  pcapkit.const.pcapng.tls_key_label.TLSKeyLabel     <- added by #890

Nine of the ten predate #890, having landed through #860's steps, so the sentence was already wrong; #890 merely adds the tenth. The test itself is fine — only its justification is stale, and "synthetic" now understates its value, since it is covering a shape the library really ships.

2. EXCLUDED_STILL_BESPOKE is a misnomer for most of its members. Its comment describes modules that override the base's generic implementation, plus TransportProtocol for never mixing in EnumRegistry at all. But of the eleven entries, five already mix in EnumRegistry and three of those are StrEnum. The set's operative meaning is "outside this file's IntEnum-only historical census, anchored at 05468a06b" — which is a legitimate thing to have, just not what the name or comment says.

Expected behavior

The docstring at :1536 states the truth: the shape is shipped by ten registries, and the test pins it rather than inventing it. And the set is renamed to what it means — something like EXCLUDED_FROM_INTENUM_CENSUS — with its eleven entries re-sorted and given per-entry reasons, since they are excluded for at least three different ones (overrides the base; never mixes in the base; not an IntEnum).

Additional context

Surfaced by the cross-review of #890, which added the eleventh entry. Deliberately not fixed there: #890 neither created either problem nor is the right place to rename a shared census set, and widening it would have put a rename of eleven entries inside a PR about a crawler. The comment #890 adds is honest in the meantime — it says outright that the exclusion is not a judgement about the file's inherited-protocol cleanliness, and names the three dynamic pkgutil.walk_packages sweeps that do cover it.

Worth doing together, since the rename is what makes the docstring's EXCLUDED_STILL_BESPOKE pointer correct rather than merely less misleading.

Activity

  1. added
    bugIssues reporting a defect (set by the bug report template; a default, not an assessment)
    testPull requests that add or correct tests (test: subject prefix)
    wipWork in flight - a covering PR is open or an agent is actively on it
    on Sep 29, 2026
  2. removed
    wipWork in flight - a covering PR is open or an agent is actively on it
    on Sep 29, 2026
  3. added this to the 1.5 milestone on Oct 6, 2026
  4. moved this to Done in PyPCAPKiton Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugIssues reporting a defect (set by the bug report template; a default, not an assessment)testPull requests that add or correct tests (test: subject prefix)

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions