Skip to content

docs(logging,pcap): correct the per-frame logging note; pin Frame.read cap_len (#2236) - #2250

Merged
JarryShaw merged 1 commit into
mainfrom
docs/2236-logging-frame-read
Oct 26, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
docs/2236-logging-frame-read

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • docs — documentation only

Description

Closes #2236.

  • logging.rst: "What DEBUG Will Tell You" says no record is logged per frame parsed or per field read. It names the two lines that fire per dropped item: PyShark's skipping tshark frame and reassembly's abandoning buffer. No other per-frame logger.debug or logger.info exists.
  • Frame.read: the false "no test pins this line" comment is gone. The comment now states why cap_len is passed: the schemas count down from it, so a truncated header warns. It cites the new test.
  • Test: test_truncated_ipv4_header_warns_whatever_the_on_wire_length builds Ethernet plus 10 IPv4 octets with orig_len 154. It asserts packet length < 0: -10 is warned, the same as for an uncut record.
  • Fail-without-fix: with the argument patched back to frame.len, the test fails ('packet length < 0: -10' not found in []).
  • Tests run: tests/protocols/misc/pcap, tests/utilities/test_logging.py and the frame-length readers give 300 passed. The docstring contract, tier guard and test_docs_* give 208 passed.

…d's cap_len (#2236)

Problem: the logging.rst DEBUG note said nothing is logged inside per-frame
loops, but PyShark logs each skipped non-packet tshark record and reassembly
logs each buffer abandoned on timeout. The Frame.read comment said no test
pins the cap_len argument and reverting it leaves the suite green, yet a frame
cut inside its IPv4 header warns "packet length < 0" only with cap_len.

Fix: the note says no record is logged per frame parsed or field read, and
names the two per-dropped-item lines. The comment states what the length
does for the schemas and why, and names the test that pins it.

Test: test_frame_read_cap_len_2236_unit asserts the SchemaWarning for that
frame, identical to an uncut frame of the same octets; it fails when the
argument is reverted to frame.len.
@JarryShaw JarryShaw added docs Pull requests that change documentation only (docs: subject prefix) test Pull requests that add or correct tests (test: subject prefix) review: running A cross-review is in flight against the current head - no verdict yet labels Oct 26, 2026
@JarryShaw JarryShaw added this to the PyPCAPKit v1.5 milestone Oct 26, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Review: GOOD TO GO at dd21c5174 (Sonnet).

  • logging.rst: the only per-dropped-item lines under pcapkit/ are PyShark's skipping tshark frame and reassembly's abandoning buffer. The per-flow lines are covered by the paragraph above the note.
  • Fail-without-fix: with frame.len patched in, the new test fails with 'packet length < 0: -10' not found in []. The comment's arithmetic holds: 24 − 14 = 10 octets against a 20-octet IPv4 header.
  • Tests: pcap, logging, the docstring contract and the tier guard give 210 passed.

@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 26, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 26, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 91.56% (unit tier, Python 3.15, dd21c5174, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 2 95.16%
pcapkit/const 19599 881 2618 867 92.11%
pcapkit/corekit 2589 74 816 37 96.51%
pcapkit/dumpkit 282 3 102 3 98.44%
pcapkit/foundation 2994 23 1038 25 98.81%
pcapkit/interface 138 4 52 3 96.32%
pcapkit/protocols 17433 218 4584 185 98.15%
pcapkit/toolkit 869 36 298 10 95.20%
pcapkit/utilities 504 7 146 5 98.15%
pcapkit/vendor 4432 1988 1018 167 51.87%

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

@JarryShaw

Copy link
Copy Markdown
Owner Author

Ready to merge at dd21c5174. Closes #2236.

  • Review: Sonnet GOOD TO GO, first round.
  • CI: all checks complete, Required checks passed SUCCESS, 0 failed.
  • KTLO: 12 legs, no warnings.
  • Ordering: merged into main and run. protocols leg: 1925 tests, 0 failures.
  • Conflicts: none against the current main.

@JarryShaw
JarryShaw merged commit 21b0b57 into main Oct 26, 2026
70 checks passed
@JarryShaw
JarryShaw deleted the docs/2236-logging-frame-read branch October 26, 2026 14:54
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 26, 2026
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Pull requests that change documentation only (docs: subject prefix) test Pull requests that add or correct tests (test: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

docs(logging,pcap): per-frame logging note and Frame.read suite claim are false

1 participant