Skip to content

fix(generators): make_samples builds fixtures from the shared checkout, not the tree it runs in - #1388

Merged
JarryShaw merged 1 commit into
mainfrom
fix-1343-generators-own-tree
Oct 8, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix-1343-generators-own-tree

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner
  • Searched for similar pull requests
  • Followed the coding style — isort and pylint (W/E) clean on the five files; mypy clean on the new test (the generators' remaining mypy errors predate this PR)
  • make test passes, and a test case covers the change — pytest tests/project and the option/dispatch generator modules, details below
  • Added a changelog entry — N/A, added centrally

What is the purpose of your pull request?

  • fix — corrects a defect

Description

Closes #1343

  • Own tree. make_samples.py, options.py, dispatch.py and pcapng.py (the generators that import pcapkit) put their tree root first on sys.path before any pcapkit import. options.py's one top-level pcapkit import moves into handler(). I used an inline insert rather than a shared _path.py because importing a sibling by name only works when the generator runs as a script. Loaded by path, from make_samples or a test, it would fail, and this directory is deliberately kept off sys.path.
  • Fail loudly. options.generate() writes what it can, then raises RuntimeError if any captured case was left out (pcapng.generate() already does this). No case is legitimately left out on a correct tree: EXPECTED_FAILURES holds only pcapng-secrets entries, and that family is not captured. CI runs make_samples.py under pipefail and gets 0 left out.

Probe (worktree, no PYTHONPATH/PYTHONSAFEPATH): before the fix, python examples/generators/make_samples.py printed 257 … 7 case(s) left out. After it prints 266 … 0 case(s) left out from the tree root, from cwd=/, and for options.py run on its own. With a stale pcapkit pre-imported, options.py now exits 1: RuntimeError: options: 7 case(s) left out ….

Tests. New tests/project/test_generators_import_their_own_tree.py gives 2 passed, 8 subtests passed. With git show origin/main: written over the four generators it gives 9 failed: each subtest reports imported pcapkit from /…/PyPCAPKit/pcapkit/__init__.py, plus RuntimeError not raised. pytest tests/project gives 391 passed, 1 skipped, and the project ordering leg gives 578 test(s), 0 failure(s). All runs used a 200 MB file-size cap.

…t, not the tree it runs in

Run as a script, a generator's sys.path[0] is examples/generators/, so
pcapkit resolved to the editable install's pinned checkout instead of the
tree being run. make_samples.py, options.py, dispatch.py and pcapng.py now
put their own tree root first on sys.path before any pcapkit import.

options.generate() also raises once its captures are written if any
captured case was left out, so a stale library fails make samples instead
of silently shrinking the fixture set.

Closes #1343
@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) review: running A cross-review is in flight against the current head - no verdict yet labels Oct 7, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review verdict on 892832f08: GOOD TO GO (Sonnet reviewed; author Opus).

  • Repro: with no PYTHONPATH, main's generators print 257 capturable case(s) … 7 case(s) left out and exit 0. With this PR they print 266 … 0 left out, and pcapkit.__file__ points into the worktree, whether run from the tree root or from cwd=/. options.py standalone does the same.
  • CI cannot trip the new raise:
    • Pipefail: all three make_samples.py steps (unit-tests.yml:280, :768, :1201) run under bash with pipefail, so a raise fails the step loudly.
    • Optional dependencies: with cryptography, pycrate_*, pyshark, dpkt, pypcap and pcapfile all blocked from import, options.generate() still yields 266 cases and 0 left out. The leanest CI install therefore cannot fail.
  • No test depends on a non-tree pcapkit. Every workflow installs with pip install -e.
  • The new test's worktree is detached under a mkdtemp directory and removed in tearDownClass, including after assertion failures. The revert run left no pcapkit-1343 entry behind. It skips when git or a repository is absent.
  • Tests:
    • The revert proof gives 9 failed / 1 passed.
    • tests/project gives 391 passed.
    • The option round-trip suite gives 6 passed (362 subtests).

Nit, worth a follow-up commit: if setUpClass raises after git worktree add (a failed copy2, say), tearDownClass never runs and the worktree leaks into the shared repo. Registering the removal with addClassCleanup immediately after worktree add closes it.

Python 3.10–3.13 and -n auto were not run locally. The case set is pure Python.

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

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Coverage: 88.97% (unit tier, Python 3.14, 892832f08, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18777 1032 2348 843 90.27%
pcapkit/corekit 1949 84 606 20 94.91%
pcapkit/dumpkit 189 1 68 1 99.22%
pcapkit/foundation 2443 102 852 39 94.39%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 16013 206 4178 177 98.06%
pcapkit/toolkit 539 74 168 3 84.58%
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 739e3da into main Oct 8, 2026
41 checks passed
@JarryShaw
JarryShaw deleted the fix-1343-generators-own-tree branch October 8, 2026 01:05
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 8, 2026
@JarryShaw JarryShaw added this to the PyPCAPKit v1.5 milestone Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(generators): make_samples builds fixtures from the shared checkout, not the tree it runs in

1 participant