Run AWS test suites only on ready pull requests and related changes

[laughingman7743]

WHAT

Changes when the Test workflow runs the AWS suites:

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

• test.yaml
  • A new lint job runs just lint offline for every Test run, including Draft and fork pull requests. Until now, most pull requests ran lint only inside the AWS jobs; the Scripts workflow runs it only for script, workflow, and dependency changes.
  • A new changes job lists the pull request's files through the GitHub API and decides whether the SQLAlchemy tests and the Spark tests run. The SQLAlchemy tests are the compliance suites plus the PyAthena suite's tests/pyathena/(aio/)sqlalchemy/; the Spark tests are its tests/pyathena/(aio/)spark/. It is skipped for Draft and fork pull requests, so the AWS jobs that need it are skipped too.
  • ready_for_review starts the AWS jobs when a pull request leaves Draft. converted_to_draft starts a run without them, which cancels a run still in progress through the concurrency group.
  • Files that trigger both suites: pyproject.toml, uv.lock, justfile, and the Test workflows. SQLAlchemy tests: pyathena/(aio/)sqlalchemy/, tests/sqlalchemy/, tests/pyathena/(aio/)sqlalchemy/, setup.cfg. Spark tests: pyathena/(aio/)spark/, tests/pyathena/(aio/)spark/.
• test-suite.yaml: new skip-spark and skip-sqla inputs, which add --ignore options for the PyAthena suite's Spark and SQLAlchemy test directories to PYTEST_ADDOPTS. pyproject.toml passes PYTEST_ADDOPTS through tox.
• docs/testing.md, AGENTS.md, and the development-workflow skill describe the new flow: reviews happen in Draft, gh pr ready starts AWS CI, and a failed AWS job returns the PR to Draft. gh workflow run test.yaml --ref <branch> gives AWS results while the PR is still Draft.

Unchanged: paths-ignore, the concurrency group, the fork guard in test-suite.yaml, and the release procedure, since dispatching on master runs every suite.

Deferred from #834: a smaller Python matrix for intermediate pushes (item 2). It will be revisited after measuring the effect of this change.

WHY

Refs #834. The test account cost about $143 in September. The cost scales with the number of Athena queries, and each Test run started 15 jobs, including runs for Draft pull requests. In a sample of 10,000 CI queries, 65% came from the compliance suites, and Spark cost about $20. Regressions in suites skipped on a pull request are caught by the weekly run and by the dispatch before a release.

TEST

Tested commit: 87aacab

• just scripts (includes just lint, actionlint, shellcheck, and scripts/tests): passed, 108 tests.
• mypy . on Python 3.12, the lint job's interpreter, passed (run on 99948ec; later commits do not change Python code).
• just docs lint and markdownlint-cli2 '.agents/skills/**/*.md' AGENTS.md: 0 errors.
• pytest --collect-only tests/pyathena with the Spark and SQLAlchemy ignores in PYTEST_ADDOPTS: 1,297 tests, none under the ignored directories. Without them: 1,889 tests.
• Ran the changes filter locally on sample file lists:
  • pyathena/cursor.py, tests/pyathena/test_cursor.py, and .github/workflows/docs.yaml → neither
  • pyathena/(aio/)sqlalchemy/base.py, tests/sqlalchemy/test_suite.py, tests/pyathena/(aio/)sqlalchemy/test_base.py, and setup.cfg → SQLAlchemy only
  • pyathena/spark/cursor.py and tests/pyathena/aio/spark/test_cursor.py → Spark only
  • uv.lock and .github/workflows/test-suite.yaml → both

Observed on this pull request (at 2c3588b):

• While it was a Draft, only lint ran; changes and the AWS jobs were skipped.
• Marking it Ready (run 36222483878): changes succeeded and all 15 AWS jobs started, because the PR changes the Test workflows.
• Converting it back to Draft (run 36222765121, lint only) cancelled those AWS jobs.
• The final Ready run for the current head is recorded in the PR checks.
• The skip path for a non-dialect PR can only be observed after merge. It will be recorded on Reduce the AWS cost of integration tests without reducing real-service coverage #834.

🤖 Generated with Claude Code

[laughingman7743]

Self-review round one (implementation behavior): FINDINGS, repaired

• Perspective: behavior and failure paths of the workflow conditions, framework contracts (GitHub Actions needs/if on reusable-workflow jobs, pull_request activity types), and simplicity.
• Base 2a107adf1f68713e7b7da2e334cc98ca543b7e7d, head 99948ec7fd9ea8524c2b997fe9700562c27aa5a0 (initial pass), repair checked at 44fab0164369491f4171cb6f55f7509a8f40fb65.
• Covered: .github/workflows/test.yaml, .github/workflows/test-suite.yaml, the tox passenv in pyproject.toml, docs/testing.md, AGENTS.md, .agents/skills/development-workflow/SKILL.md.
• Checked:
  • A Draft or fork PR skips changes, so the three needs: changes jobs are skipped. Observed on the Draft run: lint passed, changes/test/test-sqla/test-sqla-async skipped.
  • ready_for_review has draft == false. Schedule and dispatch take the non-PR branch, which sets both outputs to true.
  • A failing gh api call fails the step under bash -e. The dependent jobs are then skipped and the run is red, not silently green.
  • An empty PYTEST_ADDOPTS is a no-op. With the ignores set, pytest --collect-only tests/pyathena collects no Spark tests.
  • No branch protection requires these checks, so skipped jobs do not block merges.
• Finding: docs/testing.md:151 said lint runs on every pull request. paths-ignore means docs/Markdown-only PRs do not run the Test workflow at all. Repaired in 44fab01 (this line and the lint job comment at line 37).
• Limit: the skip path for a non-dialect ready PR can only be observed after merge.

[laughingman7743]

Self-review round two (claims and operational behavior): FINDINGS, repaired

• Base 2a107adf1f68713e7b7da2e334cc98ca543b7e7d, head 44fab0164369491f4171cb6f55f7509a8f40fb65.
• Claims checked:
  • The trigger table here matches the changes filter. Verified locally against sample file lists (dialect, compliance tests, Spark, uv.lock, workflow, docs workflow, tests/pyathena/sqlalchemy/).
  • Dispatch runs every suite, so the release procedure (dispatch on master) keeps full coverage.
  • gh pr ready --undo is the command that returns a PR to Draft.
  • The cost figures in the PR body (65% of sampled queries from the compliance suites, about $20 Spark) come from the measurements recorded on Reduce the AWS cost of integration tests without reducing real-service coverage #834.
• Finding: the PR body said lint previously ran only inside the AWS jobs. The Scripts workflow also runs just lint (via just scripts) for script, workflow, and dependency changes. PR body corrected.
• Operator view: a Draft PR now makes no AWS calls. A ready non-dialect PR runs 5 jobs instead of 15. Weekly and pre-release runs still cover everything. A regression in a skipped suite surfaces at the next weekly or release run.
• Deferred: the smaller Python matrix for intermediate pushes (Reduce the AWS cost of integration tests without reducing real-service coverage #834 item 2), pending cost measurement.

[laughingman7743]

Independent review (relayed): Codex, via the Claude Code codex:codex-rescue agent (Codex thread 01a0dc2e-d2c6-7920-a11f-1a93c81fb383): FINDINGS. This was a static review: the reviewer was told not to edit, run tests, or access GitHub, and did none of these.

• Snapshot: detached worktree at head 44fab0164369491f4171cb6f55f7509a8f40fb65, base 2a107adf1f68713e7b7da2e334cc98ca543b7e7d, literal diff supplied. No PR number, PR description, or prior findings were included in the prompt.
• Covered: the diff, the changed workflow/process files, justfile, the tox config, and the SQLAlchemy/Spark callers relevant to the path filters.
• Finding 1 (test.yaml:13 at the reviewed head): the trigger list omitted converted_to_draft. Returning a Ready PR to Draft while its AWS jobs ran started no new run, so the concurrency group did not cancel them. That contradicted the documented Draft behavior and the skill's gh pr ready --undo step. Accepted and repaired in 7787388: converted_to_draft now starts a run in which changes and the AWS jobs are skipped, and that run cancels the in-progress one. docs/testing.md states this.
• Finding 2 (path filters, line 88 at this head): shared modules such as pyathena/util.py trigger neither the compliance suites nor the Spark tests, although those callers import them. Rejected as a deliberate trade-off, not a defect. Reduce the AWS cost of integration tests without reducing real-service coverage #834's agreed direction limits the compliance-suite trigger to the dialect, its tests, and dependencies, and relies on the weekly run and the pre-release dispatch (both run every suite) to catch regressions from other modules. docs/testing.md lists exactly which paths trigger each suite.

[laughingman7743]

Repair self-review, both perspectives, for 7787388 (range 44fab0164369491f4171cb6f55f7509a8f40fb65..7787388be47f2fe656c628568567c513f7585edc): CLEAN

• Round one (behavior): a converted_to_draft event has draft == true, so changes and the three AWS jobs are skipped and only lint runs. The run shares the refs/pull/N/merge concurrency group, so cancel-in-progress cancels an in-progress AWS run. paths-ignore still applies, so docs-only PRs start no run, as before. actionlint passes.
• Round two (claims and operations): the new docs/testing.md sentence (converting back to Draft cancels the AWS jobs) and the workflow comment match this behavior. The extra cost is one offline lint job per conversion to Draft. The skill's gh pr ready --undo step now also stops any AWS jobs still running.

[laughingman7743]

Independent follow-up (relayed): Codex, via the Claude Code codex:codex-rescue agent (Codex thread 01a0dc33-0cbe-7793-9652-9b5bd11a373f), static review of 44fab01..7787388: FINDINGS

• Confirmed that the repair fixes finding 1: converted_to_draft starts a run in which changes and the AWS jobs are skipped, and the shared concurrency group cancels the in-flight run.
• New finding: paths-ignore also applies to converted_to_draft. If a Ready PR reverts its code while AWS jobs run, it is left changing only Markdown, and neither the revert nor the conversion to Draft starts a run. The unconditional claim that converting to Draft cancels the jobs in docs/testing.md was therefore too broad. Accepted; wording repaired in 1ec4635 (7787388..1ec4635): the docs sentence is now scoped to the pull requests the workflow runs for, and the workflow comment notes that paths-ignore applies. Behavior is unchanged. For a docs-only PR, the in-flight jobs finish normally and the docs-only state needs no AWS coverage.
• Repair self-review, both perspectives: CLEAN. actionlint and just docs lint pass. The sentence now follows the section's scope (docs/testing.md:151). No other prose claims unconditional cancellation.

[laughingman7743]

Independent follow-up 2 (relayed): Codex, via the Claude Code codex:codex-rescue agent (Codex thread 01a0dc35-0bd0-7522-82f0-064c7f046913), static review of 7787388..1ec4635: FINDINGS

• Confirmed that the paths-ignore wording repair resolves the previous finding.
• New finding: docs/testing.md:163 ("these pull requests") still included external forks from line 152. For a fork PR, marking it Ready starts no AWS jobs, because changes requires the head repository to match. Accepted; wording repaired in 46d3c52 (1ec4635..46d3c52): the table row and the sentence now say "a pull request from a branch of this repository". Behavior is unchanged.
• Repair self-review, both perspectives: CLEAN. just docs lint passes. The fork policy paragraph that follows (line 170) is unchanged and consistent with the new wording.

[laughingman7743]

Independent follow-up 3 (relayed): Codex, via the Claude Code codex:codex-rescue agent (Codex thread 01a0dc37-16c6-74e1-b7cd-f63b1170cee1), static review of 1ec4635..46d3c52 plus a full pass over the GitHub Actions section: FINDINGS

• Confirmed that the fork-scoping repair matches the repository check in test.yaml.
• Finding: rewording for forks dropped the docs-only scope, so the Draft-cancellation claim at docs/testing.md:163 was unconditional again, which regresses follow-up 1. Accepted; repaired in 2c3588b (46d3c52..2c3588b): the sentence now covers a pull request from a branch of this repository that still changes files other than docs/ and Markdown. Behavior is unchanged.
• Repair self-review, both perspectives: CLEAN. just docs lint passes. The sentence now states both conditions that paths-ignore and the changes job impose.

[laughingman7743]

Independent follow-up 4 (relayed): Codex, via the Claude Code codex:codex-rescue agent (Codex thread 01a0dc39-9c42-7210-b055-5a49c602d347), static review of 46d3c52..2c3588b plus a final pass over the GitHub Actions section of docs/testing.md against test.yaml and test-suite.yaml: CLEAN. The reviewer confirmed that the cancellation sentence is now limited to same-repository PRs that still change files outside docs/ and Markdown, and that the remaining claims in the section match the workflow definitions. Independent review is complete for head 2c3588ba2ceacd15737a4f60af4a508ee66c246e.

[laughingman7743]

Self-review, both rounds, of the skip-sqla change requested by the maintainer (2c3588ba2ceacd15737a4f60af4a508ee66c246e..87aacabba94436acb6d30a39a2aae044ae5ac879): CLEAN

• Round one (behavior):
  • PYTEST_ADDOPTS is a folded scalar that joins the two conditional ignore groups. With both inputs false it is whitespace only, which pytest treats as no options (checked with PYTEST_ADDOPTS=' ': 1,889 tests collected).
  • With both inputs true, 1,297 tests are collected and none come from tests/pyathena/(aio/)spark/ or tests/pyathena/(aio/)sqlalchemy/. Neither directory has a conftest.py, so ignoring them removes no shared fixtures.
  • skip-sqla reuses the sqla output, which now also matches tests/pyathena/(aio/)sqlalchemy/: editing those tests runs them, and also runs the compliance suites. This keeps a single SQLAlchemy trigger, as for Spark.
  • Only the test (pyathena) job receives skip-sqla; the compliance jobs keep their job-level if. actionlint and just scripts pass.
• Round two (claims and operations):
  • docs/testing.md now names the "SQLAlchemy tests" column and lists both parts and all trigger paths. It states that the PyAthena suite runs without the skipped directories. just docs lint passes.
  • Operator effect: a ready PR that touches neither area now runs 1,297 of the PyAthena suite's 1,889 tests. Dialect regressions caused by core modules (cursor, formatter, converter) are then caught by the weekly or pre-release run, the same trade-off already accepted for Spark and the compliance suites.
  • The PR body was updated to describe both inputs and the observed Ready/Draft runs.

[laughingman7743]

Independent follow-up (relayed): Codex, via the Claude Code codex:codex-rescue agent (Codex thread 01a0dc58-a3ed-7892-ad0b-91b3546dc52a), static review of 2c3588b..87aacab: CLEAN.

The reviewer checked:

• the folded scalar and boolean expressions (the ignore groups are space-separated, and empty groups leave only harmless whitespace);
• PYTEST_ADDOPTS propagation through the tox passenv into the pytest command;
• the filter regexes;
• that no test outside the ignored directories imports fixtures or setup from them;
• that docs/testing.md matches the workflow.

No builds or tests were run. Independent review is complete for head 87aacabba94436acb6d30a39a2aae044ae5ac879.