[laughingman7743]

WHAT

Review the PyHive-derived DB API tests that are repeated for each cursor class, on the tests that #1252, #1253, #1256, #1258, #1261, and #1265 left. Each copy is counted by its code path: the result set class and its fetch implementation, the read path (CSV, CSV in chunks, or UNLOAD), and the sync, async, or aio cursor. One test stays per path. This diff changes no file under pyathena/.

Path	Kept	Removed
AthenaResultSet, GetQueryResults rows, Cursor	test_fetchone (also the query execution properties), test_fetchmany, test_fetchall, test_arraysize	test_iterator
AthenaDictResultSet, DictCursor	test_dict_type, test_null_vs_empty_string	test_fetchone, test_fetchmany, test_fetchall
AthenaResultSet / AthenaDictResultSet, AsyncCursor / AsyncDictCursor	test_arraysize, test_query_execution, test_dict_type	test_fetchone, test_fetchmany, test_fetchall, test_iterator
pandas (CSV, CSV in chunks, UNLOAD), Arrow and Polars (CSV, UNLOAD), S3FS (CSV); sync and async cursors	test_arraysize for each read path	test_fetchone, test_fetchmany, test_fetchall, test_iterator, test_open_close
pandas (CSV), AioPandasCursor	test_fetchone	test_fetchall

test_arraysize is the test kept on each pandas, Arrow, Polars, and S3FS path because the cursor's own execute() passes arraysize to the result set. TestAsyncS3FSCursor::test_arraysize now also compares the rows of SELECT a FROM many_rows ORDER BY a LIMIT 20, because the removed copies were the last tests to check row values returned through AsyncS3FSCursor, whose _collect_result_set() passes the converter; each other async cursor keeps such a check in test_description. Every fetch on these result sets is a method of the result set. Those methods are checked offline in the test_result_set.py of that result set, or of AthenaResultSet for the methods that the Arrow and S3FS result sets inherit. The other parts of a removed copy are code that no cursor class overrides, and a kept test runs them:

• WithFetch.fetchone(), fetchmany(), and fetchall() delegate to the result set for every synchronous cursor; TestCursor checks them.
• CursorIterator.__next__() is shared by every synchronous cursor and result set; TestAthenaArrowResultSet::test_fetch_across_empty_batches iterates a result set.
• DictCursor, the pandas, Arrow, Polars, and S3FS cursors, and their async variants do not override close(); TestCursor::test_open_close and TestAsyncCursor::test_open_close check WithResultSet.close() and AsyncCursor.close().
• The execute() of a cursor class with a given read path is the one that the kept test_arraysize of that class and read path runs.

The default and aio test_fetchall no longer run SELECT * FROM one_row; the many_rows query reads the first page and the later ones.

These copies stay because they run code of their own:

• AioCursor's test_fetchone, test_fetchmany, test_fetchall, and test_arraysize, and AioDictCursor's test_fetchone: AioCursor has its own fetch methods and AthenaAioResultSet fetches asynchronously.
• test_executemany of the default, pandas, Arrow, Polars, and S3FS cursors: each result set reads the result of an INSERT through its own reader, also on a cursor with the unload option, which leaves INSERT unwrapped.
• test_bad_query: each async result set class handles a failed query in its constructor; the aio cursor raises.
• The SQLAlchemy tests (test_basic_query, test_reflect_schemas, test_has_table, and the others with PyHive names): the aio copies run the aio connection adapter, and the per-driver cases run each dialect's cursor class.

NOTICE no longer says that tests/pyathena/aio/sqlalchemy/test_base.py ports test_unicode, which #1251 removed; it names the PyHive test cases that file still ports, test_basic_query and test_reflect_schemas. It also states that the cursor tests of the other cursor classes repeat some of PyHive's DB API test cases, such as test_arraysize and test_fetchone. The PyHive-derived tests that NOTICE names remain.

	Before	After
Test functions with a PyHive test name	105	54
Collected tests under tests/pyathena/	3745	3662

The removed cases ran 97 Athena queries per run, and the trimmed test_fetchall cases 2 more.

WHY

Closes #1260. Part of #1239: the removed copies repeated the contract of a kept test on the same code path with the same kind of evidence, including copies of synchronous tests for async or aio variants that run through the same code, which "Choose what to test" in docs/testing.md excludes.

TEST

Python 3.15.0.

At 9e652a5, on 82754d2. 9e652a5 is the reviewed series (8e85cb2, on 0b18cdb) rebased without conflicts; git range-diff shows the three patches unchanged. The upstream changes in between change tests/pyathena/s3fs/test_result_set.py and tests/pyathena/test_cursor.py, outside the hunks of this change.

• just format: no changes. just lint: passed.
• Offline (--noconftest with placeholder AWS settings, tests/pyathena without filesystem/): 1763 passed, 16 failed, 607 errors, 24 skipped, 2 xfailed. At 82754d2: 1769 passed, 18 failed, 682 errors. The failures and errors are tests that need AWS; none is new at the head, and the 6 fewer passes are the removed offline test_open_close copies.
• Live, once, at 8e85cb2 after the reviews (02:18-02:26 UTC; no other Test workflow AWS job was running at the start; the AWS test job of another PR, run 39089607580, started at the same time and passed): uv run --env-file .env pytest -n 4 -q -p no:cacheprovider on the 12 changed test files: 515 passed, 3 skipped (conditional skips in those files for the Polars version and the environment), 2 xfailed, in 448 s. The full AWS suite runs in CI on this PR.

Offline mutation checks, with --noconftest and placeholder settings. Each mutation was reverted and git diff -- pyathena/ was empty after each. Before the removals, at c5356c4e, and again at 9e652a5, each mutation below failed the offline tests named; every test that failed a mutation at c5356c4e also failed it at 9e652a5:

Mutation	Failed offline tests (examples)
AthenaResultSet.fetchone() does not count the row in rownumber	3 tests, including TestAthenaArrowResultSet::test_fetch_across_empty_batches, TestAthenaResultSet::test_fetchall_across_pages
AthenaResultSet.fetchmany() without a size takes arraysize + 1 rows	1 test: TestAthenaS3FSResultSet::test_reader_null_conversion
AthenaResultSet._take_rows() takes one row fewer	3 tests, including TestAthenaArrowResultSet::test_fetch_across_empty_batches, TestAthenaS3FSResultSet::test_reader_null_conversion
AthenaResultSet._fetch() never requests the next page	5 tests, including TestAthenaResultSet::test_fetch_after_failed_fetch, TestAthenaResultSet::test_fetchall_across_pages
CursorIterator.__next__() skips a row	1 test: TestAthenaArrowResultSet::test_fetch_across_empty_batches
AthenaDictResultSet builds tuples instead of dicts	3 tests, including TestAthenaAioResultSet::test_async_iteration, TestAthenaAioResultSet::test_fetchmany_across_pages
AthenaPandasResultSet.fetchone() does not count the row	2 tests, including TestAthenaPandasResultSet::test_fetch_across_chunks, TestAthenaPandasResultSet::test_fetch_after_chunks_run_out
AthenaPandasResultSet._take_rows() takes one row fewer	1 test: TestAthenaPandasResultSet::test_fetch_across_chunks
AthenaPolarsResultSet.fetchone() does not count the row	3 tests, including TestAthenaPolarsResultSet::test_fetch_across_chunks, TestAthenaPolarsResultSet::test_fetch_after_chunks_run_out
AthenaPolarsResultSet._take_rows() takes one row fewer	1 test: TestAthenaPolarsResultSet::test_fetch_across_chunks
AthenaArrowResultSet._fetch() buffers only the first record batch	1 test: TestAthenaArrowResultSet::test_fetch_across_empty_batches
AthenaS3FSResultSet._fetch() drops the first row of each batch	4 tests, including TestAthenaS3FSResultSet::test_csv_open_options, TestAthenaS3FSResultSet::test_reader_closes_at_end_of_results
WithAsyncFetch.fetchall() runs fetchmany() in the thread	2 tests, including TestWithAsyncFetch::test_cancelled_fetch_detaches_result_set, TestWithAsyncFetch::test_fetch_runs_result_set_fetch
AthenaAioResultSet.fetchone() does not count the row	2 tests, including TestAthenaAioResultSet::test_fetchall_across_pages, TestAthenaAioResultSet::test_fetchone_across_pages
AthenaAioResultSet._async_take_rows() takes one row fewer	1 test: TestAthenaAioResultSet::test_fetchmany_across_pages
AthenaAioResultSet._async_fetch() never requests the next page	5 tests, including TestAthenaAioResultSet::test_fetch_after_cancelled_fetch, TestAthenaAioResultSet::test_fetch_after_failed_fetch
WithResultSet.close() raises	19 tests, including TestWithAsyncFetch::test_context_exit_closes_off_event_loop, TestCursor::test_open_close
AsyncCursor.close() raises	14 tests, including TestAsyncCursor::test_open_close

Breaking the delegation in WithFetch.fetchmany() or WithFetch.fetchall(), or a query execution property of WithResultSet, failed no offline test. Those methods are shared by every synchronous cursor, and TestCursor checks them against Athena; the removed copies ran the same methods.

🤖 Generated with Claude Code

[laughingman7743]

Self-review round 1 (implementation behavior). Base 0b18cdb, head a8e7439. Result: CLEAN.

Covered: all 13 changed files (NOTICE and the 12 test files), traced against the fetch, iteration, close, and execute code of Cursor, DictCursor, AsyncCursor, AsyncDictCursor, the sync and async pandas, Arrow, Polars, and S3FS cursors, and AioPandasCursor.

• Behavior: the sync cursors define no fetchone/fetchmany/fetchall/__next__/__iter__/close, so WithFetch, CursorIterator, and WithResultSet.close() are shared; the async cursors other than AsyncCursor do not override close(). The __next__/__iter__ in pandas/result_set.py and polars/result_set.py belong to the DataFrame iterators, not to the result sets.
• Data boundaries: reading many_rows with UNLOAD (several result files) is still checked by test_many_as_pandas/test_many_as_arrow/test_many_as_polars with the unload parameter. Fetching across chunks and record batches is checked offline (test_fetch_across_chunks, test_fetch_across_empty_batches). Hypothesis: S3FS refilling across _fetch() batches lost its only test with the removed 10000-row test_fetchall. Not a finding: the offline helper in s3fs/test_result_set.py uses arraysize=1, and a mutation that drops the reader after the first batch failed 11 offline tests there (reverted; git diff -- pyathena/ empty).
• Test quality: the kept test_arraysize detects a broken passthrough, because without it the result set uses DEFAULT_FETCH_SIZE (1000) and fetchmany() returns all 20 rows instead of 5.
• Docs and agent skills do not refer to the removed tests.

Limitations: the live AWS run has not been done yet; it runs once after the reviews.

[laughingman7743]

Rebased the reviewed series 8e85cb2 (on 0b18cdb) onto 82754d2 as 9e652a5 without conflicts; git range-diff 0b18cdb7..8e85cb22 82754d2f..9e652a50 shows the three patches unchanged. Upstream contracts checked: #1266 rewrote test_reader_closes_at_end_of_results and test_managed_results_preserve_api_values in s3fs/test_result_set.py, which this PR cites as offline covers; #1264 added a live test to test_cursor.py. At 9e652a5, just lint passed, the offline suite has no new failure against 82754d2, and all 22 offline mutations were rerun: every test that failed a mutation at c5356c4e also failed it at 9e652a5.

[laughingman7743]

Self-review round 2 (claims, callers, operations, evidence). Base 0b18cdb, head a8e7439 (reviewed), republished as f986774 with the same tree. Result: FINDINGS, repaired in the PR body and the commit message.

Claims checked:

• No file under pyathena/ changes: git diff --stat 0b18cdb7..HEAD -- pyathena is empty.
• test_arraysize checks the cursor's own passthrough: arraysize=self.arraysize / self._arraysize in each sync execute() and async _collect_result_set() (pandas/cursor.py:245, pandas/async_cursor.py:185, and likewise for Arrow, Polars, S3FS, and async_cursor.py:200).
• test_bad_query: the Arrow, Polars, and S3FS result set constructors branch on STATE_SUCCEEDED (arrow/result_set.py:142, polars/result_set.py:373, s3fs/result_set.py:127).
• test_executemany: each result set checks the output suffix and substatement of an INSERT result in its own CSV read (arrow/result_set.py:259, polars/result_set.py:668, s3fs/result_set.py:177).
• Counts: 105 to 54 test functions with a PyHive name, 3745 to 3662 collected tests, 97 + 2 queries; Python 3.15.0.

Findings and repairs:

1. The PR body and the commit message said that each result set's own test_result_set.py checks its fetch methods, rownumber, and iteration. The S3FS and Arrow result sets inherit fetchmany() and the row counting from AthenaResultSet, checked in tests/pyathena/test_result_set.py, and iteration is checked only through the Arrow result set. Both texts now say so.
2. The offline comparison was made against fb22d08, not 0b18cdb. Rerun at 0b18cdb: 1769 passed, 18 failed, 681 errors; at the head 1763 passed, 16 failed, 606 errors. No failure is new, and the 6 fewer passes are the removed offline test_open_close copies. The mutation results come from c5356c4e; git diff fb22d08a 0b18cdb7 touches none of the offline covers named.
3. The WHY covered only async/aio copies, and "cursors other than Cursor and AsyncCursor" was broader than checked; both are narrowed to what was checked.

Operator: the change removes 99 Athena queries per run and changes no retry or quota behavior. Documentation: no doc or skill refers to the removed tests. Limitation: the live run has not been done yet.

[laughingman7743]

Independent review (relayed). Reviewer: Codex CLI 0.163.0, model gpt-6-astra, reasoning effort max, codex exec -s read-only, session 01a14cb6-4e35-7922-abc2-c8d191602c15. Base 0b18cdb, head f986774, reviewed in a detached snapshot with the literal diff, without the PR text, commit messages, or earlier findings. Static review: no edits, tests, builds, or network. Result: FINDINGS (2).

Covered: 51 removed test methods (83 parametrized cases), the two shortened test_fetchall, and NOTICE; traced fixtures, cursor construction, execute(), _collect_result_set(), fetching, iteration, arraysize, and cleanup against the remaining live and offline tests, and compared PyHive's test files.

Finding 1 (P2): removing test_fetchone, test_fetchall, and test_open_close from TestAsyncS3FSCursor removes every check of non-empty row values returned through AsyncS3FSCursor. The remaining successful query only checks len(result_set.fetchmany()) == 5; test_read_options mocks AthenaS3FSResultSet. Scenario: _collect_result_set() passes another converter instead of self._converter, so ("1",) is returned instead of (1,), and no test fails; the tests that build the result set directly or use the sync and aio S3FS cursors bypass this collector.

Observations without change: the one_row queries removed from the kept test_fetchall were redundant; the removed pandas test_fetchall did not pass chunksize to its 10000-row query; the removed test_open_close copies keep their context-manager and close paths through fixtures and test_no_ops; no imports, helpers, fixtures, or comments became stale.

Author verification: confirmed. Every other async cursor keeps a value check through its own cursor (test_description asserts fetchall() == [(1,)] in test_async_cursor.py, pandas/, arrow/, and polars/test_async_cursor.py); s3fs/test_async_cursor.py has none.

[laughingman7743]

Repaired in 8e85cb2: test_arraysize runs SELECT a FROM many_rows ORDER BY a LIMIT 20 and asserts fetchmany() == [(i,) for i in range(5)], which checks the arraysize and the converter that _collect_result_set() passes, in the same single query. Self-review of the repair (both perspectives): a is the integer column of many_rows; the other async cursors keep their value checks in test_description; no query is added. Limitation: this is a live test, so the mutation (another converter in _collect_result_set()) is checked by reasoning, not run: the rows would be strings and not equal (0,) to (4,); the live run after the reviews checks that the assertion passes.

[laughingman7743]

Independent review (relayed), finding 2 (P2), same reviewer and session as the review on tests/pyathena/s3fs/test_async_cursor.py: deleting the whole reference to tests/pyathena/aio/sqlalchemy/test_base.py leaves recognizable PyHive material unlisted. Its test_basic_query runs SELECT * FROM one_row and checks one row, number_of_rows == 1, and one column, as PyHive's SqlAlchemyTestCase.test_basic_query does. Replace the obsolete example with the surviving test instead of removing the file's attribution.

Author verification: confirmed; the aio test_reflect_schemas also checks the test schema and default as PyHive's does. Checking the same question for the DB API tests found that test_arraysize (arraysize = 5, SELECT * FROM many_rows LIMIT 20, len(fetchmany()) == 5) and the aio test_fetchone remain PyHive's test cases in the cursor tests of the other cursor classes, which NOTICE did not mention before this change either.

[laughingman7743]

Repaired in 8e85cb2: NOTICE states that tests/pyathena/aio/sqlalchemy/test_base.py ports test_basic_query and test_reflect_schemas to the asynchronous dialect, and that the cursor tests of the other cursor classes under tests/pyathena/ repeat some of PyHive's DB API test cases, such as test_arraysize and test_fetchone. Self-review of the repair: both statements were checked against the remaining tests and PyHive ae3c9a96 (SqlAlchemyTestCase.test_basic_query/test_reflect_schemas, DBAPITestCase.test_arraysize/test_fetchone); the PR body now describes the NOTICE change.

[laughingman7743]

Independent follow-up review (relayed). Reviewer: Codex CLI 0.163.0, model gpt-6-astra, codex exec -s read-only, session 01a14cc4-353f-7f61-88bc-cd2b2879589c. Scope: the repair f986774..8e85cb2 (NOTICE and TestAsyncS3FSCursor::test_arraysize), in a detached snapshot of 8e85cb2. Static review: no edits, tests, builds, or network. Result: CLEAN.

Covered: the follow-up diff, both touched files, the cursor, converter, and result set flow, the many_rows fixtures, the previous arraysize check, and the NOTICE statements against the remaining tests and PyHive's test files. ORDER BY a makes (0,) to (4,) the first five rows; the assertion fails on wrong values, including unconverted strings; fetchmany() without a size on 20 rows still checks arraysize. NOTICE:31 describes the remaining repetitions accurately, and NOTICE:37 resolves the missing aio SQLAlchemy attribution. The snapshots and the PR worktree were unchanged after both reviews.