Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 5 additions & 1 deletion .github/workflows/test-suite.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ on:
test-type:
required: true
type: string
python-versions:
description: JSON array of the Python versions to test
required: true
type: string
max-parallel:
description: Maximum number of Python versions tested concurrently
type: number
Expand Down Expand Up @@ -55,7 +59,7 @@ jobs:
fail-fast: false
max-parallel: ${{ inputs.max-parallel }}
matrix:
python-version: ['3.10', '3.11', '3.12', '3.13', '3.14']
python-version: ${{ fromJSON(inputs.python-versions) }}

steps:
- name: Checkout
Expand Down
24 changes: 18 additions & 6 deletions .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,12 @@ jobs:
tool: just
- run: just lint

# Selects the AWS suites. Draft and external-fork pull 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
# Selects the AWS suites and Python versions. Draft and external-fork pull
# 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.
# dependencies, or this workflow change. Pull requests test the newest
# Python version only; the schedule and dispatch test every version.
changes:
if: >-
github.event_name != 'pull_request' ||
Expand All @@ -72,19 +73,27 @@ jobs:
outputs:
sqla: ${{ steps.filter.outputs.sqla }}
spark: ${{ steps.filter.outputs.spark }}
python-versions: ${{ steps.filter.outputs.python-versions }}
steps:
- id: filter
env:
GH_TOKEN: ${{ github.token }}
EVENT_NAME: ${{ github.event_name }}
REPO: ${{ github.repository }}
PR_NUMBER: ${{ github.event.pull_request.number }}
# 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"]'

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): CLEAN

Base 531185619465b809331a9a8cf2083884365de193, head a2eb20acf406a0e603ba37b562f1878c562e2c86.

Covered:

  • .github/workflows/test.yaml
  • .github/workflows/test-suite.yaml
  • docs/testing.md

Checked:

  • Version selection.
    • pull_request → jq -c '[last]' → ["3.14"].
    • Other events → the full list, emitted in the same grouped write as sqla=true and spark=true.
    • The values stay JSON strings, so fromJSON yields "3.10", not 3.1. Job names (run (3.10)) and the setup-python condition on '3.13'/'3.14' are unchanged.
  • Skipped changes job. For Draft and fork pull requests, changes is skipped. The dependent suite jobs are then skipped as before, so the required python-versions input is never evaluated with an empty value.
  • Callers. test-suite.yaml has three callers, all in test.yaml, and all three pass the input. No other workflow calls it (benchmarks.yaml and database-sweep.yaml do not).
  • Branch protection. It has no required status contexts, so fewer matrix jobs on pull requests do not leave an expected check pending.
  • actionlint (with ShellCheck on the run script) passes. SC2129 was fixed before this review.

No findings.

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 (a2eb20acf406a0e603ba37b562f1878c562e2c86 → 738d64cf0939592ef4f395eb2a15c379689242de, same base 531185619465b809331a9a8cf2083884365de193): CLEAN

  • The repair changes one comment line in .github/workflows/test.yaml, removing "which the Release workflow checks".
  • No workflow logic changed: the PYTHON_VERSIONS value, both selection branches, and the outputs are identical.

run: |
if [[ "$EVENT_NAME" != "pull_request" ]]; then
echo "sqla=true" >> "$GITHUB_OUTPUT"
echo "spark=true" >> "$GITHUB_OUTPUT"
{
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"

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-sol, reasoning effort high, codex exec -s read-only, session 01a0e0df-6730-7ba1-b3ab-ee60847bd7b4.

  • Snapshot: detached worktree at head a2eb20acf406a0e603ba37b562f1878c562e2c86, base 531185619465b809331a9a8cf2083884365de193. The prompt contained the literal diff and repository context only. The review was static: no tests, no network, no edits. The snapshot was unchanged afterwards.

Reviewer result, verbatim:

Surfaces covered: the specified diff, both Test workflows, the release workflow, Python classifiers and tox configuration, and the testing docs. This was a read-only review; I did not run tests.

Verdict: FINDINGS

  • P1 — .github/workflows/test.yaml:96: jq '[last]' wraps the last input document. Here the input is one array, so it produces a nested array rather than ["3.14"]. A ready PR therefore passes an array as its sole Python matrix value; the Python setup steps cannot use it as a version, so the PR’s AWS suites fail before testing. Use [.[-1]] to select the final array element.

  • P2 — .github/workflows/test.yaml:84: The comment says the Release workflow checks that this list matches the classifiers, but release.yaml has no such check. If a maintainer adds a supported version to pyproject.toml but misses this list, dispatch runs omit that version and the release can still publish. Add the check or remove the claim.

Non-blocking suggestion: Add an offline check of the generated PR and dispatch version arrays; this change has no test covering that output.

Author verification:

  1. jq '[last]' nests the array (P1): rejected, disproved by execution.
    • last without arguments is jq's builtin def last: .[-1]; (jq ≥ 1.5). It is not last(f) over input documents.
    • echo '["3.10", "3.11", "3.12", "3.13", "3.14"]' | jq -c '[last]' prints ["3.14"] (jq 1.8.2 locally; the ubuntu-24.04 runner's apt jq 1.7.1 has the same builtin).
    • This PR's own Ready CI will show run (3.14) jobs.
  2. "which the Release workflow checks" (P2): holds once Run the full test matrix in the Release workflow before publishing #862 is merged, which is the stated merge order.
  3. Offline test of the version arrays (non-blocking): declined. The selection is two jq expressions checked by execution above. The PR's own CI (pull_request) and the next dispatch or schedule run exercise both branches.

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-sol, reasoning effort high, codex exec -s read-only, session 01a0e21f-5ebf-76d0-bada-7de1f44f0cca.

  • Scope: git range-diff from a2eb20a to 738d64cf0939592ef4f395eb2a15c379689242de (base 531185619465b809331a9a8cf2083884365de193), plus the full current diff.
  • The review was static, and the snapshot was unchanged afterwards.

Reviewer result, verbatim:

Covered surfaces: repair delta and full base-to-head diff; all reusable-workflow callers; event gates, suite filters, JSON matrix propagation; Python classifiers, tox configuration, Release workflow, and changed documentation.

Verdict: CLEAN. The repair correctly removes the unsupported Release-verification claim. Eligible PRs select Python 3.14; schedule and dispatch select 3.10–3.14, matching the classifiers. All three callers supply the required input. No unsupported claims or workflow defects found in scope.

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 All @@ -108,6 +117,7 @@ jobs:
uses: ./.github/workflows/test-suite.yaml
with:
test-type: pyathena
python-versions: ${{ needs.changes.outputs.python-versions }}
skip-spark: ${{ needs.changes.outputs.spark != 'true' }}
skip-sqla: ${{ needs.changes.outputs.sqla != 'true' }}

Expand All @@ -117,10 +127,12 @@ jobs:
uses: ./.github/workflows/test-suite.yaml
with:
test-type: sqla
python-versions: ${{ needs.changes.outputs.python-versions }}

test-sqla-async:
needs: changes
if: needs.changes.outputs.sqla == 'true'
uses: ./.github/workflows/test-suite.yaml
with:
test-type: sqla_async
python-versions: ${{ needs.changes.outputs.python-versions }}
12 changes: 6 additions & 6 deletions docs/testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -151,11 +151,11 @@ Sanitize logs before sharing them.
The Test workflow runs for pull requests that change files other than `docs/` and Markdown.
It runs the offline checks (`just lint`) on each of them, including Drafts and external forks, and runs the AWS suites as follows:

| Trigger | PyAthena suite | SQLAlchemy tests | Spark tests |
| --- | --- | --- | --- |
| Draft pull request | No | No | No |
| Ready pull request from a branch of this repository | Yes | When related files change | When related files change |
| Weekly schedule and manual dispatch | Yes | Yes | Yes |
| Trigger | PyAthena suite | SQLAlchemy tests | Spark tests | Python versions |
| --- | --- | --- | --- | --- |
| 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 |

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, PR description corrected (no code change)

Base 531185619465b809331a9a8cf2083884365de193, head a2eb20acf406a0e603ba37b562f1878c562e2c86.

Findings:

  1. Claim: "Since Run AWS test suites only on ready pull requests and related changes #837, most Ready pull-request runs on 9/25–9/27 still ran 15 AWS jobs" was wrong. Run AWS test suites only on ready pull requests and related changes #837 merged at 2026-09-26 07:00 UTC, and the 9/25 runs predate it. Measured over the 30 Test runs after the merge:
    • 22 ran no AWS jobs.
    • 4 ran 5 AWS jobs.
    • 4 ran 15 AWS jobs.
      The description now states these counts.
  2. Unstated trade-off. uv.lock resolves pandas 2.3.3 and numpy 2.2.6 only for Python 3.10, while 3.11+ get pandas 3.0.6 (and numpy 2.4.6/2.5.3). Pull requests on 3.14 therefore never exercise pandas 2 (the pandas>=1.3.0 range), and pandas 2-only regressions move to the weekly, dispatch, or pre-release run. This is now stated in WHY. The decision itself (newest only) is Run pull-request AWS tests on the newest Python version only #850's and is not changed here.
  3. Claim: "cuts pull-request AWS cost by about 80%" is an estimate. It is reworded as one: it assumes each version issues about the same queries, and the actual effect is measured in Measure the CI cost after the #834 changes #846.
  4. Claim: "$20–35 on active days": Cost Explorer shows about $19–37 per active day, excluding the one-off EC2 benchmark on 9/25. Corrected.

Checked and held:

Not observed: a post-merge dispatch or schedule run with 15 jobs (stated in TEST).

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 (738d64cf0939592ef4f395eb2a15c379689242de): CLEAN, and the description was updated.

| Weekly schedule and manual dispatch | 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,7 +164,7 @@ 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 a branch, dispatch the workflow:
To run every suite on every supported Python version on a branch, dispatch the workflow:

```bash
gh workflow run test.yaml --ref <branch>
Expand Down
Loading