Skip to content

test(generators): HTTP/2 sample frames are all written on stream 0 - #1161

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1156-httpv2-sample-frames
Oct 7, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1156-httpv2-sample-frames

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

Please follow the guide below

  • Searched for similar pull requests
  • Followed the coding style (make pylint, make mypy, make isort) — ran pylint/isort on the two files only; no new findings on my lines, make targets not run
  • make test passes, and a test case covers the change — only the modules listed below, not the full suite
  • Added a changelog entry under docs/source/changelog/ and regenerated CHANGELOG.md, if the change is user-visible — N/A, added centrally after the wave

What is the purpose of your pull request?

  • test — tests only

Description of your pull request and other information

Closes #1156. _httpv2_build now puts SETTINGS/PING/GOAWAY on stream 0 and every other frame type on stream 1; PUSH_PROMISE promises stream 2. The Length convention needed no generator change: on 2c7a97c84 the frames already declare the payload length (frame 35 is 000000000000000000, not 000009…), since #1138 fixed HTTP.make.

Probe (options-transport.pcap, frames 35–44, one per type): before, 4 TCP:HTTP/2 and 6 TCP:Raw (35–38, 40, 44); after, all 10 TCP:HTTP/2. No other generated capture changes. Captures are gitignored and generated, so none is committed.

Test: new tests/protocols/test_option_generator_httpv2_stream_unit.py (unit tier, no capture). Without the fix: 14 failed, 4 passed, 27 subtests passed; with it: 5 passed, 40 subtests passed. No existing test pinned the Raw decoding. Also passing: test_option_roundtrip_unit, test_option_coverage_runtime, test_fields_copy_dispatch_runtime, test_tcp_http_dispatch_unit, test_option_generator_tcp_base_unit, test_generated_pcap_runtime, and tests/project (389 passed, 1 skipped).

- _httpv2_build put every frame on stream 0, so DATA, HEADERS, PRIORITY,
  RST_STREAM, PUSH_PROMISE and CONTINUATION were frames no conforming
  sender writes and decoded as TCP:Raw since #1153. SETTINGS, PING and
  GOAWAY stay on stream 0; every other type uses stream 1.
- PUSH_PROMISE promises stream 2 rather than 0.
- New tests/protocols/test_option_generator_httpv2_stream_unit.py pins the
  stream per type, the payload-only Length, and TCP:HTTP/2 decoding.

Closes #1156
@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet test Pull requests that add or correct tests (test: subject prefix) labels Oct 6, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review verdict on dcd727ac5: GOOD TO GO (ran on Sonnet; author Opus)

  • Root cause confirmed: on main, frames 35–44 all sit on stream 0 with a payload-only Length. _test_http2_frame accepts types 0/1/2/3/5/9 only on a non-zero stream, so exactly the six Raw frames were these types (35, 36, 37, 38, 40, 44).
  • Stream assignments match RFC 9113 §6.1–6.10. PUSH_PROMISE on client stream 1 promising even stream 2 is coherent under §6.6 and §5.1.1.
  • Decoding: options-transport.pcap decodes 4/6 HTTP/2 vs Raw before and 10/0 after. The other four options-* captures decode identically.
  • The new test is independent and fails without the fix: it carries its own literal RFC table, and gives 14 failed with options.py reverted.
  • Composes cleanly with fix(http): HTTP.from_data raises 'invalid HTTP version: 0' #1162: merged together on main, this PR's test, test_option_roundtrip_unit and fix(http): HTTP.from_data raises 'invalid HTTP version: 0' #1162's test all pass.
  • Follow-ups, not blocking: the sample WINDOW_UPDATE carries a zero increment, which RFC 9113 §6.9 treats as an error (this predates the PR). Also, the test docstring's example failure text would read False != True, not 0 != 1.

@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 6, 2026
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Coverage: 88.87% (unit tier, Python 3.14, dcd727ac5, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18757 1035 2342 846 90.23%
pcapkit/corekit 1878 91 580 22 94.34%
pcapkit/dumpkit 142 0 42 0 100.00%
pcapkit/foundation 2422 104 842 40 94.24%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 15688 188 3960 163 98.18%
pcapkit/toolkit 487 65 144 1 85.42%
pcapkit/utilities 429 4 122 4 98.55%
pcapkit/vendor 4409 2359 1006 158 42.84%

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

@JarryShaw
JarryShaw merged commit f17c77f into main Oct 7, 2026
41 checks passed
@JarryShaw
JarryShaw deleted the fix/1156-httpv2-sample-frames branch October 7, 2026 00:49
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 7, 2026
JarryShaw added a commit that referenced this pull request Oct 7, 2026
- 4 new 1.5.0 entries: HTTP/2 sample stream identifiers (#1161),
  TCP option re-padding (#1163, breaking), format='pcap' output
  (#1164), declared lengths on truncated captures (#1166, breaking)
- extend the PCAP Header (#1159), HIP (#1160) and from_data
  round-trip (#1162) entries rather than duplicating them
- regenerate CHANGELOG.md with util/changelog_md.py
- process.rst: entry-count pin re-measured, 189 -> 193

tests/project: conventions_doc_claims, changelog_md and
documentation_claims pass; changelog_md.py --check is in step.
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.

test(generators): HTTP/2 sample frames are all written on stream 0

1 participant