Skip to content

Run the PyAthena suite only when its files change #913

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 4 commits into from
Oct 1, 2026
Merged

Run the PyAthena suite only when its files change #913

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2c369bf
ci: run the PyAthena suite only when its files change
laughingman7743 Oct 1, 2026
dfc14e2
docs: say marking Ready starts only the selected AWS jobs
laughingman7743 Oct 1, 2026
e737245
docs: scope the sentence on skipped SQLAlchemy and Spark tests to sui…
laughingman7743 Oct 1, 2026
692d623
docs: allow for no AWS job when a Draft is marked ready
laughingman7743 Oct 1, 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
20 changes: 15 additions & 5 deletions .github/workflows/test.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,12 @@ jobs:
- run: just lint

# 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, this workflow, the top-level modules of pyathena and
# pyathena.aio, or the shared test fixtures change. Pull requests and the
# requests run none. A ready pull request runs the PyAthena suite only when
# the package, the tests, setup.cfg, the dependencies, or this workflow
# change; 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, this workflow, the top-level modules of pyathena
# and pyathena.aio, or the shared test fixtures 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:
Expand All @@ -81,6 +82,7 @@ jobs:
permissions:
pull-requests: read
outputs:
pyathena: ${{ github.event_name != 'pull_request' || steps.paths.outputs.pyathena == 'true' }}
sqla: ${{ github.event_name != 'pull_request' || steps.paths.outputs.sqla == 'true' }}
spark: ${{ github.event_name != 'pull_request' || steps.paths.outputs.spark == 'true' }}
python-versions: ${{ steps.versions.outputs.python-versions }}
Expand Down Expand Up @@ -125,6 +127,13 @@ jobs:
- pyathena/aio/spark/**
- tests/pyathena/spark/**
- tests/pyathena/aio/spark/**
# A superset of sqla and spark, so their tests never run without
# the PyAthena suite.
pyathena:
- *shared

@laughingman7743 laughingman7743 Oct 1, 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 and operational effects): FINDINGS, repaired in the PR description

- setup.cfg
- pyathena/**
- tests/**
- id: versions
env:
EVENT_NAME: ${{ github.event_name }}
Expand Down Expand Up @@ -156,6 +165,7 @@ jobs:
# parallel; each is still a separate job for "Re-run failed jobs".
test:
needs: changes
if: needs.changes.outputs.pyathena == 'true'

@laughingman7743 laughingman7743 Oct 1, 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): FINDINGS, repaired

  • Scope: full pass over git diff 468affc6417840912fd58b37712214c3a493edc3..2c369bf670a43bb65a36c26fbe38c1fb29d082e8, two files: .github/workflows/test.yaml (new pyathena filter, pyathena job output, if: on test, changes comment) and docs/testing.md (GitHub Actions section).
  • Behavior:
    • The output has the same form as sqla/spark, so it is the string true/false. For non-pull_request events the paths step is skipped and the output is true, so schedule, dispatch, and Release runs still select the suite. Inside the workflow_call from release.yaml, github.event_name is the caller's tag push, and release (needs: test) still waits on every suite.
    • Draft and fork PRs: changes is skipped, so test is skipped as before. A false output now skips test too; test-sqla/test-sqla-async do not depend on test, and master has no required status checks, so a skipped job blocks nothing.
    • dorny v4.0.3 sets <key>, <key>_count, and changes outputs per filter key (main.ts lines 273-285); pyathena collides with none of them. The comment lines inside filters parse as YAML comments, as in ci: run SQLAlchemy and Spark tests when shared core modules change #906.
    • skip-spark/skip-sqla inputs are unchanged. When test runs, they behave as before.
  • Superset: pyathena = shared + setup.cfg + pyathena/** + tests/**, which covers every core, sqla, and spark pattern. The simulation's invariant over all tracked files on master (279) and 3.x (236) holds.
  • What the suite reads: just tox (tox config in pyproject.toml) → uv sync --group dev (uv.lock) + just test pyathena (justfile, lint first) over tests/pyathena/. The benchmarks workspace member reaches it only through uv.lock. No test reads scripts/, cloudformation/, or benchmarks/ (grep).
  • Finding (repaired in dfc14e2): docs/testing.md still said marking the Draft ready "starts the AWS jobs". With this change it may start none, so the sentence now says "the selected AWS jobs".
  • Simplicity: no *core in pyathena, because pyathena/** and tests/** already contain it.
  • Checks after the repair: just docs lint passes; earlier just lint and actionlint pass.

uses: ./.github/workflows/test-suite.yaml
with:
test-type: pyathena
Expand Down
11 changes: 6 additions & 5 deletions docs/testing.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,18 +155,19 @@ It runs the offline checks (`just lint`) on each of them, including Drafts and e
| 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 |
| Ready pull request from a branch of this repository | When related files change | When related files change | When related files change | Newest 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/`.
When the SQLAlchemy or Spark tests do not run, the PyAthena suite runs without them.
For the SQLAlchemy tests, the related files are `pyathena/sqlalchemy/`, `pyathena/aio/sqlalchemy/`, `tests/sqlalchemy/`, their PyAthena suite test directories, and `setup.cfg`.
When the PyAthena suite runs but the SQLAlchemy or Spark tests do not, it runs without them.

@laughingman7743 laughingman7743 Oct 1, 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): FINDINGS, repaired in e737245

  • Reviewer: Codex CLI 0.157.1, model gpt-6-sol, reasoning effort high, --sandbox read-only, --ephemeral, session 01a0f50c-0ba1-7f92-982e-a63e640c8436. Static review only: no workflow or tests run.
  • Scope: full pass over 468affc6417840912fd58b37712214c3a493edc3..dfc14e24ae3003a4418aa90b3bed2c607ea0027c on a detached snapshot. Because the sandbox has no network, the package also contained action.yml, src/main.ts, and src/filter.ts of dorny/paths-filter at the pinned commit ceb8a2b8 (v4.0.3), outside the repository tree. The prompt omitted the PR number, description, commit messages, and prior findings. The snapshot's HEAD was unchanged afterwards; the only untracked entry was that reference directory.
  • Covered: the Test and Release workflow gates, the pinned action's parser and matching, the reusable suite, the test commands and fixtures, and docs/testing.md.
  • Finding (introduced, verified, repaired): docs/testing.md:165 said "When the SQLAlchemy or Spark tests do not run, the PyAthena suite runs without them." For a Ready PR that changes only scripts/check_license_headers.py, the run starts and lint runs, but all three AWS suites are skipped, so the sentence implied a PyAthena run that does not happen. It now reads "When the PyAthena suite runs but the SQLAlchemy or Spark tests do not, it runs without them."
  • Otherwise: the pyathena filter matches the specified paths and is a superset of sqla and spark. The output and the if: keep schedule, dispatch, and Release runs, and a skipped or failed changes does not start test. The reviewer found no other introduced or pre-existing issue in scope.

@laughingman7743 laughingman7743 Oct 1, 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.

Repair self-review (both perspectives): CLEAN

  • Scope: git diff dfc14e24ae3003a4418aa90b3bed2c607ea0027c..e737245e6ecabe1f77660e19ff6153d906933c28, one sentence of docs/testing.md.
  • Behavior: documentation only; no workflow change.
  • Claim: when test runs with skip-sqla/skip-spark true, test-suite.yaml adds --ignore for those test directories to PYTEST_ADDOPTS, so "it runs without them" holds.
  • Related prose: a search of docs/, README.md, CONTRIBUTING.md, AGENTS.md, .agents/, and .github/ finds no other statement that the PyAthena suite always runs. Line 170 was already scoped to "the selected AWS jobs" in round one.
  • Check: just docs lint passes.

For the PyAthena suite, the related files are `pyathena/`, `tests/`, and `setup.cfg`.
For the SQLAlchemy tests, they are `pyathena/sqlalchemy/`, `pyathena/aio/sqlalchemy/`, `tests/sqlalchemy/`, their PyAthena suite test directories, and `setup.cfg`.
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.
Changes to the modules directly under `pyathena/` and `pyathena/aio/`, the shared test fixtures such as `tests/pyathena/conftest.py` and `tests/resources/`, `pyproject.toml`, `uv.lock`, `justfile`, or the Test workflows run all three.
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 any AWS jobs its changed files select, and converting it back to Draft cancels AWS jobs still running.

@laughingman7743 laughingman7743 Oct 1, 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 1 (relayed): FINDINGS, repaired in 692d623

  • Reviewer: Codex CLI 0.157.1, gpt-6-sol, effort high, read-only, ephemeral, session 01a0f50c-... → follow-up session 01a0f50e-b944-7bd0-a43e-507edcd7a0e8. Static review of the repair dfc14e24..e737245e and the GitHub Actions section of docs/testing.md, against test.yaml and test-suite.yaml.
  • Result: the repair at line 165 resolves the earlier finding and is accurate.
  • New finding (verified, repaired): line 170 said marking the Draft ready "starts the selected AWS jobs". A Ready PR that changes only scripts/check_license_headers.py starts none. It now says "starts any AWS jobs its changed files select".

@laughingman7743 laughingman7743 Oct 1, 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.

Repair self-review (both perspectives) and independent follow-up 2 (relayed): CLEAN

  • Self-review of e737245e6ecabe1f77660e19ff6153d906933c28..692d623dc57bc9ef4d1a03021e8eab686dc06752 (one sentence): documentation only. "Any" covers both outcomes: a PR that changes only unlisted files starts no AWS job, and one that changes listed files starts the selected suites. The cancellation half of the sentence is unchanged. just docs lint passes.
  • Codex follow-up 2: same reviewer settings, session 01a0f50f-92d8-7563-ac95-db3c7e567b6e, static review of the repair and docs/testing.md:150-190 against test.yaml and test-suite.yaml. Result: CLEAN. The reported case now selects no AWS jobs, and the section is consistent.
  • After each run, the snapshot's HEAD matched the reviewed commit; its only untracked entry was the reference directory.

To run every suite on a branch, dispatch the workflow; it tests every supported Python version unless `python-versions` lists some of them:

```bash
Expand Down
Loading