From 5da8d5e4af9c3645a48060c3ad3169244927ecb3 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Mon, 26 Oct 2026 15:26:56 -0400 Subject: [PATCH] docs(repo): seventh whole-repo pass over util, examples, workflows and root files (#1103) Full read of util/, examples/, workflow comments and root config. - util: changelog_md sdist scope, pyshark_encap_map Unassigned handling and lxml markers, pytest_shards interpreter, python_versions KTLO outputs, release_tags/vendor_test_gate/update_python_versions/superseded claims. - examples: options.py HIP/SMF_DPD/PCAP-NG notes, pcapng.py logging, dispatch.py, legacy.py, engine lists. - workflows: unittest-ordering cap comments, lint schedule, deploy-pages leftover comment, milestone duplicate-closed, status_for cited by name. - root: Makefile captures, pyproject test extra, .gitignore wording. - Change-relative wording and counts restated throughout. --- .github/workflows/coverage-comment.yml | 2 +- .github/workflows/create-release.yml | 3 +- .github/workflows/cron-conda.yml | 2 +- .github/workflows/cron-vendor.yml | 8 +- .github/workflows/deploy-pages.yml | 2 +- .github/workflows/lint.yml | 2 +- .github/workflows/milestone.yml | 6 +- .github/workflows/project-status.yml | 2 +- .github/workflows/unit-tests.yml | 11 +- .gitignore | 25 ++-- Makefile | 5 +- examples/benchmark/benchmark.py | 3 +- examples/benchmark/report.py | 2 +- examples/generators/dispatch.py | 17 ++- examples/generators/endian.py | 4 +- examples/generators/legacy.py | 15 ++- examples/generators/options.py | 159 ++++++++--------------- examples/generators/pcapng.py | 34 ++--- examples/legacy_smoke/README.md | 3 +- examples/legacy_smoke/_engine_support.py | 2 +- examples/legacy_smoke/test_perf.py | 3 +- pyproject.toml | 19 +-- util/apt_packages.py | 12 +- util/bump_version.py | 5 +- util/changelog_md.py | 14 +- util/ci_timing.py | 4 +- util/project_status.py | 2 +- util/pyshark_encap_map.py | 32 +++-- util/pytest_shards.py | 12 +- util/python_versions.py | 16 +-- util/release_tags.py | 3 +- util/run_unittest_leg.py | 9 +- util/superseded.py | 4 +- util/update_python_versions.py | 4 +- util/vendor_test_gate.py | 7 +- 35 files changed, 202 insertions(+), 251 deletions(-) diff --git a/.github/workflows/coverage-comment.yml b/.github/workflows/coverage-comment.yml index 9ac7268a4f..00215d0841 100644 --- a/.github/workflows/coverage-comment.yml +++ b/.github/workflows/coverage-comment.yml @@ -100,7 +100,7 @@ jobs: if tests not in ('success', 'failure', 'cancelled', 'skipped'): sys.exit(f'invalid test outcome in artifact: {tests!r}') total = number(data['total']) - # The leg's version, from the artifact; one written before #1660 has none. + # The leg's version, from the artifact; one without it reads as the coverage leg. python = data.get('python') if python is not None and not (isinstance(python, str) and re.fullmatch(r'3\.\d+', python)): sys.exit(f'invalid Python version in artifact: {python!r}') diff --git a/.github/workflows/create-release.yml b/.github/workflows/create-release.yml index 627be4dc33..534d205acb 100644 --- a/.github/workflows/create-release.yml +++ b/.github/workflows/create-release.yml @@ -752,8 +752,7 @@ jobs: - macos-latest - macos-26-intel - ubuntu-latest - # #2060: win-64, once Conda Update's leg had built and smoke-tested it - # green. Its `run:` steps name `bash`, as Windows runs `pwsh`. + # #2060: win-64. Its `run:` steps name `bash`, as Windows runs `pwsh`. - windows-latest # #1633, #1660: the supported tier only. No KTLO 3.10 leg: the list # util/conda_vendor.py derives omits typing-extensions, which diff --git a/.github/workflows/cron-conda.yml b/.github/workflows/cron-conda.yml index 7a67b34d70..47277330dd 100644 --- a/.github/workflows/cron-conda.yml +++ b/.github/workflows/cron-conda.yml @@ -551,7 +551,7 @@ jobs: - macos-26-intel - ubuntu-latest # #2060: win-64, uploaded with the anaconda-client CLI as the Intel - # leg is, once its runs had built and smoke-tested it green. + # leg is. - windows-latest # #1633: the supported tier, from .github/python-versions.toml through # `conda-update` (#1660); create-release.yml's ``conda`` job says why diff --git a/.github/workflows/cron-vendor.yml b/.github/workflows/cron-vendor.yml index a30f53e19d..4515a6953b 100644 --- a/.github/workflows/cron-vendor.yml +++ b/.github/workflows/cron-vendor.yml @@ -180,10 +180,10 @@ jobs: find pcapkit/const -mindepth 2 -name '*.py' -exec isort -l100 -ppcapkit {} + # #1853: both gates in `head` judge the commit from before the crawl, so - # 1.5.0b13 was committed with a test failing on it. The owner's ruling: - # test it here under #872's rule -- a module the tests fail with goes - # back to HEAD's copy, the rest are committed. util/vendor_test_gate.py's - # docstring gives the rule. + # they pass a crawl the tests fail on, as they did 1.5.0b13's. The + # owner's ruling: test it here under #872's rule -- a module the tests + # fail with goes back to HEAD's copy, the rest are committed. + # util/vendor_test_gate.py's docstring gives the rule. # # `continue-on-error`, so the tests never fail the job; if the step fails # or times out, the next one discards every regenerated module. `.[test]` diff --git a/.github/workflows/deploy-pages.yml b/.github/workflows/deploy-pages.yml index 6326187f66..f0a39e9f42 100644 --- a/.github/workflows/deploy-pages.yml +++ b/.github/workflows/deploy-pages.yml @@ -77,7 +77,7 @@ jobs: github.event.workflow_run.head_branch == 'main' && needs.head.outputs.verified == 'true' )) - concurrency: ci-${{ github.event_name == 'workflow_dispatch' && github.run_id || github.ref }} # Recommended if you intend to make multiple deployments in quick succession. + concurrency: ci-${{ github.event_name == 'workflow_dispatch' && github.run_id || github.ref }} runs-on: macos-latest steps: - name: Checkout 🛎️ diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 0adc9aa769..12bb6ea0da 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -30,7 +30,7 @@ on: pull_request: branches: [main] schedule: - # Saturdays, an hour clear of the other Saturday jobs. + # Saturdays, at an hour no other Saturday job uses. - cron: '0 6 * * 6' workflow_dispatch: diff --git a/.github/workflows/milestone.yml b/.github/workflows/milestone.yml index e881d824e5..0086daa3eb 100644 --- a/.github/workflows/milestone.yml +++ b/.github/workflows/milestone.yml @@ -7,15 +7,15 @@ name: Milestone # "Milestone" in docs/source/contributing/workflows.rst: an open item, a merged # pull request or an issue closed as completed gets the open milestone named # after the checked-out `pcapkit.__version__`, unless it already holds another; -# an unmerged pull request or an issue closed as not planned loses whichever -# milestone it holds. A missing milestone is a warning, never created. +# an unmerged pull request or an issue closed as not planned or as a duplicate +# loses whichever milestone it holds. A missing milestone is a warning, never created. # # SECURITY. `pull_request_target` gives this run a write token on events a # stranger can cause, so it must never execute anything the pull request # supplies. The job below: # # * checks out `github.sha`, which under `pull_request_target` is the tip of -# the *base* branch, never the PR head, and only the three files it reads, +# the *base* branch, never the PR head, and only the files it reads, # without persisting the token; # * reads `__version__` from pcapkit/__init__.py as text, importing nothing; # * takes nothing from the event payload but the item number, passed through diff --git a/.github/workflows/project-status.yml b/.github/workflows/project-status.yml index 56252e8330..a7396a3708 100644 --- a/.github/workflows/project-status.yml +++ b/.github/workflows/project-status.yml @@ -2,7 +2,7 @@ name: Project Status # Keeps the project board's Status field (users/JarryShaw/projects/2) in line with # each item's state labels, which no built-in board workflow observes. The mapping -# is util/project_status.py:status_for; it and the PROJECT_TOKEN setup are +# is `status_for` in util/project_status.py; it and the PROJECT_TOKEN setup are # documented under "Milestones and the Project Board" in # docs/source/contributing/conventions/process.rst. # diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 87ef9df095..745e2ba583 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -123,7 +123,7 @@ jobs: cache: pip # DPKT (#729), crypto and NGAP (#738) are here so the unit-tier tests - # gated on them run instead of skipping; all three install cleanly. On + # gated on them run instead of skipping; each installs cleanly. On # PyPy 8.0 PyPI has no Linux wheel of cryptography, so pip builds it with # the image's Rust and libssl-dev (#1958). `cli` (emoji) is not here: its # only gated tests are in tests/integration, which this job ignores. @@ -1166,8 +1166,8 @@ jobs: runs-on: ubuntu-latest # 45, as `test`'s: this is the only test job with no xdist, so a cell # carries its whole serial cost, and a contended run once passed 20 - # minutes. The slowest cell, with #1029's split, is 8.2m; even #1029's slowdown - # window, at least 1.6x, leaves it far inside the cap. + # minutes. Each cell is held well inside it by its run step's own cap, + # which ci-timing.yml's weekly report watches (#2254). timeout-minutes: 45 strategy: fail-fast: false @@ -1224,8 +1224,9 @@ jobs: python -c "import os; print('cpu_count', os.cpu_count())" # `--verbose` names every test as it runs, so a hung cell shows where it - # was. The step cap is about twice the slowest cell's measured maximum, - # under the job's, so a hang reports as this step timing out. + # was. The step cap sits under the job's, so a hang reports as this step + # timing out. util/ci_timing.py flags a cell whose p90 reaches half of + # it, and such a cell is split, as `project` is (#2254). - name: Run tests/${{ matrix.leg }} and the root-level modules under plain unittest timeout-minutes: 20 run: python util/cap_file_size.py python util/run_unittest_leg.py ${{ matrix.leg }} ${{ matrix.exclude }} ${{ matrix.shard }} --verbose diff --git a/.gitignore b/.gitignore index 7ff2aba208..ec6e205630 100644 --- a/.gitignore +++ b/.gitignore @@ -6,20 +6,19 @@ pcapkit-temp.html # generated captures at examples/sample/, and nothing should track them. examples/sample/ examples/captures/* -# Only the two genuine *inputs* are tracked. Everything else in this directory is -# build output: the captures `examples/generators/make_samples.py` writes, and the -# reference reports `examples/legacy_smoke/` renders out of these two inputs -- -# out.json, out.plist, out.txt and pcapng.txt, via `make fixtures` there. +# Only the genuine *inputs*, re-included below, are tracked. Everything else in +# this directory is build output: the captures `examples/generators/make_samples.py` +# writes, and the reference reports `examples/legacy_smoke/` renders out of those +# inputs -- out.json, out.plist, out.txt and pcapng.txt, via `make fixtures` there. # -# Those four used to be tracked, and the reason they no longer are is that a -# tracked report goes stale silently. Nothing reads one back, so nothing notices -# when a parser change alters how a frame renders: #685 was exactly that, with -# pcapng.txt still recording `packet -> NIL` for four blocks long after #683 gave -# PCAP-NG packet blocks their captured octets back. Regenerating them on every such -# change is work with no reader, and the tier guard in `tests/_tiers.py` asks git -# which captures are tracked rather than hardcoding a list, so dropping them here -# is the whole change -- see `examples/legacy_smoke/README.md` for how to rebuild -# them locally. +# The reports are not tracked because a tracked report goes stale silently. +# Nothing reads one back, so nothing notices when a parser change alters how a +# frame renders: #685 was exactly that, with pcapng.txt still recording +# `packet -> NIL` for four blocks long after #683 gave PCAP-NG packet blocks their +# captured octets back. Regenerating them on every such change is work with no +# reader, and the tier guard in `tests/_tiers.py` asks git which captures are +# tracked rather than hardcoding a list, so ignoring them here is all it takes -- +# see `examples/legacy_smoke/README.md` for how to rebuild them locally. !examples/captures/in.pcap !examples/captures/dhcp.pcapng diff --git a/Makefile b/Makefile index 38d4c24260..ccc969c8e1 100644 --- a/Makefile +++ b/Makefile @@ -77,8 +77,9 @@ pipenv: vendor: pipenv run pcapkit-vendor -# Sample captures under examples/captures/ are not tracked (see .gitignore); -# regenerate the ones the runtime, regression and integration tests read. +# Sample captures under examples/captures/, but for the inputs .gitignore +# lists, are not tracked; regenerate the ones the runtime, regression and +# integration tests read. samples: pipenv run python examples/generators/make_samples.py diff --git a/examples/benchmark/benchmark.py b/examples/benchmark/benchmark.py index 18a1dbc70a..ba28f9b866 100644 --- a/examples/benchmark/benchmark.py +++ b/examples/benchmark/benchmark.py @@ -67,7 +67,8 @@ __all__ = ['ENGINES', 'ROUNDS', 'REPEATS', 'expected_drivers', 'preflight', 'measure', 'run'] #: Every engine ``pcapkit.extract(engine=...)`` accepts, in the order the suite -#: exercises them. Matches :data:`examples.legacy_smoke._engine_support.ENGINES`. +#: exercises them. The same engines as +#: :data:`examples.legacy_smoke._engine_support.ENGINES`, in a different order. #: ``'default'`` is ``pcapkit``'s own parser and is the cross-environment #: baseline, so it is always measured first and never dropped from the list. ENGINES = ('default', 'dpkt', 'scapy', 'pypcap', 'pcap_ct', 'pypcapfile', 'pyshark') diff --git a/examples/benchmark/report.py b/examples/benchmark/report.py index fe599dd6b2..5cac954385 100644 --- a/examples/benchmark/report.py +++ b/examples/benchmark/report.py @@ -1083,7 +1083,7 @@ def render_versions_rst(rows: 'Sequence[Row]', documents: 'Sequence[dict[str, An lines.extend(_simple_table(headers, body, numeric=True)) lines.append('') - # The owner's own sentence about the hand-maintained table, kept word for word: + # The sentence docs/source/engines.rst carries under its table, kept word for word: # it is the right warning, and a regenerated table that rewords it would show up as # a diff in the prose every time the numbers changed. It sits between the table and # the footnotes, where docs/source/engines.rst has it. diff --git a/examples/generators/dispatch.py b/examples/generators/dispatch.py index b0eed41578..805229ca80 100644 --- a/examples/generators/dispatch.py +++ b/examples/generators/dispatch.py @@ -9,10 +9,10 @@ *resolves* to the right class object, but that is a weaker property than the one that actually matters -- it is satisfied by a table whose target could not parse a packet if it tried. -:file:`tests/protocols/test_dispatch_bindings_unit.py` exists because exactly -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 +:file:`tests/protocols/test_dispatch_bindings_unit.py` asserts on the parsed +protocol chain for exactly that reason: as its own docstring records, a +protocol can be reachable from no table at all, and unable to parse a packet +if it were. But that module hand-picks its cases rather than enumerating, so it guards the entries someone remembered rather than the registries themselves. See GitHub issue #496. @@ -59,7 +59,7 @@ a minimal dispatch probe, so both use the same placeholder payload :meth:`~tests.protocols.transport.test_sctp_unit.SCTPUnitTests.test_ppid_dispatch_hook` already does, and for the same documented reason -- one PPID is DTLS-wrapped, -which pcapkit implements no DTLS for, and the other degrades identically +and pcapkit implements no DTLS; the other degrades identically because the payload is not an aligned PER encoding, exactly as that test's own docstring records. @@ -136,9 +136,8 @@ def code_name(code: 'Any', enum: 'Optional[Any]' = None) -> 'str': """Render a registry key as a name usable in a test id. Most ``__proto__`` tables are keyed entirely by enumeration members, whose - own ``.name`` is used directly. ``Link.__proto__`` and - ``Internet.__proto__`` each hold an entry keyed by a bare :class:`int` - instead (0x8137 for IPX, in ``Link``'s case) -- :func:`_resolve` does not + own ``.name`` is used directly. ``Link.__proto__`` holds an entry keyed by + a bare :class:`int` instead (0x8137 for IPX) -- :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 name. ``enum`` is consulted for that one, so the label reads @@ -188,7 +187,7 @@ def _pcapng_bytes(linktype: 'Any', packet_data: 'bytes') -> 'bytes': Built through :mod:`pcapkit`'s own :class:`~pcapkit.protocols.misc.pcapng.PCAPNG` construction API rather than hand-rolled :mod:`struct` packing, the same way :func:`examples.generators.options._pcapng_context` does -- GitHub issue - #496 judged hand-building a valid SHB+IDB+EPB stream by hand riskier than + #496 judged hand-building a valid SHB+IDB+EPB stream riskier than the check was worth, and this sidesteps that by reusing the library's own ``make`` path, which is exactly what is under test one layer further down anyway. diff --git a/examples/generators/endian.py b/examples/generators/endian.py index 4529a41775..336ad9872c 100644 --- a/examples/generators/endian.py +++ b/examples/generators/endian.py @@ -18,8 +18,8 @@ swallows the rest of the file and the read that follows is handed a negative payload length, raising ``ValueError: read length must be non-negative or -1`` out of -:file:`pcapkit/protocols/schema/schema.py`. The fix is one character, and -nothing but these fixtures can tell whether it works. +:file:`pcapkit/protocols/schema/schema.py`. The defect is one character, and +nothing but these fixtures can tell whether it is present. =============================== ========================================== Fixture Exercises diff --git a/examples/generators/legacy.py b/examples/generators/legacy.py index 03560f7eb1..2cea99493c 100644 --- a/examples/generators/legacy.py +++ b/examples/generators/legacy.py @@ -4,8 +4,9 @@ The demonstration scripts under ``examples/legacy_smoke/`` predate the test suite and read their captures out of ``examples/captures/`` by relative path. Most of them read fixtures the sibling generators in this directory already -write, but two read captures that exist nowhere in the repository, so those two -scripts cannot run at all on a fresh checkout. This module writes them, with +write, but ``test_reassembly.py`` and ``test_analyse.py`` read captures that +exist nowhere in the repository, so those scripts cannot run at all on a fresh +checkout. This module writes them, with :mod:`scapy`, deterministically and without network access. ``test.pcap`` @@ -66,7 +67,7 @@ 4. The payload of a complete datagram goes to :meth:`pcapkit.protocols.transport.transport.Transport.analyze`, which picks the application protocol from the two port numbers. Port 80 is what - maps to HTTP/1.*, so every connection in both fixtures runs on port 80. + maps to HTTP, so every connection in both fixtures runs on port 80. One absence is deliberate and worth flagging, because it looks like an oversight and is not: neither fixture contains a datagram that reassembles @@ -79,10 +80,10 @@ RST that submits the buffer. ``submit`` converts each hole's absolute bounds -- TCP sequence numbers, -``first=tcp_info.seq`` in ``pcapkit/toolkit/pcap.py`` -- into offsets into the -buffer it is reading, which is indexed from the start of the direction's data, -and decides completeness from that payload buffer's own gap list (GitHub issue -#349). Were it to slice with the absolute bounds instead, every slice would +``first=segment.seq`` in ``tcp_reassembly`` (``pcapkit/toolkit/pcap.py``) -- +into offsets into the buffer it is reading, which is indexed from the start of +the direction's data, and decides completeness from that payload buffer's own +gap list (GitHub issue #349). Were it to slice with the absolute bounds instead, every slice would start far beyond the end of the buffer for any real initial sequence number, every fragment would come out empty, and ``if data:`` would discard the datagram without a word, so no capture could reach the incomplete branch at diff --git a/examples/generators/options.py b/examples/generators/options.py index 768172f8f8..2c6a7dc402 100644 --- a/examples/generators/options.py +++ b/examples/generators/options.py @@ -95,10 +95,10 @@ orderings fighting. :func:`roundtrip` puts a :func:`signal.alarm` deadline around each case. That -is not defensive dressing: GitHub issue #431 is a parser defect that -degenerates into a loop making no progress, ``HOPOPT``'s ``SMF_DPD`` reaches it, -and without a deadline ``make samples`` does not fail -- it *hangs*, taking the -whole build with it. A signal is what interrupts it, because the loop is pure +is not defensive dressing: it guards against a parser defect that degenerates +into a loop making no progress, as GitHub issue #431 did on ``HOPOPT``'s +``SMF_DPD``. Without a deadline ``make samples`` does not fail on such a case -- +it *hangs*, taking the whole build with it. A signal is what interrupts it, because the loop is pure Python and holds the GIL for the whole of an iteration, so no watchdog thread would ever get to run. The deadline is wall-clock time, so a stalled runner can trip it on a case that normally takes milliseconds (#1428): @@ -239,10 +239,11 @@ class Outcome(NamedTuple): #: Statuses whose octets may go into a capture, i.e. the ones whose *parse* #: step ran to completion. #: -#: This is a safety property, not a tidiness one. ``HOPOPT``'s ``SMF_DPD`` -#: constructs perfectly well and then spins forever on the way back in, so its -#: octets are available and putting them in a fixture would produce a capture -#: that wedges every reader of it -- including the rest of this suite. A +#: This is a safety property, not a tidiness one. A case that constructs +#: perfectly well and then spins forever on the way back in, as ``HOPOPT``'s +#: ``SMF_DPD`` did under GitHub issue #431, has octets available, and putting +#: them in a fixture would produce a capture that wedges every reader of it -- +#: including the rest of this suite. A #: ``CONSTRUCT`` or ``PARSE`` failure is milder but no more useful: the frame #: would be one no reader can get through. So a capture carries only frames #: that are known to parse, and the cases that do not are recorded in @@ -419,10 +420,10 @@ def _named_registry(owner: 'type', enum: 'Any', prefix: 'str', #: Every key here has to be spelled the way :meth:`TCP.make #: ` declares it: an undeclared #: keyword raises :exc:`~pcapkit.utilities.exceptions.UnsupportedCall` (#617). -#: Without that check, ``make``'s ``**kwargs`` drops it silently, and GitHub -#: issue #602 is this table's keys spelled wrong and so ignored: +#: Without that check, ``make``'s ``**kwargs`` drops it silently, so a key +#: spelled wrong in this table is ignored (GitHub issue #602): #: -#: * ``'seq'`` for ``seq_no``. The mapping reads as sequence number 1 and +#: * ``'seq'`` for ``seq_no``. The mapping reads as sequence number 1 while #: every generated frame carries 0, so the captures are not the packets this #: table describes. #: * ``'urgent_pointer'`` for ``urgent``. Harmless in effect, since the value @@ -882,8 +883,8 @@ def _mh_option_overrides() -> 'dict[Any, dict[str, Any]]': LMAControlledMAGSuboption as Enum_LMAControlledMAGSuboption from pcapkit.const.mh.option import Option as Enum_Option return { - # ``{}`` makes ``_make_opt_pad`` warn and silently emit a Pad1, so the - # PadN case would otherwise be a duplicate of the Pad1 one. + # ``{}`` gives a valid zero-length PadN [RFC 6275 section 6.2.3]; a + # non-zero length exercises the padding octets as well. Enum_Option.PadN: {'length': 4}, # The authenticator has to be a multiple of 8 octets. Enum_Option.Authorization_Data: {'data': b'\xa5' * 8}, @@ -1004,74 +1005,25 @@ def _mh_extension_build(code: 'Any', kwargs: 'dict[str, Any]') -> 'Any': #: How many copies of the parameter under test go in one packet. #: -#: One (#689). What follows records why a pair is not needed, and what a pair -#: would hide. +#: One (#689), because a pair hides the length defects a single parameter +#: exposes. ``HIP.make`` computes the header's ``len`` field as +#: ``total_length // 8 + 4``, which is lossless only when the parameter octets +#: are a multiple of eight, and each parameter is padded to :rfc:`7401` +#: Section 5.2.1's ``Total Length = 11 + Length - (Length + 3) % 8`` so that it +#: is one on its own (#651). A record that is ``4 (mod 8)`` -- from padding that +#: aligns only the contents and ignores the four-octet type-and-length header +#: (#651), or from a packed length that disagrees with the declared ``Length`` +#: (#672, #679) -- makes the floor division drop four octets, and +#: ``_read_hip_param``, which compares the recovered length exactly, rejects the +#: packet. Two such records sum to a multiple of eight, so a pair makes the +#: arithmetic come out exact and the defect invisible. #: -#: A pair hides the defect #651 fixed. ``HIP.make`` computes the -#: header's ``len`` field as ``total_length // 8 + 4``, which is lossless only -#: when the parameter octets are a multiple of eight; and every padding site in -#: the two HIP modules aligned the *contents* to eight, ignoring the four-octet -#: type-and-length header, so a single parameter was always ``4 (mod 8)``, the -#: floor division always dropped those four octets, and ``_read_hip_param`` -#: -- which compares the recovered length exactly -- rejected the library's own -#: single-parameter packets. Two copies summed to a multiple of eight, so the -#: arithmetic came out exact and the parameter constructors became reachable. -#: #651 made the padding :rfc:`7401` Section 5.2.1's -#: ``Total Length = 11 + Length - (Length + 3) % 8``, so each parameter is a -#: multiple of eight on its own and the pair stopped being needed for that -- -#: measured at the time as 45 OK of 49 at one copy against 46 at two. -#: -#: What kept the pair afterwards was a short list of codes whose own packed -#: length disagreed with the ``len`` they declared, so their record was not -#: 8-aligned however the padding was computed, and a pair cancelled that -#: misalignment the same way it cancelled #651's padding defect. #672 and #679 -#: closed two of those -- ``R1_COUNTER`` (129), whose 8-octet :rfc:`7401` -#: Section 5.2.3 counter had packed as four, and ``LOCATOR_SET`` (193), whose -#: ``Length`` had been counted in 4-octet units where the RFC counts bytes -- -#: which is what moved the one-copy figure from 44 to 46, level with two copies. -#: -#: Two remained after that, neither helped by a second copy since each failed at -#: two copies as well as at one. Both are closed: -#: -#: * ``HOST_ID`` declared four octets for the single 16-bit DI-Type/DI-Length -#: word of :rfc:`7401` Section 5.2.9, so it packed two surplus octets -- 18 at -#: this table's key, where ``len=8`` makes the record 16 -- and read a -#: conformant record two octets late, taking the curve as ``Algorithm``. #1118 -#: made the field two octets. -#: * ``HIP_TRANSFORM`` is HIPv1-only -- ``_read_param_hip_transform`` raises for -#: any other version -- and is built at version 1 through -#: :data:`HIP_VERSION`, so it round-trips and needs no entry. -#: -#: A third gap here, ``R1_Counter`` (128), was closed separately by #690 and was -#: never one of the pair's cancellations above: it parsed as an -#: ``UnassignedParameter`` because the *schema* registry is keyed on the -#: ``code=`` of the class statement and ``R1CounterParameter`` declared only -#: 129 -- even though ``__parameter__``'s ``_read_param_*``/``_make_param_*`` -#: entries already existed for both codes, as two hand-written dict entries, -#: not because of any name-normalisation rule. #690 registered -#: ``R1CounterParameter`` for 128 too, so it needs no entry. -#: -#: So the pair was routing around nothing by the time #672, #679 and #690 had -#: all landed, and dropping to one copy does not change what round-trips. -#: Measured over this table's 49 HIP codes on ``5f0a1aa90`` (after #696, which -#: #689 itself waited on -- see the issue), again after #690, and again once -#: #1128 and #1118 had landed: -#: -#: ====================== ======== ========== -#: tree one copy two copies -#: ====================== ======== ========== -#: ``5f0a1aa90`` 46 OK 46 OK -#: after #690 47 OK 47 OK -#: after #1128 and #1118 49 OK 49 OK -#: ====================== ======== ========== -#: -#: ``HOST_ID`` and ``HIP_TRANSFORM`` were the only two cases either setting -#: failed at those measurements, and neither their ``status`` nor their -#: ``defect`` moved between settings, so -#: :data:`tests.protocols.test_option_roundtrip_unit.EXPECTED_FAILURES` needed no -#: change to keep recording them accurately at one copy. #1128 and #1118 closed -#: them, which takes both columns to all 49 and leaves that table with no HIP -#: entries at all. +#: ``HIP_TRANSFORM`` is HIPv1-only -- ``_read_param_hip_transform`` raises for +#: any other version -- and is built at version 1 through :data:`HIP_VERSION`. +#: ``R1_Counter`` (128) parses through ``R1CounterParameter``, which the schema +#: registry holds for both counter codes (#690). Every HIP code round-trips at +#: one copy, so :data:`tests.protocols.test_option_roundtrip_unit.EXPECTED_FAILURES` +#: has no HIP entries. #: #: The single-parameter case is asserted directly, and positively, by #: ``test_a_hip_packet_carrying_one_parameter_round_trips`` in @@ -1097,20 +1049,14 @@ def _hip_overrides() -> 'dict[Any, dict[str, Any]]': Parameter.HOST_ID: {'hi_curve': ECDSACurve.NIST_P_256}, # These two are not here because the default fails -- ``counter=0`` # constructs and parses perfectly well. They are here because a - # zero-valued field cannot discriminate a width defect from a correct - # one, which is how this parameter's width defect (#672) stayed - # hidden. The RFC-only walk over - # ``options-internet.pcap`` -- which takes its stride solely from - # :rfc:`7401` Section 5.2.1's ``Total Length = 11 + Length - (Length + - # 3) % 8`` -- read ``Length = 12``, advanced 16 over a 12-octet record, - # landed four octets inside the second copy, and found a phantom - # ``Type = 0, Length = 0`` record there whose "padding" was four zero - # octets. Every octet it mis-read was zero, so its zero-padding check - # passed and it reported no violation. Measured: patching one counter to - # ``aabbccdd`` turned that silence into ``type 0 padding not zeroed: - # aabbccdd``. With a non-zero counter the fixture cannot conceal a - # mis-stride here, whether or not the width is right -- which is the - # reason to keep this override, #672 having corrected the width. + # zero-valued field cannot discriminate a width defect (#672) from a + # correct width. An RFC-only walk over ``options-internet.pcap``, which + # takes its stride solely from :rfc:`7401` Section 5.2.1's ``Total + # Length = 11 + Length - (Length + 3) % 8``, mis-strides over a counter + # packed at the wrong width; where every octet it mis-reads is zero, its + # zero-padding check passes and it reports no violation. With a non-zero + # counter the fixture cannot conceal a mis-stride here, whether or not + # the width is right. Parameter.R1_COUNTER: {'counter': 0xaabbccdd}, Parameter.R1_Counter: {'counter': 0xaabbccdd}, } @@ -1225,13 +1171,13 @@ def _httpv2_rebuild(frame: 'Any') -> 'Any': # # 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 +# uninteresting -- it is that the construction API's defaults do not produce a # *reproducible* one. ``_make_block_shb`` takes the section's byte order from -# :data:`sys.byteorder` with no way to override it, and the TLS and WireGuard -# key-log writers stamp :meth:`datetime.datetime.now` into the block body with -# no keyword to pin it. A fixture built this way would differ between a -# little-endian and a big-endian host, and the two key-log types would differ -# between one run and the next. +# :data:`sys.byteorder` unless given ``byteorder``, and the TLS and WireGuard +# key-log writers stamp the running pcapkit version into the block body. A +# fixture built this way would differ between a little-endian and a big-endian +# host, and the two key-log types would differ between one release and the +# next. # # The PCAP-NG option space is not left uncovered by that decision: # :file:`examples/generators/pcapng.py` already builds ``profile.pcapng``, @@ -1258,9 +1204,8 @@ def _httpv2_rebuild(frame: 'Any') -> 'Any': #: 6 octets of destination, 6 of source, the EtherType, then the body. PCAPNG_PACKET = bytes.fromhex('02000000000102000000000288b50000') -#: A systemd journal export entry whose length is a multiple of four. -#: ``_make_block_systemd`` adds no padding, unlike every other block, so an -#: unaligned entry makes the block length invalid. +#: A systemd journal export entry whose length is a multiple of four, so that +#: ``_make_block_systemd`` has no padding to add to it. PCAPNG_JOURNAL = (b'__REALTIME_TIMESTAMP=1500000000000000\n_TRANSPORT=journal\n' b'PRIORITY=6\nMESSAGE=probe.....\n\n') @@ -1375,8 +1320,8 @@ def _pcapng_block_overrides() -> 'dict[Any, dict[str, Any]]': 'records': [(Enum_RecordType.nrb_record_end, {})]}, Enum_BlockType.Interface_Statistics_Block: {'timestamp': PCAPNG_TIME}, Enum_BlockType.systemd_Journal_Export_Block: {'entries': PCAPNG_JOURNAL}, - # ZigBee rather than the default TLS key log, whose writer stamps the - # current time into the body and so can never round-trip. + # ZigBee rather than the default TLS key log: the default empty + # ``secrets_data`` does not read back as a key log. Enum_BlockType.Decryption_Secrets_Block: { 'secrets_type': Enum_SecretsType.ZigBee_NWK_Key, 'secrets_data': {'nwk_key': bytes(range(16)), 'pan_id': 0x1234}}, @@ -1650,8 +1595,8 @@ def _pcapng_secrets_build(code: 'Any', kwargs: 'dict[str, Any]') -> 'Any': _hip_build, _hip_parse, _hip_extract, _hip_rebuild, capture='options-internet.pcap', envelope='ipv6', proto=139), # 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. + # above ``PCAPNG_TIME`` for why the construction API's defaults do not + # make a reproducible one, and what already covers that space instead. Family('pcapng-block', _pcapng_block_registry, _pcapng_block_overrides, _pcapng_block_build, _pcapng_parse, _pcapng_block_extract, _pcapng_block_rebuild, capture=None, envelope=None), diff --git a/examples/generators/pcapng.py b/examples/generators/pcapng.py index d36aa9f6e5..61a88fa3c6 100644 --- a/examples/generators/pcapng.py +++ b/examples/generators/pcapng.py @@ -132,10 +132,11 @@ identically at both ends, and that the block chain covers the file exactly. So the complaints above are pcapkit's readings, not malformed fixtures. -One complaint is expected rather than a defect: ``test.pcapng`` carries a -deliberately truncated packet (``captured_len`` below ``original_len``, to -exercise snapshot-length handling) and dissecting it reports ``packet length < -0: -2``, which is simply what a snapped frame looks like to a dissector. +The complaints about one packet are expected rather than a defect: +``test.pcapng`` carries a deliberately truncated packet (``captured_len`` below +``original_len``, to exercise snapshot-length handling), and dissecting it +reports ``packet length < 0: -2`` and an IPv4 header that runs past the end of +the data, which is simply what a snapped frame looks like to a dissector. Usage ----- @@ -422,8 +423,8 @@ def _journal_entry(message: 'str', realtime_us: 'int') -> 'bytes': unaligned entry's own NUL padding is read as a binary field name and raises a bare ``struct.error``. Measured on ``ef859f776``: a misaligned entry parses with no ``SchemaWarning``. The loop stays only because - removing it regenerates ``test.pcapng`` -- an untracked fixture change, - not a prose one; see `#791 + removing it would change the bytes of the generated ``test.pcapng``; see + `#791 `__. """ @@ -455,10 +456,9 @@ def _timestamp_options(writer: '_Blocks', """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 a line per missing option per packet. Declaring them explicitly keeps the fixtures - quiet as well as complete. + owning interface for each packet block, and falls back to the format's + default for each one that is absent. Declaring them explicitly keeps the + fixtures complete rather than reliant on those defaults. Args: writer: Writer whose byte order the option values are packed in. @@ -826,7 +826,7 @@ def _transient(exc: 'OSError') -> 'bool': Only HTTP 429, an HTTP 5xx, and a timeout are. Any other HTTP error is the server's final answer, and an unreachable host fails at once rather than - after a wait, which keeps an offline run as quick as it was. + after a wait, which keeps an offline run quick. :mod:`requests` raises a :exc:`~requests.Timeout` for a timeout while it connects or waits for the headers, but a timeout while the body is read @@ -977,12 +977,12 @@ def _materialise(fixture: '_Fixture', path: 'pathlib.Path') -> 'tuple[bytes, str class _LogCapture(logging.Handler): """Collect pcapkit's own log records rather than letting them print. - pcapkit reports a parse complaint twice -- once through :mod:`warnings` and - once through its ``pcapkit`` logger -- but only the logger call is reliable: - it passes a computed ``stacklevel`` to :func:`warnings.warn` that can point - 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 + pcapkit reports a parse complaint on its ``pcapkit`` logger, and a + complaint raised as a warning goes through :mod:`warnings` as well, where + the application's filters decide its fate. Only the logger sees all of + them: :func:`~pcapkit.corekit.decorators.beholder` logs a dissector + failure it recovers from without issuing a warning at all. So the logger + is what this listens to. Duplicates are dropped, because one complaint per block in a capture with many packets is not that many pieces of information. """ diff --git a/examples/legacy_smoke/README.md b/examples/legacy_smoke/README.md index 6c3dea15e2..64f4eb3dfe 100644 --- a/examples/legacy_smoke/README.md +++ b/examples/legacy_smoke/README.md @@ -56,7 +56,8 @@ to work out whether an engine can run on this host. 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 + `pyshark` engines, and test_engine and test_time the `pypcap`, `pcap_ct` and + `pypcapfile` engines too. 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`. Worth knowing: diff --git a/examples/legacy_smoke/_engine_support.py b/examples/legacy_smoke/_engine_support.py index 94fbb4d7af..aefe13885c 100644 --- a/examples/legacy_smoke/_engine_support.py +++ b/examples/legacy_smoke/_engine_support.py @@ -6,7 +6,7 @@ ``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 +loop. Creating one implicitly there was deprecated in Python 3.12, and 3.14 no longer does it, raising ``RuntimeError: There is no current event loop in thread 'MainThread'`` instead -- so on 3.14 ``pyshark`` cannot be used at all, whatever else is installed. diff --git a/examples/legacy_smoke/test_perf.py b/examples/legacy_smoke/test_perf.py index 06ff5d061e..1d6bd1f63e 100644 --- a/examples/legacy_smoke/test_perf.py +++ b/examples/legacy_smoke/test_perf.py @@ -54,7 +54,8 @@ def pyshark() -> 'None': format='tree', engine='pyshark') -#: The benchmark for each engine name in :data:`_engine_support.ENGINES`. +#: The benchmark for each engine name this demo covers, a subset of +#: :data:`_engine_support.ENGINES`. BENCHMARKS = { 'default': default, 'pyshark': pyshark, diff --git a/pyproject.toml b/pyproject.toml index b7f60a9e0e..89d4a4740a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -300,15 +300,16 @@ 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 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 runs in no - # ``unit-tests.yml`` job: every one's install line builds on ``.[test]``. - # ``cron-vendor.yml`` does run it, but needs nothing from here to: its job - # installs ``.[all,vendor]`` before ``.[test]``, and runs ``tests/vendor`` - # only when a crawl changed a constant module, by which point - # ``pcapkit-vendor`` has rewritten the IPX constant file from that same table - # (#1853). + # Every ``unit-tests.yml`` install line builds on ``.[test]``, and only the + # whole-suite set on ``latest`` adds ``vendor`` (#1958). Without these, the + # ``ALL``-tier tests that import ``pcapkit.vendor`` behind a ``requests`` + # gate would skip on every other leg rather than run: the vendor-template + # checks in ``tests/const/test_const_enum_get.py``, + # ``tests/const/test_const_enum_lookup.py`` and + # ``tests/const/test_const_enum_builtin_parity.py``, and + # ``tests/corekit/test_enum_get_exception_provenance_923_unit.py``, among + # them. ``cron-vendor.yml``'s test gate needs nothing from here: its job + # installs ``.[all,vendor]`` before ``.[test]`` (#1853). # # Both are needed, not just ``requests``. ``pcapkit.vendor.default`` imports # ``requests`` at module scope, and ``pcapkit.vendor.__init__`` then imports diff --git a/util/apt_packages.py b/util/apt_packages.py index 3366364379..3e0563488d 100644 --- a/util/apt_packages.py +++ b/util/apt_packages.py @@ -14,8 +14,8 @@ ``APT_PACKAGES``, the job's environment, names the packages, separated by whitespace, for both ``plan`` and ``install``, so the cache key cannot describe a -set other than the one installed. Each subcommand runs the commands the steps' -shell ran, ``sudo`` included, and writes the same lines and annotations: +set other than the one installed. Each subcommand runs these commands, ``sudo`` +included, and writes these lines and annotations: ``plan`` ``sudo timeout 60 apt-get update``, up to twice, with a ``::warning`` titled by @@ -43,12 +43,12 @@ installed at its version, as ``dpkg-deb -f`` and ``dpkg-query -W`` report them, and says how many it holds. A failed ``dpkg-deb`` ends the script with its status. -The bounds stay GNU's ``timeout(1)`` run under ``sudo``, as in the shell, rather than -a :mod:`subprocess` timeout: apt runs as root, so this process could signal +The bounds are GNU's ``timeout(1)`` run under ``sudo``, rather than a +:mod:`subprocess` timeout: apt runs as root, so this process could signal ``sudo`` alone and not apt's own children, its download methods, which ``timeout``, running as root in a process group of its own, ends with apt. It is given no option, so uutils' ``timeout`` on Ubuntu 26.04 behaves as GNU's (#2020). The -step's ``timeout-minutes`` stays the hang detector. The script is standard library +step's ``timeout-minutes`` is the hang detector. The script is standard library only, and runs on every interpreter the matrices hold. """ @@ -99,7 +99,7 @@ def call(command: 'Sequence[str]', **kwargs: 'object') -> 'subprocess.CompletedP def packages() -> 'list[str]': - """``APT_PACKAGES``, split on whitespace as the shell split it.""" + """``APT_PACKAGES``, split on whitespace as an unquoted shell expansion splits it.""" return os.environ['APT_PACKAGES'].split() diff --git a/util/bump_version.py b/util/bump_version.py index 685634039f..5f9c3d70f6 100644 --- a/util/bump_version.py +++ b/util/bump_version.py @@ -49,9 +49,8 @@ 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 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. +which never run this script at all. Measured at ``a41137716`` (#625), those were +about a quarter of the commits that had moved ``__version__`` on ``main``. 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 diff --git a/util/changelog_md.py b/util/changelog_md.py index 7d1e44e1d2..6a7014d4ec 100644 --- a/util/changelog_md.py +++ b/util/changelog_md.py @@ -231,9 +231,10 @@ r':(?:py:)?(' + '|'.join(sorted(PY_ROLES, key=len, reverse=True)) + r'):`([^`\n]+)`') #: Where the full rendered history lives. Must match ``[project.urls].changelog`` -#: in ``pyproject.toml`` -- ``MANIFEST.in`` prunes ``docs/``, so in a source -#: distribution this is the only route from the shipped entry to the rest of the -#: history. ``tests/project/test_changelog_md.py`` asserts the two agree. +#: in ``pyproject.toml`` -- ``MANIFEST.in`` prunes ``docs/`` but for the +#: per-version entries, so a source distribution carries the history only as +#: reStructuredText files, and this is its one route to the rendered history. +#: ``tests/project/test_changelog_md.py`` asserts the two agree. DOCS_URL = 'https://jarryshaw.github.io/PyPCAPKit/changelog.html' #: Closing note on the generated file, pointing at the rest of the history. @@ -492,7 +493,8 @@ def py_role(name: str, text: str) -> str: A code span rather than a link: the page that documents an object is known only to a built ``objects.inv``, and this script runs where there is no - documentation build -- a source distribution prunes ``docs/`` altogether. + documentation build -- a source distribution keeps nothing of ``docs/`` but + the per-version entries. The displayed name follows Sphinx's ``XRefRole`` and ``PyXRefRole`` (Sphinx 9.1.0), so an entry names the same thing in the Markdown as in the @@ -646,8 +648,8 @@ def flush() -> None: flush() # One blank row per run: the flush cycle can leave several behind, and # a run of blank lines is one paragraph break however long it is -- and - # a run before the first block is no break at all, where the ``\n{3,}`` - # substitution this replaces left an empty first line behind. + # a run before the first block is no break at all, so the output never + # opens on an empty line. if rows and rows[-1]: rows.append([]) continue diff --git a/util/ci_timing.py b/util/ci_timing.py index d5611cca42..0767b37f00 100644 --- a/util/ci_timing.py +++ b/util/ci_timing.py @@ -170,8 +170,8 @@ def _coverage_legs() -> 'tuple[str, tuple[str, ...]]': MERGEABLE_P90 = 30.0 #: H3: that p90, over the floor, rising this much against the baseline report's. #: Absolute, it measures runner-queue load as much as test time, so it pages only on -#: a regression; the absolute breach alone is W9. Initial value, to tune once weekly -#: reports exist -- W1's 20% median rise, loosened for a tail statistic. +#: a regression; the absolute breach alone is W9. Set from W1's 20% median rise, +#: loosened for a tail statistic, rather than tuned against the weekly reports. HARD_MERGEABLE_RISE = 0.25 #: H4: a linear projection reaching its cap within this many days. HARD_PROJECTION_DAYS = 14.0 diff --git a/util/project_status.py b/util/project_status.py index b27d75196b..d7536f5369 100644 --- a/util/project_status.py +++ b/util/project_status.py @@ -53,7 +53,7 @@ #: The board, by owner and number. Resolved to node ids at runtime. OWNER = 'JarryShaw' PROJECT_NUMBER = 2 -#: The ids the board had when this was written. A resolved id that differs means +#: The ids the board is known by. A resolved id that differs means #: the board was recreated, and writing to it blind is the wrong default. EXPECTED_PROJECT_ID = 'PVT_kwHOAO8M8c4Bl2gH' EXPECTED_FIELD_ID = 'PVTSSF_lAHOAO8M8c4Bl2gHzhkhthA' diff --git a/util/pyshark_encap_map.py b/util/pyshark_encap_map.py index 5a108c6931..b372e9f3e2 100644 --- a/util/pyshark_encap_map.py +++ b/util/pyshark_encap_map.py @@ -31,16 +31,16 @@ .layer_name`` and what :func:`pcapkit.toolkit.pyshark.tcp_traceflow` falls back to when a capture carries no ``frame.encap_type`` field at all. -Traps that cost real time when #850's tables were first produced ----------------------------------------------------------------- +Traps in the round trip +----------------------- **A PDML ``showname`` can hold a ``/``.** ``tshark`` renders ``frame.encap_type``'s ``showname`` as ``"Encapsulation type: NULL/Loopback (15)"`` -- the display name itself, not just this one, can contain the character. A regex that expects the name to be made of "word" characters up to the parenthesised number -- ``r'(\\w+) \\((\\d+)\\)'``, say -- stops at the -``/`` and drops the row outright. That silently lost 10 of 157 rows, -including ``null`` itself, when #850's tables were first produced. +``/`` and drops the row outright, which silently loses 10 of the 157 rows, +``null`` itself among them. :func:`parse_parenthesised_number` is anchored to the *end* of the string instead, so the name in front of the number can contain anything at all. @@ -109,7 +109,7 @@ reported on :data:`sys.stderr` and exits cleanly rather than raising, so a host without Wireshark installed does not turn "nothing to regenerate" into a traceback. It reads the PDML with lxml (#1894), which the ``test`` extra -installs except on GraalPy, which has no wheel for it. +installs on CPython alone, since PyPI has no wheel of it for GraalPy or PyPy. This cannot be run in CI to verify the committed tables, only locally against a pinned Wireshark @@ -123,8 +123,8 @@ measured and committed) -- see :class:`EncapSweepError`. Ubuntu noble's packaged Wireshark, what every ``apt``-based CI runner installs, is 4.2.2: its ``editcap -T`` accepts 224 encapsulations, not 226, so this script's own -assertion trips there every time, by design (found the hard way: the first -CI run for #851, three legs, ``AssertionError: 224 != 226``). +assertion trips there every time, by design (measured on #851's CI legs: +``AssertionError: 224 != 226``). That is harmless to the *committed* tables at runtime -- an ``ENCAP_TYPE_TO_LINKTYPE``/``FILTER_NAME_TO_LINKTYPE`` entry a 4.2.2 install @@ -187,7 +187,7 @@ EXPECTED_REFUSED = 69 #: The `` - `` shape of ``editcap -T ''``'s listing, -#: anchored to leading whitespace so the banner line -- which has none -- +#: anchored to leading whitespace so the banner lines -- which have none -- #: cannot match. See the module docstring's second trap. _ENCAP_LINE = re.compile(r'^[ \t]+([A-Za-z0-9._-]+) - .*$') @@ -407,15 +407,13 @@ def build_tables(measurements: 'Sequence[Measurement]', linktype: 'Any') -> ( notes = [] # type: list[str] # *linktype* is one of this project's registry enums: an unknown value - # does not raise out of the constructor, it mints a placeholder member - # via ``_missing_``/``extend_enum`` (#575) -- so ``LinkType(121)`` - # *succeeds*, returning a freshly minted ``Unassigned_121`` rather than - # telling us 121 was never a real member. The set of values genuinely - # registered has to be captured *before* any such lookup runs, or the - # very first miss would mint a member that every later check then finds - # already there. ``dlt in known_values`` is the check; ``linktype(dlt)`` - # is only ever called once *that* has already said yes, so it can never - # reach ``_missing_`` itself. + # does not raise out of the constructor, ``_missing_`` answers it with an + # unregistered ``Unassigned`` pseudo-member (#775) -- so ``LinkType(121)`` + # *succeeds* rather than telling us 121 was never a real member. So the + # values genuinely registered are read off the members themselves, and + # ``dlt in known_values`` is the check; ``linktype(dlt)`` is only ever + # called once *that* has already said yes, so it never reaches + # ``_missing_`` itself. known_values = frozenset(member.value for member in linktype) by_encap_type = {} # type: dict[int, list[Measurement]] diff --git a/util/pytest_shards.py b/util/pytest_shards.py index 8204d4f186..94221a3206 100644 --- a/util/pytest_shards.py +++ b/util/pytest_shards.py @@ -9,8 +9,9 @@ /usr/bin/python3 util/pytest_shards.py "$SHARDS" -- python -m pytest ... -unit-tests.yml's ``Run unit tests`` runs it the same way on PyPy's legs, whose -xdist workers' RSS grows with every test as well (#2005). +unit-tests.yml's ``Run unit tests`` runs it on PyPy's legs too, on the leg's own +interpreter and under :file:`util/cap_file_size.py`, since their xdist workers' +RSS grows with every test as well (#2005). ``SHARDS`` holds the shards, separated by whitespace, and each is a directory or a glob of the entries of one directory, such as ``tests/corekit/test_i*``. @@ -33,15 +34,16 @@ named after the shard -- ``junit-tests-protocols.xml`` beside ``junit.xml`` -- and the last run keeps the command's. -The runs share the step's one ``timeout``, scope and watcher, which run this -script, so the step's caps hold over the whole tier rather than a shard. On +On the GraalPy leg the runs share the step's one ``timeout``, scope and watcher, +which run this script, so the step's caps hold over the whole tier rather than a shard. On ``SIGINT``, which ``timeout`` sends to its whole process group, the run then in progress ends as pytest ends it, with its JUnit file written, and no later run starts. Any other signal ends this script as it would any process. The exit status is that of the last run to fail, as a shell gives it for a run a signal ended, or 0 where none did. The script runs on the image's own -interpreter, apart from the leg's, so it is standard library only. +interpreter on the GraalPy leg, and on the leg's own on PyPy's, so it is +standard library only. """ diff --git a/util/python_versions.py b/util/python_versions.py index 32849cac82..a810ecab19 100644 --- a/util/python_versions.py +++ b/util/python_versions.py @@ -30,8 +30,8 @@ and ``os`` as above, for unit-tests.yml's jobs of the same names, which nothing gating waits on (#1738). ``pypcap-parity-ktlo`` - The same, for CPython's KTLO versions below 3.12. No other unit-tests.yml - output holds a KTLO version. + The same, for CPython's KTLO versions below 3.12. Of the outputs + unit-tests.yml reads, only the ``*-ktlo`` ones hold a KTLO version. ``experimental`` Each experimental version as a leg carrying ``experimental: true``, for experimental.yml's ``experimental`` job, a workflow of its own so that nothing @@ -50,10 +50,10 @@ The supported tier's wheel tags and conda build tags, which Create Release's completeness checks count. -Each matrix keeps the shape its job had when the versions were spelled out in -the workflow -- a ``python-version`` axis, then ``include`` -- so GitHub expands -it to the same legs in the same order. -``tests/project/test_python_versions_1660.py`` compares the two. +Each matrix has the shape of the one its job spelled out by hand at the commit +``tests/project/test_python_versions_1660.py`` pins -- a ``python-version`` axis, +then ``include`` -- so GitHub expands it to the same legs in the same order, and +that test compares the two. Run without arguments it prints every output; with names, just their values. @@ -134,7 +134,7 @@ def load(path: 'pathlib.Path' = FILE) -> 'dict[str, Any]': key or tier, a misspelt version or runner, a ``latest`` that is not a supported CPython, an exemption missing a field, or a pre-release entry -- no workflow has the non-blocking pre-release legs one needs - (``allow-prereleases``, a name of its own) yet, so listing one fails here + (``allow-prereleases``, a name of its own), so listing one fails here instead of running it as something else. An exemption's reason is the comment above it, which :mod:`tomllib` does not keep. @@ -253,7 +253,7 @@ def ktlo_matrix(data: 'dict[str, Any]') -> 'dict[str, Any]': ``Required checks passed`` reads one ``needs..result`` per job, and ``unittest-ordering`` needs ``test``, so a KTLO leg that fails as a job -- a - lost runner, or the job cap -- failed both from inside ``test``. + lost runner, or the job cap -- would fail both from inside ``test``. """ return legs_matrix([leg for impl in TESTED for leg in ktlo_legs(data, impl)]) diff --git a/util/release_tags.py b/util/release_tags.py index 3b017055bd..ab39216bb4 100644 --- a/util/release_tags.py +++ b/util/release_tags.py @@ -2,7 +2,8 @@ """Which commit a release tag names, against the commit this run was created for (#1634, #1748). :file:`.github/workflows/create-release.yml` runs it from the checkout's root, -on the runner's own ``python3``, in these forms: +on ``python3``, setup-python's in ``version_check`` and the image's in the +other jobs, in these forms: ``python3 util/release_tags.py check`` The step "Check the release tags name this run's commit" of the ``github``, diff --git a/util/run_unittest_leg.py b/util/run_unittest_leg.py index 1b8117ce63..6a4dffc78d 100644 --- a/util/run_unittest_leg.py +++ b/util/run_unittest_leg.py @@ -36,10 +36,9 @@ modules, in that order, disables the very same autouse fixture directly and reproduces the identical ``subTest`` failures in about 70s on a tree without #981's fix, 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 -script was written; this paragraph records that gap rather than inventing a -reason for the choice after the fact. +Whether that single invocation would suffice in place of this dedicated +runner and its per-directory matrix has not been evaluated; this paragraph +records that gap rather than inventing a reason for the choice. Scope, and why it stops where it does -------------------------------------- @@ -176,7 +175,7 @@ #: Environment override for :data:`DEFAULT_STALL_DUMP`; ``--stall-dump`` wins. STALL_DUMP_ENV = 'PCAPKIT_UNITTEST_STALL_DUMP' -#: The module name each flat leg ``--half`` splits is cut at (#2254). Move it to +#: The module name at which ``--half`` cuts each flat leg (#2254). Move it to #: rebalance; the workflow's cells and steps ask for a half and follow. SHARD_BOUNDARIES = {'project': 'test_q'} diff --git a/util/superseded.py b/util/superseded.py index 295fd84f74..28dcadd472 100644 --- a/util/superseded.py +++ b/util/superseded.py @@ -29,7 +29,7 @@ session or, with ``--scope UNIT``, to every process of that systemd scope (``sudo -n systemctl kill``): ``COMMAND`` then starts a root-owned ``sudo``, and the tests run in the scope as the runner's user, out of reach of a signal to -the session's group (:func:`stop`). Where ``COMMAND`` exits, it writes the notice, the summary +the session's group (:func:`stop`). Where ``COMMAND`` then exits, it writes the notice, the summary line and ``stopped=true`` to ``$GITHUB_OUTPUT``, and exits 0: a stopped pytest writes no JUnit file, so the upload after it reads the output and expects none (#2073). Otherwise it exits with ``COMMAND``'s own status, ``128 + N`` for a signal ``N`` as a shell reports it, so a test @@ -91,7 +91,7 @@ #: The REST API's root, where ``GITHUB_API_URL`` does not name one. API = 'https://api.github.com' -#: The two answers. +#: The answers :func:`check` gives. CURRENT, SUPERSEDED = 'current', 'superseded' #: Seconds between two checks while ``watch`` runs its command. INTERVAL = 60.0 diff --git a/util/update_python_versions.py b/util/update_python_versions.py index d9e2dea691..f654da8321 100644 --- a/util/update_python_versions.py +++ b/util/update_python_versions.py @@ -37,7 +37,7 @@ image ``ubuntu-latest`` may mean has no ``runner``; otherwise it names the newest hosted Ubuntu image that has one. dropped - ``end-of-life`` with no build for any hosted Ubuntu image. + ``end-of-life`` with no stable build for any hosted Ubuntu image. pre-release ``feature`` or ``prerelease`` once setup-python has a build. No workflow runs pre-release legs, and :func:`python_versions.load` refuses the tier, so @@ -903,7 +903,7 @@ def computed(versions: 'dict[str, dict[str, Any]]') -> 'dict[str, dict[str, Any] def fingerprint(latest: 'str', versions: 'dict[str, Any]') -> 'str': - """A proposal's identity: its computed tiers, whatever else moved on ``main``, hand-kept entries included.""" + """A proposal's identity: its computed tiers alone, whatever else moved on ``main``, hand-kept entries included.""" canonical = json.dumps({'latest': latest, 'versions': computed(versions)}, sort_keys=True, separators=(',', ':')) return hashlib.sha256(canonical.encode('utf-8')).hexdigest()[:16] diff --git a/util/vendor_test_gate.py b/util/vendor_test_gate.py index 423ccd1c5c..6f22858058 100644 --- a/util/vendor_test_gate.py +++ b/util/vendor_test_gate.py @@ -2,9 +2,10 @@ """Test a Vendor Update regeneration before it is committed (#1853). :file:`.github/workflows/cron-vendor.yml` runs ``pcapkit-vendor`` and isort, then -commits what changed. Neither of its gates sees that tree: the Vendor test gate -runs on the schedule alone, and a ``workflow_run`` reads the Unit Tests verdict -on the commit from before the crawl. So 1.5.0b13 was committed with a pinned +commits what changed. Neither gate of its ``head`` job sees that tree: both +judge the commit from before the crawl, the test gate by running the tests on +it, on the schedule or a manual run, and ``verdict`` by reading the Unit Tests +verdict on it, on a ``workflow_run``. So 1.5.0b13 was committed with a pinned test failing on it, and ``main`` stayed red until #1820. The owner's ruling on #1853 is that the job tests the regenerated tree before it commits, under #872's rule: a failed sub-vendor's changes are discarded, the others are kept,