From 0232dd8128581816cbc6770e24652914664ac3dd Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Fri, 16 Oct 2026 15:55:21 -0400 Subject: [PATCH] docs(repo): third whole-repo pass over the repository files (#1103) Counted numbers: - Drop counts from workflow comments, the release-notes and coverage configs, README, CONTRIBUTING, MANIFEST.in, the Makefile, pyproject.toml comments, util/ docstrings, and the examples' docstrings and READMEs. - Replace lint.yml's finding tables with why the header keeps none; the measurements that justify a design choice stay, stated as measurements. - Keep the edits to line-cited files line-neutral, so the citations hold. Accuracy: - lint.yml: pcap-ct is also absent, so it is an import-error source too. - release.yml: dependencies PRs come from pyup and Dependabot, feat and fix never overlap each other, and the category is "Fixes". - CONTRIBUTING: which _OPT_Option siblings stay documented. - legacy.py's frame-range comments, _engine_support.py's engine claim and cron-conda.yml's stale create-release.yml line citation. Refs #1103 --- .github/coverage.toml | 2 +- .github/python-versions.toml | 6 +- .github/release.yml | 45 ++++--- .github/workflows/ci-timing.yml | 2 +- .github/workflows/coverage-comment.yml | 2 +- .github/workflows/create-release.yml | 54 ++++----- .github/workflows/cron-conda.yml | 4 +- .github/workflows/cron-python-versions.yml | 6 +- .github/workflows/cron-vendor.yml | 4 +- .github/workflows/lint.yml | 130 +++++++-------------- .github/workflows/project-status.yml | 2 +- .github/workflows/python-compatibility.yml | 2 +- .github/workflows/unit-tests.yml | 120 +++++++++---------- CONTRIBUTING.md | 18 +-- MANIFEST.in | 8 +- Makefile | 12 +- README.md | 8 +- examples/benchmark/Dockerfile | 4 +- examples/benchmark/README.md | 48 ++++---- examples/benchmark/benchmark.py | 12 +- examples/benchmark/entrypoint.sh | 6 +- examples/benchmark/python-images.txt | 2 +- examples/benchmark/report.py | 6 +- examples/benchmark/run.sh | 8 +- examples/benchmark/test_harness.py | 12 +- examples/generators/dispatch.py | 32 ++--- examples/generators/endian.py | 14 +-- examples/generators/legacy.py | 22 ++-- examples/generators/make_samples.py | 2 +- examples/generators/options.py | 28 ++--- examples/generators/pcap.py | 8 +- examples/generators/pcapng.py | 28 ++--- examples/legacy_smoke/Makefile | 2 +- examples/legacy_smoke/README.md | 18 +-- examples/legacy_smoke/_engine_support.py | 8 +- pyproject.toml | 40 +++---- util/bump_version.py | 11 +- util/changelog_md.py | 32 ++--- util/ci_timing.py | 4 +- util/project_status.py | 2 +- util/pyshark_encap_map.py | 6 +- util/run_unittest_leg.py | 22 ++-- util/update_python_versions.py | 6 +- util/verdict_covers_checkout.py | 14 +-- 44 files changed, 386 insertions(+), 436 deletions(-) diff --git a/.github/coverage.toml b/.github/coverage.toml index 6c0671255a..1018463b82 100644 --- a/.github/coverage.toml +++ b/.github/coverage.toml @@ -1,7 +1,7 @@ # Coverage configuration for the one coverage-measured leg of unit-tests.yml # (`test`, Python 3.15), passed with `--rcfile`. Coverage reads a single config # file, so this repeats pyproject.toml's [tool.coverage.*] settings and adds the -# four below on top. tests/project/test_coverage_rcfile.py asserts every +# settings below on top. tests/project/test_coverage_rcfile.py asserts every # pyproject setting is repeated here unchanged. # # parallel -- each process writes its own .coverage.* file, merged afterwards diff --git a/.github/python-versions.toml b/.github/python-versions.toml index fafc50472f..162b9eca7f 100644 --- a/.github/python-versions.toml +++ b/.github/python-versions.toml @@ -35,7 +35,7 @@ latest = "3.15" # no wheel, conda build or required context. The weekly job has no source for # either, so it keeps these lines as written, after the other versions of their # table. Promoting one once its leg is green is a hand edit, which -# util/python_versions.py refuses until it has those three. +# util/python_versions.py refuses until it has all of them. [versions.graalpy] "graalpy-25.4" = { tier = "experimental" } @@ -43,8 +43,8 @@ latest = "3.15" # tests/project/test_python_versions_1660.py fails on a pin not listed here, on # an entry that matches no pin, and on an entry without a comment. -# The parity matrix's one supported leg saves the apt cache. Both legs install -# one set under one key, and a second saver fails with `Unable to reserve +# Only the parity matrix's supported leg saves the apt cache. Every leg installs +# one set under one key, and another saver fails with `Unable to reserve # cache`. [[exemptions]] workflow = "unit-tests.yml" diff --git a/.github/release.yml b/.github/release.yml index a9bd48c2e6..a61a8c478d 100644 --- a/.github/release.yml +++ b/.github/release.yml @@ -12,34 +12,31 @@ # the type a pull request announces in its title cannot reach this file. # # Merged pull requests carry a type label, set by hand, which is what makes a -# category per label possible -- an earlier survey of 40 pull requests, -# mostly untyped, supported only a bare Dependencies/catch-all split. -# -# Sampling 334 merged pull requests (`gh pr list --state merged --limit 400`) -# gives the frequency below; zero carry no type label: -# -# fix 124, dependencies 97, breaking 50, ci 41, test 31, docs 28, feat 12, -# release 10, chore 9, perf 7, const 2, refactor 2 +# category per label possible -- an earlier survey of pull requests, mostly +# untyped, supported only a bare Dependencies/catch-all split. The order below +# was derived from a sample of merged pull requests (`gh pr list --state +# merged`), every one of which carried a type label, `fix` most often. # # GitHub files a pull request under the *first* category whose `labels` it -# matches, so category order is what resolves the 69 pull requests that carry +# matches, so category order is what resolves the pull requests that carry # more than one type label. This order is derived from those overlaps, traced # against the actual pull requests, not guessed: # -# * `breaking` is first, full stop. It co-occurs with `fix` (39x, +5 more with -# `test` too), `feat` (3x), `ci`+`release` (2x, +1 more with `fix` too) and -# `refactor` (1x) -- e.g. #25 "New distribution [0.14.0]" carries +# * `breaking` is first, full stop. It co-occurs with `fix` (most often, at +# times with `test` too), `feat`, `ci`+`release` (at times with `fix` too) +# and `refactor` -- e.g. #25 "New distribution [0.14.0]" carries # breaking+ci+fix+release. Any other position buries a breaking change -# under "Fixed" or "Release", which is the one mistake this file exists to +# under "Fixes" or "Release", which is the one mistake this file exists to # avoid. -# * `release` is second. Its overlaps with `ci` (3x) and with `ci`+ -# `dependencies` (1x) are all old "New distribution [x]" pull requests +# * `release` is second. Its overlaps with `ci` and with `ci`+ +# `dependencies` are all old "New distribution [x]" pull requests # (#9, #11, #21, #22) -- a version bump that happens to also touch a # workflow file or a lockfile is still, in substance, a release. `release` -# also wins its one overlap with `fix` (#14, "New distribution [0.13.3]"). -# * `feat` and `fix` follow, in that order by convention: neither overlaps -# with anything ranked above, so nothing in the data forces this half. -# `refactor` comes next; its only overlap is with `breaking`, above. +# also wins its overlap with `fix` (#14, "New distribution [0.13.3]"). +# * `feat` and `fix` follow, in that order by convention: they never overlap +# each other, so nothing in the data forces this half. +# `refactor` comes next; in the sample its only overlap was with +# `breaking`, above. # * `ci` beats `perf` (#727, subject `ci(unit-tests):`, labelled ci+perf), # `const` (#12 "Update Makefile", labelled ci+const) and `dependencies` # (#41 "Config file for pyup.io", labelled ci+dependencies) -- in each case @@ -53,9 +50,9 @@ # against). # * `dependencies` sits second-to-last rather than first -- see below. # -# `dependencies` used to be the first category, and is 97 of the 334 sampled -# pull requests, all Dependabot noise. Moving it down is a judgement call, not -# a data requirement: nothing in the overlaps above forces its position +# `dependencies` used to be the first category, and its pull requests are +# almost all bot noise, from pyup and Dependabot. Moving it down is a +# judgement call, not a data requirement: nothing in the overlaps above forces its position # relative to anything except `release` and `ci`, both of which already rank # ahead of it regardless of where it sits. The case for moving it here: a # future pull request that mixes a real change with a `dependencies` label @@ -63,12 +60,12 @@ # instead of being swallowed by bot noise -- which is also, concretely, what # keeps #22 ("New distribution [0.13.3.post2]", ci+dependencies+release) filed # under Release rather than Dependencies. The case for leaving it first, as -# before: it keeps the highest-volume, lowest-interest category out of the way +# before: it keeps a high-volume, low-interest category out of the way # of everything else on a first read. This file takes the former; flip its # position in the list below if that judgement is wrong. # # `*` catches whatever matches none of the categories above -- nothing in the -# sample, since every pull request in it carries a type label, but it stays +# sample, since every pull request in it carried a type label, but it stays # last regardless so an unlabelled future pull request still lands somewhere # instead of silently vanishing from the notes. # diff --git a/.github/workflows/ci-timing.yml b/.github/workflows/ci-timing.yml index 58c2d58544..19f4a0aa10 100644 --- a/.github/workflows/ci-timing.yml +++ b/.github/workflows/ci-timing.yml @@ -30,7 +30,7 @@ on: # Read-only: the runs, jobs and artifacts need `actions: read`, the workflow # files at each run's sha need `contents: read`. GITHUB_TOKEN allows 1,000 API -# requests an hour per repository; a default run uses about 690. +# requests an hour per repository; a default run was estimated at about 690. permissions: actions: read contents: read diff --git a/.github/workflows/coverage-comment.yml b/.github/workflows/coverage-comment.yml index deaf07daa5..41437e61d0 100644 --- a/.github/workflows/coverage-comment.yml +++ b/.github/workflows/coverage-comment.yml @@ -4,7 +4,7 @@ name: Coverage Comment # unit-tests.yml as a pull-request comment. It runs no tests. The comment cannot # be posted from unit-tests.yml itself: that file is also a reusable workflow, # and a called workflow can only narrow the token its caller passes, never widen -# it. All four callers grant `contents: read`, so asking for +# it. Every caller grants `contents: read`, so asking for # `pull-requests: write` there would break every one of them, the release gate # included. # diff --git a/.github/workflows/create-release.yml b/.github/workflows/create-release.yml index edc143c0bc..ea0f9749e1 100644 --- a/.github/workflows/create-release.yml +++ b/.github/workflows/create-release.yml @@ -32,8 +32,8 @@ concurrency: cancel-in-progress: false # Every job below with an effect outside this run declares a deployment -# ``environment:``, but only one of them -- ``github-release`` -- carries a -# required reviewer (#887). Four jobs still get four environments rather than +# ``environment:``, but only ``github-release`` carries a required reviewer +# (#887). Each such job still gets an environment of its own rather than # one shared ``release``, for credential and blast-radius separation, not for # approval count: # @@ -50,7 +50,7 @@ concurrency: # published. See :file:`docs/source/contributing/releasing.rst` for the full pipeline and the # reasoning behind one approval being enough. # -# The other three environments keep their names -- ``pypi``'s matters because +# The other environments keep their names -- ``pypi``'s matters because # PyPI trusted publishing matches its OIDC claim against it -- and keep their # secret scoping, they just hold no required reviewer of their own. Do # not add one back to any of them without re-examining the ``needs:`` chain: a @@ -60,7 +60,7 @@ concurrency: # ``tests/project/test_release_gates.py``'s ``TestGatedJobsDependOnGithub`` # asserts that every gated job's ``needs:`` closure includes ``github``, which # is the checkable half of this paragraph -- reverting the line above to -# ``needs: [ version_check ]`` fails only that one test. +# ``needs: [ version_check ]`` fails only that test. # # One environment per distinct credential and blast radius regardless of who # gates it: a single ``release`` approved once would release PyPI and Anaconda @@ -75,7 +75,7 @@ concurrency: # whereas ``tag`` spends a long-lived PAT and a deploy key to write to the # default branch. # -# Two things about this that are not self-enforcing, and #641 is the first of them: +# Some things about this are not self-enforcing, and #641 is the first of them: # # * An ``environment:`` naming an environment that does not exist is created # implicitly, with no protection rules, and the job proceeds unapproved. So @@ -129,7 +129,7 @@ concurrency: # pair does not. A cancelled ``tag`` would otherwise send ``conda`` into its # check that ``conda-+0`` names this run's commit, for a tag ``tag`` # never pushed, trading a clean skip for a failed step. ``version_check`` is held -# to a stricter bar still: all three require ``needs.version_check.result == +# to a stricter bar still: all of them require ``needs.version_check.result == # 'success'`` with no ``|| 'skipped'``, since it produces the evidence the gates # below read, and a skipped or cancelled ``version_check`` leaves them nothing # to decide on. Either spelling bypasses the default ``success()`` GitHub @@ -148,9 +148,9 @@ concurrency: # that the gate -- a ~20-minute full-suite run -- is spent only when something # is going to be published. Every successful Vendor Update triggers this # workflow, and most of them leave ``__version__`` alone, so the gate used to -# run on commits Unit Tests had already passed only for all four jobs below to +# run on commits Unit Tests had already passed only for every job below to # skip (eight times on 2026-10-05 alone). ``unit-tests`` runs when the ref -# is a ``v*`` tag or *any* of the four evidence outputs is not ``'true'``, +# is a ``v*`` tag or *any* of the evidence outputs is not ``'true'``, # which covers every case in which ``github``, ``tag``, ``pypi`` or ``conda`` # could run: each of them runs only on a ``v*`` ref or on its own evidence # reading ``'false'``. ``!= 'true'`` rather than ``== 'false'`` is deliberate -- @@ -191,11 +191,11 @@ concurrency: # and runs ``tag``'s scripts against a scratch repository. # # #1748: so a version tagged at another commit would fail every later run for -# it: ``v1.5.0b12`` names ``2025da5e5`` with 1 of its 10 Anaconda files, and -# each push-raised run spent the test gate and 10 ``conda`` legs on refusals. +# it: ``v1.5.0b12`` names ``2025da5e5`` with only part of its Anaconda files, +# and each push-raised run spent the test gate and every ``conda`` leg on refusals. # ``version_check``'s ``owner`` step resolves the ``v*`` tag as the #1634 check # does, and on a ref that is not a ``v*`` tag, ``PCAPKIT_RELEASED_ELSEWHERE`` -# reading ``'true'`` skips the gate and all four publishers -- ``github`` +# reading ``'true'`` skips the gate and every publisher -- ``github`` # mostly skips there already, the tag existing. ``release_status`` then posts # a notice when that release is complete and a warning naming what it lacks # when not, and stays green. A ``v*`` push still meets the #1634 check, and @@ -209,7 +209,7 @@ jobs: # ``success``, and Create Release took it for a real one (Vendor Update # 38650191283, a second Create Release run for ``0cdd3cdd1``). Measured: that # run's jobs read ``success``/``skipped``/``skipped``, and 38655782606, whose - # three jobs all skipped, concluded ``skipped``; ``verdict`` has to run to + # jobs all skipped, concluded ``skipped``; ``verdict`` has to run to # decide, so Vendor Update could not conclude otherwise without a failed or # cancelled run. This job reads that run's ``vendor-update`` job instead, with # "List jobs for a workflow run", whose ``filter`` defaults to ``latest``, @@ -269,7 +269,7 @@ jobs: # The outermost gate, moved here from ``unit-tests`` by #1052: a # ``workflow_run`` trigger proceeds only when the Vendor Update run that # triggered it concluded ``success``, and a tag push always proceeds. This - # job is read-only -- it installs the package and reads three public + # job is read-only -- it installs the package and reads public # endpoints -- so running it ahead of the test gate risks nothing. # # #1634: a ``v*`` push by the release App's bot skips too. That is the tag @@ -303,7 +303,7 @@ jobs: # #1660: .github/python-versions.toml, read with the runner's own python3: # `latest` for this job and the ``pypi`` and ``conda`` matrices, from this - # run's commit, and the tags the two completeness checks below count until + # run's commit, and the tags the completeness checks below count until # the version is tagged. - name: Read the Python versions id: python-versions @@ -337,7 +337,7 @@ jobs: # above legitimately uses since the ``v*`` tag *is* its own artefact. It is # the wrong question for ``tag``, ``pypi`` and ``conda``, whose artefacts # are a different tag, PyPI, and Anaconda respectively -- a half-failed - # release leaves the ``v*`` tag behind while those three are still + # release leaves the ``v*`` tag behind while those are still # incomplete, and gating them on it is what made every one of them skip # silently. Each gets its own evidence step below instead. - uses: mukunku/tag-exists-action@v1.7.0 @@ -345,7 +345,7 @@ jobs: with: tag: "conda-${{ steps.get_version.outputs.PCAPKIT_VERSION }}+0" - # #1660: the two checks below count what the version was released with. + # #1660: the checks below count what the version was released with. # Once its ``v*`` tag exists, that is the tier file at the tag's commit, # read by that commit's own util/python_versions.py, not this run's file: # a version added to the supported tier since was never built for it, and @@ -404,7 +404,7 @@ jobs: # # The per-version endpoint's own ``releases`` mapping was empty on every # request checked while writing this, including against a version that - # genuinely has all 8 files -- so this reads ``urls``, the field the + # genuinely has every file -- so this reads ``urls``, the field the # per-version endpoint actually populates for the requested version. # ``/pypi/pypcapkit/json`` (no version) does populate ``releases``, but at # ~950KB against this endpoint's ~15KB, for one release's worth of data. @@ -573,7 +573,7 @@ jobs: # effect, and a job that calls a reusable workflow may not declare one anyway. # # #1052: runs only when a release will actually happen -- a ``v*`` tag push, - # or any of the four targets still incomplete by its own evidence. See the + # or any target still incomplete by its own evidence. See the # note above ``jobs:`` for why ``!= 'true'`` and why a skip here can never # let a publisher through. #1748: nor on a version tagged at another commit, # unless this is a ``v*`` push, on the same clause as ``tag``, ``pypi`` and @@ -603,7 +603,7 @@ jobs: # ``github.sha`` (#1634) -- that every job below then verifies before # publishing under it. This is the release's only approval (#887) -- # see the note above ``jobs:``. Still gated on ``PCAPKIT_TAG_EXISTS``, - # unlike the three jobs below it: the tag that output answers for is this + # unlike the publishing jobs below it: the tag that output answers for is this # job's own artefact, not a proxy for someone else's -- see the #888 note # above ``jobs:``. ``unit-tests`` is listed because the gate follows # ``version_check`` (#1052); this ``if:`` calls no status-check function, @@ -613,8 +613,8 @@ jobs: # #1748: a tag naming another commit is a tag that exists, so # ``PCAPKIT_TAG_EXISTS`` already skips this job off a ``v*`` ref -- unless # the tag was pushed between ``check_tag`` and ``owner``, which read it - # separately. The same clause as the other three covers that too, so the - # gate runs whenever any of the four would. + # separately. The same clause as the other publishers' covers that too, so the + # gate runs whenever any publisher would. environment: github-release permissions: contents: write @@ -830,7 +830,7 @@ jobs: fetch-depth: 0 # Everything after this step writes through the deploy key loaded above. - # Kept to these lines alone: the project test runs the two steps below + # Kept to these lines alone: the project test runs the steps below # against a scratch repository, standing in for this one. - name: Configure git for the deploy key run: | @@ -1103,7 +1103,7 @@ jobs: # byte-identical -- but only an asset in state ``uploaded``: after a 502, # Upload a release asset "may leave an empty asset with a state of # starter", which ``--clobber`` deletes before uploading the file again. - # Two failures the action did not have: no release fails ``gh release + # Failures the action did not have: no release fails ``gh release # view`` with "release not found", where the action created one with the # default branch as its target, and a pattern matching nothing is an # ``::error``, where it only warned. ``conda`` runs the same script. @@ -1466,7 +1466,7 @@ jobs: packages: ${{ steps.conda_build.outputs.packages }} labels: ${{ needs.version_check.outputs.PCAPKIT_CONDA_LABEL }} - # ``pypi``'s upload script, for the same two reasons (#1634, #1640). Not + # ``pypi``'s upload script, for the same reasons (#1634, #1640). Not # gated on ``check_leg`` as the Anaconda upload is: a leg whose Anaconda # upload landed and whose GitHub upload then failed would get no GitHub # asset on any re-run. Skipping a file the release already has is what @@ -1644,11 +1644,11 @@ jobs: # and skips with work outstanding, every publisher skips behind it # and the same reconciliation reaches the loud #888 branch instead. # - # Reconcile each of the four release jobs against its OWN evidence, + # Reconcile each release job against its OWN evidence, # not against each other. A self-heal is a MIXED outcome by design # -- github/tag skip because their artefacts already existed going # in, pypi/conda run and succeed because theirs did not -- and - # demanding uniformity across all four (either all skipped, or all + # demanding uniformity across all of them (either all skipped, or all # succeeded, as an earlier revision of this job did) reads that # mix as the #888 shape itself, firing the error below on the run # that just finished the release. "Reconciled" means: this target @@ -1695,7 +1695,7 @@ jobs: fi # Every target reconciled, but at least one of them actually ran -- - # either a fresh full release (all four `success`) or a self-heal + # either a fresh full release (every one `success`) or a self-heal # (a mix of `success` and legitimately-`skipped`). Both are the # release working as intended, so both are quiet. if [ "$all_reconciled" = "true" ]; then diff --git a/.github/workflows/cron-conda.yml b/.github/workflows/cron-conda.yml index 531c3919ee..10bd7eccfe 100644 --- a/.github/workflows/cron-conda.yml +++ b/.github/workflows/cron-conda.yml @@ -325,7 +325,7 @@ jobs: # with its own `!cancelled()` does not escape it for this job. Run # 38325136710 shows it: "Update requirements.txt" succeeded, pushing # f4f94bcad, and both this job and `conda-dist` reported `skipped` with - # their own conditions true. So the five bumps #1632 lists were committed + # their own conditions true. So the bumps #1632 lists were committed # and never tagged or uploaded. # # `needs.conda-update.result == 'success'` is then what the lost implicit @@ -334,7 +334,7 @@ jobs: # update -- which still publishes the outputs of the steps that did run -- # would have this job tag from a half-finished run, e.g. one whose push # landed and whose Get Version then failed, leaving no build. Same shape as - # create-release.yml's `tag` job (:494-498). + # create-release.yml's `tag` job. if: >- !cancelled() && needs.conda-update.result == 'success' && needs.conda-update.outputs.CONDA_CHANGED == 'true' diff --git a/.github/workflows/cron-python-versions.yml b/.github/workflows/cron-python-versions.yml index df1a68f4ea..9ed06b926b 100644 --- a/.github/workflows/cron-python-versions.yml +++ b/.github/workflows/cron-python-versions.yml @@ -9,7 +9,7 @@ name: Python Versions # open one, for the owner to merge. Nothing here pushes `main` or edits ruleset # 23497679: the pull request body lists the required contexts to change by hand. # -# Only these two triggers. A push to `bot/python-versions` starts the pull +# Only these triggers. A push to `bot/python-versions` starts the pull # request's own checks, which is why `pr` spends the release App's token rather # than GITHUB_TOKEN, whose pushes start nothing; nothing it starts runs this # workflow again. @@ -61,7 +61,7 @@ jobs: - name: Install PyYAML run: python -m pip install PyYAML - # Fetches the four sources, rewrites the files, and writes the proposal to + # Fetches the sources, rewrites the files, and writes the proposal to # $RUNNER_TEMP/proposal. `publish` is false when nothing changed, or when # `bot/python-versions` has an open pull request and a commit it holds # ahead of main proposes these tiers, hand edits or merges of main on top @@ -79,7 +79,7 @@ jobs: python util/update_python_versions.py update --write --out "$RUNNER_TEMP/proposal" --proposal-branch bot/python-versions --github-output - # The three modules that pin the workflows and the prose to the file, on + # The modules that pin the workflows and the prose to the file, on # the rewritten tree. A failure here does not stop the proposal: a tier # move can need hand edits the script names, and the body lists what failed. - name: Run the audit tests diff --git a/.github/workflows/cron-vendor.yml b/.github/workflows/cron-vendor.yml index 62d55de9f3..5d0ca7e77f 100644 --- a/.github/workflows/cron-vendor.yml +++ b/.github/workflows/cron-vendor.yml @@ -18,7 +18,7 @@ on: # Least privilege by default, since `workflow_run` is in the trigger list -- # that event can be raised by a pull request's `Unit Tests` run completing, so # the ambient token this workflow gets should default to nothing rather than -# whatever the repository's default is. Both jobs below already declare their +# whatever the repository's default is. Every job below already declares its # own narrower `permissions:`, so this changes nothing they rely on; it only # closes the gap for anything added later that forgets to. permissions: {} @@ -168,7 +168,7 @@ jobs: # #910 moved those to the `dev` extra instead, which is # for the toolchain rather than for a vendor crawl. `vendor` is # installed alongside `all` for those two: `pcapkit-vendor` below - # regenerates every crawler with no target given, and six of them + # regenerates every crawler with no target given, and some of them # import `bs4` at module scope while `pcapkit.vendor.default` imports # `requests` the same way (#507). See the `vendor` extra's own # comment in pyproject.toml for the full reasoning. diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 8aab094c9a..418b80a9f5 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -1,73 +1,28 @@ name: "Lint" -# The four linters this repository already maintains -- vermin, pylint, mypy and +# The linters this repository already maintains -- vermin, pylint, mypy and # bandit -- had Makefile targets and no CI presence at all, so nothing noticed # when their output changed. (`isort` does appear in cron-vendor.yml, but as a # formatter that rewrites the generated constants, not as a check.) This workflow # is where they run. # # Every one of them is advisory -- `continue-on-error: true` on each step -- and -# that is a measurement result rather than a preference. Each line below is -# pinned to the commit it was taken at, rather than left as a bare count: #753 -# was filed when three of the four had already drifted from the figures this -# comment once recorded (on 15189abff) -- mypy and pylint by the issue's own -# numbers, and vermin too (105 recorded; 106 measured here both at 73f09aecb, -# where #753 was filed, and at 932cb48d1 below, so it had already moved before -# the issue's own text said it "was not re-checked"). Only bandit held. A bare -# number invites exactly this, with no way to tell staleness from disagreement. -# All four happen to be re-measured together below, each from the same flag -# sets the Makefile uses; re-measuring only one tool later should update only -# that line's commit. +# that is a measurement result rather than a preference. This header records no +# finding counts on purpose: #753 was filed when most of the figures it once +# recorded (on 15189abff) had already drifted -- mypy and pylint by the issue's +# own numbers, and vermin too, which had moved before the issue's own text said +# it "was not re-checked". Only bandit held. A written-down count invites +# exactly this, with no way to tell staleness from disagreement, so each step +# writes its current count to the run summary instead; read the numbers there. # -# bandit 8 findings (7 medium, 1 low, 0 high) exit 1 [932cb48d1] -# mypy 112 errors in 38 files (506 checked) exit 1 [932cb48d1] -# vermin 106 files flagged; needs 3.11, targets 3.6 in vermin.ini exit 1 [932cb48d1] -# pylint 90 E, 4765 W, 542 C -- stable; R and the total are not, see below exit 30 [1a852698b] +# pylint's R class is the one that moves without the code moving. Repeated runs +# on an identical tree were measured to disagree on it, entirely through +# `R0401` cyclic-import, whose count depends on module-processing order rather +# than on the code -- `duplicate-code` and every other R check held steady +# across all of them, as did E, W and C. A change in R between two runs is +# therefore no evidence on its own that anything changed. # -# pylint's R and total are deliberately not pinned to a single number. Before -# #767 below, repeated runs on the identical tree gave R of 645-680 and totals -# of 6044-6079 at 932cb48d1 (two runs measured here: 659/6058 and 680/6079; a -# cross-review measured four more: 662/647/647/645, totals 6061/6046/6046/6044), -# entirely from `R0401` cyclic-import, whose count depends on module-processing -# order rather than on the code -- `duplicate-code` (400) and every other R -# check held steady across all of them. That range's own basis is not this -# line's, though: every one of those runs predates #767, so each *R* already -# carries the three `R0022` the old flags spent on `Command line` (not just -# the total -- `R0022` is class R), and each *total* carries one `E0013` and -# one `W0012` on top of that: five stale messages in all, `91 + 4766 + 542 + -# R_old` where `R_old` is itself `R_real + 3`. On the corrected basis -# (`90 + 4765 + 542 + R`) that range is really R 642-677, total 6039-6074 -- -# not R 645-680, total 6044-6079 as printed above the line predating #767. -# -# After #767, re-measured on that corrected basis at 1a852698b: two runs here -# gave R 662 and 648 (coincidentally sharing a digit with one of the four -# pre-#767 cross-review runs above, which is a different measurement on a -# different basis, not a repeat of it), totals 6059 and 6045, and a -# cross-review measured five more -- 642/643/646/649/659, totals -# 6039/6040/6043/6046/6056. Combined: R 642-662, total 6039-6059 -# [1a852698b]. The floor matches the pre-#767 range's own corrected floor -# exactly (642 both ways) because it is the same tree state either side of -# that diff -- the range only looks lower because three of its messages were -# deleted, not because the code changed. Already superseded the -# same day it was measured, too: a cross-review measured `R0801 -# duplicate-code` alone moving 400 -> 573 on `merge(f31e5114f, 83b58ebda)` -- -# this branch merged onto the `main` it was rebased against before #768 -# landed there -- taking R to 823 and the total to 6220 on that merge. That -# figure is named for the tree it was actually measured on rather than -# asserted of `main` directly. This range is a moving target to re-measure, -# never a constant to defend. -# -# The pin above moved from 932cb48d1 to 1a852698b because `pcapkit/` is not -# actually unchanged between them, despite what an earlier revision of this -# comment claimed: four files moved (`pcapkit/const/reg/apptype/apptype.py`, -# `pcapkit/foundation/extraction.py`, `pcapkit/protocols/schema/schema.py`, -# `pcapkit/vendor/reg/apptype/apptype.py`; tree 7270ad50 -> 2b9ac808), #758's -# port-validation logic among them. E, W and C read identically either side of -# that diff -- 90/4765/542 both times -- but 1a852698b is the tree actually -# re-measured for this line, so that is what it is pinned to, rather than -# repeating a byte-identity claim that does not hold. -# -# Four of `PYLINT_FLAGS`' `--disable=` entries named checks pylint has +# #767 cleaned up `PYLINT_FLAGS`. Its `--disable=` list named checks pylint has # genuinely removed -- `old-division`, `no-absolute-import` and # `input-builtin` (-> `R0022 useless-option-value`, and pylint's own message # says so: "was removed from pylint") -- plus `eq-without-hash`, which is not @@ -75,52 +30,50 @@ name: "Lint" # but nothing in `--load-plugins=` ever named it, so `--disable=eq-without-hash` # was as unrecognised as if the check had been removed (-> `W0012 # unknown-option-value`). Same shape of breakage as `emptystring` below, a -# different cause. All four are dropped either way -- loading the fifth plugin -# to enable `eq-without-hash` properly found nothing when measured (`W1641` -# was 0 over `pcapkit/`), so there is nothing lost in dropping it instead. +# different cause. All of them are dropped either way -- loading that plugin +# to enable `eq-without-hash` properly found nothing when measured (no `W1641` +# over `pcapkit/`), so there is nothing lost in dropping it instead. # # `--load-plugins=` also named `pylint.extensions.emptystring`, which no # longer exists in the installed pylint (-> `E0013 bad-plugin-value` every -# run). Unlike the four above, though, the check it used to provide did not +# run). Unlike the entries above, though, the check it used to provide did not # stop running when the plugin did: `compare-to-empty-string` was folded into # the core `refactoring` checker at pylint 3.0 as # `use-implicit-booleaness-not-comparison-to-string` (C1804), reachable under # either name with no plugin load at all. #767's own premise -- that the # broken plugin reference made the check "inert" and it "has never run" -- is # wrong: `--list-msgs-enabled` run under the old flags (broken plugin -# reference included) and the new ones side by side gives the same 399-line +# reference included) and the new ones side by side gives the same # enabled-message list either way, and C1804 fires under the old flags on a # minimal repro despite the `E0013`. So this half of `PYLINT_FLAGS`' change is # flag hygiene, not enablement: it drops a dead plugin reference and keeps the # check it used to gate under that check's current name, and the only thing -# that actually changes is that `E0013` (and the other four's Command-line -# noise) stops firing. With the five gone, the job's own grep-based count (the -# `pylint` step below) and this header have nothing left to disagree about on -# the same run -- the figures above are both the raw and the file-scoped -# count, for E, W and C alike. +# that actually changes is that `E0013` (and the Command-line noise of the +# entries above) stops firing. With those gone, every message the job's own +# grep-based count (the `pylint` step below) tallies is about the code. # -# None of the four is clean, so none can block: a job that is red the day +# None of them is clean, so none can block: a job that is red the day # it lands teaches everyone to scroll past red, which costs more than the checks # are worth. Advisory steps keep the numbers visible (each writes its count to # the run summary) without that damage, and promoting one to blocking is a matter # of deleting its `continue-on-error` line once its count reaches zero. # # Promotion order, cheapest first: -# * bandit -- 8 findings, and 7 are B104 "binding to all interfaces" on +# * bandit -- most of its findings are B104 "binding to all interfaces" on # `address: ... = '0.0.0.0'` *default arguments* in MH/MIP option builders, -# which bind no socket. This package already uses `# nosec` (16 findings -# were suppressed that way when measured), so triaging 8 is the whole job. -# * mypy -- 24 of the 112 are `unused-ignore`, i.e. `# type: ignore` comments -# that are no longer needed. Deleting those is risk-free and cuts a fifth. +# which bind no socket. This package already suppresses findings with +# `# nosec`, so triaging what is left is the whole job. +# * mypy -- part of its output is `unused-ignore`, i.e. `# type: ignore` +# comments that are no longer needed. Deleting those is risk-free. # * vermin -- blocked on a decision, not on cleanup: see the step below. -# * pylint -- not realistically promotable at this rule set. 4356 of its 4765 -# warnings are `unused-argument` (2929), `redefined-builtin` (808) and -# `super-init-not-called` (619), all three of which are inherent to the +# * pylint -- not realistically promotable at this rule set. Most of its +# warnings are `unused-argument`, `redefined-builtin` and +# `super-init-not-called`, each of which is inherent to the # schema DSL: protocol fields are legitimately named `next`/`type`/`id`, and # the `if TYPE_CHECKING: def __init__(...)` stubs exist precisely so as not # to call `super().__init__`. Narrowing to `--enable=E,F` does not rescue it -# either -- that is still 87 errors, mostly `unsubscriptable-object` (36) and -# `no-member` (31) false positives off the same metaprogramming. +# either -- what remains is mostly `unsubscriptable-object` and +# `no-member` false positives off the same metaprogramming. on: # Deliberately *not* `push: [main]`, which is the trigger the other validation @@ -167,7 +120,7 @@ jobs: # `latest` from .github/python-versions.toml, like every single-interpreter # job (#1661). Moving off 3.14 changes no count. Measured at 0bc157660 # on setup-python's 3.14.8 and 3.15.0 builds: pylint 4.1.3 (astroid - # 4.3.4) reports the same 515 C, 71 E and 4970 W, message for message; + # 4.3.4) reports the same C, E and W messages on both, message for message; # only R0401 cyclic-import differs, as it does between two runs on one # interpreter (see the header). bandit, mypy and vermin print identical # reports. To test a 3.15 without C `_decimal`, use setup-python's build, @@ -187,9 +140,10 @@ jobs: # what puts `dpkt`/`scapy`/`pyshark`/`pypcapfile` back for this job -- # without it, narrowing `all` would have grown the count below rather # than left it alone. Note `pypcap` and `pypcapfile` are marked - # `python_version < '3.12'` in pyproject.toml and so are absent here by - # design -- that is the source of pylint's 8 `import-error` messages, and - # they will persist until those engines support a current Python. + # `python_version < '3.12'` in pyproject.toml, and `pcap-ct` is in no + # extra installed here, so each is absent by design -- that is the source + # of pylint's `import-error` messages, and they will persist until those + # engines are installed here. - name: Install package and lint tools run: | set -x @@ -252,8 +206,8 @@ jobs: # This one needs a decision rather than a cleanup before it can block, and # the decision is not CI's to make. vermin.ini sets `targets = 3.6` while # the code's real floor is 3.11 -- `enum.StrEnum` and - # `enum.show_flag_values` are 3.11 members, and 93 modules import - # `typing_extensions`. pyproject.toml's `requires-python` claims + # `enum.show_flag_values` are 3.11 members, and modules across the package + # import `typing_extensions`. pyproject.toml's `requires-python` claims # `>=3.6, <4`, so the declared floor and the actual one are five releases # apart. Whichever way that is resolved -- raise `requires-python`, or # raise `targets` to match it -- belongs in its own change. @@ -276,7 +230,7 @@ jobs: exit $status # Last because it is by far the slowest -- minutes over ~90k lines -- so - # the three quick verdicts reach the summary before it starts. + # the quick verdicts reach the summary before it starts. - name: pylint continue-on-error: true run: | diff --git a/.github/workflows/project-status.yml b/.github/workflows/project-status.yml index 370f164a40..5eacff56cb 100644 --- a/.github/workflows/project-status.yml +++ b/.github/workflows/project-status.yml @@ -22,7 +22,7 @@ name: Project Status # `env:` rather than interpolated into the script, and re-reads everything else # (labels, state) from the API. # -# tests/project/test_project_status.py asserts all three over this file -- no +# tests/project/test_project_status.py asserts each of them over this file -- no # checkout or `uses:`, no `${{ }}` inside a `run:` script, and no event-payload # reference outside `env:` other than the item number -- so a later edit that # breaks one fails a test rather than a security review. diff --git a/.github/workflows/python-compatibility.yml b/.github/workflows/python-compatibility.yml index 63cbfaf0d2..988abca05c 100644 --- a/.github/workflows/python-compatibility.yml +++ b/.github/workflows/python-compatibility.yml @@ -50,7 +50,7 @@ jobs: # showed two rows reading `Python 3.10`, one a 13-second import smoke test # and one a three-minute test run, with nothing to tell them apart. # - # #1633: two tiers. The supported tier, 3.11-3.15, keeps the plain name, + # #1633's tiers. The supported tier, 3.11-3.15, keeps the plain name, # which is what ruleset 23497679 requires one context per version of. The # KTLO tier, end-of-life 3.7-3.10, is suffixed ` (KTLO)`, so no required # context can match one. Its steps carry `continue-on-error`, as in diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index c50dcca29f..f14d0de4a5 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -140,8 +140,8 @@ jobs: # nothing per leg and adds no new job or leg, unlike Scapy below. # # crypto (cryptography) and NGAP (pycrate) join it for the same reason, - # covering two of #738's remaining gates -- HAS_CRYPTO (test_esp_unit.py, - # 14 methods) and HAS_PYCRATE (test_ngap_unit.py, 10 methods), both plain + # covering #738's remaining HAS_CRYPTO gate (test_esp_unit.py) and its + # HAS_PYCRATE gate (test_ngap_unit.py), both plain # unit-tier. `cli` (emoji) is deliberately NOT added here: its only # HAS_EMOJI-gated tests are in tests/integration/test_cli_subprocess.py, # which this job ignores wholesale, so installing it here would unlock @@ -154,12 +154,12 @@ jobs: # PyPCAPFile (pypcapfile) is deliberately NOT here. It used to be kept off # every job in this file: on 3.10/3.11 (where its # `python_version < '3.12'` marker lets it install), installing it turned - # 7 of the 10 HAS_PYPCAPFILE-gated methods this comment used to count + # most of the HAS_PYPCAPFILE-gated methods of the time # into failures, against a real bug in pcapkit/toolkit/pypcapfile.py's # handling of pypcapfile 0.12.0's `IP.src`/`IP.dst`. #743 and #746 fixed # that bug, and #751 gave the extra a home: the dedicated - # `engine-tests` and `pypcap-parity` jobs below install it (15 - # HAS_PYPCAPFILE-gated methods between them), deliberately apart from + # `engine-tests` and `pypcap-parity` jobs below install it, covering the + # HAS_PYPCAPFILE-gated methods between them, deliberately apart from # this job rather than added to it -- see either job's own comment for # why. # @@ -473,7 +473,7 @@ jobs: # The Scapy extra is there for examples/generators/pcap.py and legacy.py, # which build their captures with scapy; examples/generators/pcapng.py needs # nothing beyond the standard library and pcapkit itself, which it uses to - # parse each fixture back. DPKT joins it so the two dpkt-gated methods + # parse each fixture back. DPKT joins it so the dpkt-gated methods # living under tests/integration/ (test_engine_parity.py, # test_engine_runtime.py) actually run here instead of skipping -- see # #729; same reasoning as the `test` job above. @@ -481,16 +481,16 @@ jobs: # `cli` (emoji) joins it for one of #738's remaining gates. This job's # selection (tests/_tiers.py's fixture_tier_paths()) always includes the # whole of tests/integration/ -- which is where every HAS_EMOJI-gated - # method lives (test_cli_subprocess.py, 14 methods). `crypto` and `NGAP` + # method lives (test_cli_subprocess.py). `crypto` and `NGAP` # are deliberately NOT added here: test_esp_unit.py and test_ngap_unit.py # are plain unit-tier and never enter this job's selection, so installing # cryptography/pycrate here would unlock nothing -- they are already # covered, on every Python version, by the `test` job above. # PyPCAPFile is NOT added here either, even though this job's selection - # also reaches test_new_engine_parity_runtime.py's 6 HAS_PYPCAPFILE + # also reaches test_new_engine_parity_runtime.py's HAS_PYPCAPFILE # methods -- see the `test` job's comment above for the bug that used to # make installing it here a regression, fixed by #743/#746. #751's - # `pypcap-parity` job below covers those 6 methods instead, on the same + # `pypcap-parity` job below covers those methods instead, on the same # fixture-tier selection as this job but in a venv of its own. # # A KTLO leg that cannot install stops here, as in the `test` job above. @@ -526,7 +526,7 @@ jobs: exit 1 fi - # examples/generators/pcapng.py fetches two captures from the Wireshark + # examples/generators/pcapng.py fetches captures from the Wireshark # repository and synthesises stand-ins when the download fails. The # stand-ins parse, and tests/protocols/test_pcapng_regression.py only # asserts that extraction succeeds, so a degraded run goes green and @@ -551,7 +551,7 @@ jobs: # same commit. Re-running them here (a bare ``pytest -q`` used to) bought # no signal and cost ~21.75 minutes per Python version -- ~130 minutes per # PR push across the matrix (see #715). So this selects only the - # fixture-dependent tier plus the handful of unit-tier modules whose + # fixture-dependent tier plus the unit-tier modules whose # capture read is skipped, not exercised, in the `test` job -- see # tests/_tiers.py's module docstring and fixture_tier_paths() for why a # module can be unit-tier by path and still belong here. The selection is @@ -652,7 +652,7 @@ jobs: # Per-engine coverage for the third-party capture engines (#845): every # (Python version, engine) pair is checked, and a failure -- or a correctly # asserted decline -- names the one engine and interpreter responsible. Each - # pair ("cell") resolves to one of three expected outcomes, decided by the + # pair ("cell") resolves to one of these expected outcomes, decided by the # cell script's `expect` case below (anything not listed there is # `supported`): # @@ -682,12 +682,12 @@ jobs: # green and never red for the wrong reason. If it # ever starts building, the cell fails loudly. # - # One job per engine loops over the six interpreters, each in a fresh venv, - # rather than one job per cell (6 Pythons x 5 engines): the queue, not the - # runners, is the bottleneck, and 30 jobs would each pay checkout, setup and - # apt for a minute of work. One job per *engine* rather than per Python + # One job per engine loops over every interpreter, each in a fresh venv, + # rather than one job per cell (every Python times every engine): the queue, + # not the runners, is the bottleneck, and a job per cell would pay checkout, + # setup and apt for a minute of work. One job per *engine* rather than per Python # because the engine is what varies the environment: the apt packages - # are per engine (installed once per job instead of six times), and pcap-ct + # are per engine (installed once per job, not once per interpreter), and pcap-ct # and pypcap both install a top-level `pcap` module, so a per-Python job # looping over engines would need a fresh venv per engine anyway while also # carrying every engine's system packages. It also keeps `matrix.engine` as @@ -698,7 +698,7 @@ jobs: # every annotation titled by engine and Python version and a per-cell row in # the job summary. # - # #1633 splits the interpreters into two tiers, read from + # #1633 splits the interpreters into supported and KTLO tiers, read from # .github/python-versions.toml by `python-versions` (#1660). The supported # cells share one step, and any one of them failing fails the job. The KTLO # cells (3.10 alone; see below) are end-of-life, still tested on every run, @@ -766,7 +766,7 @@ jobs: # time; without it, PCAP_CT.unsupported_reason() degrades the engine to # the default parser instead of running it. # - # build-essential + libpcap-dev: PyPCAP is the one engine that compiles + # build-essential + libpcap-dev: PyPCAP compiles # (pcap.c against libpcap's headers), on every cell -- including the # `not-installable` ones, deliberately, so that failure is a genuine # compile failure against the 3.12+ C API rather than merely "no @@ -1110,14 +1110,14 @@ jobs: # Whether upstream PyPCAP is worth building in CI at all -- #751's own # instruction was "try to build and if the CI is not a good suit, then we # ripe it". This job is that attempt: a C toolchain plus libpcap headers, on - # the two Python versions its own marker allows - # ("pypcap; python_version < '3.12'", pyproject.toml:168). If a clean run of + # the Python versions its own marker allows (the PyPCAP extra's + # "pypcap; python_version < '3.12'" in pyproject.toml). If a clean run of # this job's install step goes red -- not a flake -- the fix is deleting this # job and returning HAS_PYPCAP to # tests/_dependency_gates.DEPENDENCY_GATE_EXCLUSIONS with that run linked as # the reason, per the ruling. # - # Kept apart from `engine-tests` above for two reasons: it needs a compiler + # Kept apart from `engine-tests` above: it needs a compiler # that job has no other reason to carry, and its `pcap` module would collide # with pcap-ct's if both were installed into the one venv (see # `engine-tests`'s own comment on that). The HAS_PYPCAP-gated module this job @@ -1126,10 +1126,10 @@ jobs: # docstring), so this mirrors the `integration` job's fixture-tier selection # and Scapy/DPKT/cli baseline rather than `test`'s ignore-shape one, and pays # that job's full run cost a second time on top -- accepted here rather than - # discovered, since tests/_dependency_gates.py's guard has no cheaper of its - # three known selection shapes for reaching one fixture-dependent module (see - # that module's own docstring). The same module also carries 6 of - # HAS_PYPCAPFILE's 15 gated methods, which come along for free once + # discovered, since tests/_dependency_gates.py's guard has no cheaper known + # selection shape for reaching one fixture-dependent module (see + # that module's own docstring). The same module also carries some of + # HAS_PYPCAPFILE's gated methods, which come along for free once # PyPCAPFile is on this job's install line too. # # The matrix is deliberately just 3.10 and 3.11, not every version: PyPCAP's @@ -1150,8 +1150,8 @@ jobs: # Read by the apt plan step; see `engine-tests`'s note on it. APT_PACKAGES: build-essential libpcap-dev tshark APT_SET: pypcap-parity - # Both cells install the same set under the same key; only one saves it, - # or the other's save fails with `Unable to reserve cache`. The supported + # Every cell installs the same set under the same key; only one saves it, + # or another's save fails with `Unable to reserve cache`. The supported # leg, so the cache does not depend on a leg that may not get that far. APT_CACHE_SAVE: ${{ matrix.python-version == '3.11' }} # See the `test` job above. @@ -1396,8 +1396,8 @@ jobs: # (tests/protocols/application/test_http_unit.py and # tests/test_base_class_contract.py) each pass alone, and pytest -- the # runner every job above uses -- reports the combined run green, while - # plain unittest over the same two modules in one process fails four - # subTest cases. pytest-subtests is not why -- it is not even installed in + # plain unittest over the same two modules in one process fails some of + # their subTest cases. pytest-subtests is not why -- it is not even installed in # this project (absent from the `test` extra in pyproject.toml), and plain # pytest already reports a failed subTest as its own top-level SUBFAILED # entry rather than folding it into a passing parent. The real cause is @@ -1413,7 +1413,7 @@ jobs: # # A cheaper, narrower alternative: `pytest --noconftest` over just those two # modules, in that order, disables that same autouse fixture directly and - # reproduced the identical four subTest failures in about 70s on the pre-fix + # reproduced the identical subTest failures in about 70s on the pre-fix # tree, under the pytest this CI already installs. Whether that single # invocation would have been sufficient instead of this dedicated job and its # per-directory matrix was not evaluated when this job was written -- @@ -1428,7 +1428,7 @@ jobs: # #981's own reproduction needs, since the defect is an earlier module # polluting a later one. Running the whole suite in one process is not an # option (it OOMs at 29 GB on the machine this was diagnosed on); a matrix - # of one leg per top-level directory (protocols being two cells, #1029) + # of one leg per top-level directory (protocols split across cells, #1029) # is the affordable approximation, and it # is sized for wall time rather than for memory. Peak RSS was measured for # the three cheapest legs only -- cli 152 MiB, dumpkit 237 MiB, interface @@ -1443,8 +1443,8 @@ jobs: # # GitHub issue #1029: the protocols cell measured 30.4 minutes on a CI # runner against this job's 45-minute cap (#1029 measured the next slowest - # leg at 6.2 minutes on CI), so it is two cells: `protocols/internet` and the rest of - # tests/protocols (its other five subdirectories and the modules sitting + # leg at 6.2 minutes on CI), so it is split: `protocols/internet` and the rest of + # tests/protocols (its other subdirectories and the modules sitting # directly in it), the latter expressed as `protocols` with # `--exclude protocols/internet`. run_unittest_leg.py takes a sub-path and # repeatable --exclude for this; every other cell is untouched. @@ -1470,33 +1470,33 @@ jobs: # pre-#1029 local serial table above is left as it was measured. # # What sharding costs: modules in different cells no longer share a process, - # so pollution from one to the other is invisible again. Counts below were - # derived with leg_modules() on main at 30714cd51 plus #1029's split: 51 - # modules in tests/protocols (12 under internet, 39 in the rest, of which 10 - # sit directly in tests/protocols). "Pairs" are unordered, C(n,2), since a - # single serial run realises each pair in one order. Before: 1275 pairs - # co-ran in one process. After: 807 (internet x internet 66, rest x rest - # 741); 468 do not. The 255 pairs of a protocols module followed by a - # root-level module (51 x 5 roots) all survive, since every cell still runs - # the root modules after its own. The 10 modules sitting directly in - # tests/protocols stay in the rest cell: moving them into internet would keep - # 637 pairs instead of 807 and lengthen internet, the slower cell already. + # so pollution from one to the other is invisible again. A pair of + # tests/protocols modules still co-runs when both land in the same cell + # (leg_modules() is what places each module); a pair split between + # internet and the rest no longer does, where the unsharded leg ran it in + # one process. Pairs are unordered, since a single serial run realises each + # pair in one order. Every pair of a protocols module followed by a + # root-level module survives, since every cell still runs the root modules + # after its own. The modules sitting directly in tests/protocols stay in the + # rest cell: moving them into internet would split more of the pairs that + # still co-run than it would join, and lengthen internet, the slower cell + # already. # # tests/vendor was first left out of ``leg`` because running it this way # surfaced an instance of the very defect this job exists to catch (issue # #985): a stale VendorRuntimeWarning generation made assertWarnsRegex fail # in tests/vendor/test_vendor_snapshot_restore_unit.py. That was fixed by - # re-resolving the class fresh per test, so the directory is a leg (297 - # tests in the leg, 118 of them tests/vendor's own, measured on this venv). + # re-resolving the class fresh per test, so the directory is a leg like + # the others. # - # Two directories are deliberately absent from ``leg``, for reasons that + # These directories are deliberately absent from ``leg``, for reasons that # are not this job's to fix: # # * tests/corekit -- already known and already accepted, not a new # finding. tests/_support.py's own purge_modules() docstring measures - # and documents exactly this: "python -m unittest discover -s - # tests/corekit fails 5 of that class's identity subtests, because - # test_module sorts first and purges". pytest is "the supported + # and documents exactly this: plain `python -m unittest discover -s + # tests/corekit` fails identity subtests there, because + # test_module sorts first and purges. pytest is "the supported # runner" for precisely this reason -- tests/conftest.py's autouse # restore_module_table fixture reconciles the module-identity drift # that plain unittest has no way to see coming. Running this leg @@ -1507,7 +1507,7 @@ jobs: # build, and is already run whole, under pytest, by the `integration` # job above. The exclusion lives here, in this matrix, and nowhere else: # util/run_unittest_leg.py does not refuse the directory, it collects - # nothing from it. tests._tiers.is_unit_tier() rejects all 12 of its + # nothing from it. tests._tiers.is_unit_tier() rejects every one of its # test_*.py files, so leg_modules('integration') returns an empty tuple # and the leg would run the root-level modules alone -- green, while # covering none of tests/integration. Adding it back here would buy that @@ -1518,8 +1518,8 @@ jobs: # maintainer's call to make separately from landing it. unittest-ordering: name: Plain unittest ordering (${{ matrix.label || matrix.leg }}) - # Every push to main and no PR (#1052 item H): not required, and its 11 - # cells would be a sixth of each PR push's jobs. `needs: [test]` (#1538) + # Every push to main and no PR (#1052 item H): not required, and its + # cells would add a large share of each PR push's jobs. `needs: [test]` (#1538) # queues them behind this push's own legs; `!cancelled()` still runs them # if `test` fails, so a red cell still fails the run that gates publishing. if: ${{ !cancelled() && inputs.gate-only != true && github.event_name != 'pull_request' }} @@ -1619,7 +1619,7 @@ jobs: # # ``tests/project/test_changelog_md.py`` asserts the same identity, so the # matrix covers it too -- but that test ``skipTest``s when either file is - # absent, and reports a mismatch as two 17 KB strings. ``--check`` cannot skip, + # absent, and reports a mismatch as two whole-file strings. ``--check`` cannot skip, # exits 1 when the file is missing, and prints a unified diff. changelog: name: Changelog drift @@ -1791,7 +1791,7 @@ jobs: # DPKT joins Scapy here for the same reason as the `integration` job # above -- this is the full, unfiltered suite, so it is the one place # where skipping the dpkt-gated tests would be most misleading. crypto, - # cli and NGAP join it for the same reason and cover the three of #738's + # cli and NGAP join it for the same reason and cover those of #738's # remaining gates they were added for, since an unfiltered `pytest` run (no # --ignore, no tier selection) reaches every one of them regardless of # which tier they live in -- unlike the `test` and `integration` jobs @@ -1852,7 +1852,7 @@ jobs: # lists, because rulesets match check names literally and support no # wildcard: requiring each matrix cell by name would mean hand-editing the # ruleset every time a matrix changes. On a docs-only pull request it - # accepts `skipped` for the four code legs (see below). The `Compat` legs + # accepts `skipped` for the code legs (see below). The `Compat` legs # are required on their own because a job cannot `needs:` a job in another # workflow file, so nothing here can stand in for # them. @@ -1882,14 +1882,14 @@ jobs: # call, not a PR, which is not where ruleset 23497679 applies anyway. # * The check itself inspects every dependency explicitly and prints each # one's result before failing, so a red run names which leg broke - # instead of leaving that to be re-discovered from the four jobs' own + # instead of leaving that to be re-discovered from the jobs' own # logs. # # Note for whoever adds a caller of this reusable workflow: under # `workflow_call`, GitHub prefixes a called workflow's check names with the # calling job's own name (` / Required checks passed`), which # would not match a ruleset entry naming just `Required checks passed`. - # Harmless for the four callers -- create-release, cron-conda, cron-vendor and + # Harmless for the callers -- create-release, cron-conda, cron-vendor and # deploy-pages each pass `gate-only: true`, which skips this job via the guard # above before that renaming would ever matter -- but a caller that does NOT # set `gate-only: true` would produce a differently-named, unrequired check. @@ -1926,7 +1926,7 @@ jobs: # `pypcap-parity` are skipped by their own `if:`, and `project-tests` runs # instead. A job-level `if:` rather than workflow-level `paths-ignore`, # because `paths-ignore` would not run this workflow at all and leave this - # required context pending forever. So `skipped` is accepted for those four + # required context pending forever. So `skipped` is accepted for those # legs **only** when `changes` succeeded and said `code == 'false'`, and then # `project-tests` must have succeeded. Every other combination keeps the # strict rule: `changes` failing, its output missing, or any leg skipped diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 734129331b..b27e6d2024 100755 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -34,8 +34,8 @@ Thanks for your contributions! ## Running the tests -The suite runs in two tiers. The tier is a property of a module's *path*, not of the command that -collects it; `tests/_tiers.py` is the rule. +The suite runs in a unit tier and a fixture-dependent tier. The tier is a property of a module's +*path*, not of the command that collects it; `tests/_tiers.py` is the rule. ```shell make test # the unit tier -- the selection CI runs @@ -59,7 +59,7 @@ Entries live in `docs/source/changelog/.rst`, one file per version, lis the toctree of `docs/source/changelog.rst`. That tree is the single source of the project's history. **`CHANGELOG.md` is generated — never edit it by hand.** `util/changelog_md.py` derives it from the -newest entry alone, because its two consumers (the `Create Release` workflow's release body and the +newest entry alone, because its consumers (the `Create Release` workflow's release body and the source distribution) read Markdown rather than reStructuredText. Add your bullet to the `.rst` entry, then regenerate: @@ -114,13 +114,13 @@ it, which is correct — they are reachable only through that class. The sweep runs **in both directions**: a must-implement member with no directive is the same defect as a recipe body with one, only quieter. Add the missing directive rather than aiming for a small diff. -Two mechanical points that decide real cases: +Mechanical points that decide real cases: - **A private base class must stay documented when a documented subclass carries `:show-inheritance:`.** Sphinx renders that subclass's `Bases:` line as a link into the private class's page, so dropping the directive breaks a link a public page renders. This keeps - `pcapkit.protocols.schema.misc.pcapng._OPT_Option` and its five siblings, plus `_IPField`, - `_IPInterfaceField` and `_TextField`. + `pcapkit.protocols.schema.misc.pcapng._OPT_Option` and the other per-block option bases with + documented subclasses, plus `_IPField`, `_IPInterfaceField` and `_TextField`. - **Dunders keep their directives.** A `__dunder__` is reached through public syntax, not by name (`len()` calls `__len__`, `obj[key]` calls `__getitem__`), so overriding one changes behaviour a caller observes without writing the name. PyPCAPKit's own `__proto__`, `__option__`, `__schema__` @@ -149,7 +149,7 @@ make bandit # security lint make vermin # minimum-Python-version check ``` -Four of them — `pylint`, `mypy`, `bandit` and `vermin` — also run in CI as the `Lint` job in +`pylint`, `mypy`, `bandit` and `vermin` also run in CI as the `Lint` job in `.github/workflows/lint.yml`: on every pull request against `main`, on a weekly Saturday schedule and on demand through `workflow_dispatch`, on the newest supported Python alone (`latest` in `.github/python-versions.toml`) rather than across the test matrix. @@ -158,8 +158,8 @@ The job drives them through the `Makefile` rather than restating their flags (`v variables, so a check cannot come out clean locally and red in CI because the two drifted. **Those steps are advisory, not a gate.** Each carries `continue-on-error: true`, so a finding lands -as a non-blocking annotation and a red linter will not fail your pull request; none of the four is -clean. The workflow's header records the counts and what each tool would need before +as a non-blocking annotation and a red linter will not fail your pull request; none of them is +clean. The workflow's header records what each tool would need before its `continue-on-error` line could be deleted. Read the job's run summary, where each step writes its verdict, and try not to add to the numbers. diff --git a/MANIFEST.in b/MANIFEST.in index 96a45939be..4564c73d33 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -41,9 +41,9 @@ prune temp # So the failure mode is real in principle; it just cannot be reached by removing # this line, because the README is in the sdist regardless. # -# Of the three ``include`` lines this file originally carried, only -# ``include CHANGELOG.md`` is load-bearing: deleting it alone drops exactly one -# entry from the listing, and nothing else changes. +# Of the ``include`` lines this file originally carried, only +# ``include CHANGELOG.md`` is load-bearing: deleting it alone drops just that +# file from the listing, and nothing else changes. # # These two are kept anyway, as an explicit statement of intent that does not # depend on a setuptools default staying where it is -- a much weaker reason than @@ -68,7 +68,7 @@ include LICENSE # distribution". include CITATION.cff -# The changelog needs two lines of its own, and neither is redundant. +# The changelog needs lines of its own, and none of them is redundant. # # ``global-include *.py`` above matches only ``*.py``, so the root # ``CHANGELOG.md`` is picked up by nothing and would be absent from every source diff --git a/Makefile b/Makefile index dd328d61c0..1d794abe37 100644 --- a/Makefile +++ b/Makefile @@ -102,16 +102,16 @@ coverage: samples # since the whole point is that the measuring environments are the pinned ones # inside the images rather than whatever is installed here. # -# Expect around two hours at the defaults: five interpreters, seven engines, +# Expect around two hours at the defaults, every default interpreter and engine, # and almost all of the measuring time is pyshark, which spawns a tshark process # per extraction. Cut it with --engines, --pythons, or --quick. bench: examples/benchmark/run.sh -# A smoke check that the harness works end to end, not a measurement -- two -# interpreters and the two cheapest engines, which is enough to exercise both +# A smoke check that the harness works end to end, not a measurement -- the +# interpreters and cheapest engines named below, which is enough to exercise both # virtualenvs, the matrix loop and both emitted tables. The full matrix at --quick -# would still build five images, and building is most of a quick run's cost. +# would still build every image, and building is most of a quick run's cost. bench-quick: examples/benchmark/run.sh --quick --pythons 3.11,3.12 --engines default,dpkt @@ -145,8 +145,8 @@ isort: RUN ?= pipenv run # Flag sets are variables rather than literals for the same reason -- `vermin` -# needs two recipes (see below) and would otherwise carry two copies of its -# flags. Note the `pcapkit` before the flags as well as after: that is how this +# needs more than one recipe (see below) and would otherwise carry a copy of its +# flags in each. Note the `pcapkit` before the flags as well as after: that is how this # recipe has always read, and vermin de-duplicates the paths (it reports the # same number of files analyzed either way), so it is preserved verbatim rather # than tidied. diff --git a/README.md b/README.md index 7692e94e3a..e57fb812d5 100644 --- a/README.md +++ b/README.md @@ -10,8 +10,8 @@ network packets and [PCAP](https://en.wikipedia.org/wiki/Pcap) files, with Unlike popular PCAP extractors such as [Scapy](https://scapy.net), [DPKT](https://dpkt.readthedocs.io) and [PyShark](https://kiminewt.github.io/pyshark), `pcapkit` reports more detail about each packet through a more *Pythonic* interface. -Where that depth is not needed, the same interface also drives six third-party -extraction engines. +Where that depth is not needed, the same interface also drives third-party extraction +engines. The whole project declares **Python 3.6** or later. CI supports 3.11 to 3.15, and still tests the end-of-life 3.7 to 3.10 without letting a failure block anything; @@ -41,7 +41,7 @@ pip install pypcapkit[all] # core addons only: cli + crypto + NGAP (pyc ``` Engines are on demand; `all` bundles only the core addons the library needs for -full functionality. Four engines also need something beyond `pip install` -- a +full functionality. Some engines also need something beyond `pip install` -- a `tshark` binary, a C compiler, `libpcap` headers, or an older interpreter -- and `pypcap`/`pcap-ct` must never be installed together. The [installation guide](https://jarryshaw.github.io/PyPCAPKit/#installation) gives each @@ -89,7 +89,7 @@ reference. Pages worth knowing by name: | Page | What is in it | |---|---| | [API reference](https://jarryshaw.github.io/PyPCAPKit/pcapkit/index.html) | Every module, protocol and constant | -| [Module structure](https://jarryshaw.github.io/PyPCAPKit/#module-structure) | What each of the nine subpackages is for | +| [Module structure](https://jarryshaw.github.io/PyPCAPKit/#module-structure) | What each subpackage is for | | [Engine comparison](https://jarryshaw.github.io/PyPCAPKit/#engine-comparison) | Which engines exist, which Python versions they run on, and measured speed per packet | | [Engine support](https://jarryshaw.github.io/PyPCAPKit/pcapkit/foundation/engines/index.html) | What each engine does *not* support, and how the gap is surfaced | | [Installation](https://jarryshaw.github.io/PyPCAPKit/#installation) | Extras, engine prerequisites and the local development setup | diff --git a/examples/benchmark/Dockerfile b/examples/benchmark/Dockerfile index de71bdaf56..0b788ec9c3 100644 --- a/examples/benchmark/Dockerfile +++ b/examples/benchmark/Dockerfile @@ -46,7 +46,7 @@ ENV PIP_DISABLE_PIP_VERSION_CHECK=1 \ VENV_PYPCAP=/opt/venv-pypcap \ VENV_PCAP_CT=/opt/venv-pcap_ct -# Three things beyond the base image, each needed by exactly one engine: +# What this image adds beyond the base image, each needed by exactly one engine: # # build-essential + libpcap-dev -- `pypcap` is an sdist with a C extension, and # its setup.py hard-fails unless it can find both `pcap.h` and an @@ -122,7 +122,7 @@ RUN set -eux; \ # not name. So its failure is recorded rather than fatal. # # The alternative is worse: a fatal failure here means `run.sh` produces no table at -# all, and the six engines that do work go unreported because the seventh did not +# all, and the engines that do work go unreported because one did not # build. Recording it keeps the promise that a missing engine is an explained row # rather than an absent one -- benchmark.py reads this file and reports the build # error as the reason `pypcap` was not measured. diff --git a/examples/benchmark/README.md b/examples/benchmark/README.md index 102e7081dc..f71452346a 100644 --- a/examples/benchmark/README.md +++ b/examples/benchmark/README.md @@ -14,12 +14,12 @@ machine, and because the numbers in it were not reproducible. Both problems are same problem -- the environment was never pinned and could not hold every engine at once -- so the fix is to define the environment precisely and ship it. Once the environment is a container, the number of them is a loop, which is what makes a -complete table a single command rather than five hand-run measurements pasted -together. +complete table a single command rather than one hand-run measurement per version +pasted together. ## The matrix -**One image per Python version**, five of them by default, each pinned by digest in +**One image per Python version**, each pinned by digest in [python-images.txt](python-images.txt) -- which is the matrix, and is a table rather than logic so that a reader can check it. Inside each image, **one or two virtualenvs**, so an "environment" here is a *(Python version, pcap provider)* @@ -31,19 +31,19 @@ pair: | 3.12, 3.13, 3.14 | `-pcap_ct` only | `pypcap` cannot be installed at all, so a second virtualenv would compile for minutes to produce nothing | | 3.15 | opt-in, `-pcap_ct` only | no released image; see below | -Seven environments for a default run, and a version named with `--pythons` that is -not in the file is a typo rather than a column. +A version named with `--pythons` that is not in the file is a typo rather than a +column. ## Why this cannot be one virtualenv, or one Python -Two hard constraints shape everything here. Neither is a `pcapkit` defect and -neither can be worked around from inside `pcapkit`: +Hard constraints shape everything here. None of them is a `pcapkit` defect, and +none can be worked around from inside `pcapkit`: 1. **pypcap and pcap-ct are mutually exclusive.** Both distributions install a top-level module named `pcap`. With both present, `import pcap` resolves to `pcap-ct`'s package directory and upstream's extension module is shadowed and unreachable, so `engine='pypcap'` cannot run at all. No install order changes - that. **So no single environment can hold all seven engines**, and an image that + that. **So no single environment can hold every engine**, and an image that can hold `pypcap` carries two virtualenvs -- `/opt/venv-pypcap` and `/opt/venv-pcap_ct`, identical but for which `pcap` provider they have. 2. **3.11 is the** *last* **interpreter on which every engine runs**, and 3.10 is the @@ -129,10 +129,10 @@ This is not hypothetical; it is what the first full run of this harness did. `pyshark` spawns a `tshark` process per extraction, so a default run spawns **three thousand** of them per environment, and one of them died -- `TSharkCrashException ... retcode: 255`, on the third pass. The exception propagated, `benchmark.py` exited, no JSON was -written, and the run produced **no table at all**. Six working engines went unreported -because the seventh hiccupped once. +written, and the run produced **no table at all**. Every working engine went unreported +because one hiccupped once. -So failure is contained at two levels, and counted at both: +So failure is contained, and counted, at each level below: - **A failed extraction** is discarded and the pass carries on, up to `TOLERATED_FAILURES` (5) per engine per pass. A crashed extraction is a @@ -144,9 +144,9 @@ So failure is contained at two levels, and counted at both: with what was lost. If *every* pass failed, the engine becomes `*not measured*` with the exception as its reason. -Two things are never tolerated, because tolerating them would mean reporting a number +Some things are never tolerated, because tolerating them would mean reporting a number that is not true: the escalated `EngineWarning` that means the engine fell back to -`pcapkit`'s parser, and a capture that yielded no packets. Both end the run. +`pcapkit`'s parser, and a capture that yielded no packets. Either ends the run. The same principle covers the build. `pypcap` is the only engine in the set that compiles, and therefore the only one whose *install* can fail for reasons these pins @@ -159,7 +159,7 @@ so a failure installing it is a real error and stays fatal. ## A missing column is held to the same standard as a missing row -With five images there are five chances for something outside these pins to go wrong: +Each image is a chance for something outside these pins to go wrong: a base image withdrawn, a wheel that stops publishing for one interpreter, a daemon that runs out of disk halfway through. **None of those ends the run**, and none of them silently narrows the table: @@ -177,7 +177,7 @@ them silently narrows the table: **The exit status says which happened**, because a scheduled invocation reads nothing else: `0` when every requested version was measured, `3` **when the tables were written but a version was lost**, `1` when no table was produced at all, and `2` -for bad arguments. A `make bench` that measured one column out of five is not a +for bad arguments. A `make bench` that measured only some of its columns is not a failure -- the tables are there -- but it is not success either, and it should not be indistinguishable from it. @@ -213,7 +213,7 @@ does not apply to it would be the wrong kind of caution. The text report additionally prints each shared engine's median *per environment*: if two environments disagree about `dpkt`, the join on `default` is not doing its -job, and that is the line that shows it. With seven environments that line is also the +job, and that is the line that shows it. That line is also the quickest way to see whether an engine behaves differently on one interpreter. ## Departures from the legacy methodology @@ -227,7 +227,7 @@ milliseconds per packet. What changed, and why: | Change | Why | | --- | --- | | The whole set is measured `--repeats` times, not once | one pass gives no way to tell a real gap from noise, which is most of what was wrong with the old table | -| Every Python version is measured in one run, not one per sitting | the legacy script measures whichever interpreter invokes it, so a complete table was five separate sittings hand-assembled; a column measured last week is not comparable in absolute terms with one measured today, which is the property the per-version table depends on | +| Every Python version is measured in one run, not one per sitting | the legacy script measures whichever interpreter invokes it, so a complete table was one sitting per version, hand-assembled; a column measured last week is not comparable in absolute terms with one measured today, which is the property the per-version table depends on | | A second table reports ratios, with one absolute footnote | absolute times are not comparable across machines, and "which engine is faster" is a question about engines rather than about this host | | The driver is asserted on every extraction | the legacy script preflights once; an engine that stops being itself mid-run would go unnoticed | | Progress is printed to stderr every 100 rounds, not to stdout every round | stdout carries the JSON document, and a flushed write per round is jitter bought for nothing | @@ -284,7 +284,7 @@ compared with anything. Pinned exactly: - every base image, **by digest**, in `python-images.txt` rather than by tag: - `3.11-slim-bookworm` and its four siblings are mutable and rebuilt whenever Debian + `3.11-slim-bookworm` and its siblings are mutable and rebuilt whenever Debian or CPython ships a patch, so a tag pin is a pin in name only. They are all **slim-bookworm**, which is a measurement decision rather than a packaging one -- the per-version table's columns are compared with each other, so everything but the @@ -349,17 +349,17 @@ would undo that, and would leak machine identity into a public README. **A full default matrix is on the order of two hours**, and it splits roughly in two: -- **build: 15-25 minutes.** Five images, each installing the same pinned set, plus a +- **build: 15-25 minutes.** One image per version, each installing the same pinned set, plus a `pypcap` C extension compiled twice (3.10 and 3.11). Docker's layer cache does not help across versions -- a different base image is a different layer chain -- so this cost is per column. Measured on this suite: a 3.12 image, which compiles nothing, takes about a minute and a half; a 3.11 image, which does, takes about three. -- **measure: 1.5-2 hours.** Seven environments x 1,000 rounds x 3 passes, and **almost +- **measure: 1.5-2 hours.** 1,000 rounds x 3 passes in every environment, and **almost all of it is** `pyshark`, which spawns a `tshark` process per extraction and is two to three orders of magnitude slower than anything else in the table. It runs in - six of the seven environments (everything but 3.14, where it cannot run at all). + every environment but 3.14's, where it cannot run at all. -Three ways to cut it, in descending order of how much they save: +Ways to cut it, in descending order of how much they save: ```shell # drop pyshark and the run is minutes rather than hours @@ -452,8 +452,8 @@ back to `pcapkit`'s parser refuses to report a number. to report "running". - The `pypcap` layer takes minutes on arm64 -- expected, and explained above: its `setup.py` walks `/usr` recursively to find a library its search list does - not name. It is built for 3.10 and 3.11 only, so this cost is paid twice, not five - times. + not name. It is built for 3.10 and 3.11 only, so this cost is paid on those images, + not on every one. - `pyshark` reports not measured, "the tshark binary ... is not installed" -- the image installs `tshark`, so this means the apt install was skipped or failed; rebuild without `--no-build`. diff --git a/examples/benchmark/benchmark.py b/examples/benchmark/benchmark.py index 55a6b80c04..9dcc46d99c 100644 --- a/examples/benchmark/benchmark.py +++ b/examples/benchmark/benchmark.py @@ -24,7 +24,7 @@ milliseconds per packet. The deliberate departures are listed in :file:`README.md` under "Departures from the legacy methodology". -Two properties matter more than the numbers themselves. +Some properties matter more than the numbers themselves. **The engine that ran is asserted, not assumed.** A missing or unusable engine does not raise: :meth:`Extractor.run @@ -404,7 +404,7 @@ def preflight(engine: 'str', capture: 'str') -> 'tuple[Optional[str], Optional[s through a thousand extractions throws the whole repeat away, and an engine this environment does not have is not a measurement at all. - Three sources are consulted, in this order, and the order is the point: what the + These sources are consulted, in this order, and the order is the point: what the image recorded about not installing the engine at all (:func:`_not_attempted`), then the engine's own ``unsupported_reason()``, then an actual attempt. The first wins outright where it exists, since an engine the interpreter rules out cannot @@ -483,8 +483,8 @@ def measure(engine: 'str', capture: 'str', rounds: 'int', fastidiousness. ``pyshark`` spawns a :program:`tshark` process per extraction, so a default run spawns three thousand of them, and measured here: one of them crashed with ``TSharkCrashException ... retcode: 255`` partway through the - third pass and took the entire run with it -- six working engines went - unreported because the seventh hiccupped once. A crashed extraction is a + third pass and took the entire run with it -- every working engine went + unreported because one hiccupped once. A crashed extraction is a *non-measurement* rather than a slow measurement, so discarding its sample is correct; what would not be honest is discarding it silently, which is why the count and the reasons are returned and reported. @@ -629,7 +629,7 @@ def run(capture: 'str', engines: 'tuple[str, ...]', rounds: 'int', repeats: 'int facts = _capture_facts(capture) # Preflight once, up front. An engine ruled out here is ruled out for every - # repeat, and reporting the reason once beats reporting it three times. + # repeat, and reporting the reason once beats reporting it on every repeat. status = {} # type: dict[str, Optional[str]] for engine in engines: reason, driver = preflight(engine, capture) @@ -660,7 +660,7 @@ def run(capture: 'str', engines: 'tuple[str, ...]', rounds: 'int', repeats: 'int except Exception as exc: # pylint: disable=broad-except # A third-party engine gave up partway through this pass. Measured: # `tshark` crashed on the third pass of a real run and, before this - # branch existed, took the six working engines down with it. The + # branch existed, took the working engines down with it. The # pass is lost; the run is not, and the loss is recorded rather than # quietly rounded away. failures[engine].append(f'pass {repeat + 1}: {type(exc).__name__}: {exc}') diff --git a/examples/benchmark/entrypoint.sh b/examples/benchmark/entrypoint.sh index 07ecc0cd2e..a25f86ce5e 100755 --- a/examples/benchmark/entrypoint.sh +++ b/examples/benchmark/entrypoint.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash # -# Inside the image, in one of two modes. +# Inside the image, in one of the modes below. # # `BENCH_MODE=measure` (the default) times every environment this image holds and # writes one JSON document per environment. Which environments those are is read @@ -60,8 +60,8 @@ if [ "$MODE" = measure ]; then # The locks record the resolved transitive closure, which the pinned # requirements files only partly fix. Copied out under the environment's own # label rather than the virtualenv's, because the matrix has one lock per - # (version, `pcap` provider) pair and `pypcap.txt` from five images would be - # four files overwriting each other. + # (version, `pcap` provider) pair and `pypcap.txt` from several images would + # be files overwriting each other. cp -f "$lock" "$OUT/$label.txt" 2>/dev/null || true done 3< /opt/environments exit 0 diff --git a/examples/benchmark/python-images.txt b/examples/benchmark/python-images.txt index fe9fff93d8..f8931274c5 100644 --- a/examples/benchmark/python-images.txt +++ b/examples/benchmark/python-images.txt @@ -31,7 +31,7 @@ # ## Which engines run on which interpreter # # Measured, not assumed. `default` (pcapkit's own parser), `dpkt` and `scapy` run -# everywhere. The other four have ceilings, and only the first of them is a +# everywhere. The others have ceilings, and only the first of them below is a # *build* problem, which is why only it needs a column here: # # | engine | interpreters | why it stops | diff --git a/examples/benchmark/report.py b/examples/benchmark/report.py index cc49414e8c..3e23a246e9 100644 --- a/examples/benchmark/report.py +++ b/examples/benchmark/report.py @@ -350,7 +350,7 @@ def _ordered(documents: 'Sequence[dict[str, Any]]') -> 'list[dict[str, Any]]': Presentation only -- :func:`collect` keeps the order it was given, since the order ratios are pooled in does not change them. This exists so that what the report *says* does not depend on the order the JSON files happened to arrive in, which - for a matrix of nine environments is otherwise a diff between two runs of the same + for a matrix of environments is otherwise a diff between two runs of the same thing. Args: @@ -798,7 +798,7 @@ def _provenance(documents: 'Sequence[dict[str, Any]]', image: 'Optional[str]', # "pass" throughout rather than "repeat", so this row and the table's "Samples" # column are counting the same unit. They are not the same *number*, and cannot be: # a pass happens once per environment, so the table's count is this figure times the - # number of environments -- 3 passes over 7 environments is 21 samples per engine. + # number of environments. # The column was called "Passes" while there were two environments and one # interpreter, where the difference was easy to overlook; across a matrix it reads as # a contradiction, hence the rename. @@ -990,7 +990,7 @@ def render_versions_rst(rows: 'Sequence[Row]', documents: 'Sequence[dict[str, An continue by_reason.setdefault(reason, []).append(series) # Grouped by reason rather than one bullet per cell: the usual case is one - # sentence true of three consecutive versions, and repeating it three times + # sentence true of several consecutive versions, and repeating it for each # makes the notes longer than the table they annotate. for reason, series_list in by_reason.items(): notes.append(f'* ``{row.label}`` on {", ".join(series_list)} -- {_escape(reason)}') diff --git a/examples/benchmark/run.sh b/examples/benchmark/run.sh index 92bb019c24..0e6f5ef6d0 100755 --- a/examples/benchmark/run.sh +++ b/examples/benchmark/run.sh @@ -275,7 +275,7 @@ trap cleanup EXIT INT TERM # bullet under the table, so the person reading the table months later sees the same # reason as the person who watched the run. # -# "a gap" rather than "not measured", because the two callers below that record a note +# "a gap" rather than "not measured", because the callers below that record a note # *after* copying results out may well have collected an environment or two first, and # the reporting pass tells those cases apart -- a column with figures in it and a note # attached is partly measured, not unmeasured. @@ -306,10 +306,10 @@ for version in ${VERSIONS}; do if [ "${DO_BUILD}" -eq 1 ]; then echo "==> building for ${PLATFORM} (pcapkit ${REVISION}, base ${base_image})" >&2 - # The one place a build failure is caught rather than propagated. Five - # versions means five chances for a base image to have gone missing or a + # The one place a build failure is caught rather than propagated. Every + # version is a chance for a base image to have gone missing or a # wheel to have stopped publishing for one interpreter, and none of those is - # a reason to abandon the four columns that would have worked. + # a reason to abandon the columns that would have worked. if ! docker build \ --platform "${PLATFORM}" \ --file "${SCRIPT_DIR}/Dockerfile" \ diff --git a/examples/benchmark/test_harness.py b/examples/benchmark/test_harness.py index 4949a63a27..8485f3a53b 100644 --- a/examples/benchmark/test_harness.py +++ b/examples/benchmark/test_harness.py @@ -588,8 +588,8 @@ class TestFlakyEngines: Not hypothetical. On a real 1,000-round run, ``tshark`` crashed on the third pass -- ``TSharkCrashException ... retcode: 255``, after roughly two thousand - process spawns -- and the run produced no table at all. Six working engines went - unreported because the seventh hiccupped once. + process spawns -- and the run produced no table at all. Every working engine went + unreported because one hiccupped once. """ @@ -1001,7 +1001,7 @@ def test_a_fully_measured_engine_is_not_marked(self): def test_one_reason_covering_several_versions_is_stated_once(self): """Notes are grouped by reason, not one bullet per empty cell. - With five columns and seven engines the ungrouped form runs to more lines than + Over a full matrix the ungrouped form runs to more lines than the table it annotates, and reads as though each repetition were a separate finding. @@ -1023,14 +1023,14 @@ def test_a_missing_version_does_not_blame_the_engines(self): """No engine is marked for a column that never ran. The column's own note explains every blank in it, and tagging each engine as - well would attribute someone else's build failure to seven engines that were + well would attribute someone else's build failure to engines that were never given the chance to fail. """ snippet, _, rows = self._snippet(versions=('3.11',), missing=[('3.15', 'the image failed to build')]) assert f'3.15 {report.UNMEASURED_MARK}' in snippet - # Every engine, not a sample of two: on 3.11 all seven are measurable, so the + # Every engine, not a sample: on 3.11 every engine is measurable, so the # only mark in the whole table should be the one on the 3.15 column heading. for row in rows: assert f'``{row.label}`` {report.UNMEASURED_MARK}' not in snippet @@ -1130,7 +1130,7 @@ def test_a_partly_measured_version_says_partly(self): assert 'Python 3.11 (partly measured)' in snippet def test_pooling_across_interpreters_is_declared(self): - """A ratio pooled over five interpreters must not read as one interpreter's.""" + """A ratio pooled over several interpreters must not read as one interpreter's.""" docs = matrix() snippet = report.render_rst(report.collect(docs), docs) assert 'pooled across every environment' in snippet diff --git a/examples/generators/dispatch.py b/examples/generators/dispatch.py index aa907e9078..957e14380d 100644 --- a/examples/generators/dispatch.py +++ b/examples/generators/dispatch.py @@ -1,9 +1,9 @@ # -*- coding: utf-8 -*- -"""Enumerate the eight ``__proto__`` dispatch registries, and probe every entry. +"""Enumerate the ``__proto__`` dispatch registries, and probe every entry. :file:`pcapkit/foundation/registry/protocols.py` documents the registries that decide *which* :class:`~pcapkit.protocols.protocol.Protocol` subclass parses the -next layer -- 47 entries across eight tables. Nothing enumerated them: several +next layer. Nothing enumerated them: several test modules reference ``__proto__``, and :file:`tests/protocols/test_registry_runtime.py` checks that a table entry *resolves* to the right class object, but that is a weaker property than the one @@ -13,7 +13,7 @@ that shipped once: its own docstring records that :class:`~pcapkit.protocols.application.ospf.OSPF` "was reachable from no table at all and could not have parsed a packet if it had been". But that module hand-picks -its eleven cases rather than enumerating, so it guards the entries someone +its cases rather than enumerating, so it guards the entries someone remembered rather than the registries themselves. See GitHub issue #496. This module is the ``__proto__`` counterpart to @@ -24,17 +24,17 @@ round-trips construct -> parse -> construct and compares octets; dispatch has no such symmetry to exploit, so :func:`probe` instead builds one minimal envelope per code, decodes it the way the library itself would (through -:func:`pcapkit.extract` for the six packet-level tables, or by handing the +:func:`pcapkit.extract` for the packet-level tables, or by handing the :class:`~pcapkit.protocols.misc.pcap.frame.Frame` bytes straight to -:func:`pcapkit.extract` for the two pcap/pcapng link-type tables, which is the +:func:`pcapkit.extract` for the pcap/pcapng link-type tables, which is the same call with a different global header), and asks whether the resulting :class:`~pcapkit.corekit.protochain.ProtoChain` actually contains the class :data:`PINNED_TARGETS` says the code is *supposed* to reach -- not merely that the alias string looks right, since several of these classes rename themselves on the wire (:class:`~pcapkit.protocols.application.rarp.RARP` reports ``'ARP'`` for ``oper in (1, 2)``; :class:`~pcapkit.protocols.internet.hip.HIP` reports -``'HIPv2'``) and one table entry's target *is* itself -:class:`~pcapkit.protocols.misc.raw.Raw` rather than a defect. +``'HIPv2'``), and a table entry whose target *is* +:class:`~pcapkit.protocols.misc.raw.Raw` itself is not a defect. The expected class is deliberately **not** read out of the registry under test. An earlier version of this module did exactly that @@ -51,9 +51,9 @@ so a mismatch between the two is reported as what it is rather than folded into "dispatch failed". -Two of the 47 entries are known to degrade to :class:`Raw`, for reasons +Some entries are known to degrade to :class:`Raw`, for reasons recorded in :data:`KNOWN_DEGRADED` rather than fixed here -- this module reports -defects, it does not carry workarounds for them. Both are +defects, it does not carry workarounds for them. They are :class:`~pcapkit.protocols.transport.sctp.SCTP` payload protocol identifiers reaching :class:`~pcapkit.protocols.application.ngap.NGAP`, and neither is a defect: building a well-formed PER-encoded ``NGAP-PDU`` is out of scope for @@ -139,7 +139,7 @@ def code_name(code: 'Any', enum: 'Optional[Any]' = None) -> 'str': Most ``__proto__`` tables are keyed entirely by enumeration members, whose own ``.name`` is used directly. ``Link.__proto__`` and - ``Internet.__proto__`` each hold one entry keyed by a bare :class:`int` + ``Internet.__proto__`` each hold an entry keyed by a bare :class:`int` instead (0x8137 for IPX, in ``Link``'s case) -- :func:`_resolve` does not care, since dict lookup does not distinguish an :class:`~enum.IntEnum` member from the plain :class:`int` it equals, but a case label needs a @@ -511,8 +511,8 @@ def _linktype_payload(code: 'Any') -> 'tuple[bytes, bool]': Returns: The record's octets, and whether they still need an Ethernet wrapper (true for :attr:`~pcapkit.const.reg.linktype.LinkType.ETHERNET`, false - for the two raw-IP link types, which carry no link layer at all, and - for the two loopback link types, whose header is built here). + for the raw-IP link types, which carry no link layer at all, and + for the loopback link types, whose header is built here). """ from pcapkit.const.reg.linktype import LinkType @@ -905,10 +905,10 @@ def outcomes(families: 'Optional[tuple[Family, ...]]' = None) -> 'list[Outcome]' #: names has been fixed; a case *not* listed here must reach its target, or a #: fresh regression has gone unnoticed. KNOWN_DEGRADED = { - # NGAP is registered for both PPIDs (SCTP.__proto__ is a defaultdict of - # exactly two entries). Building a well-formed PER-encoded NGAP-PDU is out - # of scope for a minimal dispatch probe -- this uses the same placeholder - # payload (``b'ngap-pdu'``) test_sctp_unit.py::test_ppid_dispatch_hook + # NGAP is registered for both of its PPIDs. Building a well-formed + # PER-encoded NGAP-PDU is out of scope for a minimal dispatch probe -- + # this uses the same placeholder payload (``b'ngap-pdu'``) + # test_sctp_unit.py::test_ppid_dispatch_hook # does, which is not an aligned PER encoding and fails inside NGAP's own # decoder. The *reason* reported depends on the environment: where the # optional ``pycrate`` dependency is installed the decoder runs and reports diff --git a/examples/generators/endian.py b/examples/generators/endian.py index d8e5812c2e..3196d70f9a 100644 --- a/examples/generators/endian.py +++ b/examples/generators/endian.py @@ -28,14 +28,14 @@ =============================== ========================================== ``big_endian.pcap`` magic ``a1 b2 c3 d4`` -- big-endian, microsecond timestamps -``little_endian.pcap`` magic ``d4 c3 b2 a1`` -- the same three +``little_endian.pcap`` magic ``d4 c3 b2 a1`` -- the same records in the little-endian container ``big_endian_nanosecond.pcap`` magic ``a1 b2 3c 4d`` -- big-endian, nanosecond timestamps =============================== ========================================== The little-endian twin is the point of the set rather than a spare. It carries -the *same three records* as ``big_endian.pcap`` -- identical timestamps, +the *same records* as ``big_endian.pcap`` -- identical timestamps, identical lengths, byte-identical packet data -- so a test can assert that the two files parse to the same values instead of only that the big-endian one parses to values hardcoded in the test. That is the property the byte-order @@ -43,7 +43,7 @@ The packet data comes from :mod:`scapy`, as it does in :file:`pcap.py`, so the frames carry real headers and real checksums. The *containers* are packed here -by hand with :mod:`struct`, for two reasons: :func:`scapy.utils.wrpcap` writes +by hand with :mod:`struct`, for these reasons: :func:`scapy.utils.wrpcap` writes the host's byte order and offers no way to ask for the other one, and the byte order of the container is the whole subject of these fixtures -- spelling it out where it can be read is worth more than delegating it. @@ -103,8 +103,8 @@ EPOCH = 1500000000 #: The fixtures this module writes, as ``(file name, byte order, nanosecond -#: flag)``. The two microsecond files carry byte-identical records in the two -#: containers; the nanosecond one holds the same packets, timed more finely. +#: flag)``. The microsecond files carry byte-identical records in both byte +#: orders; the nanosecond one holds the same packets, timed more finely. FIXTURES = ( ('big_endian.pcap', 'big', False), ('little_endian.pcap', 'little', False), @@ -116,7 +116,7 @@ class _Record(NamedTuple): """One record of a capture: a record header's four fields, and its data. The record header is *not* stored as octets here, because the octets are - what differs between the three fixtures. :func:`_pack` turns these numbers + what differs between the fixtures. :func:`_pack` turns these numbers into a header in whichever byte order it was asked for. """ @@ -156,7 +156,7 @@ def _filler(length: 'int', tag: 'bytes') -> 'bytes': def _records(nanosecond: 'bool' = False) -> 'list[_Record]': - """Build the three records every fixture in this module carries. + """Build the records every fixture in this module carries. Args: nanosecond: Whether the timestamp fractions are nanoseconds. Only the diff --git a/examples/generators/legacy.py b/examples/generators/legacy.py index 84c08aeb8a..d6bf684ac4 100644 --- a/examples/generators/legacy.py +++ b/examples/generators/legacy.py @@ -8,7 +8,7 @@ scripts cannot run at all on a fresh checkout. This module writes them, with :mod:`scapy`, deterministically and without network access. -``test.pcap`` (34 frames) +``test.pcap`` Read by ``examples/legacy_smoke/test_reassembly.py``, which extracts it with ``tcp=True``, ``reasm_strict=True`` and ``reassembly=True`` and prints every reassembled TCP datagram. Two connections to port 80, one over IPv4 @@ -18,22 +18,22 @@ delivered out of order, with one of them retransmitted; the IPv6 request body is spread over three segments delivered in order. The IPv4 connection closes with a FIN from each end, the IPv6 one with a FIN from the server - and an abortive RST from the client, so the capture covers both of the - events that make :mod:`pcapkit` submit a datagram. + and an abortive RST from the client, so the capture covers each event + that makes :mod:`pcapkit` submit a datagram. -``http6.cap`` (26 frames) +``http6.cap`` Read by ``examples/legacy_smoke/test_analyse.py``, which pretty-prints the application layer :mod:`pcapkit` finds in each reassembled datagram. Two HTTP/1.1 connections over IPv6 to port 80: a page fetch whose response body spans three segments, and a conditional request for a stylesheet answered - ``304 Not Modified`` with no body at all. All four datagrams -- two - requests, two responses -- analyse as HTTP. + ``304 Not Modified`` with no body at all. Every datagram, requests and + responses alike, analyses as HTTP. That script does not run to completion even with this fixture in place, and the reason is the script rather than the capture: it asks for ``tcp=True`` and ``reasm_strict=True`` but never for ``reassembly=True``, so reading ``extraction.reassembly`` raises ``UnsupportedCall: 'Extractor(reassembly= - False)' object has no attribute 'reassembly'``. It extracts all 26 frames + False)' object has no attribute 'reassembly'``. It extracts every frame first, and adding that one keyword makes the rest of it work; the fixture is built for the script as it will read once repaired. @@ -45,7 +45,7 @@ How pcapkit's TCP reassembly shapes these captures -------------------------------------------------- -The layouts above are not arbitrary. Four properties of +The layouts above are not arbitrary. These properties of :class:`pcapkit.foundation.reassembly.tcp.TCP` decide what a capture has to look like for a datagram to come out of it at all: @@ -673,7 +673,7 @@ def _write_test(dest: 'pathlib.Path') -> 'pathlib.Path': delay=0.014772) # ------------------------------------------------------------------ - # 2-16: the IPv4 page fetch + # 2-20: the IPv4 page fetch # ------------------------------------------------------------------ page = _Flow(_CLIENT4, _SERVER4, 49812, 80) request = _http_request('GET', host4, '/index.html', @@ -714,7 +714,7 @@ def _write_test(dest: 'pathlib.Path') -> 'pathlib.Path': capture.add(page.ack(False)) # ------------------------------------------------------------------ - # 17-22: the IPv6 telemetry upload + # 21-33: the IPv6 telemetry upload # ------------------------------------------------------------------ upload = _Flow(_CLIENT6, _SERVER6, 49814, 80, client_window=32768) report = _http_request('POST', host6, '/v1/telemetry', @@ -755,7 +755,7 @@ def _write_http6(dest: 'pathlib.Path') -> 'pathlib.Path': conditional request the browser makes for the stylesheet the page references, which the server answers ``304 Not Modified`` -- a response with a header block, no body and no ``Content-Length``. Both connections - close from both ends, so all four datagrams are submitted. + close from both ends, so every datagram is submitted. Args: dest: Destination directory. diff --git a/examples/generators/make_samples.py b/examples/generators/make_samples.py index 19c1c707b7..1f90ea1937 100644 --- a/examples/generators/make_samples.py +++ b/examples/generators/make_samples.py @@ -2,7 +2,7 @@ """Regenerate the sample captures used by the test suite and the examples. The captures under ``examples/captures/`` are not tracked in git, apart from the -two inputs ``in.pcap`` and ``dhcp.pcapng`` (see ``.gitignore``), yet the runtime, +inputs ``in.pcap`` and ``dhcp.pcapng`` (see ``.gitignore``), yet the runtime, regression and integration tests read them and pin their contents, as do the demonstration scripts in ``examples/legacy_smoke/``. This script rebuilds the generated captures, so a fresh clone can run ``pytest`` without any ignore flags: diff --git a/examples/generators/options.py b/examples/generators/options.py index eaa6adfa01..c24383d85e 100644 --- a/examples/generators/options.py +++ b/examples/generators/options.py @@ -27,8 +27,8 @@ than a packet assembled by :mod:`scapy` from its own protocol model. That is the point: the octets under test are the library's own output, so a capture is a record of what this version of ``pcapkit`` builds, and re-parsing it exercises -``_read_*`` against ``_make_*``. The one frame with more in it is HOPOPT's Jumbo -Payload case, a genuine jumbogram -- see :func:`_jumbogram_datagram`. +``_read_*`` against ``_make_*``. HOPOPT's Jumbo Payload case is the exception, +carrying a genuine jumbogram -- see :func:`_jumbogram_datagram`. Only the codes that construct *successfully* can appear in a capture -- there are no octets for the ones that do not. A failing case in a captured family @@ -66,8 +66,8 @@ ----------- The captures have to regenerate byte-identically; a fixture that churns is -worse than none, because every unrelated change then shows up as a diff. Four -things would otherwise vary between runs and are pinned: +worse than none, because every unrelated change then shows up as a diff. These +would otherwise vary between runs, and are pinned: 1. Frame timestamps, from :data:`EPOCH` rather than the clock. 2. The order cases are emitted in, from :func:`sort_key` rather than from @@ -79,13 +79,13 @@ 4. Anything reading the host. No case here uses PCAP-NG's ``_make_option_if_os``, which defaults to the live :mod:`platform` string. -Two deliberate deviations from the sibling generators ----------------------------------------------------- +Deliberate deviations from the sibling generators +------------------------------------------------- Every import of :mod:`pcapkit` and of :mod:`scapy` in this module is inside the function that needs it, which is why :mod:`pylint`'s ``import-outside-toplevel`` is switched off for the file rather than argued with -forty times. Both are load-bearing. :mod:`scapy` is needed only by +at every import. Both are load-bearing. :mod:`scapy` is needed only by :func:`generate`, while the case table and :func:`roundtrip` are useful without it -- the unit-tier test uses both and has to run on a checkout that installed no :mod:`scapy`, so a top-level import would make that test skip for a dependency it @@ -416,7 +416,7 @@ def _named_registry(owner: 'type', enum: 'Any', prefix: 'str', #: ` declares it, because ``make`` #: takes ``**kwargs`` and *silently drops* whatever it does not declare -- no #: ``UnknownFieldWarning``, no ``TypeError``, nothing. GitHub issue #602 is -#: three keys that were spelled wrong and therefore ignored: +#: the keys below, spelled wrong and therefore ignored: #: #: * ``'seq'`` is ``seq_no``. The mapping read as sequence number 1 and every #: generated frame carried 0, which is what made the defect worth an issue: @@ -448,7 +448,7 @@ def _tcp_registry() -> 'Any': def _tcp_overrides() -> 'dict[Any, dict[str, Any]]': from pcapkit.const.tcp.option import Option as Enum_Option return { - # Each of these four defaults to an empty variable-length field, which + # Each of these defaults to an empty variable-length field, which # is the one shape that never runs the field's packing loop. Enum_Option.SACK: {'sack': [(1, 2), (3, 4)]}, Enum_Option.TCP_Alternate_Checksum_Data: {'data': b'\x01\x02'}, @@ -804,7 +804,7 @@ def _sctp_cause_overrides() -> 'dict[Any, dict[str, Any]]': return { Enum_CauseCode.Missing_Mandatory_Parameter: { 'types': [Enum_Parameter.State_Cookie]}, - # The four causes below carry an embedded TLV, and each one's default + # The causes below carry an embedded TLV, and each one's default # is a zero-length value, i.e. not a TLV. Enum_CauseCode.Unresolvable_Address: { 'value': b'\x00\x0b\x00\x0elocalhost\x00'}, @@ -896,7 +896,7 @@ def _mh_option_overrides() -> 'dict[Any, dict[str, Any]]': Enum_Option.Permanent_Home_Keygen_Token: {'token': b'\xbb' * 8}, Enum_Option.Experimental_Mobility_Option: {'data': b'\xcc' * 4}, Enum_Option.Binding_Authorization_Data_for_FMIPv6: {'data': b'\xb5'}, - # The four below are not workarounds for defects: each option has a + # The entries below are not workarounds for defects: each option has a # minimum content its RFC requires, and the no-argument default is not a # well-formed instance of it. The constructors refuse it, correctly, so # the arguments here are what make the code reachable at all. @@ -1125,7 +1125,7 @@ def _hip_build(code: 'Any', kwargs: 'dict[str, Any]') -> 'Any': # ``extension=True`` is not decoration. ``HIP.alias`` reads ``self._info``, # which ``__post_init__`` only assigns after ``read`` returns, and the sole # path that returns before touching it is the ``if extension`` early exit. - # Without it every one of the 49 codes fails with ``AttributeError: 'HIP' + # Without it every HIP code fails with ``AttributeError: 'HIP' # object has no attribute '_info'``. return HIP(parameters=[(code, kwargs)] * HIP_COPIES, extension=True, **_hip_base(code)) @@ -1220,7 +1220,7 @@ def _httpv2_rebuild(frame: 'Any') -> 'Any': # PCAP-NG -- blocks, block options, name records and decryption secrets ############################################################################### # -# These four families are exercised by :func:`roundtrip` but deliberately write +# These families are exercised by :func:`roundtrip` but deliberately write # no capture, and the reason is not that a PCAP-NG capture would be # uninteresting -- it is that the construction API cannot produce a # *reproducible* one. ``_make_block_shb`` takes the section's byte order from @@ -1646,7 +1646,7 @@ def _pcapng_secrets_build(code: 'Any', kwargs: 'dict[str, Any]') -> 'Any': Family('hip-parameter', _hip_registry, _hip_overrides, _hip_build, _hip_parse, _hip_extract, _hip_rebuild, capture='options-internet.pcap', envelope='ipv6', proto=139), - # The four PCAP-NG families write no capture -- see the section comment + # The PCAP-NG families write no capture -- see the section comment # above ``PCAPNG_TIME`` for why the construction API cannot make a # reproducible one, and what already covers that space instead. Family('pcapng-block', _pcapng_block_registry, _pcapng_block_overrides, diff --git a/examples/generators/pcap.py b/examples/generators/pcap.py index d79a9b482a..be5a267004 100644 --- a/examples/generators/pcap.py +++ b/examples/generators/pcap.py @@ -2,8 +2,8 @@ """Generate the ``.pcap`` sample captures used by the test suite. The tests under ``tests/`` extract capture files from ``examples/captures/``, but -``.gitignore`` keeps that directory empty except for a handful of committed -files, so a fresh checkout cannot run them. This module rebuilds the missing +``.gitignore`` keeps that directory empty except for the files committed +there, so a fresh checkout cannot run them. This module rebuilds the missing ``.pcap`` fixtures with :mod:`scapy`, deterministically and without network access, so every machine gets byte-identical captures. @@ -67,8 +67,8 @@ indexes frames 114, 117, 393, 556, 587, 629 and 1113 directly -- hence the ``_HTTPBuilder.fill`` calls that pace the surrounding traffic. -Two details are worth flagging, because both look like sleight of hand and -neither is: +These details are worth flagging, because each looks like sleight of hand and +none is: 1. Some tests pin a UDP checksum. A checksum is a function of the datagram, so the datagram is chosen to have that checksum: a single free 16-bit word diff --git a/examples/generators/pcapng.py b/examples/generators/pcapng.py index 88d962a502..dd206f19be 100644 --- a/examples/generators/pcapng.py +++ b/examples/generators/pcapng.py @@ -1,10 +1,10 @@ # -*- coding: utf-8 -*- """Provision the ``.pcapng`` sample fixtures used by the test suite. -The test suite (``tests/protocols/test_pcapng_regression.py``) reads five +The test suite (``tests/protocols/test_pcapng_regression.py``) reads PCAP-NG captures out of ``examples/captures/`` that :file:`.gitignore` deliberately keeps out of the repository. This module puts them back on any machine, without a -checkout of anything private, by two routes: +checkout of anything private, by one of these routes: * **Downloaded** -- fetched from a public source, with the source URL and the SHA-256 of the exact bytes pinned below. The digest is checked after every @@ -33,7 +33,7 @@ Provenance and licensing of the downloaded captures --------------------------------------------------- -Both downloads come from the Wireshark source tree's ``test/captures`` +The downloads come from the Wireshark source tree's ``test/captures`` directory (https://gitlab.com/wireshark/wireshark, mirrored on GitHub), which is distributed under the GNU GPL v2 or later. They are fetched into ``examples/captures/``, which :file:`.gitignore` excludes, so this project never redistributes them -- @@ -50,7 +50,7 @@ Block types deliberately left out --------------------------------- -Two block types once raised on spec-conformant input and parse since their +Some block types once raised on spec-conformant input and parse since their fixes, so they are absent from these fixtures only because the fixtures have not been extended to carry them -- not because pcapkit cannot read them: @@ -73,7 +73,7 @@ no attribute '_info'`` (GitHub issue #345). The block MUST NOT appear in new files, so a fixture carrying one would be documenting the past. -Two further constructs were once avoided rather than covered; both are +Further constructs were once avoided rather than covered; they are **fixed**, so the fixtures carry them as regression cover: * **``if_IPv6addr``** -- ``IPv6InterfaceField.post_process`` in @@ -93,11 +93,11 @@ simple packet block in the two-interface first section, which is the case that used to be rejected. -Three further defects were exercised on purpose, because they warned rather than +Further defects were exercised on purpose, because they warned rather than raised, and a fixture that covers the code path is what will catch a future -crash there. All three were option-area sizing errors in -``pcapkit/protocols/schema/misc/pcapng.py``, all three were confirmed by -bisecting one block type at a time, and all three are **fixed** -- so the +crash there. Each was an option-area sizing error in +``pcapkit/protocols/schema/misc/pcapng.py``, each was confirmed by +bisecting one block type at a time, and each is **fixed** -- so the fixtures that reached them are regression cover rather than known-bad input: * **Interface Statistics Block** -- ``options`` was sized ``length - 20`` though @@ -458,12 +458,12 @@ def _tls_key_log() -> 'bytes': def _timestamp_options(writer: '_Blocks', resolution: 'int' = 6) -> 'list[tuple[int, bytes]]': - """The three timestamp options every synthesised interface carries. + """The timestamp options every synthesised interface carries. pcapkit looks ``if_tsresol``, ``if_tzone`` and ``if_tsoffset`` up on the owning interface for each packet block and logs a ``MissingKeyError`` for each one that is absent -- harmless, but it buries the interesting output - under three lines per packet. Declaring them explicitly keeps the fixtures + under a line per missing option per packet. Declaring them explicitly keeps the fixtures quiet as well as complete. Args: @@ -672,7 +672,7 @@ def build_test() -> 'bytes': def build_many_interfaces() -> 'bytes': - """Build a capture with eight interface description blocks. + """Build a capture with many interface description blocks. The offline stand-in for the downloaded Wireshark capture. Packets are dealt round-robin across the interfaces so every interface index is referenced by @@ -795,7 +795,7 @@ class _Fixture(NamedTuple): sha256: 'Optional[str]' = None -#: The five fixtures ``tests/protocols/test_pcapng_regression.py`` needs. +#: The fixtures ``tests/protocols/test_pcapng_regression.py`` needs. FIXTURES = ( _Fixture( name='dhcp_big_endian.pcapng', @@ -973,7 +973,7 @@ class _LogCapture(logging.Handler): outside the stack, and the record then never reaches a :func:`warnings.catch_warnings` recorder. So the logger is what this listens to. Duplicates are dropped, because one complaint per block in a capture - with sixty-four packets is not sixty-four pieces of information. + with many packets is not that many pieces of information. """ diff --git a/examples/legacy_smoke/Makefile b/examples/legacy_smoke/Makefile index 6b5ca2407b..30b5374320 100644 --- a/examples/legacy_smoke/Makefile +++ b/examples/legacy_smoke/Makefile @@ -11,7 +11,7 @@ PYTHON ?= python #: README for the reason. FIXTURE_TZ ?= UTC -## Build the four reference reports in ../captures/. Git tracks none of them -- they +## Build the reference reports in ../captures/. Git tracks none of them -- they ## are generated output, so a fresh clone has to run this before it has them (#685). ## out.json, out.plist, out.txt <- test_extractor.py, from ../captures/in.pcap ## pcapng.txt <- test_pcapng.py, from ../captures/dhcp.pcapng diff --git a/examples/legacy_smoke/README.md b/examples/legacy_smoke/README.md index 8b15157093..a58cb8f582 100644 --- a/examples/legacy_smoke/README.md +++ b/examples/legacy_smoke/README.md @@ -58,8 +58,8 @@ Most of these run with nothing but `pcapkit`. These do not: - **test_engine, test_time, test_perf** exercise the `dpkt`, `scapy` and `pyshark` engines. Those are optional packages, and `pyshark` additionally drives Wireshark's `tshark` binary. An engine that is missing is reported as - skipped, with the reason, and the rest still run -- see `_engine_support.py`. Two - things worth knowing: + skipped, with the reason, and the rest still run -- see `_engine_support.py`. + Worth knowing: - `pyshark` 0.6 does not work on Python 3.14 at all. It asks `asyncio.get_event_loop_policy().get_event_loop()` for a loop on a thread that @@ -85,9 +85,9 @@ Most of these run with nothing but `pcapkit`. These do not: ## Regenerating the reference reports -Four files in `../captures/` are reports rather than inputs -- `out.json`, -`out.plist`, `out.txt` and `pcapng.txt` -- and they are produced by two of the -scripts here. **Git does not track them**, so a fresh clone does not have them. Build +Some files in `../captures/` are reports rather than inputs -- `out.json`, +`out.plist`, `out.txt` and `pcapng.txt` -- and they are produced by the scripts +named below. **Git does not track them**, so a fresh clone does not have them. Build them the way you would any other generated capture, and again after a change to how `pcapkit` renders one: @@ -106,19 +106,19 @@ Both *inputs* are committed, so this works on a fresh clone with no `make sample first. Use `make fixtures PYTHON=../../.venv/bin/python` to pick a specific interpreter. -These four were tracked once, and #685 is why they are not any more. Nothing reads a +These reports were tracked once, and #685 is why they are not any more. Nothing reads a report back -- no test asserts on one, and the docs only quote them as prose -- so a parser change that alters how a frame renders leaves the committed copy quietly wrong, with no failure anywhere to say so. `pcapng.txt` sat recording `packet -> NIL` for -four blocks that had started carrying their captured octets, and the staleness was +blocks that had started carrying their captured octets, and the staleness was found by reading the file rather than by anything breaking. Generating them on demand means the copy on your disk describes the tree you are standing in, which was the only thing that made them worth looking at. `TZ=UTC` **is not decoration.** `pcapkit` renders frame timestamps in the host's -local zone, so an unpinned run rewrites every timestamp line in all four files with +local zone, so an unpinned run rewrites every timestamp line in every report with wherever it happened to be run -- which is how these reports came to disagree with -each other while they were still tracked, the PCAP three having been generated at +each other while they were still tracked, the PCAP reports having been generated at UTC-05:00 and `pcapng.txt` at UTC+08:00. Pinning UTC makes the output reproducible on any host, so two runs can be diffed against each other. It also happens to be the only zone in which PCAP-NG's `timestamp_epoch` is the true UNIX epoch: diff --git a/examples/legacy_smoke/_engine_support.py b/examples/legacy_smoke/_engine_support.py index fe3b7399de..94fbb4d7af 100644 --- a/examples/legacy_smoke/_engine_support.py +++ b/examples/legacy_smoke/_engine_support.py @@ -1,10 +1,10 @@ # -*- coding: utf-8 -*- """Which extraction engines can actually run on this host. -``pcapkit`` ships four extraction engines and three of them lean on something the -host may simply not have: ``dpkt``, ``scapy`` and ``pyshark`` are optional -third-party packages, and ``pyshark`` additionally drives the external ``tshark`` -binary from Wireshark. On top of that, ``pyshark`` 0.6 calls +Most of ``pcapkit``'s extraction engines lean on something the host may simply +not have: every engine but ``default`` wraps an optional third-party package, and +``pyshark`` additionally drives the external ``tshark`` binary from Wireshark. +On top of that, ``pyshark`` 0.6 calls ``asyncio.get_event_loop_policy().get_event_loop()`` on a thread with no running loop. Creating one implicitly there was deprecated years ago and Python 3.14 no longer does it, raising ``RuntimeError: There is no current event loop in thread diff --git a/pyproject.toml b/pyproject.toml index b05057359f..7b881d1ddf 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -38,7 +38,7 @@ license = { text="BSD 3-Clause License" } # ``poseur.py:744`` in ``_process_classdef``, which does ``cls_ctx=name.name`` # where ``name`` is already a parso ``Name`` leaf and wants ``.value``. It then # **exits 0**, so ``refactor()``'s ``check_call`` cannot see the failure, and the -# 12 positional-only parameters in ``pcapkit/corekit/io.py`` survive into the +# positional-only parameters in ``pcapkit/corekit/io.py`` survive into the # installed package -- making ``import pcapkit`` fail with ``SyntaxError`` on an # interpreter below 3.8. # @@ -48,7 +48,7 @@ license = { text="BSD 3-Clause License" } # reason to withdraw the declaration. Until it lands, sub-3.8 support is intent # rather than something that works. # -# Independently, CI has two tiers (#1633), declared in .github/python-versions.toml +# Independently, CI sorts versions into tiers (#1633), declared in .github/python-versions.toml # (#1660), which a tier change edits. **3.11 through 3.15** are supported and # blocking. **3.7 through 3.10** are KTLO: end-of-life, still built and tested, never # blocking, and a failure there is a warning on a green leg (#1672). Each 3.7 leg @@ -152,7 +152,7 @@ DPKT = [ "dpkt" ] # The floor is measured, #1572: on scapy 2.5.0 a PCAP-NG interface's # ``if_tsresol`` is lost after a comment option without a newline, so a # nanosecond interface reads as microseconds, and ``packet2dict`` reports DNS -# records and field order differently. 2.6.0, the next release, fixes all three. +# records and field order differently. 2.6.0, the next release, fixes them all. # It needs Python 3.7, hence the marker, as ``PCAP_CT``'s below. Scapy = [ "scapy>=2.6.0; python_version >= '3.7'" ] PyShark = [ "pyshark" ] @@ -171,7 +171,7 @@ PyShark = [ "pyshark" ] # its own, and is unmaintained. PyPCAP = [ "pypcap; python_version < '3.12'" ] # ``pcap-ct`` is what covers 3.12+, and it is a *separate extra* rather than a -# complementary marker on ``PyPCAP`` above. Two reasons, and the first is +# complementary marker on ``PyPCAP`` above, for the reasons below, the first # decisive: # # * They are two engines, not one engine with two backends -- ``engine='pypcap'`` @@ -226,10 +226,10 @@ PCAP_CT = [ PyPCAPFile = [ "pypcapfile; python_version < '3.12'" ] # for developers -- installs pcapkit.vendor's crawlers, run via # `pcapkit-vendor` to regenerate pcapkit.const. Includes ``pycrate`` so -# pcapkit.vendor.ngap's two crawlers (#880) can run, and ``requests``/ -# ``beautifulsoup4`` for the six crawlers that import them at module scope +# pcapkit.vendor.ngap's crawlers (#880) can run, and ``requests``/ +# ``beautifulsoup4`` for the crawlers that import them at module scope # (c.f. the ``test`` extra's own comment below for which, and #507). #910 -# narrowed ``all`` to core addons only, so this extra names all three +# narrowed ``all`` to core addons only, so this extra names each of them # itself rather than leaning on ``all`` to also carry them -- ``pycrate`` # happens to be a core addon in its own right post-#910 (see the ``all`` # extra's comment below), but ``requests``/``beautifulsoup4`` are not, having @@ -241,7 +241,7 @@ vendor = [ "requests[socks]", "beautifulsoup4[html5lib]", "pycrate" ] # than everything an end user might conceivably want. An earlier ruling on the # same issue read the latter and added ``pycrate`` on that basis; the ruling that # stuck narrowed it again, this time by kind: ``cli``, ``crypto`` and ``pycrate`` -# (NGAP) are core addons and stay, and the four 3rd-party capture engines that +# (NGAP) are core addons and stay, and the 3rd-party capture engines that # used to be here -- ``dpkt``, ``scapy``, ``pyshark``, ``pypcapfile`` -- are # installed one at a time on demand, as ``PyPCAP`` and ``PCAP_CT`` always were, # and move out to their own extras below. Install one explicitly: ``pip install pypcapkit[DPKT]`` / @@ -262,11 +262,11 @@ all = [ "emoji", "cryptography>=3.4", "pycrate" ] # ``dpkt``/``scapy``/``pyshark``/``pypcapfile`` newly unresolvable to those # tools. This extra is what a workflow installs *alongside* ``all`` -- # ``.[all,dev]`` -- to see everything ``all`` used to carry, for that purpose -# only. ``.github/workflows/lint.yml``'s own comment tracks exactly 8 +# only. ``.github/workflows/lint.yml``'s own comment traces pylint's # ``import-error`` messages, from ``pypcap``/``pcap-ct`` (via ``PyPCAP`` and # ``PCAP_CT`` above, both absent on the Python it lints) and from # ``pypcapfile`` being excluded there by the same marker this extra repeats -# below; this is the extra that keeps that count at 8 rather than growing it, +# below; this is the extra that keeps those from growing in number, # and it is not a place to add ``pypcap``/``pcap-ct`` just to silence a # finding CI has already decided to carry as advisory. dev = [ @@ -302,32 +302,32 @@ test = [ "pytest-timeout", "typing-extensions", # Enough of the ``vendor`` extra for a test to import a vendor crawler. - # Without these, ``tests/vendor/test_ipx_socket_unit.py`` reports "9 skipped" - # rather than failing, so the byte-for-byte regeneration check that is the + # Without these, ``tests/vendor/test_ipx_socket_unit.py`` reports its tests as + # skipped rather than failing, so the byte-for-byte regeneration check that is the # only guard on the hand-maintained IPX socket registry never runs on any CI # path: every ``unit-tests.yml`` job's install line builds on ``.[test]``, # and ``cron-vendor.yml`` installs ``.[all,vendor]`` but never invokes pytest. # # Both are needed, not just ``requests``. ``pcapkit.vendor.default`` imports # ``requests`` at module scope, and ``pcapkit.vendor.__init__`` then imports - # every crawler unconditionally -- six of which import ``bs4`` at module + # every crawler unconditionally -- some of which import ``bs4`` at module # scope. Its ``try: import bs4`` guard suppresses only the warning, not the # ``ModuleNotFoundError`` that the crawler imports below it raise anyway. # # Plain, without the ``vendor`` extra's ``[socks]`` and ``[html5lib]``: - # nothing in the test suite goes through a proxy, and the one unit-tier + # nothing in the test suite goes through a proxy, and the unit-tier # test that parses HTML (``tests/vendor/test_ftp_return_code_unit.py``) # does so from an inline fixture with the ``vendor`` extra's own # ``html5lib``, not through anything this extra provides, so neither # parser nor proxy support is reachable from here. Their absence - # makes :mod:`pcapkit.vendor` log two ``VendorWarning``s at import, which + # makes :mod:`pcapkit.vendor` log ``VendorWarning``s at import, which # pytest captures and shows only on failure; they change no result. See #507. "requests", "beautifulsoup4", - # Same reason as the two above, for a different test: without it + # Same reason as the entries above, for a different test: without it # ``tests/project/test_isort_clean.py`` skipped on every CI leg, so the only - # guard on ``make isort``'s four recipe lines -- and on the target-resolving - # globs behind them, wrong three times over (#757, #765, #767) -- ran + # guard on ``make isort``'s recipe lines -- and on the target-resolving + # globs behind them, wrong more than once (#757, #765, #767) -- ran # nowhere. #766. # # A lint tool in the ``test`` extra rather than in a ``lint`` extra of its @@ -337,11 +337,11 @@ test = [ # anything, and carrying mypy would silently close the ``HAS_MYPY`` gap that # #779 decided to leave open -- reversing a ruling from an unrelated change. # - # Unlike the other four linters, isort has no CI check of its own to defer + # Unlike the other linters, isort has no CI check of its own to defer # to: ``.github/workflows/lint.yml`` installs "vermin pylint mypy bandit" # and says in its own header that isort appears only in # ``cron-vendor.yml``, as a formatter rather than as a check. Cheap, too -- - # a pure-Python wheel, and all four lines together measure about 1.5s. + # a pure-Python wheel, and the recipe lines together measure about 1.5s. # # Unpinned, matching ``requests`` and ``beautifulsoup4`` above rather than # inventing an unmeasured floor (verified against 9.0.1). A future isort diff --git a/util/bump_version.py b/util/bump_version.py index cf3fe6ec9b..cd74b4c248 100644 --- a/util/bump_version.py +++ b/util/bump_version.py @@ -8,7 +8,7 @@ Where it is invoked from, and why that matters ---------------------------------------------- -Exactly one caller: the ``Bump Version`` step of +Its caller is the ``Bump Version`` step of :file:`.github/workflows/cron-vendor.yml`, between the crawl and the ``git commit``. Completion of that workflow is itself the ``workflow_run`` trigger of :file:`.github/workflows/create-release.yml`, which reads ``pcapkit.__version__`` @@ -57,13 +57,14 @@ actually going out. Until then this is the better of the two available answers, and the ``version`` half is guarded by a test that fails when the file drifts from ``pcapkit.__version__`` -- which also covers the version changes made by hand, -which never run this script at all. Those are 40 of the 167 commits that have -moved ``__version__`` on ``main``: a quarter over the project's life, and 11 of -the most recent 25. +which never run this script at all. Those are about a quarter of the commits +that have moved ``__version__`` on ``main`` over the project's life, and a larger +share of the recent ones. The rewrite is line-oriented rather than a :mod:`yaml` load-and-dump. A round trip through a YAML library would reorder keys, normalise quoting, and drop the -sixteen lines of comment that explain why the file omits ``doi`` and ``orcid`` -- +comment header, which among other things explains why the file omits ``doi`` +and ``orcid`` -- none of which is a change anyone asked for. Matching ``^version:`` and ``^date-released:`` at column zero is enough to identify the two top-level keys, since every nested key in the file is indented and ``cff-version`` does not diff --git a/util/changelog_md.py b/util/changelog_md.py index 21f5da8275..9bc8885dc7 100644 --- a/util/changelog_md.py +++ b/util/changelog_md.py @@ -4,8 +4,8 @@ ``docs/source/changelog/.rst`` is the single source of every changelog entry, and Sphinx renders the lot as the project's history. ``CHANGELOG.md`` is a *derivative* of exactly one of them -- the version being released -- because its -two consumers, the ``Create Release`` workflow's release body and the source -distribution, both read Markdown rather than reStructuredText. +consumers, the ``Create Release`` workflow's release body and the source +distribution, read Markdown rather than reStructuredText. Nothing edits ``CHANGELOG.md`` by hand. That is the point: there is no "promote the latest entry into the history" step to forget, because the entry was written @@ -14,7 +14,7 @@ which is generated from ``pcapkit/vendor/**``. Markdown at the repository root is a deliberate exception to the "documentation is -reStructuredText" rule, justified by those two consumers. +reStructuredText" rule, justified by those consumers. Which entry is "newest" ----------------------- @@ -37,7 +37,7 @@ -------------- The entries are written in a deliberately small reStructuredText subset, so the -conversion is six mechanical rules rather than a document converter: +conversion is the mechanical rules below rather than a document converter: 1. a setext heading becomes an ATX heading, one level per underline character: ``=`` (the version heading) is ``##``, ``-`` (a module @@ -64,7 +64,7 @@ column of short ragged lines. A paragraph with no newline inside it renders the same either way, which is why unwrapping is the safe direction. -Six regexes cannot recognise everything reStructuredText can express, and a rule +Rules this simple cannot recognise everything reStructuredText can express, and a rule that does not fire copies its construct straight through as literal text. That is how a release body ends up with ``.. note::`` printed in it -- published to GitHub and PyPI, where it is expensive and irreversible. So :func:`residual` re-reads the @@ -91,15 +91,15 @@ above it by rule 6 -- the second is how a sub-heading would silently disappear into a paragraph. -Two things it deliberately does **not** catch. A bare ``name_`` reference on its +What it deliberately does **not** catch: a bare ``name_`` reference on its own, because a pattern loose enough to spot a trailing underscore in prose fires on ordinary identifiers; its target line is caught instead, as above. And a construct that rule 6 joins into the *middle* of a line rather than the start of one, since every line-anchored pattern here then has nothing to anchor to -- a field list -buried inside a bullet, say. Both would need the guard to run before unwrapping, -which is a larger change than the insurance is worth. +buried inside a bullet, say. Catching either would need the guard to run before +unwrapping, which is a larger change than the insurance is worth. -Every pattern was measured against all 37 committed entries and matches none of +Every pattern was measured against every committed entry and matches none of them, so the guard costs nothing until an entry actually leaves the subset. Where a complaint points @@ -111,7 +111,7 @@ It is not the frame the guard finds the construct in. Rule 6 joins a wrapped bullet onto one line, so the converted body is a fraction of the file's length and a number counted in it is not a location at all: measured on the 1.5.0 -entry, the body is 55 lines against the file's 580, and four roles written on +entry, the body was 55 lines against the file's 580, and the roles written on source lines 467 and 468 all reported as ``line 46``. The bound alone settles it -- a number that cannot exceed 55 cannot name a line in a 580-line file. @@ -261,7 +261,7 @@ #: reStructuredText that should be gone by the time conversion finishes. Checked #: against the masked text, and each one means a rule did not fire. Every pattern -#: matches none of the 37 committed entries; see the module docstring for what is +#: matches none of the committed entries; see the module docstring for what is #: covered and what is deliberately not. _RESIDUAL = ( (re.compile(r':[a-zA-Z][a-zA-Z0-9_+:.-]*:' + _SPAN), @@ -301,7 +301,7 @@ class ResidualMarkupError(RuntimeError): """Conversion finished with reStructuredText still in the output. - Deliberately fatal. The six rules cover the subset the entries are written + Deliberately fatal. The conversion rules cover the subset the entries are written in; anything outside it would otherwise be copied through as literal text and render as itself in a release body, which is a silent defect in a published artefact. Failing here instead names the construct, and the line of the entry @@ -468,7 +468,7 @@ def extlink(name: str, text: str, def convert(rst: str) -> str: - """Apply the six rules to one per-version entry. + """Apply the conversion rules to one per-version entry. Args: rst: The entry's reStructuredText. @@ -481,7 +481,7 @@ def convert(rst: str) -> str: def convert_traced(rst: str) -> tuple[str, list[tuple[int, int]]]: - """Apply the six rules, and record where each piece of the output came from. + """Apply the conversion rules, and record where each piece of the output came from. What :func:`convert` returns, plus the bookkeeping :func:`residual` needs to report a location in *rst* rather than in its own much shorter output. Rules 1 @@ -619,9 +619,9 @@ def flush() -> None: def residual(markdown: str, sources: Optional[Sequence[tuple[int, int]]] = None) -> list[str]: - """Report reStructuredText left in *markdown* that the six rules did not convert. + """Report reStructuredText left in *markdown* that the conversion rules did not convert. - See the module docstring for the guarded set, for the two constructs left + See the module docstring for the guarded set, for the constructs left deliberately unguarded, and for why a complaint's line number belongs to the entry file rather than to *markdown*. diff --git a/util/ci_timing.py b/util/ci_timing.py index 5d22866f75..6fc5dc1bea 100644 --- a/util/ci_timing.py +++ b/util/ci_timing.py @@ -3,7 +3,7 @@ This is the script :file:`.github/workflows/ci-timing.yml` runs every Monday, the periodic review GitHub issue #1538 asked for. It reads the Actions API for the last -``--days`` days and writes three things: ``report.json`` (the next run's baseline), a +``--days`` days and writes ``report.json`` (the next run's baseline), a Markdown summary, and one ``::error::`` or ``::warning::`` annotation per threshold that fired. Exit status: 0 when no HARD threshold fired, 1 when one did, 2 when the API could not be read or the arguments are wrong, 3 for anything else. A HARD @@ -106,7 +106,7 @@ #: The aggregate job the ruleset requires; its completion is "mergeable". REQUIRED_JOB = 'Required checks passed' -#: The Python version tiers (#1660), which the two names below are read from. This +#: The Python version tiers (#1660), which the names below are read from. This #: script runs on ``latest`` (ci-timing.yml), so :mod:`tomllib`, 3.11+, is there. TIERS = os.path.join(os.path.dirname(os.path.dirname(os.path.abspath(__file__))), '.github', 'python-versions.toml') diff --git a/util/project_status.py b/util/project_status.py index c250e6a18d..668a086666 100644 --- a/util/project_status.py +++ b/util/project_status.py @@ -9,7 +9,7 @@ is the only place it is written in code -- the tests parse that table and hold the two to each other. -Two modes: +It runs in one of these modes: * ``--item N`` -- one issue or pull request, for an ``issues`` or ``pull_request_target`` event. Its labels and state are re-read through the API diff --git a/util/pyshark_encap_map.py b/util/pyshark_encap_map.py index 8aa4041134..d332d9fbcb 100644 --- a/util/pyshark_encap_map.py +++ b/util/pyshark_encap_map.py @@ -31,8 +31,8 @@ .layer_name`` and what :func:`pcapkit.toolkit.pyshark.tcp_traceflow` falls back to when a capture carries no ``frame.encap_type`` field at all. -Two traps, both costing real time when #850's tables were first produced --------------------------------------------------------------------------- +Traps that cost real time when #850's tables were first produced +---------------------------------------------------------------- **A PDML ``showname`` can hold a ``/``.** ``tshark`` renders ``frame.encap_type``'s ``showname`` as ``"Encapsulation type: NULL/Loopback @@ -666,7 +666,7 @@ def main(argv: 'Optional[Sequence[str]]' = None) -> int: # newline='\n' rather than the default None: every string this # script builds already ends its lines in '\n', so on Windows the # default would translate each into '\r\n' on the way out and - # rewrite all ~210 lines rather than the handful that actually + # rewrite every line rather than the few that actually # changed. .gitattributes' `* text=auto` keeps CRLF from reaching # a commit either way, so this is hardening against a noisy local # diff, not a fix for output this repository would ever ship. diff --git a/util/run_unittest_leg.py b/util/run_unittest_leg.py index be420546e5..e0147e9ed2 100644 --- a/util/run_unittest_leg.py +++ b/util/run_unittest_leg.py @@ -5,7 +5,7 @@ :mod:`tests.protocols.application.test_http_unit` and :mod:`tests.test_base_class_contract` -- each pass alone and :program:`pytest` reports the whole run green when they are collected together, but plain -:mod:`unittest` over the same two modules in one process fails -- four +:mod:`unittest` over the same two modules in one process fails some of their ``subTest`` cases. ``pytest-subtests`` is *not* why: that plugin is not installed in this project at all (absent from the ``test`` extra in ``pyproject.toml``), and plain :program:`pytest` (9.1.1 in this checkout) @@ -34,7 +34,7 @@ A cheaper alternative exists, and is recorded here rather than left undocumented: running ``pytest --noconftest`` over just those same two modules, in that order, disables the very same autouse fixture directly and -reproduces the identical four ``subTest`` failures in about 70s on the pre-fix +reproduces the identical ``subTest`` failures in about 70s on the pre-fix tree, under the exact :program:`pytest` version this CI already installs. Whether that single invocation would have been sufficient instead of this dedicated runner and its per-directory matrix was not evaluated when this @@ -50,14 +50,12 @@ :file:`.github/workflows/unit-tests.yml`'s ``unittest-ordering`` job calls once per entry of its own matrix -- the leg list lives in that workflow, not as a module-level constant here. :mod:`tests.protocols` is the largest leg; -its timings, on three different bases (a local serial run, and CI before and +its timings, on different bases (a local serial run, and CI before and after it was sharded), are tabulated in that workflow's own comment rather than restated here. At the other end sit -two legs of two unit-tier modules each, with no clean ordering between them -- -:mod:`tests.dumpkit` (199 tests, ~39s) and :mod:`tests.interface` (201 tests, -~37s), dumpkit carrying two fewer tests but running two seconds slower. -Neither small leg is :mod:`tests.const` (478 tests, ~268s), which is larger on -both axes. +:mod:`tests.dumpkit` and :mod:`tests.interface`, small legs with no clean +ordering between them by test count or by wall time (~39s and ~37s). Neither +small leg is :mod:`tests.const` (~268s), which is larger by both measures. Peak RSS was measured for the three cheapest legs only, via :func:`resource.getrusage` on ``RUSAGE_CHILDREN`` in this venv: @@ -84,11 +82,11 @@ GitHub issue #1029 -- sharding a leg: ``directory`` may be a deeper path such as ``protocols/internet``, and ``--exclude SUBDIR`` (repeatable) drops a subdirectory, so ``protocols --exclude protocols/internet`` is "the rest". The -workflow uses that pair to split :mod:`tests.protocols` into two cells. A leg +workflow uses that pair to shard :mod:`tests.protocols`. A leg invoked without ``--exclude`` runs every unit-tier module under its directory. The price is that modules in different shards do not share a process, so -pollution between them is invisible; the workflow comment records the measured -pair counts. +pollution between them is invisible; the workflow comment records which +pairs that loses. What this still does not catch: a defect running the *other* direction (a root module polluting a directory module), interference between two @@ -230,7 +228,7 @@ def leg_modules(directory: 'str', exclude: 'Sequence[str]' = ()) -> 'tuple[str, exclude: subdirectories of ``directory`` (also relative to :data:`TESTS_ROOT`) whose modules are left out, so that one shard can be "everything in ``directory`` except its sibling shards". - Empty by default, which is every leg but one. + Empty by default, as for every unsharded leg. Returns: Dotted module names, in path-sorted order. diff --git a/util/update_python_versions.py b/util/update_python_versions.py index 774ab4fa0a..64190c9e01 100644 --- a/util/update_python_versions.py +++ b/util/update_python_versions.py @@ -2,7 +2,7 @@ """Recompute the Python version tiers in :file:`.github/python-versions.toml` (#1660). :file:`.github/workflows/cron-python-versions.yml` runs this weekly. It reads -four sources: +these sources: ``release-cycle`` python.org's release cycle, ``https://peps.python.org/api/release-cycle.json``: @@ -66,7 +66,7 @@ does it retire a job whose matrix the new tiers leave empty: the proposal names the job as a manual step, for the owner to retire or remove first (#1747). -Three commands: +Its commands: ``update`` Recompute, print what changed, and with ``--write`` rewrite the files. @@ -397,7 +397,7 @@ def axes(spec: 'dict[str, Any]') -> 'tuple[str, ...]': def evaluate(expression: 'str', leg: 'dict[str, Any]') -> 'str': - """The three expression shapes job names use: ``matrix.K``, ``matrix.K && 'A' || 'B'``, ``matrix.K || 'A'``.""" + """The expression shapes job names use: ``matrix.K``, ``matrix.K && 'A' || 'B'``, ``matrix.K || 'A'``.""" match = re.fullmatch(r'matrix\.([\w-]+)', expression) if match: value = leg.get(match.group(1)) diff --git a/util/verdict_covers_checkout.py b/util/verdict_covers_checkout.py index 5c759e358d..94f4723191 100644 --- a/util/verdict_covers_checkout.py +++ b/util/verdict_covers_checkout.py @@ -20,9 +20,9 @@ lost by that: ``HEAD``'s own ``Unit Tests`` run raises the workflow again when it completes, bot commits included -- since #401 (bb900906b, 2026-09-15) they are pushed with a deploy key, not with ``GITHUB_TOKEN``, whose events "will not create -a new workflow run" (GitHub, "Triggering a workflow from a workflow"). All 21 bot -commits on ``main`` since have their own ``push`` run of ``Unit Tests``, -f4f94bcad's being 38325349953; the three before #401, 5d5d8af4d, 48bc11384 and +a new workflow run" (GitHub, "Triggering a workflow from a workflow"). Every bot +commit on ``main`` since has its own ``push`` run of ``Unit Tests``, +f4f94bcad's being 38325349953; the ones before #401, 5d5d8af4d, 48bc11384 and e4d0823db, have none. The second case only saves waiting for that run where it cannot change the answer. @@ -55,10 +55,10 @@ BOT_EMAIL = 'github-actions[bot]@users.noreply.github.com' #: Files a bot commit may touch and still be built under its parent's verdict. -#: These are what Conda Update ("Bumped build to N", 137 commits on ``main``) and -#: Create Release's ``tag`` job ("Reset conda build number", 9) commit, and no -#: test reads either. Vendor Update's "Bumped version to N" commits are left out -#: on purpose: all 123 rewrite ``pcapkit/``, and e58618bdf's Unit Tests run +#: These are what Conda Update ("Bumped build to N") and Create Release's +#: ``tag`` job ("Reset conda build number") commit, and no test reads either. +#: Vendor Update's "Bumped version to N" commits are left out on purpose: every +#: one of them rewrites ``pcapkit/``, and e58618bdf's Unit Tests run #: 36355214959 reported "12 failed" on each Python leg, where its parent #: 70fa92010's run 36354135989 passed every job. BOT_MANAGED_PATHS = frozenset({'conda/build', 'conda/requirements.txt'})