Skip to content

refactor(corekit): share one PortEnumField across TCP, UDP and SCTP (#1516) - #1526

Merged
JarryShaw merged 1 commit into
mainfrom
refactor/1516-port-enum-field
Oct 9, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
refactor/1516-port-enum-field

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

Please follow the guide below

What is the purpose of your pull request?

  • refactor — changes neither behaviour nor performance

Description of your pull request and other information

The three PortEnumField classes in schema/transport/{tcp,udp,sctp}.py were AST-identical apart from the TransportProtocol member in post_process. They become one public PortEnumField in corekit/fields/numbers.py, beside EnumField, with a keyword-only proto. Each schema passes its own. It is documented on the numbers page and in the fields class diagram.

  • Byte-identical: all 1398 TCP, UDP and SCTP layers in examples/captures/* (1261/103/34) give the same octets and port members before and after. That covers parse→rebuild, from_data(info.to_dict()), and make→parse from members and from bare ports. Digest sha256 010bd641… both times.
  • Tests: tests/corekit/test_fields_numbers_port_enum_field_1516_unit.py has 7 tests, and all 7 fail on main. A mutant that ignores proto fails 5 subtests. Updated: the existing callers of the old classes, the field round-trip table, and the composite-proto sweep in tests/const.
  • Not re-exported from corekit.fields: doing so forces PortEnumField into corekit/__init__.py's __all__, which refactor(corekit)!: Info.to_dict() returns an OrderedMultiDict; add MultiInfo and OrderedMultiInfo (#1484) #1523 has in flight (test_public_api fails otherwise). SwitchField sets the precedent.
  • Selections run, not the whole suite: tests/corekit, tests/protocols/transport, each tests/protocols/test_*roundtrip*, tests/project, tests/test_docstring_contract.py, tests/test_tier_guard.py, plus the touched tests/const and tests/dumpkit modules. All pass.

Refs #1516

…1516)

- corekit.fields.numbers: add a public PortEnumField beside EnumField. It
  takes a keyword-only proto, the TransportProtocol member its ports belong
  to; its pre_process and post_process are the three copies' bodies, with
  that member in place of the hard-coded one.
- schema/transport/{tcp,udp,sctp}: drop the local PortEnumField classes,
  which an AST diff shows differed only in that member, and pass each
  schema's own transport to the shared field.
- docs: document it on the numbers page and in the fields class diagram;
  repoint the cross-references to the removed classes.
- tests: new unit tests pin the per-transport lookup, an unassigned port,
  pack and unpack; the existing callers pass proto, the field round-trip
  table gets one case per transport, and the composite-proto sweep follows
  the lookups into numbers.py and pins the six constructor calls.

Behaviour is byte-identical on the 1398 TCP, UDP and SCTP layers of
examples/captures/*.

Refs #1516
@JarryShaw JarryShaw added refactor Restructuring for its own sake — neither a fix nor a new capability (refactor: prefix) review: running A cross-review is in flight against the current head - no verdict yet labels Oct 9, 2026
@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Coverage: 90.10% (unit tier, Python 3.14, c68689e63, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18795 842 2356 756 92.09%
pcapkit/corekit 2186 67 710 34 96.24%
pcapkit/dumpkit 242 2 88 2 98.79%
pcapkit/foundation 2665 98 934 42 95.00%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 17021 229 4598 189 98.04%
pcapkit/toolkit 539 74 168 3 84.58%
pcapkit/utilities 431 4 124 4 98.56%
pcapkit/vendor 4409 2342 1006 157 43.25%

Per-file detail: the coverage-html artifact of this run.

@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at c68689e63. Cross-reviewed on Sonnet; the author ran Opus.

  • The three old classes are AST-identical once docstrings are stripped. The only difference is the Enum_TransportProtocol.<proto> member.
  • Field behaviour is identical. A sweep of post_process over every port 0–65535 and out-of-range values, plus unpack, pack, pre_process and pickling, compared 2,163,609 entries per run with transports in both orders. The only differences are the class's __module__ and the new _proto. 900 TCP/UDP ports and 6,155 TCP/SCTP ports resolve differently, so the sweep is not vacuous. No registry entries are minted.
  • Captures are byte-identical. On 1,398 TCP/UDP/SCTP layers, canonical to_dict(), from_data, pack({}) and three make→parse paths give identical output. The 69 json/tree/plist dumps are identical too.
  • The lazy AppType import is not a hot-path cost. extract(http.pcap) makes 0 pre_process calls.
  • Fail-before and mutation. The new tests give 29 failures. A mutant that ignores proto fails exactly 5 subtests.
  • The other changed tests are updated, not weakened.

Non-blocking:

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 9, 2026
@JarryShaw
JarryShaw merged commit f96368b into main Oct 9, 2026
41 of 42 checks passed
@JarryShaw
JarryShaw deleted the refactor/1516-port-enum-field branch October 9, 2026 23:47
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Restructuring for its own sake — neither a fix nor a new capability (refactor: prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant