Skip to content

test: run the PCAP-NG JSON report test now that keys are escaped (#1795) - #1809

Open
JarryShaw wants to merge 1 commit into
mainfrom
test/1795-unskip-pcapng-json
Open

JarryShaw wants to merge 1 commit into
mainfrom
test/1795-unskip-pcapng-json

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

Please follow the guide below

What is the purpose of your pull request?

Tick the commit type your subject line carries.

  • fix — corrects a defect
  • feat — adds a feature
  • perf — changes performance, not behaviour
  • refactor — changes neither behaviour nor performance
  • test — tests only
  • docs — documentation only
  • ci — workflows or build tooling
  • release — bumps the version or rolls up a distribution
  • chore — anything else

Summary

tests/integration/test_pcapng_end_to_end.py skipped PcapngUnescapedKeyTests.test_json_report_of_a_decryption_secrets_block_parses as "blocked on dictdumper". #1158 (for #1152) made pcapkit escape JSON mapping keys itself, so the block was no longer real.

  • Skip removed. The class and test docstrings now say what the test guards: the JSON report parses because pcapkit escapes keys, although dictdumper still writes them raw. Line-number citations are replaced by symbol names.
  • HAS_RUNTIME guard added, matching the sibling classes, so a checkout without the runtime dependencies skips rather than fails. tests/integration/_helpers.py now says every subclass carries the guard.
  • Bite: with escape_key's JSON branch turned into a no-op, the test fails with JSONDecodeError. With HAS_RUNTIME false, it skips.
  • Runs: the file has 7 passed; tests/project and tests/test_tier_guard.py pass.

Closes #1795

- Drop the "blocked on dictdumper" skip from
  PcapngUnescapedKeyTests.test_json_report_of_a_decryption_secrets_block_parses.
  #1158 made pcapkit.dumpkit escape JSON mapping keys itself, so the
  test.pcapng report parses although dictdumper 0.8.4.post6 still writes
  keys raw (JarryShaw/DictDumper#121).
- Give the class the HAS_RUNTIME guard its siblings carry; _helpers.py to match.
- Rewrite the class and test docstrings to say what the test now guards,
  citing symbols instead of the stale pcapng.py:2244 / common.py:351 lines,
  and fix the PCAPNG_SECTIONS comment that called the report unreadable.

Bite: with escape_key's JSON branch returning the key unchanged, the test
fails with "JSONDecodeError: Expecting ':' delimiter"; restored, it passes.
The file, tests/project and tests/test_tier_guard.py pass.

Closes #1795
@JarryShaw JarryShaw added the test Pull requests that add or correct tests (test: subject prefix) label Oct 16, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at 58da7d377. A Sonnet cross-review and re-check (author: Opus). The skip removal is right: #1158 escapes the keys. The new HAS_RUNTIME guard matches its siblings, and an AST scan finds no unguarded subclass left. The test fails with escaping disabled and skips without the runtime dependencies. tests/project and the tier guard pass.

@JarryShaw JarryShaw added the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 16, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 16, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review: good-to-go Cross-review at the current head says ready; CI state is separate test Pull requests that add or correct tests (test: subject prefix)

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

test: the PCAP-NG JSON report test is still skipped after #1158 fixed its cause

1 participant