Skip to content

Run the full test matrix in the Release workflow before publishing #862

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
laughingman7743 merged 2 commits into from
Sep 27, 2026
+80 −16
Merged

Run the full test matrix in the Release workflow before publishing #862

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9b79418
Run the full test matrix in the Release workflow before publishing
laughingman7743 Sep 27, 2026
b1abeb5
Document only released version tags
laughingman7743 Sep 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions .github/workflows/docs-trigger.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,21 @@
#
# SPDX-License-Identifier: MIT

name: Trigger Docs on Tag
name: Trigger Docs on Release

# Rebuilds the documentation after the Release workflow succeeds, so a tag it
# refuses to publish does not trigger a documentation build.
on:
push:
tags: ['v*']
workflow_run:
workflows: [Release]
types: [completed]

permissions:
actions: write

jobs:
trigger-docs:
if: github.event.workflow_run.conclusion == 'success'

@laughingman7743 laughingman7743 Sep 27, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review round two (claims, callers, operations): FINDINGS, repaired

Base 531185619465b809331a9a8cf2083884365de193. Reviewed head 600b13da7abd522ecda1e90d8fe17ba0482b4818; repair head 345ad32029f8e5f7b30a20d342c120406edbef99.

Findings:

  1. Operational: the docs build was not gated. .github/workflows/docs-trigger.yaml:10-12 dispatched docs.yaml on every v* tag push. docs/conf.py and docs.yaml build 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_run of Release with conclusion == 'success', and the workflow is renamed "Trigger Docs on Release".
    • docs.yaml also builds on every push to master, so a refused tag that is left in place would still appear later. docs/testing.md now says to delete a refused tag, run the tests, and push the tag again.
  2. Claim: "about 30 minutes". The 2026-09-25 dispatch run 36107643451 took 19 minutes. Corrected to about 20 minutes.
  3. Claim: the 3.x note was vague. It was replaced with measured facts. origin/3.x has the same test, test-sqla, and test-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 073635f shows 1 file, and 4ef4d3e has 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's changes job gates the suites for pull_request only.
  • "No AWS cost": the gate makes GitHub API calls only.
  • Docs sentence (every suite × every classifier version, schedule or dispatch): matches find_full_run and expected_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.

runs-on: ubuntu-latest
steps:
- uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0
Expand Down
17 changes: 17 additions & 0 deletions .github/workflows/docs.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,23 @@ jobs:
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
fetch-depth: 0 # Fetch all history for sphinx-multiversion
# 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.

@laughingman7743 laughingman7743 Sep 27, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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, base 3cb9663de659d7cb8cb6e049a017c43e2b428b3a. 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–13 still 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 at docs/testing.md:177 to 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.yaml builds on every master push, waits in the pages concurrency group (cancel-in-progress: false, builds take about 18 minutes), and checks out all tags with fetch-depth: 0. docs/conf.py:218 whitelists every vX.Y.Z tag.
  • 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.yaml deletes local v* 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.Z tag has a release).
  • docs/testing.md and the description were updated. The "delete the tag before the next master push" caveat is gone.

@laughingman7743 laughingman7743 Sep 27, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/pipefail behavior, API failures, empty results, and tag deletion; local sphinx-multiversion 0.2.4 ref discovery, version labels, dropdown, and master redirect; release publication ordering and docs/testing.md wording.

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.

- name: Drop unreleased version tags
env:
GH_TOKEN: ${{ github.token }}
run: |
released=$(gh api --paginate "repos/$GITHUB_REPOSITORY/releases?per_page=100" \
--jq '.[] | select(.draft | not) | .tag_name')
if [[ -z "$released" ]]; then
echo "::error::No published GitHub releases found"
exit 1
fi
git tag --list 'v*' | while read -r tag; do
grep -qxF "$tag" <<< "$released" || git tag --delete "$tag"
done
- name: Setup Pages
uses: actions/configure-pages@45bfe0192ca1faeb007ade9deae92b16b8254a0d # v6.0.0
- uses: astral-sh/setup-uv@37802adc94f370d6bfd71619e3f0bf239e1f3b78 # v7.6.0
Expand Down
17 changes: 14 additions & 3 deletions .github/workflows/release.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,24 @@ on:
tags:
- 'v*'

permissions:
id-token: write
contents: write
permissions: {}

jobs:
# Runs every suite on every supported Python version for the tagged commit;
# nothing is built or published unless all of them pass.
test:
uses: ./.github/workflows/test.yaml
permissions:
contents: read
id-token: write
pull-requests: read

release:
needs: test

@laughingman7743 laughingman7743 Sep 27, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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." release needs: test. The called workflow's jobs all run on push with the full list: lint (just lint), and test, test-sqla, and test-sqla-async for 3.10–3.14. A failed or cancelled job fails test, so release is 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-pyathena trusts sub StringLike repo:pyathena-dev/PyAthena:* (read-only iam get-role).
    • The description first said the deployed role was unchecked. It was corrected with this evidence.
  • The docs caveat. docs/conf.py:218 has smv_tag_whitelist = r"^v\d+\.\d+\.\d+$", and docs.yaml builds on every push to master. So a refused tag left in place would appear as a version, as docs/testing.md says.
  • "Replaces the manual dispatch step; full matrix once per release." It holds for master tags. For 3.x tags, 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 case in test.yaml:98-114.
    • The example gh workflow run ... -f python-versions=3.11,3.14 matches 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).

@laughingman7743 laughingman7743 Sep 27, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Round-two follow-up on the repair (b1abeb5c73a927db9506c41c671f3ef9e1522993): CLEAN

  • "A GitHub release means published": in release.yaml, softprops/action-gh-release is the last step of release, after uv build and pypa/gh-action-pypi-publish, and release needs: test.
  • "No current version disappears": git tag -l 'v*' against gh release list shows 183 = 183, with no tag missing a release.
  • Docs. docs/testing.md now 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.

runs-on: ubuntu-latest
permissions:
id-token: write
contents: write

env:
PYTHON_VERSION: '3.12'
Expand Down
42 changes: 34 additions & 8 deletions .github/workflows/test.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,21 @@ on:
- 'docs/**'
- '**.md'
# The scheduled run executes every suite, including the ones that pull
# requests only run when related files change.
# requests only run when related files change, on the newest Python version.
schedule:
- cron: '0 0 * * 0'
# Runs every suite on the selected branch: before a release, on demand for
# a pull request, and to refresh the README status badge after a transient
# failure on the default branch.
# Runs every suite on the selected branch: on demand for a pull request, and
# to refresh the README status badge after a transient failure on the
# default branch.
workflow_dispatch:
inputs:
python-versions:
description: Comma-separated Python versions, such as 3.12 or 3.11,3.14; empty for every supported version
type: string
default: ''
# The Release workflow runs every suite on every supported Python version
# before publishing.
workflow_call:

@laughingman7743 laughingman7743 Sep 27, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.yaml
  • docs/testing.md

Checked:

  • Called-workflow context. GitHub documents that in a reusable workflow "the github context is always associated with the caller workflow". So a Release call has github.event_name == 'push'.
    • changes runs (its if only restricts pull_request).
    • The * branch selects every version, and sqla/spark are true.
    • test-suite.yaml's fork guard passes.
  • Inputs. inputs.python-versions is defined only for workflow_dispatch. Under workflow_call and schedule it is empty, and those branches ignore it.
  • Dispatch validation, run locally with bash -eo pipefail:
    • Whitespace, empty items, and duplicates are removed (unique sorts, and 3.1x strings sort correctly).
    • An unsupported item makes jq exit 5. Because of -e in the assignment, the step fails before python-versions is written, so no suite starts.
  • Permissions. Workflow-level {} in release.yaml. The test job grants contents: read, id-token: write, and pull-requests: read, which covers test.yaml's top level (id-token: write, contents: read) and the changes job (pull-requests: read). Called workflows can only lower permissions, and they do not raise them here. release keeps id-token: write and contents: write.
  • Concurrency. The group resolves to Release-refs/tags/<tag> under the caller's context. release.yaml defines no concurrency group of its own, so the caller cannot be cancelled by its callee.
  • Side workflows. database-sweep.yaml still follows only Test workflow_run events with event == '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.

@laughingman7743 laughingman7743 Sep 27, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Round-one follow-up on the repair (9b7941825291a862c3db9dec6b05e162048f0a0f → b1abeb5c73a927db9506c41c671f3ef9e1522993, one commit: docs.yaml and docs/testing.md): CLEAN

  • Release lookup. gh api --paginate applies --jq to each page, so every non-draft tag_name is collected, beyond 100. contents: read, which docs.yaml already has, can read releases.
  • Failure handling. Under bash -eo pipefail, an API failure fails the assignment and the step. An empty list fails explicitly. The grep -qxF … || git tag --delete loop 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's git describe --tags for 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 did just docs lint.


permissions:
id-token: write
Expand Down Expand Up @@ -60,8 +68,9 @@ jobs:
# requests run none. A ready pull request always runs the PyAthena suite; it
# runs the SQLAlchemy tests (the compliance suites and the PyAthena suite's
# SQLAlchemy tests) and the Spark tests only when their code, tests,
# dependencies, or this workflow change. Pull requests test the newest
# Python version only; the schedule and dispatch test every version.
# dependencies, or this workflow change. Pull requests and the schedule test
# the newest Python version; a dispatch tests the requested versions or
# every version, and the Release workflow every version.
changes:
if: >-
github.event_name != 'pull_request' ||
Expand All @@ -81,19 +90,36 @@ jobs:
EVENT_NAME: ${{ github.event_name }}
REPO: ${{ github.repository }}
PR_NUMBER: ${{ github.event.pull_request.number }}
REQUESTED_VERSIONS: ${{ inputs.python-versions }}
# Every supported version, oldest first; keep in sync with the
# pyproject.toml classifiers.
PYTHON_VERSIONS: '["3.10", "3.11", "3.12", "3.13", "3.14"]'
run: |
case "$EVENT_NAME" in
pull_request | schedule)
versions=$(jq -c '[last]' <<< "$PYTHON_VERSIONS")
;;
workflow_dispatch)
versions=$(jq -c --arg requested "$REQUESTED_VERSIONS" '
($requested | split(",") | map(gsub("\\s"; "")) | map(select(. != "")) | unique) as $selected
| if $selected == [] then .
elif ($selected - .) == [] then $selected
else error("unsupported Python versions: \($selected - . | join(", "))")
end' <<< "$PYTHON_VERSIONS")
;;
*)
# The Release workflow (a workflow_call from a tag push).
versions=$(jq -c '.' <<< "$PYTHON_VERSIONS")
;;
esac
echo "python-versions=$versions" >> "$GITHUB_OUTPUT"
if [[ "$EVENT_NAME" != "pull_request" ]]; then
{
echo "python-versions=$(jq -c '.' <<< "$PYTHON_VERSIONS")"
echo "sqla=true"
echo "spark=true"
} >> "$GITHUB_OUTPUT"
exit 0
fi
echo "python-versions=$(jq -c '[last]' <<< "$PYTHON_VERSIONS")" >> "$GITHUB_OUTPUT"
files=$(gh api "repos/$REPO/pulls/$PR_NUMBER/files" --paginate --jq '.[].filename')
printf 'Changed files:\n%s\n' "$files"
shared='^(\.github/workflows/test(-suite)?\.yaml|justfile|pyproject\.toml|uv\.lock)$'
Expand Down
10 changes: 8 additions & 2 deletions docs/testing.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,9 @@ It runs the offline checks (`just lint`) on each of them, including Drafts and e
| --- | --- | --- | --- | --- |
| Draft pull request | No | No | No | None |
| Ready pull request from a branch of this repository | Yes | When related files change | When related files change | Newest supported |
| Weekly schedule and manual dispatch | Yes | Yes | Yes | All supported |
| Weekly schedule | Yes | Yes | Yes | Newest supported |
| Manual dispatch | Yes | Yes | Yes | Requested, or all supported |
| Release tag (Release workflow) | Yes | Yes | Yes | All supported |

The SQLAlchemy tests are the compliance suites and the PyAthena suite's `tests/pyathena/sqlalchemy/` and `tests/pyathena/aio/sqlalchemy/`.
The Spark tests are the PyAthena suite's `tests/pyathena/spark/` and `tests/pyathena/aio/spark/`.
Expand All @@ -164,12 +166,16 @@ For the SQLAlchemy tests, the related files are `pyathena/sqlalchemy/`, `pyathen
For the Spark tests, they are `pyathena/spark/`, `pyathena/aio/spark/`, and their PyAthena suite test directories.
Changes to `pyproject.toml`, `uv.lock`, `justfile`, or the Test workflows run both.
For a pull request from a branch of this repository that still changes files other than `docs/` and Markdown, marking the Draft ready for review starts the AWS jobs, and converting it back to Draft cancels AWS jobs still running.
To run every suite on every supported Python version on a branch, dispatch the workflow:
To run every suite on a branch, dispatch the workflow; it tests every supported Python version unless `python-versions` lists some of them:

```bash
gh workflow run test.yaml --ref <branch>
gh workflow run test.yaml --ref <branch> -f python-versions=3.11,3.14
```

The Release workflow runs the same suites on every supported Python version for the tagged commit before building, and publishes nothing unless all of them pass.
If they fail, nothing is published, and the documentation leaves out the tag because it only lists tags with a GitHub release; delete the tag, fix the failure, and push the tag again.

Project policy excludes external-fork pull requests from AWS integration CI.
Maintainers do not approve those jobs as a substitute for contributor testing.
Checks without AWS access may still run on a fork pull request.
Expand Down
Loading