From c7a047efc2e6e3e55d7705f5e47cb46e39508a96 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Mon, 5 Oct 2026 23:43:03 -0400 Subject: [PATCH] ci: cut queue pressure in unit-tests (#1052) - engine-tests: fold the 36 Python x engine jobs into one job per engine that loops over 3.10-3.15 in fresh venvs; 3.15 is a step with continue-on-error. - changes/project-tests: classify PR diffs; a docs-only PR skips test, integration, engine-tests and pypcap-parity and runs tests/project instead. - required-checks: accept skipped legs only when changes reports a docs-only diff, and then require project-tests. - unittest-ordering: off pull requests; --verbose and a 20-minute step cap on its run step. - apt steps: 5-minute cap with one bounded retry of update+install. - docs: refresh the unit-tests.yml citations in workflows.rst and releasing.rst; add tests/project/test_unit_tests_queue.py. --- .github/workflows/unit-tests.yml | 778 ++++++++++++++----------- docs/source/contributing/releasing.rst | 4 +- docs/source/contributing/workflows.rst | 45 +- tests/project/test_unit_tests_queue.py | 575 ++++++++++++++++++ 4 files changed, 1029 insertions(+), 373 deletions(-) create mode 100644 tests/project/test_unit_tests_queue.py diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 8e265261bb..c06e8017f6 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -48,7 +48,8 @@ jobs: # and skipped the gate everywhere. An input is the only reliable way for a # reusable workflow to know how it was invoked. name: Python ${{ matrix.python-version }} - if: ${{ inputs.gate-only != true }} + if: ${{ inputs.gate-only != true && needs.changes.outputs.code != 'false' }} + needs: [changes] runs-on: ubuntu-latest timeout-minutes: 45 strategy: @@ -218,7 +219,8 @@ jobs: integration: name: Integration Python ${{ matrix.python-version }} - if: ${{ inputs.gate-only != true }} + if: ${{ inputs.gate-only != true && needs.changes.outputs.code != 'false' }} + needs: [changes] runs-on: ubuntu-latest timeout-minutes: 45 strategy: @@ -329,32 +331,12 @@ jobs: echo "Fixture-dependent selection: $selection" python -m pytest -q -n auto --dist load $selection - # Per-engine coverage for the third-party capture engines -- rebuilt in #845 - # as a genuine two-dimension matrix (Python version x engine), replacing the - # single "install everything, run the unit tier once per Python version" job - # #751 landed. That shape had a real gap #845 measured on current `main`: - # PyPCAPFile's own "python_version < '3.12'" marker (pyproject.toml) meant - # editable-installing the old job's combined extras line silently resolved - # PyPCAPFile to nothing for that extra on 3.12/3.13/3.14 (deliberately not - # spelled out as the literal `pip install` form: tests/_dependency_gates.py's - # job_sections() slices on `^ ([\w-]+):`, so a comment sitting between two - # jobs is textually part of the one above it, and that guard's pytest_jobs() - # requires exactly one such literal per job section -- a second one here, - # even in a comment, makes it think this job has two install lines and - # cannot tell which extras the run would have. See #845's cross-review.), - # so "Engines Python 3.12/3.13/3.14" reported - # green while never exercising PyPCAPFile at all on three of five legs -- - # yet all three were promoted to ruleset 23497679's required checks anyway - # (finding 3), on the reasoning that the matrix rebuild would close finding - # 2 rather than leaving it hidden behind a passing gate. - # - # The maintainer's own ruling (issue #845) is what sets this job's shape: - # one leg per (Python version, engine) pair rather than one per Python - # version installing every engine together, so a failure -- or a correctly - # asserted decline -- names the one engine responsible instead of an - # ambiguous "Engines Python 3.12". Each cell resolves to one of three - # outcomes, tracked via the matrix's own `expect` field (defaulting to - # `supported` for any (python, engine) pair not listed in `include` below): + # Per-engine coverage for the third-party capture engines (#845): every + # (Python version, engine) pair is checked, and a failure -- or a correctly + # asserted decline -- names the one engine and interpreter responsible. Each + # pair ("cell") resolves to one of three expected outcomes, decided by the + # cell script's `expect` case below (anything not listed there is + # `supported`): # # * `supported` -- the engine installs via its normal pyproject.toml # extra and its dedicated unit-tier tests must @@ -363,7 +345,7 @@ jobs: # its own PYTHON_CEILING means it declines at run # time (PyShark on 3.14+; PyPCAPFile on 3.12+, whose # pip marker would otherwise stop it ever installing - # here -- so this job installs the raw distribution + # here -- so the cell installs the raw distribution # past that marker, deliberately, rather than via the # extra). The same test files run; their own # version-aware assertions (e.g. @@ -374,61 +356,53 @@ jobs: # a runtime refusal (no wheel; pcap.c does not # compile against the 3.12+ C API -- PyPCAP carries # no PYTHON_CEILING at all, so there is no decline to - # assert). This job still installs the raw + # assert). The cell still installs the raw # distribution past pyproject.toml's marker to prove # the failure is real rather than assumed, and treats - # that failure as the expected, passing outcome for - # the cell -- distinctly logged via `::notice`, never - # silently green and never red for the wrong reason. - # If it ever starts building, the job fails loudly - # (see the install step) rather than reporting a - # vacuous pass. + # that failure as the expected, passing outcome -- + # distinctly logged via `::notice`, never silently + # green and never red for the wrong reason. If it + # ever starts building, the cell fails loudly. # - # Scoped deliberately out of this rebuild: the "and integ, if any" half of - # the ruling. DPKT and PyShark also have integration-tier coverage - # (tests/integration/test_engine_parity.py, test_engine_runtime.py) and - # PyPCAP has tests/foundation/engines/test_new_engine_parity_runtime.py, but - # all three need example captures regenerated first (examples/generators/), - # which itself needs Scapy and DPKT together -- awkward to do per single- - # engine cell without installing engines this job is deliberately keeping - # apart. That tier stays covered in aggregate by the `integration` and - # `pypcap-parity` jobs above rather than being duplicated per engine here; - # worth the maintainer's separate call if per-engine integration coverage is - # wanted too. + # #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 + # 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. # - # Also scoped out: the multi-engine HAS_RUNTIME gate - # (tests/foundation/engines/test_runtime_engines.py) that the old job - # exercised as a side effect of installing DPKT+Scapy+PyShark together, and - # the incidental crypto/NGAP/vendor coverage (#738) the old job carried on - # this leg because it happened to be the one non-required-check job touching - # every Python version. Splitting by engine removes the "everything - # installed at once" venv those relied on; both are pre-existing coverage - # this rebuild does not restore, and are unaffected by anything already - # required, but are worth the maintainer's attention -- not silently - # dropped, but not solved here either. + # 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 + # `continue-on-error: true`, so a 3.15 failure shows as a failed step and an + # error annotation while the job -- and `required-checks`, which reads this + # job's result -- stays green. The 3.10-3.14 cells share one step, and any + # one of them failing fails the job. # - # `include` only needs to name the cells that are NOT `supported`; every - # other (python-version, engine) pair defaults to it, per the install step's - # `${EXPECT:-supported}`. + # Scoped deliberately out (#845, unchanged by #1052): 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 + # carried. engine-tests: - name: Engines Python ${{ matrix.python-version }} (${{ matrix.engine }}) - if: ${{ inputs.gate-only != true }} + name: Engines (${{ matrix.engine }}) + if: ${{ inputs.gate-only != true && needs.changes.outputs.code != 'false' }} + needs: [changes] runs-on: ubuntu-latest timeout-minutes: 30 - # 3.15 is experimental-prerelease per #845's ruling: present so a - # regression is visible, but never blocking -- a failure on this leg - # reports without failing the run or the required-check set. - continue-on-error: ${{ matrix.python-version == '3.15' }} + env: + ENGINE: ${{ matrix.engine }} strategy: fail-fast: false matrix: - python-version: - - "3.10" - - "3.11" - - "3.12" - - "3.13" - - "3.14" - - "3.15" engine: - DPKT - Scapy @@ -436,164 +410,146 @@ jobs: - PyPCAPFile - PyPCAP - PCAP_CT - include: - # PyShark's PYTHON_CEILING (pcapkit/foundation/engines/pyshark.py) - # is (3, 14): no pip marker stops it installing here, it just - # declines once constructed. - - { python-version: "3.14", engine: PyShark, expect: unsupported } - - { python-version: "3.15", engine: PyShark, expect: unsupported } - # PyPCAPFile's PYTHON_CEILING is (3, 12) -- see - # pcapkit/foundation/engines/pypcapfile.py. Its own pyproject.toml - # marker ("python_version < '3.12'") is exactly what #845 finding 2 - # is about: it stops the extra installing here at all, which is why - # the install step below reaches for the raw distribution on these - # four legs instead. - - { python-version: "3.12", engine: PyPCAPFile, expect: unsupported } - - { python-version: "3.13", engine: PyPCAPFile, expect: unsupported } - - { python-version: "3.14", engine: PyPCAPFile, expect: unsupported } - - { python-version: "3.15", engine: PyPCAPFile, expect: unsupported } - # PyPCAP carries no PYTHON_CEILING -- pyproject.toml's own - # "python_version < '3.12'" marker is the only thing that keeps pip - # from attempting the build here, because the build itself fails - # (no wheel; pcap.c does not compile against the 3.12+ C API). - - { python-version: "3.12", engine: PyPCAP, expect: not-installable } - - { python-version: "3.13", engine: PyPCAP, expect: not-installable } - - { python-version: "3.14", engine: PyPCAP, expect: not-installable } - - { python-version: "3.15", engine: PyPCAP, expect: not-installable } steps: - uses: actions/checkout@v7 + # All six interpreters in one step; each is then reachable as + # `python3.X`. setup-python does not ship 3.15 as a release build yet, so + # `allow-prereleases` is needed for it to resolve at all (as in + # python-compatibility.yml's compatibility-nightly job); for 3.10-3.14 a + # GA release exists and wins. - uses: actions/setup-python@v7 with: - python-version: ${{ matrix.python-version }} - # setup-python does not ship a 3.15 interpreter as a release build - # yet; python-compatibility.yml's compatibility-nightly job already - # established that 3.15 needs this flag to resolve at all. - allow-prereleases: ${{ matrix.python-version == '3.15' }} - cache: pip - - # Only the engines that need a system package get one, rather than - # installing tshark/libpcap/a C toolchain on every cell regardless of - # which engine it is testing. + python-version: | + 3.10 + 3.11 + 3.12 + 3.13 + 3.14 + 3.15 + allow-prereleases: true + + # Only the engines that need a system package get one: # - # tshark: PyShark is a wrapper around it and does no parsing of its - # own -- see the debconf pre-seed note below for why the two apt lines - # are shaped the way they are. + # tshark: PyShark is a wrapper around it and does no parsing of its own. + # The debconf pre-seed stops tshark's postinst blocking on the "allow + # non-superusers to capture packets" prompt. # # libpcap0.8: PCAP_CT's ctypes loader calls find_library("pcap") at run # time; without it, PCAP_CT.unsupported_reason() degrades the engine to # the default parser instead of running it. # # build-essential + libpcap-dev: PyPCAP is the one engine that compiles - # (pcap.c against libpcap's headers), on every leg -- including the + # (pcap.c against libpcap's headers), on every cell -- including the # `not-installable` ones, deliberately, so that failure is a genuine # compile failure against the 3.12+ C API rather than merely "no # compiler was present to try". # # `timeout-minutes` bounds this step because an apt stall in it used to - # eat the whole job. Per #974, on two docs-only PRs within forty minutes - # a PCAP_CT cell fetched its InRelease metadata and then went silent for - # 29m52s, until this job's own `timeout-minutes: 30` killed it -- which - # surfaces as `cancelled`, and `cancelled` correctly fails the - # `Required checks passed` gate, so an unrelated mirror turned a green - # docs PR red. The other five PCAP_CT cells of that same run finished, - # job and all, in 39-81 seconds. + # 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`. # - # 20 minutes rather than the couple of minutes that bounding a stalled - # mirror tightly would suggest, because the healthy tail here is - # genuinely long. Across 12 runs (230 non-zero observations of this - # step) p95 was 338s and the slowest *successful* run 508s, both on - # PyShark cells; this change's own CI then produced a healthy 646s - # PyShark run, so treat those figures as a floor on the tail rather - # than its ceiling -- tshark plus wireshark-common is a large download - # and archive.ubuntu.com's throughput varies enormously. A 5-minute cap - # would have failed ~4% of healthy legs, which is a worse defect than - # the one it fixes. 20 is ~1.9x the largest healthy run yet seen, and - # was raised from an initial 15 on review: that 646s observation moved - # the known maximum 27% on a single sample, and a bound sized at ~1.4x - # a quantity still moving that much risks reproducing this very defect - # at smaller scale -- a healthy-but-slow run killed and reported as a - # step timeout, still blocking the pull request. 20 stays under both - # job budgets (30 here, 45 on `pypcap-parity`), so the headroom is - # free. A hang costs 20 minutes instead of 30 and - # -- the actual win -- reports as this named step exceeding its timeout - # rather than as an opaque job cancellation with no attributable cause. + # 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 + # `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 + # budget, so a hang reports as this named step timing out rather than as + # an opaque job cancellation. `dpkg --configure -a` before the second + # attempt recovers an install killed mid-configure. # - # Deliberately *not* also passing `-o Acquire::Retries=3 -o + # Deliberately *not* passing `-o Acquire::Retries=3 -o # Acquire::http::Timeout=30`, which #974 proposed: both are already the - # defaults in the apt that noble ships (verified in 2.8.3, the current - # noble-updates version; 2.7.14's tarball is no longer in the pool -- - # `Retries(_config->FindI("Acquire::Retries", 3))` in - # apt-pkg/acquire-item.cc, and `TimeOut(30)` in methods/basehttp.cc, - # read back by `ConfigFindI("Timeout", TimeOut)` in methods/http.cc), so - # passing them would change no behaviour while reading like protection - # this step had gained. Their already being in force is also why the - # stall ran to 29m52s instead of failing after 30s and three retries: - # whatever hung is not something apt's data timeout governs, since that - # value bounds each individual `select()` wait rather than the transfer - # as a whole. Wrapping these invocations in `timeout` and retrying is no - # safer at these numbers, because a legitimate 646s run cannot be told - # from a stall inside any per-attempt budget short enough to be worth - # retrying -- the step timeout below is the one guard that holds however - # the hang is caused. + # defaults in noble's apt (2.8.3: `Acquire::Retries` 3 in + # apt-pkg/acquire-item.cc, `TimeOut(30)` in methods/basehttp.cc), so they + # would change nothing while reading like protection. Their being in + # force is also why the #974 stall ran to 29m52s: whatever hung is not + # something apt's per-`select()` data timeout governs, which is why the + # bound here is `timeout(1)` around the whole command. - name: Install this engine's system packages - timeout-minutes: 20 + timeout-minutes: 5 run: | set -eu - ENGINE="${{ matrix.engine }}" + apt_install() { + for attempt in 1 2; do + if sudo timeout 45 apt-get update \ + && sudo DEBIAN_FRONTEND=noninteractive timeout 90 apt-get install -y --no-install-recommends "$@"; then + return 0 + fi + echo "::warning title=apt attempt $attempt of 2 failed (${ENGINE})::apt-get update or install failed or exceeded its per-attempt bound; retrying once." + sudo dpkg --configure -a || true + done + return 1 + } if [ "$ENGINE" = "PyShark" ]; then - sudo apt-get update echo "wireshark-common wireshark-common/install-setuid boolean false" | sudo debconf-set-selections - sudo DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends tshark + apt_install tshark elif [ "$ENGINE" = "PCAP_CT" ]; then - sudo apt-get update - sudo apt-get install -y --no-install-recommends libpcap0.8 + apt_install libpcap0.8 elif [ "$ENGINE" = "PyPCAP" ]; then - sudo apt-get update - sudo apt-get install -y --no-install-recommends build-essential libpcap-dev + apt_install build-essential libpcap-dev fi + # The per-cell script: one (Python version, engine) cell, start to + # finish, in a venv of its own. Written to a file so each cell runs as a + # separate `bash` process -- `set -e` is suspended inside a function + # called from an `if`, so an inline function would let a failed install + # carry on silently. + # # `expect` decides not just the assertion but *how* the engine gets # installed: `supported` uses the ordinary pyproject.toml extra, whose # own marker (if any) already resolves to "install" on this Python # version; `unsupported` and `not-installable` install the raw # distribution directly, deliberately bypassing whichever marker would - # otherwise have skipped it silently -- see this job's own comment - # above for why that bypass is the point of this rebuild. + # otherwise have skipped it silently (#845 finding 2). # # `set +e` is scoped to the one install command whose failure is - # sometimes expected, not to the baseline above it: upgrading pip - # itself and editable-installing this repo's own `test` extra failing - # is never expected on any cell (deliberately not spelled out as the - # literal `pip install` form below -- see this job's own comment - # earlier in this file, and tests/_dependency_gates.py's - # job_sections()/pytest_jobs(), for why a second such literal in a - # comment here would make that guard think this section holds two - # baseline install lines), and running them unchecked used to mean a - # broken base install on a `not-installable` cell read as "expected" - # while the job had installed and run nothing at all (#845's - # cross-review). They now run under this step's own explicit `set -eu` - # below -- not "the shell's default", since GitHub's default for a - # Linux `run:` step is plain `bash -e {0}` with no `pipefail`, and - # this step never sets `shell: bash` to change that -- so either - # failing aborts the step immediately and loudly. - - name: Install package and this engine's dependency - id: install + # sometimes expected. Upgrading pip and editable-installing this repo's + # `test` extra failing is never expected on any cell, so those run under + # the script's `set -eu` (#845's cross-review: running them unchecked + # once let a broken base install on a `not-installable` cell read as + # "expected"). + # + # Runs the SAME test files whether `expect` is `supported` or + # `unsupported`: the difference is in what those files assert once they + # see this interpreter's version (PyPCAPFile's + # test_unsupported_reason_tracks_the_running_interpreter, PyShark's + # test_the_reason_tracks_the_running_interpreter), which is what proves + # the decline fired. `not-installable` cells run no tests. + - name: Write the per-cell script run: | + cat > "$RUNNER_TEMP/engine-cell.sh" <<'CELL' set -eu + PY="$1" + CELL_NAME="$ENGINE on Python $PY" + + case "$ENGINE:$PY" in + PyShark:3.14|PyShark:3.15) EXPECT=unsupported ;; + PyPCAPFile:3.12|PyPCAPFile:3.13|PyPCAPFile:3.14|PyPCAPFile:3.15) EXPECT=unsupported ;; + PyPCAP:3.12|PyPCAP:3.13|PyPCAP:3.14|PyPCAP:3.15) EXPECT=not-installable ;; + *) EXPECT=supported ;; + esac + echo "Cell: $CELL_NAME, expecting '$EXPECT'" + + "python$PY" -m venv "$RUNNER_TEMP/venv-$PY" + # shellcheck disable=SC1090 + . "$RUNNER_TEMP/venv-$PY/bin/activate" + python --version + python -m pip install -U pip setuptools wheel python -m pip install -e '.[test]' - ENGINE="${{ matrix.engine }}" - EXPECT="${{ matrix.expect }}" - EXPECT="${EXPECT:-supported}" - # RAW_PKGS is an array, not a string, so PCAP_CT's two-package case - # ("pcap-ct libpcap") reaches pip as two argv entries rather than by - # relying on unquoted word-splitting (shellcheck SC2086) -- quoting - # the string form instead would hand pip one invalid "pcap-ct - # libpcap" argument and break exactly that case. + # ("pcap-ct libpcap") reaches pip as two argv entries. case "$ENGINE" in DPKT) EXTRA=DPKT; RAW_PKGS=(dpkt); IMPORT_NAME=dpkt ;; Scapy) EXTRA=Scapy; RAW_PKGS=(scapy); IMPORT_NAME=scapy ;; @@ -614,89 +570,42 @@ jobs: set -e echo "$output" - echo "status=$status" >> "$GITHUB_OUTPUT" - if [ "$status" -ne 0 ] && [ "$EXPECT" != "not-installable" ]; then - echo "::error title=$ENGINE failed to install on Python ${{ matrix.python-version }}::This cell expects '$EXPECT', which requires a successful install. pip exited $status." + echo "::error title=$ENGINE failed to install on Python $PY::This cell expects '$EXPECT', which requires a successful install. pip exited $status." exit 1 fi if [ "$status" -eq 0 ] && [ "$EXPECT" = "not-installable" ]; then - echo "::error title=$ENGINE now installs on Python ${{ matrix.python-version }}::This cell is recorded as not-installable in this workflow's include: list (and in pyproject.toml's marker for $ENGINE). It installed cleanly -- update both rather than leaving this green for the wrong reason." + echo "::error title=$ENGINE now installs on Python $PY::This cell is recorded as not-installable in this job's expect case (and in pyproject.toml's marker for $ENGINE). It installed cleanly -- update both rather than leaving this green for the wrong reason." exit 1 fi if [ "$status" -ne 0 ]; then - # A not-installable cell's whole point is a genuine C-API compile - # failure (see this job's own comment above), not a flaky index - # or a dropped connection -- treating *any* nonzero exit as that - # expected failure would let a PyPI 503 pass as though the - # engine had been proven unbuildable. Grep the captured output - # for the shape of a transient failure instead of a compile one. - # - # The transient list is pip's own retry/network vocabulary, not a - # bare "5xx or 429" number scan: pip prints a "(NNN kB)" download - # size on essentially every sdist fetch, and a Cython-generated - # .c file runs to thousands of lines, so an unanchored bare-number - # match (#845's cross-review) fires on ordinary download-size - # lines and on compiler diagnostics quoting a line number in that - # range -- e.g. "Downloading pypcap-1.3.0.tar.gz (500 kB)" and - # "pcap.c:501:24: error: ..." both matched, while the actual - # compile-failure markers matched nothing. A cell that hits that - # false positive would exit 1 on a transcript no re-run can ever - # change, so this is a loud, permanent false red, not a silent - # one -- see this job's own comment on "fails loudly in both - # directions" above for why that distinction still matters: a - # loud false red still blocks every PR just as surely as a - # silent false green would. Requiring the *absence* of a compile - # marker alongside a transient one is the second half of the - # fix: a message that happens to carry both is treated as the - # compile failure it also is, not the transient one it merely - # resembles. + # A not-installable cell's point is a genuine C-API compile + # failure, not a flaky index: grep for pip's own retry/network + # vocabulary, and require the *absence* of a compile marker + # alongside it. A bare "5xx or 429" number scan is not used, + # because pip prints "(NNN kB)" download sizes and compiler + # diagnostics quote line numbers in that range (#845's + # cross-review), which would make a permanent false red. if echo "$output" | grep -qiE \ 'read timed out|connection (reset|refused|aborted)|temporary failure in name resolution|max retries exceeded|could not fetch url|newconnectionerror|readtimeouterror|httpsconnectionpool|retrying \(retry\(|http error (5[0-9][0-9]|429)|status code 429|too many 429 error responses|no matching distribution found' \ && ! echo "$output" | grep -qiE \ 'failed building wheel|error: command .* failed with exit code|subprocess-exited-with-error'; then - echo "::error title=$ENGINE's install failure looks transient, not a compile failure::pip exited $status, but the output matches a network/index failure pattern rather than the C-API compile failure this cell records as expected. Re-run the job rather than trusting this as a genuine 'not-installable' result." + echo "::error title=$ENGINE's install failure on Python $PY looks transient, not a compile failure::pip exited $status, but the output matches a network/index failure pattern rather than the C-API compile failure this cell records as expected. Re-run the job rather than trusting this as a genuine 'not-installable' result." exit 1 fi - echo "::notice title=$ENGINE is not installable on Python ${{ matrix.python-version }} (expected)::pip exited $status, matching this cell's recorded expectation ('not-installable'), and the output does not match a transient-failure pattern. No tests run for this cell -- this is a deliberate skip, not a pass." + echo "::notice title=$ENGINE is not installable on Python $PY (expected)::pip exited $status, matching this cell's recorded expectation ('not-installable'), and the output does not match a transient-failure pattern. No tests run for this cell -- this is a deliberate skip, not a pass." + echo "| $PY | $ENGINE | $EXPECT | not installable (expected) |" >> "$GITHUB_STEP_SUMMARY" + exit 0 fi - # #845 finding 2, reproduced structurally rather than left to be - # re-found by enumeration: a `supported` cell whose extra resolves - # to nothing (an unmet environment marker, a typo'd extra name) - # still exits 0, and every one of this cell's tests would then - # skip rather than fail, going green having proven nothing. This - # cell's own point is exercising $IMPORT_NAME, so require it. - if [ "$status" -eq 0 ]; then - if ! python -c "import $IMPORT_NAME" 2>/dev/null; then - echo "::error title=$ENGINE installed but $IMPORT_NAME does not import on Python ${{ matrix.python-version }}::pip exited 0 but the module this cell exists to exercise is not importable -- a green install that unlocked nothing." - exit 1 - fi + # #845 finding 2: a `supported` cell whose extra resolves to nothing + # still exits 0 and every test would then skip, so require the + # module this cell exists to exercise. + if ! python -c "import $IMPORT_NAME" 2>/dev/null; then + echo "::error title=$ENGINE installed but $IMPORT_NAME does not import on Python $PY::pip exited 0 but the module this cell exists to exercise is not importable -- a green install that unlocked nothing." + exit 1 fi - # See the `test` job above for why this step exists. - - name: Report available parallelism - if: ${{ steps.install.outputs.status == '0' }} - run: | - nproc - python -c "import os; print('cpu_count', os.cpu_count())" - - # Runs the SAME test files whether `expect` is `supported` or - # `unsupported`: the difference is not in what runs here but in what - # those files themselves assert once they see this interpreter's - # version, e.g. PyPCAPFile's own - # test_unsupported_reason_tracks_the_running_interpreter and PyShark's - # test_the_reason_tracks_the_running_interpreter (added for PyShark's - # real-capture coverage) both branch on sys.version_info against the - # engine's own PYTHON_CEILING. - # That is what proves the decline actually fired and named the - # interpreter, rather than this job asserting it from the outside. - # Skipped entirely for `not-installable` cells -- see the install step. - - name: Run this engine's test suite - if: ${{ steps.install.outputs.status == '0' }} - run: | - set -eu - ENGINE="${{ matrix.engine }}" case "$ENGINE" in DPKT) TEST_PATHS="tests/toolkit/test_dpkt_unit.py" ;; Scapy) TEST_PATHS="tests/toolkit/test_scapy_unit.py tests/foundation/engines/test_scapy_engine.py tests/protocols/transport/test_sctp_unit.py" ;; @@ -706,7 +615,58 @@ jobs: PCAP_CT) TEST_PATHS="tests/toolkit/test_pcap_ct_unit.py tests/foundation/engines/test_pcap_ct_engine.py tests/foundation/engines/test_pcap_backend.py" ;; esac echo "Running: $TEST_PATHS" - python -m pytest -q -n auto --dist load $TEST_PATHS + # shellcheck disable=SC2086 + if ! python -m pytest -q -n auto --dist load $TEST_PATHS; then + echo "::error title=$ENGINE tests failed on Python $PY::See the 'Python $PY ($ENGINE)' group in this step's log." + exit 1 + fi + echo "| $PY | $ENGINE | $EXPECT | passed |" >> "$GITHUB_STEP_SUMMARY" + CELL + { + echo "### Engine cells: $ENGINE" + echo + echo "| Python | Engine | Expect | Result |" + echo "| --- | --- | --- | --- |" + } >> "$GITHUB_STEP_SUMMARY" + + # See the `test` job above for why this step exists. + - name: Report available parallelism + run: | + nproc + python -c "import os; print('cpu_count', os.cpu_count())" + + # Every cell runs even after one fails, so one red run reports all of + # them; the step fails at the end if any did. + - name: Run the 3.10-3.14 cells + shell: bash + run: | + failed=() + for py in 3.10 3.11 3.12 3.13 3.14; do + echo "::group::Python $py ($ENGINE)" + if ! bash "$RUNNER_TEMP/engine-cell.sh" "$py"; then + failed+=("$py") + echo "| $py | $ENGINE | | FAILED |" >> "$GITHUB_STEP_SUMMARY" + fi + echo "::endgroup::" + done + if [ "${#failed[@]}" -ne 0 ]; then + echo "::error title=$ENGINE failed on Python ${failed[*]}::See each cell's group in this step's log and the job summary." + exit 1 + fi + + # Non-blocking by #845's ruling -- see this job's comment above. + - name: Run the 3.15 cell (experimental, non-blocking) + shell: bash + continue-on-error: true + run: | + echo "::group::Python 3.15 ($ENGINE)" + if ! bash "$RUNNER_TEMP/engine-cell.sh" 3.15; then + echo "| 3.15 | $ENGINE | | FAILED (non-blocking) |" >> "$GITHUB_STEP_SUMMARY" + echo "::endgroup::" + echo "::error title=$ENGINE failed on Python 3.15 (non-blocking)::3.15 is experimental per #845; this does not fail the job." + exit 1 + fi + echo "::endgroup::" # Whether upstream PyPCAP is worth building in CI at all -- #751's own # instruction was "try to build and if the CI is not a good suit, then we @@ -739,7 +699,8 @@ jobs: # fixture-tier run for zero new coverage. pypcap-parity: name: PyPCAP/PyPCAPFile parity Python ${{ matrix.python-version }} - if: ${{ inputs.gate-only != true }} + if: ${{ inputs.gate-only != true && needs.changes.outputs.code != 'false' }} + needs: [changes] runs-on: ubuntu-latest timeout-minutes: 45 strategy: @@ -770,27 +731,31 @@ jobs: # declines the engine, the extractor silently falls back to the default # parser, and the test's assertions used to pass on that fallback's # output -- a vacuous pass, invisible in skip counts. The debconf - # pre-seed and DEBIAN_FRONTEND mirror `engine-tests`'s "Install system - # libpcap and tshark" step exactly, for the same reason: tshark's + # pre-seed and DEBIAN_FRONTEND mirror `engine-tests`'s "Install this + # engine's system packages" step exactly, for the same reason: tshark's # postinst otherwise blocks on the "allow non-superusers to capture # packets" prompt. # - # `timeout-minutes` for the reason #974 gives about `engine-tests`'s own - # install step -- see the long note there, which this step shares - # wholesale. It was never the leg that stalled, but it installs the - # union of the packages that one does (build-essential, libpcap-dev and - # tshark together) from the same mirrors with the same absence of any - # bound, so the hazard is identical and fixing only the leg that - # happened to bite would leave it here. The same 20 applies: this step's - # slowest *successful* observed run was 405s, over a smaller sample (22) - # than `engine-tests` gives, and its heavier package set should if - # anything tail longer than the 646s seen there. + # `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 + # 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. - name: Install libpcap headers, a C toolchain, and tshark - timeout-minutes: 20 + timeout-minutes: 5 run: | - sudo apt-get update + set -eu echo "wireshark-common wireshark-common/install-setuid boolean false" | sudo debconf-set-selections - sudo DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends build-essential libpcap-dev tshark + for attempt in 1 2; do + if sudo timeout 45 apt-get update \ + && sudo DEBIAN_FRONTEND=noninteractive timeout 90 apt-get install -y --no-install-recommends build-essential libpcap-dev tshark; then + exit 0 + fi + echo "::warning title=apt attempt $attempt of 2 failed::apt-get update or install failed or exceeded its per-attempt bound; retrying once." + sudo dpkg --configure -a || true + done + exit 1 - name: Install package, test and generator dependencies run: | @@ -962,7 +927,10 @@ jobs: # maintainer's call to make separately from landing it. unittest-ordering: name: Plain unittest ordering (${{ matrix.label || matrix.leg }}) - if: ${{ inputs.gate-only != true }} + # 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 + # 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 # 45, not 20: `timeout-minutes` is job-level, so it has to cover # checkout/setup/install (~39s) as well as the leg itself, and this is @@ -1027,8 +995,13 @@ 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 + # 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. - name: Run tests/${{ matrix.leg }} and the root-level modules under plain unittest - run: python util/run_unittest_leg.py ${{ matrix.leg }} ${{ matrix.exclude }} + timeout-minutes: 20 + run: python util/run_unittest_leg.py ${{ matrix.leg }} ${{ matrix.exclude }} --verbose # ``CHANGELOG.md`` is generated from the newest entry under # ``docs/source/changelog/`` by ``util/changelog_md.py``, so it falls out of step @@ -1072,6 +1045,119 @@ jobs: exit 1 fi + # #1052: 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. + # + # A file is docs iff it is under `docs/` or ends in `.md`, and is not under + # `.github/`, `pcapkit/`, `tests/`, `util/` or `examples/` -- so a Markdown + # file in one of those (a PR template, a test fixture) is code, and so is + # every other file (`pyproject.toml`, `setup.py`, ...). The diff is the PR + # merge commit against its first parent, which is exactly what merging would + # add; `--no-renames` lists both sides of a rename, so moving a code file + # under `docs/` is still a code change. + # + # Only this workflow's own `pull_request` run is classified. Every other + # event reports `code: true`, including a `workflow_call` from a caller + # triggered by a pull request: inside a called workflow `github.event_name` + # is the caller's, but `github.workflow` is the caller's name too (see the + # concurrency comment at the top), and no caller is named "Unit Tests". + changes: + name: Classify changed files + if: ${{ inputs.gate-only != true }} + runs-on: ubuntu-latest + timeout-minutes: 5 + outputs: + code: ${{ steps.classify.outputs.code }} + + steps: + - uses: actions/checkout@v7 + if: ${{ github.event_name == 'pull_request' && github.workflow == 'Unit Tests' }} + with: + fetch-depth: 2 + + - name: Classify the diff + id: classify + env: + OWN_PR: ${{ github.event_name == 'pull_request' && github.workflow == 'Unit Tests' }} + run: | + python3 - <<'PY' + import os + import subprocess + + CODE_DIRS = ('.github/', 'pcapkit/', 'tests/', 'util/', 'examples/') + + def is_docs(path): + if path.startswith(CODE_DIRS): + return False + return path.startswith('docs/') or path.endswith('.md') + + code = 'true' + if os.environ['OWN_PR'] != 'true': + print("Not this workflow's own pull_request run: everything runs.") + else: + # -z: names are NUL-terminated, so a name holding whitespace + # (or one git would otherwise quote) stays one path. + diff = subprocess.run(['git', 'diff', '--name-only', '-z', '--no-renames', 'HEAD^1', 'HEAD'], + capture_output=True, text=True) + files = [path for path in diff.stdout.split('\0') if path] + if diff.returncode != 0 or not files: + print(f'::warning title=Could not list the changed files::git diff exited {diff.returncode} ' + f'with {len(files)} files: {diff.stderr.strip()}. Treating the diff as code.') + else: + code_files = [path for path in files if not is_docs(path)] + print(f'{len(files)} changed files, {len(code_files)} of them code:') + for path in files: + print(f" {'code' if path in code_files else 'docs'} {path}") + if not code_files: + code = 'false' + print(f'code={code}') + with open(os.environ['GITHUB_OUTPUT'], 'a', encoding='utf-8') as file: + print(f'code={code}', file=file) + PY + + # #1052: 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 + # finds every module outside those two that names a docs/ or Markdown path + # and fails until it is listed there, so a new reader cannot go unrun. Runs + # only when `changes` says the diff is docs-only, so it never duplicates + # `test` on a code change. It uses + # util/run_unittest_leg.py's `project` leg -- tests/project plus the + # root-level modules, with the extras `unittest-ordering`'s `project` cell + # runs green on main -- rather than a pytest invocation of its own, which + # tests/_dependency_gates.py would then have to model as a new selection + # shape. + project-tests: + name: Project tests (docs-only) + needs: [changes] + if: ${{ inputs.gate-only != true && needs.changes.outputs.code == 'false' }} + runs-on: ubuntu-latest + timeout-minutes: 20 + + steps: + - uses: actions/checkout@v7 + + - uses: actions/setup-python@v7 + with: + python-version: "3.14" + cache: pip + + - name: Install package and test dependencies + run: | + python -m pip install -U pip setuptools wheel + python -m pip install -e '.[test,DPKT,crypto,NGAP]' + + - name: Run tests/project and the root-level modules under plain unittest + timeout-minutes: 15 + run: python util/run_unittest_leg.py project --verbose + + - name: Run the other tests that read docs/ or Markdown + timeout-minutes: 5 + run: python -m unittest -v tests.corekit.test_sentinel_exports_unit + # The verdict a shipping workflow asks for: the full suite, fixtures and # all, on one interpreter. It is deliberately not the matrix -- the matrix # has already run against this very commit, from this workflow's own push or @@ -1187,7 +1273,7 @@ jobs: # the silently-worse-than-no-gate failure this job exists to prevent. # That page's own recommended fix is this job's shape: "Use always() # with needs for required checks that depend on other jobs." - # * `inputs.gate-only != true` mirrors the four jobs this needs: below. + # * `inputs.gate-only != true` mirrors the jobs this needs: below. # They are all skipped by their own `if:` whenever this workflow is # called with `gate-only: true` (from create-release.yml), so without # this guard every needs.*.result below would read `skipped` on that @@ -1217,99 +1303,89 @@ jobs: # 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. + # `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). # - # `engine-tests`'s 6 `unsupported` and 4 `not-installable` cells are not a - # gap here, even though "some matrix cells succeed and others decline" - # sounds like it should risk a false red. It doesn't, because that job's - # own install step (see its comment) deliberately exits 0 for a cell that - # declined exactly as expected, and only skips that cell's own later - # *steps* via a step-level `if:` -- so every legitimately-declined cell - # still contributes `success` to `needs.engine-tests.result`, not - # `skipped`. + # `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. # - # `engine-tests` also carries `continue-on-error: ${{ matrix.python-version - # == '3.15' }}` (its own comment above explains why: 3.15 is experimental - # per #845's ruling, and a failure there must report without failing the - # run or the required-check set -- `python-compatibility.yml` states the - # same policy for its own 3.15 job). Whether a genuine 3.15-cell failure - # still surfaces as `failure` in `needs.engine-tests.result` here, or is - # neutralised to `success` the same way `continue-on-error` neutralises the - # *workflow run's* own conclusion, is NOT stated by GitHub's docs for jobs. - # The `continue-on-error` reference has four sentences: one scoping the - # flag to a single job, one about a failing job letting its matrix - # siblings keep running, two about letting the *workflow run* pass - # despite this job failing -- none of the four say anything about what a - # sibling job's `needs..result` sees. - # Suggestive, not conclusive, but the asymmetry favours caution: the - # Contexts reference documents this exact neutralisation for *steps* - # (`steps..conclusion` is defined as the step's result "after - # `continue-on-error` is applied", explicitly distinct from `.outcome`, - # which stays `failure`) and says nothing of the kind for - # `needs..result`, which lists only - # `success`/`failure`/`cancelled`/`skipped` with no `outcome`/`conclusion` - # split at all -- documented where it applies to steps, silent where the - # same question would apply to jobs. One could still argue `result` is - # simply the job-level analogue of `conclusion` and behaves the same way; - # that argument is evidence for that reading, not proof of it. Requiring - # exactly `success` below therefore carries one open, unverified risk: if - # the undocumented case resolves to `failure`, a 3.15-only engine regression - # would turn this sole required context red and block every merge -- - # worse than the gate it replaces, and directly against #845's ruling that - # 3.15 must never block. + # 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. # - # Two ways to remove the dependence on that undocumented behaviour - # outright, both considered and neither taken here. Moving - # `continue-on-error` from the job level down to the individual steps - # inside `engine-tests` that can fail on a 3.15 leg would close the gap in - # about two lines, but it changes what "non-blocking" means: a step-level - # flag makes a failing 3.15 leg's *step* report `success` too, so the leg - # reads green instead of red -- hiding the regression rather than merely - # declining to block on it, which throws away the "a regression stays - # visible" half of #845's ruling that the current job-level flag - # preserves. Splitting the 3.15 legs into their own, non-`needs:`-ed job - # keeps that visibility, but means duplicating this job's system-package - # install, per-engine install logic, and test invocation -- measured at - # 182 raw lines across those three steps, and GitHub Actions has no YAML - # anchors to shrink that with -- into a second job immediately after #845 - # rebuilt this one, to close a single unverified edge. Flagging the risk - # here instead: if a `required-checks` run ever goes red with only a 3.15 - # `engine-tests` leg failing underneath it, that is this exact gap and not - # a genuine regression -- re-run to confirm, and revisit one of the two - # options above then. - # - # Excepting that one flagged case, there is no other path in this workflow - # where a real dependency legitimately reports anything other than - # `success` on a PR run, so requiring exactly `success` below is otherwise - # the accurate choice, not merely the cautious one. (GitHub's docs never - # spell out how one matrix job's several instances collapse into the - # single result a downstream `needs..result` sees; the reasoning - # above is from how `engine-tests` itself is built, not from a documented - # aggregation rule.) + # #1052's 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 + # instead. A job-level `if:` rather than workflow-level `paths-ignore`, + # because `paths-ignore` would not run this workflow at all and leave this + # required context pending forever. So `skipped` is accepted for those four + # legs **only** when `changes` succeeded and said `code == 'false'`, and then + # `project-tests` must have succeeded. Every other combination keeps the + # strict rule: `changes` failing, its output missing, or any leg skipped + # while `code` is `true` fails this gate. `changes` reports `code: true` for + # every event but this workflow's own `pull_request`, so `push` and + # `workflow_call` runs never take this path. required-checks: name: Required checks passed - needs: [test, integration, engine-tests, pypcap-parity] + needs: [changes, test, integration, engine-tests, pypcap-parity, project-tests] if: ${{ always() && inputs.gate-only != true }} runs-on: ubuntu-latest timeout-minutes: 5 steps: - name: Check every required job succeeded + env: + CHANGES: ${{ needs.changes.result }} + CODE: ${{ needs.changes.outputs.code }} + TEST: ${{ needs.test.result }} + INTEGRATION: ${{ needs.integration.result }} + ENGINE_TESTS: ${{ needs.engine-tests.result }} + PYPCAP_PARITY: ${{ needs.pypcap-parity.result }} + PROJECT_TESTS: ${{ needs.project-tests.result }} run: | set -eu - echo "test: ${{ needs.test.result }}" - echo "integration: ${{ needs.integration.result }}" - echo "engine-tests: ${{ needs.engine-tests.result }}" - echo "pypcap-parity: ${{ needs.pypcap-parity.result }}" + echo "changes: $CHANGES (code: '$CODE')" + echo "test: $TEST" + echo "integration: $INTEGRATION" + echo "engine-tests: $ENGINE_TESTS" + echo "pypcap-parity: $PYPCAP_PARITY" + echo "project-tests: $PROJECT_TESTS" failed=0 - [ "${{ needs.test.result }}" = "success" ] || { echo "::error title=test did not succeed::result was '${{ needs.test.result }}', not 'success'."; failed=1; } - [ "${{ needs.integration.result }}" = "success" ] || { echo "::error title=integration did not succeed::result was '${{ needs.integration.result }}', not 'success'."; failed=1; } - [ "${{ needs.engine-tests.result }}" = "success" ] || { echo "::error title=engine-tests did not succeed::result was '${{ needs.engine-tests.result }}', not 'success'."; failed=1; } - [ "${{ needs.pypcap-parity.result }}" = "success" ] || { echo "::error title=pypcap-parity did not succeed::result was '${{ needs.pypcap-parity.result }}', not 'success'."; failed=1; } + if [ "$CHANGES" != "success" ] || { [ "$CODE" != "true" ] && [ "$CODE" != "false" ]; }; then + echo "::error title=changes did not classify the diff::result was '$CHANGES' and code was '$CODE'; without a classification nothing may be skipped." + CODE=true + failed=1 + fi + + # Docs-only (code == false): a leg may be skipped, and project-tests + # is what must have passed. Otherwise every leg must have succeeded. + if [ "$CODE" = "false" ]; then + allowed="success skipped" + [ "$PROJECT_TESTS" = "success" ] || { echo "::error title=project-tests did not succeed::result was '$PROJECT_TESTS'; on a docs-only diff it is the test that has to pass."; failed=1; } + else + allowed="success" + fi + for pair in "test=$TEST" "integration=$INTEGRATION" "engine-tests=$ENGINE_TESTS" "pypcap-parity=$PYPCAP_PARITY"; do + job="${pair%%=*}" + result="${pair#*=}" + case " $allowed " in + *" $result "*) ;; + *) echo "::error title=$job did not succeed::result was '$result'; accepted here: $allowed (code: '$CODE')."; failed=1 ;; + esac + done if [ "$failed" -ne 0 ]; then - echo "::error title=Required checks did not all succeed::One or more of test, integration, engine-tests, pypcap-parity did not report 'success' -- see the per-job lines above. A skipped or cancelled dependency is not a pass; see this job's own comment for why that distinction is the point of this job existing." + echo "::error title=Required checks did not all succeed::See the per-job lines above. A skipped or cancelled dependency is only a pass on a docs-only diff; see this job's own comment." exit 1 fi - echo "All required jobs reported success." + echo "All required jobs reported an accepted result." diff --git a/docs/source/contributing/releasing.rst b/docs/source/contributing/releasing.rst index 16946c82c1..25b45d2c9b 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:1079-1148``). +subset an ordinary push runs (its ``gate`` job, ``unit-tests.yml:1165-1234``). 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:1052-1073``) fails outright if +process: the ``changelog`` job (``unit-tests.yml:1025-1046``) 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 f80c191ad3..3c7cc70382 100644 --- a/docs/source/contributing/workflows.rst +++ b/docs/source/contributing/workflows.rst @@ -230,19 +230,23 @@ repository target the same reusable workflow, with the same input: * ``.github/workflows/deploy-pages.yml:47`` -- job ``unit-tests`` calls ``./.github/workflows/unit-tests.yml`` with ``gate-only: true``. -``gate-only: true`` selects **two** of ``unit-tests.yml``'s **eight** jobs, +``gate-only: true`` selects **two** of ``unit-tests.yml``'s **ten** jobs, not one: ``gate`` (the full suite, one Python version) and ``changelog`` (checks ``CHANGELOG.md`` against its source entry). It skips the other -**six** -- the five matrix jobs, ``test`` (five Python legs), -``integration`` (five), ``engine-tests`` (a Python x engine matrix), -``pypcap-parity`` (two) and ``unittest-ordering`` (eleven matrix cells), -each of which has already run once for this commit from Unit Tests' own -``push``/``pull_request`` triggers, so running any of them again per caller -would test the same commit several times over -- and -``required-checks`` (see `Required Status Checks`_ below), gated out by its -own ``if:`` rather than by having already run. ``changelog`` carries no +**eight** -- the five matrix jobs, ``test`` (five Python legs), +``integration`` (five), ``engine-tests`` (one job per engine, each looping +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 +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:1040-1046``): ``create-release.yml`` feeds +comment (``unit-tests.yml:1013-1019``): ``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 @@ -381,7 +385,7 @@ grepping every workflow file for its name: - Where * - ``Required checks passed`` - job ``required-checks`` - - ``unit-tests.yml:1290`` + - ``unit-tests.yml:1338`` * - ``Compat Python 3.10`` - job ``compatibility``, matrix leg ``3.10`` - ``python-compatibility.yml:31,41`` @@ -399,16 +403,16 @@ grepping every workflow file for its name: - ``python-compatibility.yml:31,45`` ``Required checks passed`` is defined by exactly one job -(``unit-tests.yml:1290``). ``Compat Python`` is defined by two in +(``unit-tests.yml:1338``). ``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 in -``unit-tests.yml`` (``:1151``) mentions the string without defining it. +``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 (``:1150-1171``) accounts for the +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 @@ -417,19 +421,20 @@ 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: [test, integration, engine-tests, -pypcap-parity]`` plus an explicit per-dependency check -(``unit-tests.yml:1289-1315``). The five ``Compat`` contexts stay required +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 }}`` (``:1292``), so it is never +``if: ${{ always() && inputs.gate-only != true }}`` (``unit-tests.yml:1340``), 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 ``unit-tests.yml``'s matching one for its own, differently-matrixed -``test``/``integration`` jobs (``:58-61``, ``:228``) both say why 3.15 is +``test``/``integration`` jobs (``unit-tests.yml:59-62``, ``unit-tests.yml:230``) both say why 3.15 is excluded from every *required* matrix in this repository -- the ruleset's required-checks list stops at 3.14, so a 3.15 leg cannot gate a merge and is kept advisory (``continue-on-error: true`` in ``python-compatibility.yml``'s diff --git a/tests/project/test_unit_tests_queue.py b/tests/project/test_unit_tests_queue.py new file mode 100644 index 0000000000..852484715c --- /dev/null +++ b/tests/project/test_unit_tests_queue.py @@ -0,0 +1,575 @@ +# -*- 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: + +* ``required-checks`` may accept a ``skipped`` leg **only** when ``changes`` + classified the diff as docs-only. Its shell step is executed below against + every combination that matters, rather than read, because the one way it goes + wrong -- a skip accepted on a code change -- is a green gate over untested code. +* The ``changes`` classifier must call anything outside ``docs/`` and ``*.md`` + code, and a Markdown file under a code directory code too. +* ``unittest-ordering`` stays off pull requests and out of the required set, + and ``engine-tests``'s 3.15 cell stays non-blocking through a step-level + ``continue-on-error`` rather than a job-level one. +* The classifier, the engine cell loops and the apt retry are executed too: the + classifier against a scratch merge commit, the others with their commands + stubbed on ``PATH``. +* Every test outside :file:`tests/project` that reads ``docs/`` or Markdown is + run by ``project-tests``, so a docs-only PR cannot break it unseen. + +The workflow is sliced as text, not parsed with :mod:`yaml`, which is in no +extra of :file:`pyproject.toml`; the same split +:file:`tests/project/test_workflow_apt_timeouts.py` uses. + +""" + +from __future__ import annotations + +import ast +import os +import pathlib +import re +import shutil +import subprocess +import tempfile +import textwrap +import unittest +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from typing import Callable, Optional + +ROOT = pathlib.Path(__file__).resolve().parents[2] +WORKFLOW = ROOT / '.github' / 'workflows' / 'unit-tests.yml' + +#: The four legs a docs-only pull request may skip. +LEGS = ('test', 'integration', 'engine-tests', 'pypcap-parity') + + +def job(name: 'str') -> 'str': + """The text of job ``name``, from its header to the next top-level job.""" + text = WORKFLOW.read_text(encoding='utf-8') + match = re.search(rf'(?ms)^ {re.escape(name)}:\n(.*?)(?=^ [\w-]+:\n|\Z)', text) + if match is None: + raise AssertionError(f'no job named {name!r} in {WORKFLOW}') + return match.group(1) + + +def run_block(section: 'str') -> 'str': + """The dedented ``run: |`` script of the single step in ``section`` that has one.""" + blocks = re.findall(r'(?ms)^ run: \|\n(.*?)(?=^ - |\Z)', section) + if len(blocks) != 1: + raise AssertionError(f'expected one run: | block, found {len(blocks)}') + # A job's slice runs on to the next job header, so it can end in the + # comment above that job; the block ends at its first less-indented line. + lines = [] + for line in blocks[0].splitlines(): + if line.strip() and not line.startswith(' ' * 10): + break + lines.append(line) + return textwrap.dedent('\n'.join(lines)) + + +def required_needs() -> 'set[str]': + """The job names in ``required-checks``'s ``needs:`` list.""" + match = re.search(r'(?m)^ needs: \[(.*)\]$', job('required-checks')) + if match is None: + raise AssertionError('required-checks has no one-line needs: list') + return {name.strip() for name in match.group(1).split(',')} + + +def header(section: 'str') -> 'str': + """The job-level keys of ``section``, i.e. everything before ``steps:``.""" + return section.split('\n steps:', 1)[0] + + +class TestRequiredChecksSkips(unittest.TestCase): + """The gate's shell step, executed against each result combination.""" + + def setUp(self) -> None: + if shutil.which('bash') is None: # pragma: no cover + self.skipTest('bash is not available') + self.script = run_block(job('required-checks')) + + def gate(self, code: 'str', legs: 'str', project: 'str', changes: 'str' = 'success', + **override: 'str') -> 'int': + """The gate's exit status with every leg reporting ``legs`` unless overridden.""" + results = {'TEST': legs, 'INTEGRATION': legs, 'ENGINE_TESTS': legs, 'PYPCAP_PARITY': legs} + results.update(override) + env = dict(os.environ, CHANGES=changes, CODE=code, PROJECT_TESTS=project, **results) + return subprocess.run(['bash', '-c', self.script], env=env, check=False, + capture_output=True, text=True).returncode + + def test_needs_every_leg_and_the_docs_only_helpers(self) -> None: + """The gate reads every leg plus ``changes`` and ``project-tests``.""" + self.assertEqual(required_needs(), set(LEGS) | {'changes', 'project-tests'}) + + def test_code_change_requires_every_leg_to_succeed(self) -> None: + """On a code change nothing but ``success`` passes.""" + self.assertEqual(self.gate('true', 'success', 'skipped'), 0) + for result in ('skipped', 'failure', 'cancelled'): + with self.subTest(result=result): + self.assertNotEqual(self.gate('true', result, 'skipped'), 0) + + def test_docs_only_accepts_skipped_legs_when_project_tests_pass(self) -> None: + """A docs-only diff may skip the legs when ``project-tests`` passed.""" + self.assertEqual(self.gate('false', 'skipped', 'success'), 0) + self.assertEqual(self.gate('false', 'success', 'success'), 0) + + def test_docs_only_still_needs_project_tests(self) -> None: + """A docs-only diff without a passing ``project-tests`` fails.""" + for result in ('skipped', 'failure', 'cancelled'): + with self.subTest(result=result): + self.assertNotEqual(self.gate('false', 'skipped', result), 0) + + def test_docs_only_still_rejects_a_failed_leg(self) -> None: + """Skips are accepted on a docs-only diff, failures are not.""" + self.assertNotEqual(self.gate('false', 'failure', 'success'), 0) + + def test_every_leg_is_checked_on_its_own(self) -> None: + """One failing leg among passing ones fails the gate, on either path.""" + for env_name in ('TEST', 'INTEGRATION', 'ENGINE_TESTS', 'PYPCAP_PARITY'): + with self.subTest(leg=env_name): + for code, legs, project, odd in (('true', 'success', 'skipped', 'failure'), + ('true', 'success', 'skipped', 'skipped'), + ('false', 'skipped', 'success', 'failure')): + self.assertNotEqual(self.gate(code, legs, project, **{env_name: odd}), 0) + + def test_an_unclassified_diff_skips_nothing(self) -> None: + """A failed, skipped or garbled ``changes`` makes every skip a failure.""" + for changes, code in (('failure', ''), ('skipped', ''), ('success', ''), + ('success', 'maybe'), ('failure', 'false')): + with self.subTest(changes=changes, code=code): + self.assertNotEqual(self.gate(code, 'skipped', 'success', changes), 0) + + +class TestChangesClassifier(unittest.TestCase): + """``is_docs`` from the ``changes`` job, executed on sample paths.""" + + def setUp(self) -> None: + script = run_block(job('changes')) + source = textwrap.dedent(script.split("<<'PY'\n", 1)[1].split("\ncode = 'true'", 1)[0]) + namespace = {} # type: dict[str, object] + exec(compile(source, f'{WORKFLOW}:changes', 'exec'), namespace) # pylint: disable=exec-used + is_docs = namespace['is_docs'] + self.is_docs = is_docs # type: Callable[[str], bool] # type: ignore[assignment] + + def test_docs_and_markdown_are_docs(self) -> None: + """``docs/**`` and top-level Markdown are docs.""" + for path in ('docs/source/index.rst', 'docs/source/conf.py', 'README.md', + 'CONTRIBUTING.md', 'CHANGELOG.md'): + with self.subTest(path=path): + self.assertTrue(self.is_docs(path)) + + def test_everything_else_is_code(self) -> None: + """Anything else, and Markdown under a code directory, is code.""" + for path in ('pyproject.toml', 'setup.py', 'pcapkit/__init__.py', 'Makefile', + '.github/workflows/unit-tests.yml', '.github/PULL_REQUEST_TEMPLATE.md', + 'tests/project/README.md', 'util/notes.md', 'examples/README.md', + 'pcapkit/vendor/README.md'): + with self.subTest(path=path): + self.assertFalse(self.is_docs(path)) + + def test_only_this_workflows_own_pull_request_is_classified(self) -> None: + """Other events, and callers, always run everything.""" + section = job('changes') + self.assertIn("github.event_name == 'pull_request' && github.workflow == 'Unit Tests'", + section) + self.assertIn("'--no-renames'", section) + + +class TestJobGating(unittest.TestCase): + """Which jobs run on which events, read off their job-level keys.""" + + def test_the_four_legs_skip_only_on_a_docs_only_diff(self) -> None: + """Each leg is gated on ``changes`` by a job-level ``if:``.""" + for name in LEGS: + with self.subTest(job=name): + head = header(job(name)) + self.assertIn("needs.changes.outputs.code != 'false'", head) + self.assertRegex(head, r'(?m)^ needs: \[changes\]$') + + def test_project_tests_runs_only_on_a_docs_only_diff(self) -> None: + """``project-tests`` never duplicates ``test`` on a code change.""" + self.assertIn("needs.changes.outputs.code == 'false'", header(job('project-tests'))) + + def test_project_tests_failures_are_not_softened(self) -> None: + """No ``continue-on-error`` anywhere in ``project-tests``: a red step must fail it.""" + self.assertNotIn('continue-on-error', job('project-tests')) + + def test_unittest_ordering_is_off_pull_requests_and_not_required(self) -> None: + """H: main pushes only, and never a required leg.""" + head = header(job('unittest-ordering')) + self.assertIn("github.event_name != 'pull_request'", head) + self.assertNotIn('unittest-ordering', required_needs()) + + def test_unittest_ordering_runs_verbose_under_a_step_cap(self) -> None: + """A and C: ``--verbose`` and a step-level cap on the run step.""" + section = job('unittest-ordering') + run_step = section.split('- name: Run tests/', 1)[1] + self.assertIn('--verbose', run_step) + self.assertRegex(run_step, r'(?m)^ timeout-minutes: \d+$') + + def test_engine_tests_is_one_job_per_engine_with_a_non_blocking_315_step(self) -> None: + """E: matrix on engine only; 3.15 blocks nothing, 3.10-3.14 do.""" + section = job('engine-tests') + head = header(section) + self.assertNotIn('python-version', head) + self.assertNotIn('continue-on-error', head) + self.assertRegex(section, r'(?m)^ engine:$') + experimental = section.split('- name: Run the 3.15 cell', 1)[1] + self.assertRegex(experimental, r'(?m)^ continue-on-error: true$') + gating = section.split('- name: Run the 3.10-3.14 cells', 1)[1].split(' - name:', 1)[0] + self.assertNotIn('continue-on-error', gating) + self.assertIn('for py in 3.10 3.11 3.12 3.13 3.14;', gating) + + +def step(section: 'str', name: 'str') -> 'str': + """The text of the step called ``name`` in ``section``, up to the next step.""" + match = re.search(rf'(?ms)^ - name: {re.escape(name)}\n(.*?)(?=^ - |\Z)', section) + if match is None: + raise AssertionError(f'no step named {name!r}') + return match.group(1) + + +def step_script(section: 'str', name: 'str') -> 'str': + """The dedented ``run: |`` script of step ``name``.""" + body = step(section, name).split(' run: |\n', 1)[1] + lines = [] + for line in body.splitlines(): + if line.strip() and not line.startswith(' ' * 10): + break + lines.append(line) + return textwrap.dedent('\n'.join(lines)) + '\n' + + +def step_cap(section: 'str', name: 'str') -> 'int': + """Step ``name``'s own ``timeout-minutes``.""" + match = re.search(r'(?m)^ timeout-minutes: (\d+)$', step(section, name)) + if match is None: + raise AssertionError(f'step {name!r} declares no timeout-minutes') + return int(match.group(1)) + + +def bash(script: 'str', env: 'dict[str, str]', cwd: 'Optional[str]' = None) -> 'int': + """Run ``script`` under ``bash -e`` (GitHub's default shell) and return its status.""" + return subprocess.run(['bash', '-e', '-c', script], env=dict(os.environ, **env), cwd=cwd, + check=False, capture_output=True, text=True).returncode + + +class _Scratch(unittest.TestCase): + """A temporary directory per test, and ``bash`` required.""" + + def setUp(self) -> None: + if shutil.which('bash') is None: # pragma: no cover + self.skipTest('bash is not available') + tmp = tempfile.TemporaryDirectory(prefix='pcapkit-1052-') # pylint: disable=consider-using-with + self.addCleanup(tmp.cleanup) + self.tmp = pathlib.Path(tmp.name) + + +class TestClassifierAgainstARealMerge(_Scratch): + """The whole ``Classify the diff`` script, run in a scratch repository. + + The repository mirrors a pull-request checkout: ``HEAD`` is a merge commit + whose first parent is the base branch, and the base branch has moved on with + a *code* change since the PR branched, so diffing against any other base + than ``HEAD^1`` reads the PR as code. + + """ + + def setUp(self) -> None: + super().setUp() + if shutil.which('git') is None: # pragma: no cover + self.skipTest('git is not available') + self.script = run_block(job('changes')) + self.repo = self.tmp / 'repo' + self.repo.mkdir() + self.git('init', '-q', '-b', 'main') + self.write('README.md', 'base\n') + self.write('pcapkit/x.py', 'X = 1\n') + self.git('add', '-A') + self.git('commit', '-q', '-m', 'base') + self.git('checkout', '-q', '-b', 'pr') + self.git('checkout', '-q', 'main') + self.write('pcapkit/y.py', 'Y = 1\n') + self.git('add', '-A') + self.git('commit', '-q', '-m', 'main moves on with code') + self.git('checkout', '-q', 'pr') + + def git(self, *args: 'str') -> None: + """Run git in the scratch repository, isolated from user config.""" + env = dict(os.environ, GIT_CONFIG_GLOBAL=os.devnull, GIT_CONFIG_NOSYSTEM='1') + config = ('-c', 'user.name=t', '-c', 'user.email=t@t', '-c', 'commit.gpgsign=false') + subprocess.run(['git', *config, *args], cwd=self.repo, env=env, check=True, + capture_output=True) + + def write(self, name: 'str', text: 'str') -> None: + """Write ``text`` to ``name`` in the scratch repository.""" + path = self.repo / name + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text, encoding='utf-8') + + def pr(self, *files: 'str') -> None: + """Commit ``files`` on the PR branch and check out its merge into main.""" + for name in files: + self.write(name, 'changed\n') + self.git('add', '-A') + self.git('commit', '-q', '--allow-empty', '-m', 'pr') + self.git('checkout', '-q', 'main') + self.git('merge', '-q', '--no-ff', '--no-edit', 'pr') + + def classify(self, own_pr: 'str' = 'true', cwd: 'Optional[pathlib.Path]' = None) -> 'str': + """Run the classifier and return the line it wrote to ``GITHUB_OUTPUT``.""" + output = self.tmp / 'output' + status = bash(self.script, {'OWN_PR': own_pr, 'GITHUB_OUTPUT': str(output)}, + cwd=str(cwd or self.repo)) + self.assertEqual(status, 0) + return output.read_text(encoding='utf-8').strip() + + def test_a_docs_only_pull_request_is_not_code(self) -> None: + """Only ``HEAD^1`` sees the PR's own diff; ``HEAD^2`` would see main's code.""" + self.pr('docs/source/index.rst', 'CONTRIBUTING.md') + self.assertEqual(self.classify(), 'code=false') + + def test_a_code_file_makes_it_code(self) -> None: + """One code file among docs is enough.""" + self.pr('docs/source/index.rst', 'pcapkit/x.py') + self.assertEqual(self.classify(), 'code=true') + + def test_a_name_holding_whitespace_is_one_path(self) -> None: + """``notes.md docs/y`` is a root file, not ``notes.md`` and ``docs/y``.""" + self.pr('docs/source/index.rst', 'notes.md docs/y') + self.assertEqual(self.classify(), 'code=true') + + def test_another_event_always_runs_everything(self) -> None: + """``OWN_PR`` false (a push, or a caller's pull request) is code.""" + self.pr('docs/source/index.rst') + self.assertEqual(self.classify(own_pr='false'), 'code=true') + + def test_an_empty_diff_is_code(self) -> None: + """Nothing to classify is not evidence of a docs-only change.""" + self.pr() + self.assertEqual(self.classify(), 'code=true') + + def test_a_failed_diff_is_code(self) -> None: + """Outside a repository ``git diff`` fails, and that must not skip anything.""" + elsewhere = self.tmp / 'not-a-repo' + elsewhere.mkdir() + self.assertEqual(self.classify(cwd=elsewhere), 'code=true') + + +class TestEngineCellLoops(_Scratch): + """The two cell-loop steps of ``engine-tests``, with the cell script stubbed.""" + + def run_loop(self, name: 'str', fail: 'str') -> 'tuple[int, list[str]]': + """Run loop step ``name`` with the cell for ``fail`` failing; status and cells run.""" + (self.tmp / 'engine-cell.sh').write_text( + 'echo "$1" >> "$RUNNER_TEMP/calls"\n[ "$1" != "$FAIL_PY" ]\n', encoding='utf-8') + status = bash(step_script(job('engine-tests'), name), + {'RUNNER_TEMP': str(self.tmp), 'ENGINE': 'DPKT', 'FAIL_PY': fail, + 'GITHUB_STEP_SUMMARY': str(self.tmp / 'summary')}) + calls = self.tmp / 'calls' + return status, calls.read_text(encoding='utf-8').split() if calls.exists() else [] + + def test_every_gating_cell_runs_and_passing_passes(self) -> None: + """All five gating interpreters run; none failing exits 0.""" + status, calls = self.run_loop('Run the 3.10-3.14 cells', fail='none') + self.assertEqual(status, 0) + self.assertEqual(calls, ['3.10', '3.11', '3.12', '3.13', '3.14']) + + def test_one_failing_cell_fails_the_step_after_the_rest_ran(self) -> None: + """A failure is not swallowed, and does not stop the later cells.""" + status, calls = self.run_loop('Run the 3.10-3.14 cells', fail='3.12') + self.assertNotEqual(status, 0) + self.assertEqual(calls, ['3.10', '3.11', '3.12', '3.13', '3.14']) + + def test_a_failing_315_cell_fails_its_own_step(self) -> None: + """The step is red; only its ``continue-on-error`` keeps the job green.""" + status, calls = self.run_loop('Run the 3.15 cell (experimental, non-blocking)', + fail='3.15') + self.assertNotEqual(status, 0) + self.assertEqual(calls, ['3.15']) + + +#: The two apt steps, as (job, step name, extra env). +APT_STEPS: 'tuple[tuple[str, str, dict[str, str]], ...]' = ( + ('engine-tests', "Install this engine's system packages", {'ENGINE': 'PyShark'}), + ('pypcap-parity', 'Install libpcap headers, a C toolchain, and tshark', {}), +) + +#: Stubs for the commands the apt steps run, put first on PATH. +STUBS = { + 'sudo': 'exec env "$@"\n', + 'timeout': 'echo "$1" >> "$STUB_LOG.timeouts"\nshift\nexec "$@"\n', + 'apt-get': ('echo "$1" >> "$STUB_LOG.apt"\n' + 'if [ "$1" = update ]; then\n' + ' n=$(grep -c update "$STUB_LOG.apt")\n' + ' [ "$n" -gt "$FAIL_UPDATES" ]\n' + 'fi\n'), + 'dpkg': 'exit 0\n', + 'debconf-set-selections': 'cat > /dev/null\n', +} + + +class TestAptRetry(_Scratch): + """Both apt steps, with ``sudo``/``timeout``/``apt-get`` stubbed.""" + + def run_apt(self, name: 'str', job_name: 'str', extra: 'dict[str, str]', + fail_updates: 'int') -> 'tuple[int, list[str], list[int]]': + """Run apt step ``name`` with the first ``fail_updates`` updates failing.""" + bin_dir = self.tmp / 'bin' + bin_dir.mkdir(exist_ok=True) + for command, body in STUBS.items(): + path = bin_dir / command + path.write_text('#!/bin/bash\n' + body, encoding='utf-8') + path.chmod(0o755) + log = self.tmp / 'stub' + for suffix in ('.apt', '.timeouts'): + pathlib.Path(f'{log}{suffix}').write_text('', encoding='utf-8') + status = bash(step_script(job(job_name), name), + dict(extra, PATH=f'{bin_dir}{os.pathsep}{os.environ["PATH"]}', + STUB_LOG=str(log), FAIL_UPDATES=str(fail_updates))) + apt = pathlib.Path(f'{log}.apt').read_text(encoding='utf-8').split() + timeouts = pathlib.Path(f'{log}.timeouts').read_text(encoding='utf-8') + bounds = [int(n) for n in timeouts.split()] + return status, apt, bounds + + def test_a_clean_run_tries_once(self) -> None: + """No failure, one update and one install.""" + for job_name, name, extra in APT_STEPS: + with self.subTest(job=job_name): + status, apt, _ = self.run_apt(name, job_name, extra, fail_updates=0) + self.assertEqual((status, apt), (0, ['update', 'install'])) + + def test_one_stall_is_retried_once(self) -> None: + """A failed first update is retried, and the retry can succeed.""" + for job_name, name, extra in APT_STEPS: + with self.subTest(job=job_name): + status, apt, _ = self.run_apt(name, job_name, extra, fail_updates=1) + self.assertEqual((status, apt), (0, ['update', 'update', 'install'])) + + def test_two_stalls_fail_after_exactly_two_attempts(self) -> None: + """One retry, not more: the second failure fails the step.""" + for job_name, name, extra in APT_STEPS: + with self.subTest(job=job_name): + status, apt, _ = self.run_apt(name, job_name, extra, fail_updates=2) + self.assertNotEqual(status, 0) + self.assertEqual(apt, ['update', 'update']) + + def test_both_attempts_fit_inside_a_five_minute_step_cap(self) -> None: + """Every apt call is bounded, and two full attempts fit the 5-minute cap.""" + for job_name, name, extra in APT_STEPS: + with self.subTest(job=job_name): + status, apt, bounds = self.run_apt(name, job_name, extra, fail_updates=0) + self.assertEqual(status, 0) + self.assertEqual(len(bounds), len(apt), 'an apt-get call runs without timeout') + self.assertEqual(step_cap(job(job_name), name), 5) + self.assertLessEqual(2 * sum(bounds), 5 * 60) + + +#: Matches a string literal that is a path into docs/ or names Markdown files. +_DOC_PATH = re.compile(r'^(?:docs(?:/\S*)?|\S*\.md)$') + + +def docs_readers() -> 'set[str]': + """Dotted names of test modules outside tests/project and tests/ itself that + name a ``docs/`` or Markdown path in code (docstrings excluded).""" + found = set() + for path in sorted((ROOT / 'tests').rglob('*.py')): + relative = path.relative_to(ROOT / 'tests') + if len(relative.parts) < 2 or relative.parts[0] == 'project': + continue + tree = ast.parse(path.read_text(encoding='utf-8')) + docstrings = set() + for node in ast.walk(tree): + if isinstance(node, (ast.Module, ast.ClassDef, ast.FunctionDef, ast.AsyncFunctionDef)): + if node.body and isinstance(node.body[0], ast.Expr) \ + and isinstance(node.body[0].value, ast.Constant): + docstrings.add(id(node.body[0].value)) + if any(isinstance(node, ast.Constant) and isinstance(node.value, str) + and id(node) not in docstrings and _DOC_PATH.match(node.value) + for node in ast.walk(tree)): + found.add('.'.join(('tests',) + relative.with_suffix('').parts)) + return found + + +class TestDocsOnlyPathRunsEveryDocsReader(unittest.TestCase): + """A docs-only PR must still run every test that reads what it changed.""" + + def listed(self) -> 'set[str]': + """The modules project-tests' second run step names.""" + script = step(job('project-tests'), 'Run the other tests that read docs/ or Markdown') + command = re.search(r'(?m)^ run: python -m unittest -v (.+)$', script) + self.assertIsNotNone(command) + return set(command.group(1).split()) # type: ignore[union-attr] + + def test_every_reader_outside_tests_project_is_run(self) -> None: + """tests/project and root modules run via the project leg; the rest are listed.""" + missing = docs_readers() - self.listed() + self.assertFalse(missing, f'add {sorted(missing)} to project-tests in unit-tests.yml') + + def test_every_listed_module_exists(self) -> None: + """A renamed or deleted module would make the step fail on import anyway.""" + for name in self.listed(): + with self.subTest(module=name): + self.assertTrue((ROOT / (name.replace('.', '/') + '.py')).is_file()) + + def test_the_scan_is_not_blind(self) -> None: + """The one known reader is found, so a passing run is not vacuous.""" + self.assertIn('tests.corekit.test_sentinel_exports_unit', docs_readers()) + + +#: Every ``unit-tests.yml:N`` or ``unit-tests.yml:N-M`` citation in these documents, +#: as (text the cited first line must contain, text the last line must contain). +#: A citation the table does not account for fails, and so does a table entry no +#: citation matches -- so moving a job without renumbering the prose fails here. +CITATIONS: 'dict[str, list[tuple[str, Optional[str]]]]' = { + 'docs/source/contributing/releasing.rst': [ + (' gate:', 'run: python -m pytest -q -n auto --dist load'), + (' changelog:', 'fi'), + ], + 'docs/source/contributing/workflows.rst': [ + ('# Deliberately *not* gated on ``gate-only``', '# this is neither expensive'), + ('name: Required checks passed', None), + ('five for `Compat Python 3.10`', None), + ("# Ruleset 23497679's required_status_checks", '# them.'), + (' required-checks:', 'echo "All required jobs reported an accepted result."'), + ('if: ${{ always() && inputs.gate-only != true }}', None), + ('# 3.15 deliberately excluded', '(non-blocking, schedule-only)'), + ('# See the `test` job above for why 3.15', None), + ], +} + + +class TestDocCitationsResolve(unittest.TestCase): + """The prose's ``unit-tests.yml`` line numbers point at what it says they do.""" + + def test_every_citation_lands_on_its_anchor(self) -> None: + """Each citation's range starts and ends on the line its anchor names.""" + lines = WORKFLOW.read_text(encoding='utf-8').splitlines() + documents = sorted({str(p.relative_to(ROOT)) for p in (ROOT / 'docs').rglob('*.rst')} + | {str(p.relative_to(ROOT)) for p in ROOT.glob('*.md')}) + for document in documents: + text = (ROOT / document).read_text(encoding='utf-8') + cited = [(int(m.group(1)), int(m.group(2) or m.group(1))) + for m in re.finditer(r'unit-tests\.yml:(\d+)(?:-(\d+))?', text)] + anchors = CITATIONS.get(document, []) + unmatched = list(anchors) + for first, last in cited: + with self.subTest(document=document, citation=f'{first}-{last}'): + self.assertLessEqual(last, len(lines)) + hit = next((a for a in anchors if a[0] in lines[first - 1] + and (a[1] is None or a[1] in lines[last - 1])), None) + self.assertIsNotNone( + hit, f'{document} cites unit-tests.yml:{first}-{last}, which is ' + f'{lines[first - 1].strip()!r} .. {lines[last - 1].strip()!r}') + if hit in unmatched: + unmatched.remove(hit) + with self.subTest(document=document): + self.assertFalse(unmatched, f'{document}: no citation lands on {unmatched}') + + +if __name__ == '__main__': + unittest.main()