Run the full test matrix in the Release workflow before publishing - #862
Conversation
| return items | ||
|
|
||
|
|
||
| def find_full_run(repo: str, sha: str, token: str, expected: set[str]) -> dict[str, Any] | None: |
There was a problem hiding this comment.
Self-review round one (implementation behavior): CLEAN
Base 531185619465b809331a9a8cf2083884365de193, head 600b13da7abd522ecda1e90d8fe17ba0482b4818.
Covered:
scripts/verify_release_tests.pyscripts/tests/test_verify_release_tests.py.github/workflows/release.yamldocs/testing.md
Checked:
- Run selection. The
status=successfilter plus the event check leaves onlyworkflow_dispatchandscheduleruns. The run listing is newest first. An incomplete newer run falls through to an older complete one (covered bytest_find_full_run_skips_incomplete_runs). - Re-runs.
jobs?filter=latestevaluates only the latest attempt. A job that failed in attempt 1 and passed on re-run counts as success. A run with a re-run still in progress is notsuccess, so it is not listed. - Job matching.
incomplete_jobscompares exact job names. It reports missing jobs, and jobs that are skipped, failed, or still in progress. Non-suite jobs (lint,changes) are not required individually, but a failure in either makes the run's conclusion non-success. - Failure paths fail closed.
- An HTTP error raises and exits non-zero.
- Missing
GITHUB_TOKENexits with code 2. - Classifiers with no
3.xversion raiseValueError. - No qualifying run exits with code 1.
In every casereleasedoes not start, because itneeds: verify.
- Commit identity. The tag checkout's
git rev-parse HEADis the commit even for annotated tags, andhead_shaneeds the full SHA. - Permissions. Workflow-level
{}.verifyhas read-onlyactionsandcontents.releasekeepsid-token: write(trusted publishing) andcontents: write(GitHub release). - Tests. They assert behavior (the returned run, exit codes, summary text, the requested query parameters) through a patched
_paginate. The real API response shape was checked separately by running the script read-only against real runs (see the TEST section).
No findings.
There was a problem hiding this comment.
Round-one follow-up on the round-two repair (600b13da7abd522ecda1e90d8fe17ba0482b4818 → 345ad32029f8e5f7b30a20d342c120406edbef99, one commit changing .github/workflows/docs-trigger.yaml and docs/testing.md): CLEAN
- Trigger.
workflow_run.workflows: [Release]matchesrelease.yaml'sname: Release. - Condition.
if: github.event.workflow_run.conclusion == 'success'skips runs that failed, were cancelled, or were refused byverify. A refused tag makes the Release run concludefailure, becauseverifyexits 1 andreleaseis then skipped. - Actor. A Release run started by the maintainer's tag push is not a
GITHUB_TOKEN-initiated event, soworkflow_runfires. - Where the definition comes from.
workflow_runuses the default branch's definition. That is the one merged here, and it still dispatchesdocs.yamlonmasteras before. - Unchanged. Permissions (
actions: write) and the dispatch script. - Checks. actionlint passed within
just scripts, and so didjust docs lint.
|
|
||
| jobs: | ||
| trigger-docs: | ||
| if: github.event.workflow_run.conclusion == 'success' |
There was a problem hiding this comment.
Self-review round two (claims, callers, operations): FINDINGS, repaired
Base 531185619465b809331a9a8cf2083884365de193. Reviewed head 600b13da7abd522ecda1e90d8fe17ba0482b4818; repair head 345ad32029f8e5f7b30a20d342c120406edbef99.
Findings:
- Operational: the docs build was not gated.
.github/workflows/docs-trigger.yaml:10-12dispatcheddocs.yamlon everyv*tag push.docs/conf.pyanddocs.yamlbuild tag versions with sphinx-multiversion. So a tag that the new gate refuses, or one whose PyPI upload fails, would still get a documentation build for an unpublished version.- Repair: the trigger now runs on
workflow_runof Release withconclusion == 'success', and the workflow is renamed "Trigger Docs on Release". docs.yamlalso builds on every push to master, so a refused tag that is left in place would still appear later.docs/testing.mdnow says to delete a refused tag, run the tests, and push the tag again.
- Repair: the trigger now runs on
- Claim: "about 30 minutes". The 2026-09-25 dispatch run 36107643451 took 19 minutes. Corrected to about 20 minutes.
- Claim: the 3.x note was vague. It was replaced with measured facts.
origin/3.xhas the sametest,test-sqla, andtest-sqla-async/run (<version>)job names, and a 3.10–3.14 matrix equal to its classifiers.
Checked and held:
- "v3.36.0 was moved to Update gh-action-pypi-publish to v1.14.2 for metadata 2.5 support #765, which changed only
release.yaml":git show --stat 073635fshows 1 file, and4ef4d3ehas no Test run. The gate would have stopped that re-tag, which is stated as a consequence. - "Pull-request runs can skip suites":
test.yaml'schangesjob gates the suites forpull_requestonly. - "No AWS cost": the gate makes GitHub API calls only.
- Docs sentence (every suite × every classifier version, schedule or dispatch): matches
find_full_runandexpected_jobs.
Validation after the repair: just scripts (actionlint, 124 script tests) and just docs lint passed. Neither trigger has run through a real tag push; this is stated in the PR's TEST section.
| if run["event"] not in _EVENTS or run["conclusion"] != "success": | ||
| continue | ||
| jobs = _paginate( | ||
| f"{_API}/repos/{repo}/actions/runs/{run['id']}/jobs?filter=latest&per_page=100", |
There was a problem hiding this comment.
Independent review (relayed): Codex CLI 0.157.0, model gpt-6-sol, reasoning effort high, codex exec -s read-only, session 01a0e0df-674a-7c62-9a05-a274baed7450.
- Snapshot: detached worktree at head
345ad32029f8e5f7b30a20d342c120406edbef99, base531185619465b809331a9a8cf2083884365de193. The prompt contained the literal diff and repository context only (no PR number, description, or prior findings). The review was static: no tests, no network, no edits. The snapshot was unchanged afterwards.
Reviewer result, verbatim:
Surfaces covered: the Release, Test, reusable test suite, Docs, and docs-trigger workflows; the verifier and its offline tests; the relevant documentation and version configuration. This was a static review; I ran no tests or network commands.
Verdict: FINDINGS
P2 — scripts/verify_release_tests.py:152:
filter=latestrequests jobs from the latest run attempt. After a maintainer uses Re-run failed jobs, that attempt contains only the rerun jobs. Earlier successful matrix jobs appear missing toincomplete_jobs, so the release is refused even when the Test workflow’s final conclusion is success.P2 — .github/workflows/docs-trigger.yaml:12: Gating this trigger does not prevent documentation for a rejected tag from being published. The separate Docs workflow still runs on every master push and builds every matching
vX.Y.Ztag. If verification rejects a tag and it remains in the repository when master next changes, that Docs run publishes the unreleased version.Non-blocking suggestions: None.
Author verification:
filter=latestafter "Re-run failed jobs" (P2): rejected, disproved by measurement.- Run 36252040653 (PR Require SQLAlchemy 2.0 or later for the SQLAlchemy dialect #858) is on attempt 2, with only
test-sqla-async / run (3.11)re-run. GET /actions/runs/36252040653/jobs?filter=latestreturns all 17 jobs withrun_attempt: 2. The jobs that succeeded in attempt 1 are carried into the latest attempt, so they are not reported missing.
- Run 36252040653 (PR Require SQLAlchemy 2.0 or later for the SQLAlchemy dialect #858) is on attempt 2, with only
- Docs for a refused tag via
docs.yamlon master pushes (P2): known limitation, deferred.- It is documented in
docs/testing.md(delete a refused tag, run the tests, push the tag again) and in the PR's consequences. - Fully preventing it would mean making sphinx-multiversion select only published versions, which is outside Require a full test run on the release commit before publishing #851. The trigger change still stops the immediate build on a refused tag or a failed upload.
- It is documented in
The Test workflow can now be called by other workflows. The Release workflow calls it for the tagged commit and builds and publishes only if every suite passes on every supported Python version. The docs trigger runs only after a successful Release. The weekly schedule now tests the newest Python version only, since a release runs the full matrix. A manual dispatch tests every version, or the versions given in its new python-versions input. Closes #851 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
345ad32 to
9b79418
Compare
| default: '' | ||
| # The Release workflow runs every suite on every supported Python version | ||
| # before publishing. | ||
| workflow_call: |
There was a problem hiding this comment.
Self-review round one (implementation behavior), full pass on the rework: CLEAN
Base 3cb9663de659d7cb8cb6e049a017c43e2b428b3a (master after #863), head 9b7941825291a862c3db9dec6b05e162048f0a0f. This replaces the earlier script-based gate, so every file was reviewed afresh.
Covered:
.github/workflows/test.yaml.github/workflows/release.yaml.github/workflows/docs-trigger.yamldocs/testing.md
Checked:
- Called-workflow context. GitHub documents that in a reusable workflow "the
githubcontext is always associated with the caller workflow". So a Release call hasgithub.event_name == 'push'.changesruns (itsifonly restrictspull_request).- The
*branch selects every version, andsqla/sparkaretrue. test-suite.yaml's fork guard passes.
- Inputs.
inputs.python-versionsis defined only forworkflow_dispatch. Underworkflow_callandscheduleit is empty, and those branches ignore it. - Dispatch validation, run locally with
bash -eo pipefail:- Whitespace, empty items, and duplicates are removed (
uniquesorts, and3.1xstrings sort correctly). - An unsupported item makes
jqexit 5. Because of-ein the assignment, the step fails beforepython-versionsis written, so no suite starts.
- Whitespace, empty items, and duplicates are removed (
- Permissions. Workflow-level
{}inrelease.yaml. Thetestjob grantscontents: read,id-token: write, andpull-requests: read, which coverstest.yaml's top level (id-token: write,contents: read) and thechangesjob (pull-requests: read). Called workflows can only lower permissions, and they do not raise them here.releasekeepsid-token: writeandcontents: write. - Concurrency. The group resolves to
Release-refs/tags/<tag>under the caller's context.release.yamldefines no concurrency group of its own, so the caller cannot be cancelled by its callee. - Side workflows.
database-sweep.yamlstill follows onlyTestworkflow_runevents withevent == 'schedule'. A called Test inside Release is not a separate Test run, so the sweep behavior is unchanged. - Nesting. Release → Test → Test Suite is 3 levels, within the documented limit of 10.
No findings.
There was a problem hiding this comment.
Round-one follow-up on the repair (9b7941825291a862c3db9dec6b05e162048f0a0f → b1abeb5c73a927db9506c41c671f3ef9e1522993, one commit: docs.yaml and docs/testing.md): CLEAN
- Release lookup.
gh api --paginateapplies--jqto each page, so every non-drafttag_nameis collected, beyond 100.contents: read, whichdocs.yamlalready has, can read releases. - Failure handling. Under
bash -eo pipefail, an API failure fails the assignment and the step. An empty list fails explicitly. Thegrep -qxF … || git tag --deleteloop cannot fail on a released tag. - Effect on the build. Deleted tags are local
refs/tags, which is what sphinx-multiversion lists (smv_tag_whitelist).docs/conf.py'sgit describe --tagsfor the master label then sees only released tags. - Checks. Verified on a local clone: 185 → 183 tags, only the two fake tags removed. actionlint passed within
just scripts, and so didjust docs lint.
| pull-requests: read | ||
|
|
||
| release: | ||
| needs: test |
There was a problem hiding this comment.
Self-review round two (claims, callers, operations), full pass: CLEAN after a description fix
Base 3cb9663de659d7cb8cb6e049a017c43e2b428b3a, head 9b7941825291a862c3db9dec6b05e162048f0a0f.
Claims checked:
- "Nothing is built or published unless every suite passes on every version."
releaseneeds: test. The called workflow's jobs all run onpushwith the full list:lint(just lint), andtest,test-sqla, andtest-sqla-asyncfor 3.10–3.14. A failed or cancelled job failstest, soreleaseis skipped. - OIDC from a tag ref.
- GitHub's OIDC documentation for reusable workflows says the token carries the calling workflow's standard claims, and the called workflow only in
job_workflow_ref. - The deployed role
github-actions-oidc-pyathenatrustssubStringLikerepo:pyathena-dev/PyAthena:*(read-onlyiam get-role). - The description first said the deployed role was unchecked. It was corrected with this evidence.
- GitHub's OIDC documentation for reusable workflows says the token carries the calling workflow's standard claims, and the called workflow only in
- The docs caveat.
docs/conf.py:218hassmv_tag_whitelist = r"^v\d+\.\d+\.\d+$", anddocs.yamlbuilds on every push to master. So a refused tag left in place would appear as a version, asdocs/testing.mdsays. - "Replaces the manual dispatch step; full matrix once per release." It holds for master tags. For
3.xtags, the old workflows apply and dispatching is still needed, which the description states. - Docs table.
- The schedule and PRs use the newest version.
- Dispatch uses the requested versions or all of them.
- Release uses all of them.
- This matches the
caseintest.yaml:98-114. - The example
gh workflow run ... -f python-versions=3.11,3.14matches the input name.
Operational:
- A release now takes about 20 minutes longer before the upload, because the test matrix runs first.
- A failing release leaves a tag to delete. The steps are documented.
- Neither is a regression from the old manual flow, which needed the same dispatch first.
Not exercised: a real tag-push Release run (stated in TEST).
There was a problem hiding this comment.
Round-two follow-up on the repair (b1abeb5c73a927db9506c41c671f3ef9e1522993): CLEAN
- "A GitHub release means published": in
release.yaml,softprops/action-gh-releaseis the last step ofrelease, afteruv buildandpypa/gh-action-pypi-publish, andreleaseneeds: test. - "No current version disappears":
git tag -l 'v*'againstgh release listshows 183 = 183, with no tag missing a release. - Docs.
docs/testing.mdnow says a failed tag is left out of the documentation because only tags with a GitHub release are listed. The earlier "delete before the next master push" caveat was removed, because it no longer applies. - Description. It explains why the race matters more after this PR (the dispatch step is gone), and records the measurement.
The Docs workflow builds every version tag in its checkout. A master push whose Docs run starts after a new tag is pushed would document that version while its Release run is still testing, or after its tests fail. Drop tags without a published GitHub release before building, since the Release workflow creates the release only after its tests and PyPI upload succeed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
| # sphinx-multiversion builds every version tag in the checkout. A tag | ||
| # gets its GitHub release only after the Release workflow's tests and | ||
| # PyPI upload succeed, so drop tags without one: a release still in | ||
| # progress or refused by its tests is not documented. |
There was a problem hiding this comment.
Independent review (relayed): Codex CLI 0.157.0, model gpt-6-astra, reasoning effort high, codex exec -s read-only, session 01a0e2ea-5b94-7441-bcf1-3ae56c75f382.
- Snapshot: detached worktree at
9b7941825291a862c3db9dec6b05e162048f0a0f, base3cb9663de659d7cb8cb6e049a017c43e2b428b3a. The prompt contained the literal diff and neutral context only (no PR number, description, or prior findings). The review was static, and the snapshot was unchanged afterwards.
Reviewer result, verbatim:
Covered surfaces: exact diff, reusable-workflow context and tagged checkout, permissions/OIDC, concurrency, version parsing and failure propagation, suite matrices, docs triggering, database sweep, and documentation accuracy. Static review only; no tests or network access.
Verdict: FINDINGS
- P2 — .github/workflows/docs-trigger.yaml:13: The successful-Release gate does not prevent unpublished tags from reaching documentation.
docs.yaml:12–13still triggers on master pushes, fetches all tags, and builds every matching version tag (docs/conf.py:218). Concrete scenario: push master, then push its release tag before the queued Docs checkout runs. Docs can build and deploy that version while Release tests are pending—even if those tests subsequently fail. The instruction atdocs/testing.md:177to delete the tag before the next master push cannot prevent this existing run. Gate version inclusion on successful publication, or otherwise ensure every docs execution path excludes unpublished tags.Non-blocking suggestions: None.
Author verification: confirmed, and made more likely by this PR.
docs.yamlbuilds on every master push, waits in thepagesconcurrency group (cancel-in-progress: false, builds take about 18 minutes), and checks out all tags withfetch-depth: 0.docs/conf.py:218whitelists everyvX.Y.Ztag.- The old "dispatch, then tag" step (about 20 minutes) used to separate a merge from its tag. This PR removes that step, which makes the race realistic.
Repair in b1abeb5 (this line):
- Before building,
docs.yamldeletes localv*tags that have no published (non-draft) GitHub release. - The Release workflow creates the release only after the tests and the PyPI upload succeed.
- An empty release list fails the step, so a broken lookup cannot silently drop every version.
- Verified on a local clone: two fake tags were deleted, and all 183 released tags were kept (every existing
vX.Y.Ztag has a release). docs/testing.mdand the description were updated. The "delete the tag before the next master push" caveat is gone.
There was a problem hiding this comment.
Independent follow-up (relayed): Codex CLI 0.157.0, model gpt-6-astra, reasoning effort high, codex exec -s read-only, session 01a0e2ed-fab7-7cf0-b44b-4eca9f0a6131.
- Scope: the repair diff
9b7941825291a862c3db9dec6b05e162048f0a0f..b1abeb5c73a927db9506c41c671f3ef9e1522993, plus the prior finding. - The review was static, and the snapshot was unchanged afterwards.
Reviewer result, verbatim:
Covered: GitHub token permissions, releases API,
gh --paginate --jq, draft exclusion, and exact tag matching; shell-e/pipefailbehavior, API failures, empty results, and tag deletion; local sphinx-multiversion 0.2.4 ref discovery, version labels, dropdown, and master redirect; release publication ordering anddocs/testing.mdwording.Verdict: CLEAN. The repair closes the reported path: tags without published GitHub releases are deleted before version discovery, and sphinx-multiversion does not fetch them again. Retained tag names and labels remain intact; master remains buildable. The documentation accurately describes the failed-release case.
WHAT
test.yamlcan be called by other workflows (workflow_call).release.yamlfirst runs it as thetestjob for the tagged commit. Thereleasejob (build, PyPI publish, GitHub release)needs: test, so nothing is built or published unless every suite passes on every supported Python version.changesjob from the singlePYTHON_VERSIONSlist:python-versionsinput, such as3.12or3.11,3.14workflow_callfrom a tag push)PYTHON_VERSIONS:changesjob, so no suite starts.release.yamlare now{}.testjob grants what the Test workflow needs:contents: read,id-token: writefor OIDC, andpull-requests: readfor thechangesjob.releasekeepsid-token: writeandcontents: write.docs-trigger.yaml("Trigger Docs on Tag" → "Trigger Docs on Release") runs onworkflow_runof Release withconclusion == 'success', instead of on everyv*tag push.docs.yamlgets a new step, before building, that deletes localv*tags that have no published (non-draft) GitHub release. The step fails if it finds no releases at all.docs/testing.mdcovers:WHY
Closes #851, part of #834.
release.yamlpublished without checking anything.pagesconcurrency group behind an earlier ~18-minute build, can check out after the tag is pushed. It would then document a version whose Release run is still testing, or whose tests fail.Consequences:
vX.Y.Ztags have a GitHub release, so no current version disappears from the documentation.${{ github.workflow }}-${{ github.ref }}. In a called workflow, thegithubcontext belongs to the caller, so a release run usesRelease-refs/tags/<tag>. It cancels only a previous run of the same tag, and it can overlap a Test run on master.github-actions-oidc-pyathenatruststoken.actions.githubusercontent.com:subStringLikerepo:pyathena-dev/PyAthena:*(read withiam get-role), as the template does. GitHub's OIDC token for a reusable workflow carries the caller's claims insub, with the called workflow only injob_workflow_ref. So a tag-push Release run'srepo:pyathena-dev/PyAthena:ref:refs/tags/<tag>matches.3.xmaintenance branch keep that branch's workflows, because a tag push runs the workflow files from the tagged commit. On 3.x, keep dispatching before tagging, unless this is backported.TEST
Tested commits: 9b79418 (rebased on master after #863 was merged), and b1abeb5 (docs tag filter).
just scriptspassed (including actionlint with ShellCheck on therunscripts), and so didjust docs lint.changesjob's selection script was extracted from the workflow file and run locally withbash -eo pipefail:EVENT_NAMEpython-versionsinputpull_request["3.14"]schedule["3.14"]push(Release call)["3.10","3.11","3.12","3.13","3.14"]workflow_dispatch["3.10","3.11","3.12","3.13","3.14"]workflow_dispatch3.12["3.12"]workflow_dispatch3.14 , 3.11,3.11["3.11","3.14"]workflow_dispatch3.9workflow_dispatch3.12,foodocs.yamland run on a local clone with two extra tags (v99.0.0andvfoo), withGH_TOKENandGITHUB_REPOSITORY=pyathena-dev/PyAthena. It deleted exactly those two tags and kept all 183 released tags (185 → 183), includingv3.36.0.Not run:
workflow_call, the actual OIDC exchange from a tag ref, and the docs trigger have not been exercised. The docs tag filter first runs on the next master push after merge. A tag push that passes the tests would publish to PyPI. The first real run is the next release (4.0.0), unless it is tried first with a test tag on a fork.AWS CI on head b1abeb5 (Test run 36321766073): success, with 3 AWS jobs on Python 3.14 (
test,test-sqla,test-sqla-async). All offline checks passed, including the docs build.🤖 Generated with Claude Code