ci: run the unit tier once per event, not once per caller - #392
Conversation
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 <v>", 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.
There was a problem hiding this comment.
🟢 Approval recommended
The changes consistently address the reusable-workflow guard bug, reduce redundant CI execution, and appear internally consistent across all updated workflows.
Pull request overview
This PR fixes CI workflow behavior around reusable workflows by replacing incorrect github.event_name guards with an explicit workflow_call input, so the Unit Tests matrix runs only when intended and callers can request a cheap single-version “gate” instead of re-running the full matrix. It also reduces unnecessary CI fan-out by removing CodeQL’s dependency on tests, tightening Pages workflow PR triggering, adding consistent concurrency controls, and disambiguating check/job display names to avoid collisions.
Changes:
- Add a
workflow_callboolean input (gate-only) tounit-tests.ymland use it to select between full matrix vs. single gate job. - Remove CodeQL’s test gating and add concurrency groups/cancellation behavior across workflows.
- Tighten triggers (e.g., Pages PR base branch) and rename jobs to avoid ambiguous/duplicated check names.
File summaries
| File | Description |
|---|---|
| .github/workflows/unit-tests.yml | Introduces explicit gate-only input, fixes invocation logic for reusable workflow, and adds workflow-level concurrency. |
| .github/workflows/python-compatibility.yml | Adds concurrency and renames matrix job to avoid check-name collisions with Unit Tests. |
| .github/workflows/deploy-pages.yml | Limits PR triggering to main, adds concurrency, skips test gate on PRs, and allows docs build to proceed when the gate is skipped. |
| .github/workflows/cron-vendor.yml | Adds non-cancelling concurrency and switches unit test invocation to gate-only to avoid re-running full matrices. |
| .github/workflows/cron-conda.yml | Adds non-cancelling concurrency, switches unit test invocation to gate-only, and disambiguates job names. |
| .github/workflows/create-release.yml | Adds non-cancelling concurrency and switches unit test invocation to gate-only; disambiguates job names. |
| .github/workflows/codeql-analysis.yml | Removes Unit Tests gating, adds concurrency, and runs CodeQL analysis independently. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The bug
github.event_nameinside a called (reusable) workflow is the caller's event, neverworkflow_call. Both guards inunit-tests.ymlwere built on the opposite assumption:if: github.event_name != 'workflow_call'on the matrix jobs — always trueif: github.event_name == 'workflow_call'on the gate job — always falseSo the gate added in #340 has never run anywhere, and every caller ran the full twelve-job matrix instead of the cheap gate it asked for.
Verified rather than reasoned about. CodeQL run
34866499707— apull_requestevent, whose only reason to callunit-tests.ymlwas the gate — produced:Twelve jobs where one was intended, and the intended one skipped.
Fixes
Replace the guards with an explicit
workflow_callinput.gate-only: truesays "the gate, and only the gate"; onpushandpull_requestthe input does not exist, dereferences to'', and the matrix runs as before. A caller now states its intent instead of the callee inferring it from something that does not mean what it looks like.CodeQL no longer calls the unit tier at all. It reads source; its findings do not depend on the tests passing. The old
needs:meant one failing test silently suppressed the security analysis for that commit — the opposite of what you want from a security workflow.deploy-pagesran onpull_requestfor every branch. Nowmainonly, and its gate is skipped on pull requests since nothing publishes there; the dependent job tolerates the skip rather than cascading.Concurrency on every workflow. PR-branch runs cancel superseded ones. The publishing paths —
create-release,cron-conda,cron-vendor— usecancel-in-progress: false, because cancelling a run mid-tag or mid-upload is worse than letting it finish. Theunit-testsgroup is prefixed so a callee can never share a group with its caller and cancel it.Disambiguated the check names, which is what made this look like duplication in the first place. "Python 3.10" appeared twice per PR from two different workflows — the 13-second row was Python Compatibility, not a second Unit Tests. Compat jobs are now
Compat Python <v>, and each gate names its caller. Two pre-existing collisions betweencreate-releaseandcron-condaare fixed too; both run on every push tomain.Effect
Measured before, reasoned after:
mainpushmainpushAll six Python versions and the integration tier still run — once per PR and once per push to
main.Validation
All seven workflow files parse; zero duplicate keys at any nesting level (the #375 bug class, checked with a recursive walk over the YAML node tree rather than
safe_load, which silently keeps the last of a duplicate pair); everyneeds:target resolves; everyuses:path exists; no display-name collisions remain.actionlintis not available in this environment, so the checks above are hand-rolled equivalents.