From f73b1880f7fc3b9daceec0be4f1775b2105e9e1e Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Tue, 6 Oct 2026 09:40:35 -0400 Subject: [PATCH] docs(contributing): tighten the prose from the post-sweep merges (#719) - Cut issue/PR numbers, dates and "used to" history from the comments the queue-cut, coverage and project-status changes added to unit-tests.yml, project-status.yml, coverage-comment.yml, pyproject.toml and util/run_unittest_leg.py, keeping each rationale. - Correct statements the per-class re-import made stale: coverage.toml's "thousands of tests purge in setUp" and pyproject's "a generation per test" for the retained pytest-timeout timer. - State the ruleset's current six required contexts in unit-tests.yml and workflows.rst, with why the aggregate and the separate Compat legs exist. - Fix counts: coverage.toml and test_coverage_rcfile.py name three added keys (parallel, patch, core), not two; test_unit_tests_queue.py's docstring lists five pins, not "three". - Fix unit-tests.yml's stale pyproject.toml:151 marker reference (157) and renumber the unit-tests.yml citations in workflows.rst and releasing.rst. Comments, docstrings and prose only: AST / parsed-YAML / parsed-TOML equal to origin/main for every touched file; tests/project passes; Sphinx -n shows no new warnings. --- .github/coverage.toml | 16 ++-- .github/workflows/coverage-comment.yml | 2 +- .github/workflows/project-status.yml | 17 ++-- .github/workflows/unit-tests.yml | 126 +++++++++++-------------- docs/source/contributing/releasing.rst | 4 +- docs/source/contributing/workflows.rst | 61 ++++++------ pyproject.toml | 9 +- tests/project/test_coverage_rcfile.py | 4 +- tests/project/test_unit_tests_queue.py | 2 +- util/run_unittest_leg.py | 41 ++++---- 10 files changed, 130 insertions(+), 152 deletions(-) diff --git a/.github/coverage.toml b/.github/coverage.toml index f2dee3e48..d581e9c19 100644 --- a/.github/coverage.toml +++ b/.github/coverage.toml @@ -1,8 +1,8 @@ # Coverage configuration for the one coverage-measured leg of unit-tests.yml # (`test`, Python 3.14), passed with `--rcfile`. Coverage reads a single config # file, so this repeats pyproject.toml's [tool.coverage.*] settings and adds the -# two the CI run needs on top. tests/project/test_coverage_rcfile.py asserts -# every pyproject setting is repeated here unchanged. +# three 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 # by `coverage combine`. @@ -10,12 +10,12 @@ # child Python process. pytest-xdist's workers are such children # (execnet popen), so without it only the controller is measured # and the -n auto run reports next to nothing. -# core -- "ctrace", not the sys.monitoring default on 3.12+. Thousands of -# tests purge and re-import pcapkit in setUp, and sysmon keeps -# state for every generation's code objects: measured on -# test_http_unit.py alone, peak RSS 0.40 GiB without coverage, -# 4.17 GiB under sysmon and 0.41 GiB under ctrace. Under -# -n auto that growth killed the runner (#1064). +# core -- "ctrace", not the sys.monitoring default on 3.12+. Tests +# purge and re-import pcapkit, and sysmon keeps state for every +# generation's code objects: measured on test_http_unit.py +# alone, re-importing per test, peak RSS 0.40 GiB without +# coverage, 4.17 GiB under sysmon and 0.41 GiB under ctrace. +# Under -n auto that growth killed the runner. [tool.coverage.run] source = [ diff --git a/.github/workflows/coverage-comment.yml b/.github/workflows/coverage-comment.yml index 07f8a679f..843b29f6e 100644 --- a/.github/workflows/coverage-comment.yml +++ b/.github/workflows/coverage-comment.yml @@ -1,6 +1,6 @@ name: Coverage Comment -# #1063: posts the coverage the `test` job's Python 3.14 leg measured in +# Posts the coverage the `test` job's Python 3.14 leg measured in # 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 diff --git a/.github/workflows/project-status.yml b/.github/workflows/project-status.yml index 09e0fd587..370f164a4 100644 --- a/.github/workflows/project-status.yml +++ b/.github/workflows/project-status.yml @@ -3,7 +3,7 @@ name: Project Status # Keeps the *Status* field of the project board (users/JarryShaw/projects/2) in # line with each item's state labels. The board's built-in workflows add items and # set Done on close and merge, but no built-in workflow observes a label change, so -# every other transition used to be made by hand. The mapping lives in +# this one makes every other transition. The mapping lives in # util/project_status.py:status_for and is documented under "Milestones and the # Project Board" in docs/source/contributing/conventions/process.rst; the tests in # tests/project/test_project_status.py hold the two to each other. @@ -42,7 +42,7 @@ name: Project Status # # PROJECT_TOKEN is a classic PAT with `project` and `repo` scopes, because # GITHUB_TOKEN cannot write a user-owned project. Without it every run skips with a -# `::notice::` instead of failing, so the workflow can land before the secret does. +# `::notice::` instead of failing. on: issues: @@ -68,13 +68,12 @@ permissions: {} # Issues and pull requests share one number space, so the key cannot collide # across the two event types. # -# Nothing in a group is cancelled. `cancel-in-progress` used to cancel the older -# run of a label swap, and a cancelled run stays on the PR head as a failed Sync -# Status check although the newer run fixed the board (#1066). Dropping it is not -# enough on its own: by default a group holds one pending run and cancels it when -# a newer one arrives. `queue: max` lets up to 100 wait instead; only a run past -# that is cancelled. GitHub documents both, and that the two keys together are a -# validation error: +# Nothing in a group is cancelled: a cancelled run stays on the PR head as a +# failed Sync Status check even after a newer run has fixed the board. So there +# is no `cancel-in-progress`, and that is not enough on its own: by default a +# group holds one pending run and cancels it when a newer one arrives. +# `queue: max` lets up to 100 wait instead; only a run past that is cancelled. +# GitHub documents both, and that the two keys together are a validation error: # https://docs.github.com/en/actions/how-tos/write-workflows/choose-when-workflows-run/control-workflow-concurrency # # The board still ends right, whatever order the queue runs in -- GitHub states diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index c06e8017f..d07b9299c 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -65,9 +65,9 @@ jobs: - "3.12" - "3.13" - "3.14" - # #1063: exactly one leg runs under coverage, so measuring it adds no - # leg to the queue #1052 is cutting. 3.14 because it had the most - # headroom under the 45-minute cap on main (15.6, 15.8 and 16.8 minutes + # Exactly one leg runs under coverage, so measuring it adds no leg to + # the queue. 3.14 because it had the most headroom under the 45-minute + # cap on main (15.6, 15.8 and 16.8 minutes # in runs 37393532176, 37392964804 and 37391296686, against 12.8-22.0 # for the other four), and because coverage's sys.monitoring core, # which measures branches only from 3.14 on, is its cheapest tracer. @@ -364,21 +364,21 @@ jobs: # green and never red for the wrong reason. If it # ever starts building, the cell fails loudly. # - # #1052 folded the 36 one-cell jobs (6 Pythons x 6 engines) into one job per - # engine that loops over the six interpreters, each in a fresh venv. The - # queue, not the runners, was the bottleneck: 36 jobs each paid 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 + # One job per engine loops over the six interpreters, each in a fresh venv, + # rather than one job per cell (6 Pythons x 6 engines): the queue, not the + # runners, is the bottleneck, and 36 jobs would each 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 # 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 # the one matrix axis, which is what tests/_dependency_gates.py's - # _engine_matrix_variants() reads this job as. No cell is dropped: the cell - # script below is the old per-cell install-and-test logic unchanged, run once - # per interpreter as its own process (so `set -e` holds inside it), under a - # `::group::` named for the cell, with every annotation titled by engine and - # Python version and a per-cell row in the job summary. + # _engine_matrix_variants() reads this job as. The cell script below is one + # cell's install-and-test logic, run once per interpreter as its own process + # (so `set -e` holds inside it), under a `::group::` named for the cell, with + # every annotation titled by engine and Python version and a per-cell row in + # the job summary. # # 3.15 is experimental-prerelease per #845's ruling: present so a regression # is visible, but never blocking. It runs as its own step with step-level @@ -387,7 +387,7 @@ jobs: # job's result -- stays green. The 3.10-3.14 cells share one step, and any # one of them failing fails the job. # - # Scoped deliberately out (#845, unchanged by #1052): per-engine integration + # Scoped deliberately out (#845): per-engine integration # coverage (needs regenerated captures, so it stays in aggregate in # `integration` and `pypcap-parity`), and the multi-engine HAS_RUNTIME gate # plus the incidental crypto/NGAP/vendor coverage the pre-#845 shared venv @@ -450,16 +450,14 @@ jobs: # eat the whole job: per #974, a PCAP_CT cell fetched its InRelease # metadata and then went silent for 29m52s until the job's own 30-minute # cap killed it, which reports `cancelled` and so fails `Required checks - # passed` -- an unrelated mirror turning a green PR red. The 2026-10-01 - # stalls (#1052) had the same all-or-nothing shape, in `apt-get update`. + # passed` -- an unrelated mirror turning a green PR red. Stalls in + # `apt-get update` have had the same all-or-nothing shape. # - # 5 minutes, with one retry inside it, rather than the 20 this step used - # to carry. #974 sized 20 against a healthy tail of up to 646s, but that - # tail is gone: across 6 runs (120 non-zero observations of this step and - # `pypcap-parity`'s) on 2026-10-04/05 the slowest was 34s, p99 33s, all - # on PyShark cells, and #1052 measured p99 0.47 minutes. A stall is - # therefore distinguishable from a slow mirror well inside a short - # per-attempt bound, which makes retrying worth it -- 45s for + # 5 minutes, with one retry inside it. Across 6 runs (120 non-zero + # observations of this step and `pypcap-parity`'s) the slowest was 34s, + # p99 33s, all on PyShark cells. A stall is therefore distinguishable + # from a slow mirror well inside a short per-attempt bound, which makes + # retrying worth it -- 45s for # `apt-get update` and 90s for `apt-get install`, two attempts, 270s in # the worst case, under the 5-minute step cap that still holds however # the hang is caused. The step cap stays well under the 30-minute job @@ -672,7 +670,7 @@ jobs: # 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:151). If a clean run of + # ("pypcap; python_version < '3.12'", pyproject.toml:157). 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 @@ -736,9 +734,9 @@ jobs: # postinst otherwise blocks on the "allow non-superusers to capture # packets" prompt. # - # `timeout-minutes` and the retry for the reason #974 and #1052 give about - # `engine-tests`'s own install step -- see the note there, which this - # step shares wholesale, including the 5-minute cap and per-attempt + # `timeout-minutes` and the retry for the reason given on `engine-tests`'s + # own install step -- see the note there, which this step shares + # wholesale, including the 5-minute cap and per-attempt # bounds. It was never the leg that stalled, but it installs the union of # the packages that one does from the same mirrors, so the hazard is # identical; its observations are in the 120-sample figure quoted there. @@ -927,8 +925,8 @@ jobs: # maintainer's call to make separately from landing it. unittest-ordering: name: Plain unittest ordering (${{ matrix.label || matrix.leg }}) - # Not on pull requests (#1052): it is not a required check, and its 11 - # cells were a sixth of every PR push's jobs. It still runs on every push + # Not on pull requests: it is not a required check, and its 11 cells + # would be a sixth of every PR push's jobs. It still runs on every push # to main, which is the run that tests what actually lands. if: ${{ inputs.gate-only != true && github.event_name != 'pull_request' }} runs-on: ubuntu-latest @@ -995,7 +993,7 @@ jobs: nproc python -c "import os; print('cpu_count', os.cpu_count())" - # `--verbose` (#1052) names every test as it runs, so a slow or hung cell + # `--verbose` names every test as it runs, so a slow or hung cell # shows where it was. The step cap of 20 is about twice the slowest # cell's measured maximum (9.8 minutes; the other shard 8.9), and sits # under the job's 45 so a hang reports as this step timing out. @@ -1045,7 +1043,7 @@ jobs: exit 1 fi - # #1052: which of a pull request's changed files are code. This decides + # Which of a pull request's changed files are code. This decides # whether the expensive legs run at all (see `required-checks` for how their # skips are then judged), so it fails safe: anything it cannot classify is # code. @@ -1117,7 +1115,7 @@ jobs: print(f'code={code}', file=file) PY - # #1052: the docs-only replacement for `test`, running every test that reads + # The docs-only replacement for `test`, running every test that reads # docs/ or a Markdown file, since a docs-only diff can still break those: # tests/project and the root-level modules, plus the modules elsewhere under # tests/ named in the second run step. tests/project/test_unit_tests_queue.py @@ -1233,27 +1231,16 @@ jobs: - name: Run full test suite run: python -m pytest -q -n auto --dist load - # Ruleset 23497679's required_status_checks names 22 exact contexts: five - # each for `test` and `integration`, five for `Compat Python 3.10`-`3.14` - # (live -- emitted by the `compatibility` job in - # `.github/workflows/python-compatibility.yml`, on the same push/ - # pull_request triggers as this file, confirmed by reading that file -- - # NOT stale, and NOT something a `needs:` in this file could ever cover, - # since a job cannot depend on a job in a different workflow file), five - # for the old single-cell `Engines Python ` name that - # `engine-tests` stopped producing once it became a Python x engine matrix - # (the only five of the 22 that are genuinely dead), and two for - # `pypcap-parity`. GitHub rulesets match check names literally and support - # no wildcard, so the required list has to be hand-edited regardless -- to - # drop the five dead `Engines Python ` names and to either - # re-list every gating (3.10-3.14) `engine-tests` matrix cell name (30 of - # them, growing or shrinking with the matrix) or replace that slot with - # something that does not change shape when the matrix does. The - # maintainer's ruling was the latter: one aggregate job that `needs:` - # every gating job in *this* file, so the ruleset requires only this one - # context in place of the `test`, `integration`, `Engines`, and - # `pypcap-parity` slots (17 of the 22) -- the five live `Compat` names - # stay required exactly as they are, since nothing here can stand in for + # Ruleset 23497679's required_status_checks requires six contexts: this + # job's `Required checks passed`, and five for `Compat Python 3.10`-`3.14` + # from the `compatibility` job in `.github/workflows/python-compatibility.yml`. + # This job aggregates the gating jobs of *this* file, the ones its `needs:` + # 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 + # are required on their own because a job cannot `needs:` a job in another + # workflow file, so nothing here can stand in for # them. # # `if: ${{ always() && inputs.gate-only != true }}` and the explicit @@ -1297,31 +1284,26 @@ jobs: # is called with `gate-only: true` and is otherwise skipped by its own # `if:`, so depending on it would leave this job permanently skipped on # every ordinary PR -- the one case this job has to actually gate. - # `changelog` is deliberately left out too: it is a drift check, not one of - # the 22 contexts this job replaces, and folding it in here would silently - # widen what merging requires beyond what this job exists to cover -- the - # maintainer's call to make separately, not something to sneak in. - # `test`, `integration`, `engine-tests` and `pypcap-parity` are the four - # jobs in this file whose names the 17 live/dead contexts above (all but - # `Compat`) stand in for; `changes` and `project-tests` are needed only for - # the docs-only path below. #1052 renamed the engine jobs to - # `Engines ()`; no ruleset context names an engine job, so nothing - # required was orphaned (checked against the ruleset on 2026-10-05). + # `changelog` is deliberately left out too: it is a drift check, not a + # gating leg, and folding it in would widen what merging requires -- a + # change for the maintainer to decide, not one to slip in here. + # `test`, `integration`, `engine-tests` and `pypcap-parity` are the gating + # legs; `changes` and `project-tests` are needed only for the docs-only + # path below. # # `engine-tests`'s `unsupported` and `not-installable` cells are not a gap # here: the cell script exits 0 for a cell that declined exactly as # expected, so they contribute to that job's `success` like any other cell. # - # Nor is its 3.15 cell, since #1052. It used to be a separate matrix job - # with job-level `continue-on-error`, and GitHub does not document whether - # `needs..result` sees such a job's failure as `failure` or `success` - # -- an open risk that a 3.15-only regression could block every merge. The - # 3.15 cell is now a step with step-level `continue-on-error: true` inside - # each engine's job, whose effect on the job's result *is* documented (a - # step's `conclusion` is its result after `continue-on-error` is applied), so - # the job, and this gate, read `success` while the failed step stays visible. + # Nor is its 3.15 cell. It is a step with step-level `continue-on-error: + # true` inside each engine's job, whose effect on the job's result *is* + # documented (a step's `conclusion` is its result after `continue-on-error` + # is applied), so the job, and this gate, read `success` while the failed + # step stays visible. Not a job-level `continue-on-error`: GitHub does not + # document whether `needs..result` sees such a job's failure as + # `failure` or `success`, so a 3.15-only regression could block every merge. # - # #1052's docs-only path. On a pull request whose diff touches nothing but + # The docs-only path. On a pull request whose diff touches nothing but # `docs/**` and `*.md` files outside the code directories (the `changes` # job's classifier), `test`, `integration`, `engine-tests` and # `pypcap-parity` are skipped by their own `if:`, and `project-tests` runs diff --git a/docs/source/contributing/releasing.rst b/docs/source/contributing/releasing.rst index 25b45d2c9..b06cd24cf 100644 --- a/docs/source/contributing/releasing.rst +++ b/docs/source/contributing/releasing.rst @@ -46,7 +46,7 @@ Editing ``__version__`` by hand without also moving :file:`CITATION.cff`'s (``:494-514``) asserts the two agree, and ``create-release.yml``'s ``unit-tests`` job (``:362-376``) calls ``unit-tests.yml`` with ``gate-only: true``, which runs the **full** suite rather than the tiered -subset an ordinary push runs (its ``gate`` job, ``unit-tests.yml:1165-1234``). +subset an ordinary push runs (its ``gate`` job, ``unit-tests.yml:1163-1232``). That gate runs whenever a release will -- a moved ``__version__`` is exactly what makes ``version_check``'s evidence read "not yet published" -- and every publishing job requires it to have succeeded (``:392,493,573,674``), so a @@ -56,7 +56,7 @@ Either run the script, or move both fields by hand in the same commit. This is the **only** edit a person makes to get a release started -- the tag, the Release, and every upload are the workflow's job from here. Two other checks run unconditionally on the same path, though neither is a step in *this* -process: the ``changelog`` job (``unit-tests.yml:1025-1046``) fails outright if +process: the ``changelog`` job (``unit-tests.yml:1023-1044``) fails outright if ``CHANGELOG.md`` has drifted from its source entry under :file:`docs/source/changelog/`, and the release-body step warns, without failing, if that entry's heading still reads "unreleased" diff --git a/docs/source/contributing/workflows.rst b/docs/source/contributing/workflows.rst index 3c7cc7038..c55d41a3e 100644 --- a/docs/source/contributing/workflows.rst +++ b/docs/source/contributing/workflows.rst @@ -238,15 +238,14 @@ not one: ``gate`` (the full suite, one Python version) and ``changelog`` over six Python versions), ``pypcap-parity`` (two) and ``unittest-ordering`` (eleven matrix cells), each of which has already run once for this commit from Unit Tests' own ``push`` trigger (and, all but ``unittest-ordering``, -which runs on ``main`` pushes only since -`#1052 `__, from its -``pull_request`` trigger), so running any of them again per caller would +which runs on ``main`` pushes only, from its ``pull_request`` trigger), so +running any of them again per caller would test the same commit several times over -- the docs-only path's ``changes`` and ``project-tests``, and ``required-checks`` (see `Required Status Checks`_ below), gated out by their own ``if:`` rather than by having already run. ``changelog`` carries no ``if:`` at all and runs on every path regardless -- deliberately, per its own -comment (``unit-tests.yml:1013-1019``): ``create-release.yml`` feeds +comment (``unit-tests.yml:1011-1017``): ``create-release.yml`` feeds ``CHANGELOG.md`` to the GitHub Release body, so the release path is exactly where a drifted file must not go unchecked. Confirmed on run `36210743295 `__ (a @@ -351,10 +350,11 @@ its unchanged pytest selection under ``coverage run :file:`pyproject.toml`'s ``[tool.coverage.*]`` settings (``tests/project/test_coverage_rcfile.py`` asserts it) and adds ``parallel`` and ``patch = ["subprocess"]``, without which only the xdist controller is -measured, not its workers, plus ``core = "ctrace"``: under the default -``sys.monitoring`` core, the suite's purge-and-reimport of pcapkit grew one -module's peak RSS from 0.40 to 4.17 GiB and killed the runner. The leg then writes the total and a per-package -table to the job summary and uploads the HTML report as the ``coverage-html`` +measured, not its workers, plus ``core = "ctrace"``: with the suite purging +and re-importing pcapkit, one module's peak RSS was 4.17 GiB under the default +``sys.monitoring`` core against 0.41 GiB under ``ctrace``, and that growth +killed the runner. The leg then writes the total and a per-package table to the +job summary and uploads the HTML report as the ``coverage-html`` artifact (30 days), with the bare numbers as the ``coverage-summary`` artifact. The PR comment comes from **Coverage Comment** (:file:`coverage-comment.yml`), @@ -385,7 +385,7 @@ grepping every workflow file for its name: - Where * - ``Required checks passed`` - job ``required-checks`` - - ``unit-tests.yml:1338`` + - ``unit-tests.yml:1320`` * - ``Compat Python 3.10`` - job ``compatibility``, matrix leg ``3.10`` - ``python-compatibility.yml:31,41`` @@ -403,33 +403,30 @@ grepping every workflow file for its name: - ``python-compatibility.yml:31,45`` ``Required checks passed`` is defined by exactly one job -(``unit-tests.yml:1338``). ``Compat Python`` is defined by two in +(``unit-tests.yml:1320``). ``Compat Python`` is defined by two in ``python-compatibility.yml``: the required ``compatibility`` job (``:31``, ``Compat Python ${{ matrix.python-version }}``, expanding to the five required legs at ``:41-45``) and the non-required ``compatibility-nightly`` job (``:69``, ``Compat Python 3.15 (scheduled)``, see below). A comment at -``unit-tests.yml:1237`` mentions the string without defining it. - -``Required checks passed`` is itself an aggregate, not a single check run -- -but it stands in for **17** of the ruleset's originally-named 22 contexts, -not all 22. ``unit-tests.yml``'s own comment (``unit-tests.yml:1236-1257``) accounts for the -22: five each for ``test`` and ``integration``, five for the live ``Compat -Python 3.10``-``3.14`` contexts (emitted by ``python-compatibility.yml``, a -*different* workflow file -- nothing in ``unit-tests.yml`` could ever stand -in for them, since a job cannot depend on a job living elsewhere), five for -the now-dead single-cell ``Engines Python `` name ``engine-tests`` -stopped producing once it became a Python x engine matrix, and two for -``pypcap-parity``. ``Required checks passed`` replaces only the -``test``/``integration``/dead-``Engines``/``pypcap-parity`` slots -- 5 + 5 + -5 + 2 = 17 -- via its own ``needs:`` on those four jobs plus an explicit per-dependency -check (``unit-tests.yml:1337-1391``), which accepts them as ``skipped`` only -on a docs-only pull request, where ``project-tests`` -- every test that -reads ``docs/`` or Markdown -- must pass instead. The five ``Compat`` contexts stay required -exactly as they are and are listed separately in the table above. This job -runs on Unit Tests' own ``push``/``pull_request`` triggers; the -``gate-only: true`` reusable calls documented above skip it via -``if: ${{ always() && inputs.gate-only != true }}`` (``unit-tests.yml:1340``), so it is never -produced -- and never expected -- on those paths. +``unit-tests.yml:1235`` mentions the string without defining it. + +``Required checks passed`` is an aggregate, not a single check run. The ruleset +requires it and the five ``Compat Python 3.10``-``3.14`` legs, nothing else, as +``unit-tests.yml``'s own comment (``unit-tests.yml:1234-1244``) records. It +``needs:`` the gating jobs of ``unit-tests.yml`` -- ``test``, ``integration``, +``engine-tests`` and ``pypcap-parity``, plus ``changes`` and ``project-tests`` +for the docs-only path -- and checks each result explicitly +(``unit-tests.yml:1319-1373``). Rulesets match check names literally, with no +wildcard, so requiring the matrix cells one by one would mean editing the +ruleset whenever a matrix changes; the aggregate keeps the required list fixed. +It accepts the four legs as ``skipped`` only on a docs-only pull request, where +``project-tests`` -- every test that reads ``docs/`` or Markdown -- must pass +instead. The ``Compat`` legs stay required separately because they come from +``python-compatibility.yml``, and a job cannot ``needs:`` a job in another +workflow file. This job runs on Unit Tests' own ``push``/``pull_request`` +triggers; the ``gate-only: true`` reusable calls documented above skip it via +``if: ${{ always() && inputs.gate-only != true }}`` (``unit-tests.yml:1322``), +so it is never produced -- and never expected -- on those paths. ``Compat Python 3.10``-``3.14`` are five ordinary matrix legs, not an aggregate: ``python-compatibility.yml``'s own comment (``:37-40``) and diff --git a/pyproject.toml b/pyproject.toml index 8e4e714f8..b921684c3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -278,7 +278,7 @@ test = [ # test_xdist_run_reports_the_diagnostic_and_fails goes red rather than # skipped, since `importlib.util.find_spec('xdist')` still succeeds. "pytest-xdist>=3.6.1", - # A per-test backstop (#1052): a hung test fails by name, with every + # A per-test backstop: a hung test fails by name, with every # thread's stack, instead of running until the job's ``timeout-minutes`` # cancels it with nothing in the log. The value and method live in # [tool.pytest.ini_options] below. Unpinned, like ``isort`` below, rather @@ -360,14 +360,15 @@ testpaths = [ python_files = [ "test_*.py", ] -# pytest-timeout (#1052). 900s per test is a hang backstop, not a speed limit: +# pytest-timeout. 900s per test is a hang backstop, not a speed limit: # the slowest test measured locally was 21s (in test_pyshark_encap_map.py), so # even the 10-15x slowdown seen on CI (315s) leaves it about 3x short, while a # hang still fails by name well inside the jobs' 45-minute cap. ``thread``, not # the POSIX default ``signal``: a pending SIGALRM fails # TimeLimitTests.test_nothing_is_re_armed_when_nothing_was_pending, measured. -# ``thread`` leaks a pcapkit generation per test unless tests/conftest.py's -# pytest_timeout_cancel_timer hook drops the retained timer. +# ``thread`` keeps each test's timer for the session, pinning the pcapkit +# generation live when it started, unless tests/conftest.py's +# pytest_timeout_cancel_timer hook drops it. timeout = 900 timeout_method = "thread" diff --git a/tests/project/test_coverage_rcfile.py b/tests/project/test_coverage_rcfile.py index 0203aa974..f87cf0b6d 100644 --- a/tests/project/test_coverage_rcfile.py +++ b/tests/project/test_coverage_rcfile.py @@ -4,8 +4,8 @@ #1063: the coverage leg of :file:`.github/workflows/unit-tests.yml` runs with ``--rcfile=.github/coverage.toml``, and coverage reads exactly one config file. So the ``[tool.coverage.*]`` tables in :file:`pyproject.toml` are *not* read on -that run; the rcfile has to repeat them, plus the two keys the xdist run needs -(``parallel`` and ``patch``). A setting changed in :file:`pyproject.toml` alone +that run; the rcfile has to repeat them, plus the keys only the CI run needs +(``parallel``, ``patch`` and ``core``). A setting changed in :file:`pyproject.toml` alone would silently not apply in CI, which is the drift asserted against here. """ diff --git a/tests/project/test_unit_tests_queue.py b/tests/project/test_unit_tests_queue.py index 852484715..9907e7bfe 100644 --- a/tests/project/test_unit_tests_queue.py +++ b/tests/project/test_unit_tests_queue.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- """Tests for #1052's queue cuts in :file:`.github/workflows/unit-tests.yml`. -Three things there fail silently if they drift, so they are pinned here: +These fail silently if they drift, so they are pinned here: * ``required-checks`` may accept a ``skipped`` leg **only** when ``changes`` classified the diff as docs-only. Its shell step is executed below against diff --git a/util/run_unittest_leg.py b/util/run_unittest_leg.py index 378981dee..a60fecc85 100644 --- a/util/run_unittest_leg.py +++ b/util/run_unittest_leg.py @@ -100,25 +100,24 @@ because it is already run whole, under :program:`pytest`, by the ``integration`` job. -GitHub issue #1052 -- a stalled leg: ``--stall-dump SECONDS`` (or -``PCAPKIT_UNITTEST_STALL_DUMP``) arms :func:`faulthandler.dump_traceback_later` -so that a leg still running after that long writes every thread's stack to -stderr once, then carries on. The default, 1080s, is just under a 20-minute step -cap, so a leg that is about to be killed says where it was first; ``0`` disables -it. - -#1052 also found the memory growth behind the slow ``test_mh_unit`` tail. Every -purge-then-import leaves the previous generation as cyclic garbage, and the -interpreter's own collector intermittently falls behind: three plain runs of -``protocols/internet`` peaked at 1008, 781 and 1650 MiB, the last with ``test_mh_unit`` -tests at up to 7.3s rather than 1.2-1.8s. ``--gc-every N`` (default 10) therefore -runs :func:`gc.collect` after every N tests: 491 MiB peak at the same 420s wall. -Every test cost 80s more for 320 MiB. - -It is a collect, deliberately *not* the :data:`sys.modules` restore #1052 first -proposed. The loader imports every module before any test runs, so a module's -import-time bindings belong to the generation live at load; restoring that -snapshot hands it back, which is the very skew this leg exists to expose. On +A stalled leg: ``--stall-dump SECONDS`` (or ``PCAPKIT_UNITTEST_STALL_DUMP``) +arms :func:`faulthandler.dump_traceback_later`, so a leg still running after +that long writes every thread's stack to stderr once, then carries on. The +default, 1080s, is just under the 20-minute step cap, so a leg about to be +killed says where it was first; ``0`` disables it. + +Memory: every purge-then-import leaves the previous generation as cyclic +garbage, and the interpreter's own collector intermittently falls behind -- +three plain runs of ``protocols/internet`` peaked at 1008, 781 and 1650 MiB, the +last with ``test_mh_unit`` tests at up to 7.3s rather than 1.2-1.8s. So +``--gc-every N`` (default 10) runs :func:`gc.collect` after every N tests: +491 MiB peak at the same 420s wall. Collecting after every test cost 80s more +for 320 MiB. + +It is a collect, deliberately *not* a :data:`sys.modules` restore. The loader +imports every module before any test runs, so a module's import-time bindings +belong to the generation live at load; restoring that snapshot hands it back, +which is the very skew this leg exists to expose. On #981's own reproduction (``32bcfba15^``, ``test_http_unit`` then ``test_base_class_contract``) no restore gives 1 failure and 3 errors; restoring after each module, or around each test, gives 0 and 0; a collect after each test @@ -164,14 +163,14 @@ #: this script exists to catch. _EXCLUDED_ROOT_MODULES = frozenset({'tests.test_tier_guard_xdist'}) -#: Seconds before a still-running leg dumps every thread's stack (#1052): just +#: Seconds before a still-running leg dumps every thread's stack: just #: under the 20-minute step cap, so the dump lands before the kill. DEFAULT_STALL_DUMP = 1080 #: Environment override for :data:`DEFAULT_STALL_DUMP`; ``--stall-dump`` wins. STALL_DUMP_ENV = 'PCAPKIT_UNITTEST_STALL_DUMP' -#: Run a full :func:`gc.collect` after every this many tests (#1052); see the +#: Run a full :func:`gc.collect` after every this many tests; see the #: module docstring for the measurements behind it. ``--gc-every 0`` disables. DEFAULT_GC_EVERY = 10