From a5ca612b31e80e98aeedf7aaaea19961d0ed7dd2 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Tue, 15 Sep 2026 01:01:45 -0400 Subject: [PATCH] ci: run the unit tier once per event, not once per caller github.event_name inside a called workflow is the *caller's* event, never 'workflow_call'. So unit-tests.yml's guards were both wrong in the same way: `!= 'workflow_call'` was always true and `== 'workflow_call'` always false. The consequence is measurable rather than theoretical - on PR #371's head every caller ran the full twelve-job matrix and the gate job was skipped in every context, so the gate added in #340 has never actually run anywhere. A CodeQL run on that commit produced Unit Tests / Python 3.10 through 3.15 plus all six Integration jobs, with Write workflow gate skipped. Replaced the guards with an explicit workflow_call input, gate-only, so a caller asks for the gate and only the gate. On push and pull_request the input does not exist, dereferences to empty string, and the matrix jobs run as before. CodeQL no longer calls the unit tier at all. It reads source; its findings do not depend on the tests passing, and the old `needs` meant a failing test silently suppressed the security analysis for that commit. deploy-pages ran on pull_request for every branch; now main only, and its gate is skipped on pull requests since nothing publishes there. The dependent job tolerates the skip rather than cascading. Added concurrency to every workflow: PR-branch runs cancel superseded ones, while the publishing paths - create-release, cron-conda, cron-vendor - use cancel-in-progress: false, because cancelling a run that tags or uploads is worse than letting it finish. The unit-tests group is prefixed so a callee can never share a group with its caller. Also disambiguated the check names. "Python 3.10" appeared twice per PR from two different workflows, which is what made this look like a duplicate run: the 13s row was Python Compatibility, not a second Unit Tests. Compat jobs are now "Compat Python ", and the four gates name their caller. Two pre-existing collisions between create-release and cron-conda are fixed too, both of which run on every main push. Measured before, reasoned after, one PR push: 47 jobs -> 22, unit tier 3x -> 1x. Main push: 96 -> 28 jobs, unit tier 6x -> 1x. All six Python versions and the integration tier still run once per PR and once per main push. Validated: all seven files parse, zero duplicate keys at any nesting level (the #375 bug class), every needs: target resolves, every uses: path exists, no display-name collisions remain. actionlint is not installed here. --- .github/workflows/codeql-analysis.yml | 19 ++++++----- .github/workflows/create-release.yml | 23 +++++++++++-- .github/workflows/cron-conda.yml | 16 +++++++-- .github/workflows/cron-vendor.yml | 12 +++++-- .github/workflows/deploy-pages.yml | 26 ++++++++++++--- .github/workflows/python-compatibility.yml | 12 +++++-- .github/workflows/unit-tests.yml | 39 +++++++++++++++++++--- 7 files changed, 121 insertions(+), 26 deletions(-) diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index d15f45f5fb..2c173a337c 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -2,24 +2,27 @@ name: "CodeQL" on: push: - branches: [main, ] + branches: [main] pull_request: # The branches below must be a subset of the branches above branches: [main] schedule: - cron: '0 2 * * 6' -jobs: - unit-tests: - name: Unit Tests - permissions: - contents: read - uses: ./.github/workflows/unit-tests.yml +concurrency: + group: codeql-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} +jobs: + # This workflow used to call ./.github/workflows/unit-tests.yml and gate + # `analyze` behind it. That re-ran the entire unit and integration matrix + # against a commit the Unit Tests workflow is already testing, and it bought + # nothing: CodeQL reads the source, so its findings do not depend on the test + # suite passing. Worse, it meant a failing test suppressed the security + # analysis for that commit. Static analysis now runs on its own. analyze: name: Analyze runs-on: ubuntu-latest - needs: [ unit-tests ] permissions: actions: read contents: read diff --git a/.github/workflows/create-release.yml b/.github/workflows/create-release.yml index ffbd5b12bd..7ade36e146 100644 --- a/.github/workflows/create-release.yml +++ b/.github/workflows/create-release.yml @@ -12,13 +12,27 @@ on: name: Create Release +# A `workflow_run` trigger runs against the tip of the default branch, not +# against the sha of the run that triggered it, so two Vendor Update runs +# completing close together produce two Create Release runs evaluating the +# *same* tip -- observed on 78ec96afb (runs 34894022730 and 34894646592, six +# minutes apart) and on 1e0ebc53b. Both would then read PCAPKIT_TAG_EXISTS +# before either had tagged, which is a double-publish race. Serialise instead: +# never cancel a release run, just make the second wait and find the tag +# already there. +concurrency: + group: create-release-${{ github.ref }} + cancel-in-progress: false + jobs: unit-tests: - name: Unit Tests + name: Release test gate if: ${{ github.event_name != 'workflow_run' || github.event.workflow_run.conclusion == 'success' }} permissions: contents: read uses: ./.github/workflows/unit-tests.yml + with: + gate-only: true version_check: name: Check Version @@ -93,7 +107,10 @@ jobs: token: "${{ secrets.GITHUB_TOKEN }}" tag: - name: Conda Tag + # "(release)" distinguishes this from the identically named job in + # cron-conda.yml; both workflows run on a push to main, so the two produced + # two check rows reading `Conda Tag` with nothing to tell them apart. + name: Conda Tag (release) runs-on: ubuntu-latest permissions: {} needs: [ version_check ] @@ -224,7 +241,7 @@ jobs: token: "${{ secrets.GITHUB_TOKEN }}" conda: - name: Conda deployment of package for platform ${{ matrix.os }} with Python ${{ matrix.python-version }} + name: Conda deployment (release) on ${{ matrix.os }} with Python ${{ matrix.python-version }} runs-on: ${{ matrix.os }} permissions: contents: write diff --git a/.github/workflows/cron-conda.yml b/.github/workflows/cron-conda.yml index f7b445c4fe..b7baf93ec2 100644 --- a/.github/workflows/cron-conda.yml +++ b/.github/workflows/cron-conda.yml @@ -9,12 +9,20 @@ on: push: branches: [main] +# This workflow commits and pushes, so a cancelled run can leave the bump +# half-applied -- queue superseded runs rather than killing them. +concurrency: + group: conda-update-${{ github.ref }} + cancel-in-progress: false + jobs: unit-tests: - name: Unit Tests + name: Conda test gate permissions: contents: read uses: ./.github/workflows/unit-tests.yml + with: + gate-only: true conda-update: name: Update requirements.txt @@ -93,7 +101,9 @@ jobs: PCAPKIT_BUILD: ${{ steps.get_version.outputs.PCAPKIT_BUILD }} conda-tag: - name: Conda Tag + # See create-release.yml: "(update)" keeps this apart from the Conda Tag job + # there, which runs on the same commits. + name: Conda Tag (update) runs-on: ubuntu-latest needs: [ conda-update ] if: needs.conda-update.outputs.CONDA_CHANGED == 'true' @@ -124,7 +134,7 @@ jobs: tags: true conda-dist: - name: Conda deployment of package for platform ${{ matrix.os }} with Python ${{ matrix.python-version }} + name: Conda deployment (update) on ${{ matrix.os }} with Python ${{ matrix.python-version }} runs-on: ${{ matrix.os }} needs: [ conda-tag, conda-update ] if: needs.conda-update.outputs.CONDA_CHANGED == 'true' diff --git a/.github/workflows/cron-vendor.yml b/.github/workflows/cron-vendor.yml index e42f2344ec..8cf79e015e 100644 --- a/.github/workflows/cron-vendor.yml +++ b/.github/workflows/cron-vendor.yml @@ -4,14 +4,22 @@ on: schedule: - cron: '0 10 * * 6' # everyday at 10am push: - branches: [main, ] + branches: [main] + +# This workflow commits and pushes, so a cancelled run can leave the bump +# half-applied -- queue superseded runs rather than killing them. +concurrency: + group: vendor-update-${{ github.ref }} + cancel-in-progress: false jobs: unit-tests: - name: Unit Tests + name: Vendor test gate permissions: contents: read uses: ./.github/workflows/unit-tests.yml + with: + gate-only: true vendor-update: runs-on: macos-latest diff --git a/.github/workflows/deploy-pages.yml b/.github/workflows/deploy-pages.yml index 76d7c39f51..0ed33d625b 100644 --- a/.github/workflows/deploy-pages.yml +++ b/.github/workflows/deploy-pages.yml @@ -2,25 +2,43 @@ name: GitHub Pages on: push: - branches: [main, ] + branches: [main] schedule: - cron: '0 2 * * 6' pull_request: - branches: - - '**' + # Was '**', which matched pull requests against every branch and made this + # the one workflow that fired outside the main-targeting set the others + # use. Every pull request here targets main, so '**' only ever added runs. + branches: [main] permissions: contents: write +concurrency: + group: pages-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + jobs: + # Nothing is published on a pull request -- the Deploy step below is skipped + # -- so the gate only earns its runners when a real publish follows. The docs + # build still runs on pull requests, which is the part that actually + # validates a documentation change. unit-tests: - name: Unit Tests + name: Docs test gate + if: ${{ github.event_name != 'pull_request' }} permissions: contents: read uses: ./.github/workflows/unit-tests.yml + with: + gate-only: true deploy-pages: needs: [ unit-tests ] + # `needs` cannot itself be conditional, so a skipped gate (pull requests) + # has to be accepted explicitly -- by default this job would inherit the + # skip. `!cancelled()` rather than `always()` so that cancelling the run + # still cancels the docs build; a gate that actually failed still blocks. + if: ${{ !cancelled() && needs.unit-tests.result != 'failure' }} concurrency: ci-${{ github.ref }} # Recommended if you intend to make multiple deployments in quick succession. runs-on: macos-latest steps: diff --git a/.github/workflows/python-compatibility.yml b/.github/workflows/python-compatibility.yml index 2c699b3f8e..893f9f0b6c 100644 --- a/.github/workflows/python-compatibility.yml +++ b/.github/workflows/python-compatibility.yml @@ -2,7 +2,7 @@ name: "Python Compatibility" on: push: - branches: [main, ] + branches: [main] pull_request: branches: [main] schedule: @@ -11,9 +11,17 @@ on: permissions: contents: read +concurrency: + group: python-compatibility-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + jobs: compatibility: - name: Python ${{ matrix.python-version }} + # Named "Python ${{ matrix.python-version }}" until now, which collided + # exactly with the Unit Tests matrix job of the same name: `gh pr checks` + # showed two rows reading `Python 3.10`, one a 13-second import smoke test + # and one a three-minute test run, with nothing to tell them apart. + name: Compat Python ${{ matrix.python-version }} runs-on: ubuntu-latest continue-on-error: ${{ matrix.experimental == true }} strategy: diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index d0bb5023b6..8d3335f539 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -6,14 +6,41 @@ on: pull_request: branches: [main] workflow_call: + inputs: + gate-only: + description: >- + Run only the single-version gate job instead of the full matrix. + Callers that merely need a pass/fail verdict before shipping set + this to true: the full matrix already runs once per commit from + this workflow's own push and pull_request triggers, so running it + again per caller tests the same commit several times over. + type: boolean + required: false + default: false permissions: contents: read +# `github.workflow` is the *caller's* workflow name inside a called workflow, +# so the `unit-tests-` prefix is what stops a called run from ever sharing a +# group with the caller that invoked it -- if they shared one, the callee +# would cancel its own caller. Only pull-request runs are cancellable: on +# main, on a tag, or on the release path a cancelled run is worse than a slow +# one. +concurrency: + group: unit-tests-${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + jobs: test: + # `github.event_name` is NOT 'workflow_call' inside a called workflow -- + # the github context is inherited from the caller, so it reads + # 'pull_request', 'push', 'schedule' or 'workflow_run'. Guarding on it (as + # this workflow used to) therefore ran the whole matrix in every caller + # 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: ${{ github.event_name != 'workflow_call' }} + if: ${{ inputs.gate-only != true }} runs-on: ubuntu-latest continue-on-error: ${{ matrix.experimental == true }} timeout-minutes: 30 @@ -55,7 +82,7 @@ jobs: integration: name: Integration Python ${{ matrix.python-version }} - if: ${{ github.event_name != 'workflow_call' }} + if: ${{ inputs.gate-only != true }} runs-on: ubuntu-latest continue-on-error: ${{ matrix.experimental == true }} timeout-minutes: 30 @@ -120,9 +147,13 @@ jobs: - name: Run full test suite run: python -m pytest -q + # 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 + # pull_request trigger, before anything reaches a release path. gate: - name: Write workflow gate - if: ${{ github.event_name == 'workflow_call' }} + name: Gate (full suite, Python 3.14) + if: ${{ inputs.gate-only == true }} runs-on: ubuntu-latest timeout-minutes: 30