Skip to content

test(generators): HIP_TRANSFORM is generated into a HIPv2 packet although it is HIPv1-only - #1128

Merged
JarryShaw merged 1 commit into
mainfrom
test/1122-hip-transform-v1
Oct 6, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
test/1122-hip-transform-v1

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 6, 2026 •

Copy link
Copy Markdown
Owner
  • Searched for similar pull requests
  • Followed the coding style (make pylint, make mypy, make isort) — not run; isort --check-only reports a pre-existing diff in options.py outside these lines
  • make test passes, and a test case covers the change — only the modules listed below
  • Added a changelog entry — 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 #1122. HIP_VERSION gets 577: 1, so the generator builds HIP_TRANSFORM (RFC 5201 only) at version 1. Its EXPECTED_FAILURES entry is removed; that entry blamed length arithmetic, but the real cause was the version. No library change.

  • make_samples.py before: [left out] hip-parameter/HIP_TRANSFORM: CONSTRUCT: ProtocolError: HIPv2: [ParamNo 577] invalid parameter (8 left out)
  • After: HIP_TRANSFORM is no longer left out (7 left out; HOST_ID remains, fix(hip): HOST_ID packs DI-Type/DI-Length as 4 octets where RFC 7401 has one 16-bit word #1118)
  • Proof that the entry removal depends on the fix: reverting 577: 1 with the entry removed gives SUBFAILED(case='hip-parameter/HIP_TRANSFORM'), 1 failed, 6 passed
  • Audit: I built every generated HIP code at v1 and at v2. Only 128 (R1_Counter) and 577 behave differently by version, and both are now generated at v1. The v2-only codes (511, 579, 715, 2049) and the v2 forms of PUZZLE and SOLUTION are generated at v2, which is correct.
  • Tests: test_option_roundtrip_unit.py 6 passed, 360 subtests passed; test_option_coverage_runtime.py 3 passed; internet/test_hip_unit.py 34 passed; tests/project 379 passed, 1 skipped

@JarryShaw JarryShaw added review: pending No verdict for the current head - never reviewed, or the head moved since the last one 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 and removed review: pending No verdict for the current head - never reviewed, or the head moved since the last one review: running A cross-review is in flight against the current head - no verdict yet labels Oct 6, 2026
@JarryShaw JarryShaw added the review: needs-changes Cross-review at the current head says changes are required; see the verdict comment label Oct 6, 2026
…ough it is HIPv1-only

HIP_TRANSFORM (577) is RFC 5201 only and the library rejects it at version 2,
but HIP_VERSION had no entry for it, so the option generator built it at the
default version 2 and left it out of the captures. Build it at version 1 and
drop its EXPECTED_FAILURES entry, which had misattributed the gap to length
arithmetic.

Closes #1122
@JarryShaw
JarryShaw force-pushed the test/1122-hip-transform-v1 branch from 8865ad6 to a227fea Compare October 6, 2026 19:17
@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment labels Oct 6, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review verdict on a227fea52: GOOD TO GO (ran on Sonnet; author Opus, round 2)

  • HIP_TRANSFORM: it is HIPv1-only. RFC 7401 marks 577 "v1 only", and hip.py:1343 enforces it. With 577: 1 added, make_samples.py stops leaving the case out (8 left out before, 7 after).
  • Failing without the fix: with the EXPECTED_FAILURES entry removed and 577: 1 reverted, test_option_roundtrip_unit gives SUBFAILED on that case. With the fix restored, 360 subtests pass.
  • Round 2: the two stale comments are fixed: the "two gaps" sentence and the HOST_ID cross-reference. The change touches comments only.

Nit: the following sentence still says "them", though one entry remains. Left for #1103.

Separately, HIPv2-only parameters are accepted in v1 packets. That is filed as #1133.

@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.74% (unit tier, Python 3.14, a227fea52, 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 1874 91 578 22 94.33%
pcapkit/dumpkit 136 0 40 0 100.00%
pcapkit/foundation 2422 143 842 34 92.62%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 15653 187 3942 162 98.19%
pcapkit/toolkit 487 71 144 3 84.15%
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 2ca586c into main Oct 6, 2026
40 checks passed
@JarryShaw
JarryShaw deleted the test/1122-hip-transform-v1 branch October 6, 2026 20:37
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label 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.

test(generators): HIP_TRANSFORM is generated into a HIPv2 packet although it is HIPv1-only

1 participant