[laughingman7743]

WHAT

Implements the test_s3_async.py rows of the #1328 audit (comment). Only tests/pyathena/filesystem/test_s3_async.py changes; pyathena/filesystem/ does not.

Removed (each named behavior fails another test, offline unless noted):

Test	Tests that fail without the behavior
register_async_filesystem (fixture), test_pandas_read_csv	The string registration loses to the S3FileSystem class that import pyathena.pandas registers, so the pandas test read through S3FileSystem; sync test_pandas_read_csv (S3) fails without the registration
test_copy_object_with_multipart_upload_waits_for_running_parts	..._cancelled, ..._loop_shutdown[part], ..._failed_part_with_threads_taken (same cleanup path)
test_copy_object_with_multipart_upload_failed_part_stops_running_parts	sync test of the same name, ..._failed_part_with_threads_taken (session code)
test_copy_object_with_multipart_upload_cancelled_creation_stops_retries	passed with the whole cleanup removed; ..._cancelled_creation fails
test_multipart_copy_uses_creation_algorithm	sync test of the same name, TestS3Core::test_complete_multipart_upload_uses_creation_algorithm
TestAioS3File::test_open_parts_in_flight, test_open_failed_part_while_waiting	TestS3File::test_write_parts_in_flight (5 cases), test_write_failed_part_while_waiting (4 cases)
test_put_file_mode	the _put_file cases of the delegation test; sync test_put_file_mode
test_touch_sync_wrapper	replaced by touch cases of the delegation test
test_find, test_glob, test_rm_recursive, test_upload_cp_file (S3)	delegated code: the delegation test, test_expand_path_glob_lists_stem_prefix, the offline rm tests, test_cp_file_multipart_parameters[10]; on S3, sync test_glob, test_rm_recursive, test_upload_cp_file

Rewritten:

• test_copy_object_with_multipart_upload_cancelled_queued_completion: bounded await; the completion is detected whichever pool method queues it.
• test_cp_file_multipart_parameters: the bound is checked with overlapping_calls instead of a time.sleep(0.01) overlap.
• test_copy_version: asserts the CopyObject request through _serve_keys instead of the awaited _cp_file arguments.
• test_transaction_pipe_file_compression (was test_pipe_file_compression): only the transaction cases, whose pipe_file() is the asyncio filesystem's own.
• test_transaction_pipe_put_file_create_existing: no internal exists() call count.
• test_write_cancelled: bounded part wait and await.
• TestAioS3FileSystem::fs: closes the filesystem.
• test_flush_cancelled_while_queued_then_exit_with_exception: bounded pool shutdown.
• The worker gates of cmu_cancelled_queued_completion, test_write_cancelled, test_commit_cancelled and the queued flush test are bounded and record whether they timed out, so a regression that blocks the event loop fails instead of hanging or passing.

Merged: test_close_cancelled and test_flush_cancelled_then_exit_with_exception into test_commit_cancelled[close|flush], with a bounded pool shutdown instead of asyncio.sleep(0.1).

Added: the delegation test covers _find(detail=True) (its entries are built by the asyncio method), touch, _mkdir, _makedirs, _rmdir, rmdir, getxattr, setxattr, list_multipart_uploads, clear_multipart_uploads, created, modified, and sign; test_invalidate_cache; test_expand_path_version_maxdepth.

WHY

Part of #1328: tests that repeat another test's contract, depend on unsynchronized timing, or have unbounded waits, and asyncio delegations that no test called or whose mutations no test failed. Rewriting the fs._call seams stays with #1340.

TEST

At 5004add7 (the offline full run at 47da52cb; 5004add7 changes only the fake of the sign case and the maxdepth test, rerun below):

• just lint: passed.
• Offline, uv run --env-file <offline env> pytest --noconftest -p no:rerunfailures -n 8 tests/pyathena/filesystem (every AWS endpoint pointed at a closed port): 1,050 passed; the failures are exactly the tests that need S3 (the baseline at 766d79d0 plus the new test_fsspec_conformance.py), without the five removed S3 tests.
• Mutations of pyathena/filesystem/, reverted after each run:
  • Each added case fails under its own mutation: _mkdir dropping **kwargs, _makedirs dropping exist_ok, _rmdir/rmdir doing nothing, sign dropping expiration or **kwargs (rerun at 5004add7, also without an endpoint override), getxattr dropping **kwargs, setxattr dropping copy_kwargs, list_multipart_uploads returning [], clear_multipart_uploads doing nothing, created/modified returning None, touch dropping truncate or **kwargs, invalidate_cache doing nothing, the asyncio maxdepth check removed (rerun at 5004add7), _copy not pairing a version.
  • The rewritten tests still fail under the mutations they caught: the copy ignoring max_workers, begin_abort aborting a COMPLETING write (test_commit_cancelled[flush] and the queued flush test), the completion not cancelled.
  • The asyncio _rm passing recursive=True fails the offline rm tests.
• On S3 (uv run --env-file .env pytest -n 1), with the synchronous code mutated: sync test_rm_recursive (_delete_paths always recursive), test_upload_cp_file (no CopyObject), test_pandas_read_csv (no registration), and test_glob (_find ignoring withdirs) failed. Sync test_find passes when _find drops prefix, as the removed asyncio test_find, with the same assertions, would; test_expand_path_glob_lists_stem_prefix fails under it. The maxdepth line of the removed test is covered offline: _find ignoring maxdepth fails test_find_matches_fsspec, test_glob_matches_fsspec and test_find_directory_without_extra_requests, and the _find delegation case checks that the asyncio method passes it.
• At f35cb2dd (repairs of the independent review): just lint passed; offline test_s3_async.py 142 passed, the 5 failures are its S3 tests; _find(detail=True) returning to_dict() entries fails the new case, which no test failed before; invalidate_cache doing nothing still fails test_invalidate_cache, which now also passes with FSSPEC_S3='{"use_listings_cache": false}' (both cases failed at 5004add7); the copy ignoring max_workers, begin_abort aborting a COMPLETING write, and the completion left uncancelled still fail their tests.
• At beb30b11: just lint passed; offline test_s3_async.py 142 passed, the 5 failures are its S3 tests; _wait_ready blocking the loop on the part waiter fails test_write_cancelled[1,2] in 20 s (it hung at the base), and the copy cleanup blocking the loop on the queued abort fails cmu_cancelled_queued_completion in 10 s.
• On S3 at 5004add7: the five remaining S3 tests of TestAioS3FileSystem that use the closing fs fixture passed.
• Rerun at 5004add7: the removal evidence for the cleanup, session-stop and checksum tests (_S3WriteSession.abort not waiting, _stop_requests not setting the stop, the shielded abort skipped, the part checksum dropped) and the _put_file delegation of mode.
• Not run: the full AWS suite; the Ready CI runs it.

🤖 Generated with Claude Code

[laughingman7743]

Self-review round one (behavior and implementation), base ad48ad5271d35148f267c5e4cdd44455d41bb8c6, head 47da52cb55fadbedf355d1c8e5149a2bfe9ad606. Covered: every removed, rewritten, merged and added test of test_s3_async.py, the _serve_keys fake, the fs fixture teardown, and the mutation evidence of the PR body.

FINDINGS:

1. The sign case compared signed URLs after dropping X-Amz-Date/X-Amz-Signature only for https:// URLs. botocore signs with Signature Version 2 by default (absolute Expires, Signature), and with the offline environment's endpoint the URL starts with http://, so the whole URL, including the time of signing, was compared: the case passed only when both runs signed in the same second, and without an endpoint override (as in CI) query.pop("X-Amz-Date") raised KeyError. Repaired in 5004add7: _serve_keys records the arguments of S3Core.generate_presigned_url, signing's only input from the filesystem, as a request, and the URL normalization and the sign exception of the request guard are gone. Rechecked: sign dropping expiration or **kwargs fails the case; three repeated runs and a run without AWS_ENDPOINT_URL pass.
2. test_expand_path_version_maxdepth (line 1744) added a use of the _call seam that Remove the duplication between S3FileSystem and AioS3FileSystem #1340 removes, and matched the undocumented message. Repaired in 5004add7: the connection=MagicMock() filesystem sends nothing anyway, and ValueError alone fails the mutation (# noqa: PT011, as elsewhere in the file).

Checked without findings: the delegation cases with allow_bucket_creation/allow_bucket_deletion (no other case creates or deletes buckets); the double spy of cmu_cancelled_queued_completion (submit_to reaches the patched submit with item.run, which does not match); test_commit_cancelled waits for f._pool, in which the commit and a discard run; the class fs fixture's close() (the five remaining S3 tests of the class pass on S3).

[laughingman7743]

Self-review round two (claims, callers, evidence), base ad48ad5271d35148f267c5e4cdd44455d41bb8c6, head 5004add7c1ca692e69ef92d7aad17470759c6f6b. Claims of the PR body and both commit messages checked against the code and rerun mutations.

FINDINGS:

1. The PR body said each added case was the only detector of its mutation; the touch cases replace test_touch_sync_wrapper, which detected some of the same mutations. Narrowed to delegations that no test called or whose mutations no test failed.
2. The TEST section described 47da52cb. It now names 5004add7, says which runs were repeated there (the sign and maxdepth mutations, the five remaining S3 tests with the closing fixture, the removal evidence below), and that the full offline run belongs to 47da52cb, whose only difference is the fake of the sign case and the maxdepth test.
3. The message of 5004add7 says that botocore signs with Signature Version 2 "unless an endpoint or the configuration selects version 4"; with AWS_ENDPOINT_URL set, the URL was still version 2, so only the configuration selects version 4 here. The branch is not rewritten for it; the repair itself does not depend on the signature version.

Verified claims: the removal evidence reruns at this head (_S3WriteSession.abort not waiting fails cmu_cancelled, cmu_loop_shutdown and TestS3File tests; _stop_requests not setting the stop fails the sync cmu_failed_part_stops_running_parts and cmu_failed_part_with_threads_taken; skipping the shielded abort fails cmu_cancelled* and nine others; dropping the part checksum fails the sync and TestS3Core creation-algorithm tests; _put_file dropping mode fails the delegation case); outside a transaction the asyncio pipe_file runs S3FileSystem.pipe_file (s3_async.py:224), so the compression test keeps only the transaction cases; no other file references the removed or renamed tests. Callers: tests only, no public behavior changes. AWS: five S3 tests fewer per run of the class, and the class fixture now closes its threads.

[laughingman7743]

Independent review (relayed): Codex CLI 0.164.0, model gpt-6-astra, codex exec --sandbox read-only, session 01a178f8-94ee-7c42-aa6b-88a35fb8abdb; static review of the diff ad48ad5271d35148f267c5e4cdd44455d41bb8c6..5004add7c1ca692e69ef92d7aad17470759c6f6b on a detached snapshot, with docs/testing.md "Choose what to test" and no PR text, commit messages or prior findings. The first run hit its 30-minute limit before the report; the same session was resumed only to output it. Result: FINDINGS (4). Its coverage table found every other removed test covered with the same kind of evidence, and the added and rewritten tests protective. The snapshot and the PR worktree were unchanged.

Finding 1 (P2), verified and repaired in f35cb2dd. Reviewer: removing the real-S3 test_find loses the only check of the entries that AioS3FileSystem._find(detail=True) builds itself (s3_async.py:797); returning {f.name: f.to_dict() ...} would break callers that read .name/.size. Confirmed: with that mutation no offline test failed. Repair: this detail=True case, and comparable() compares S3Object entries by type and content (a dict equals an S3Object as a Mapping). The mutation now fails this case.

[laughingman7743]

Self-review of the repair 5004add7..f35cb2dd (both perspectives). Behavior: comparable() now recurses into dicts and compares S3Object by type and content; S3Object is a MutableMapping, not a dict, so the branches do not overlap, and the other delegation cases (_info, _ls(detail=True), get_tags, touch) compare as before (all 37 selected cases pass). The two gates are released by the tests' finally blocks in a correct run, so the bound changes only a regression's outcome. The cache arguments are fsspec's DirCache options. Claims: the commit message and the PR body's Added/Rewritten/TEST entries match the repair and the reruns listed there. CLEAN.

[laughingman7743]

Finding 2 (P2), verified and repaired in f35cb2dd. Reviewer: the worker gates of cmu_cancelled_queued_completion (here) and test_flush_cancelled_while_queued_then_exit_with_exception used an unbounded release.wait() while holding the only thread; a regression that blocks the event loop on the queued work would keep the loop from reaching release.set(), and the bounded await could not fire. Both gates now wait at most EVENT_TIMEOUT. Rechecked: the completion left uncancelled still fails this test, and begin_abort aborting a COMPLETING write still fails the flush tests.

[laughingman7743]

Independent follow-up (relayed): Codex CLI 0.164.0, model gpt-6-astra, read-only, session 01a1791b-493b-76e2-b3e2-dcd906b37351, static review of 5004add7..f35cb2dd. Findings 1 and 3: resolved; comparable() keeps the other cases' comparisons. New finding (P2) on this repair: a bounded gate lets a regression that blocks the event loop end, and the timeout then releases the held call, so the test can pass; for example the copy cleanup waiting for the queued abort on the loop.

Verified and repaired in beb30b11: the gates of cmu_cancelled_queued_completion, test_write_cancelled, test_commit_cancelled and test_flush_cancelled_while_queued_then_exit_with_exception record release.wait(EVENT_TIMEOUT)'s result and the tests assert that none timed out. Rechecked: _wait_ready blocking the loop on the part waiter fails test_write_cancelled[1,2] in 20 s (it hung before this PR), and the copy cleanup blocking the loop on submit(_abort).result() fails cmu_cancelled_queued_completion in 10 s; the six affected tests pass; just lint passed.

[laughingman7743]

Self-review of f35cb2dd..beb30b11 (both perspectives): in a correct run each gate is released by the test's finally and appends True before the asserted point (the abort behind the only worker, the abort after both parts, the joined pool), so the assertions do not depend on timing; the commit message describes the change as made. CLEAN.

Independent follow-up (relayed): Codex CLI 0.164.0, model gpt-6-astra, read-only, session 01a17921-a7f8-7001-9ec8-4556e8ee52e8, static review of f35cb2dd..beb30b11 on a detached snapshot: CLEAN; a regression that blocks the event loop until a gate times out records False, so the assertions fail, and the appends precede the assertions at lines 1039, 2634, 2682 and 2742. No tests were run by the reviewer. Offline at beb30b11: test_s3_async.py 142 passed, the 5 failures are its S3 tests.

[laughingman7743]

Finding 3 (P3), verified and repaired in f35cb2dd. Reviewer: test_invalidate_cache assumed fsspec's default listing cache; with FSSPEC_S3='{"use_listings_cache": false}' its premise fails. Confirmed: at 5004add7 both cases failed with that variable. The test now passes use_listings_cache=True, listings_expiry_time=None, passes with the variable set, and still fails when invalidate_cache does nothing.

[laughingman7743]

Finding 4 (P3), not changed. Reviewer: overlapping_calls(1) releases the first call after its 0.5 s grace, so a regression whose second part copy starts later than that would pass. This is the documented design of the shared helper (tests/pyathena/util.py:310), used by the other bound tests: an exceeded bound can be missed under load but never fails a correct implementation, and waiting for a second call without the grace would make every correct run wait the full timeout. The mutation that ignores max_workers fails this test.