Audit the filesystem tests changed since the #1239 audit #1328

Activity

1. laughingman7743

   added

   enhancement

   on Oct 24, 2026
2. laughingman7743

   added a parent issue on Oct 24, 2026

   • Review the filesystem package before a standalone release #1326
3. laughingman7743 commented on Oct 24, 2026

   

   laughingman7743

   on Oct 24, 2026

   MemberAuthor

   More actions

   Audit for this issue at 766d79d0 (master after #1321), read-only.

   Scope correction. 59e00fbf is the merge of #1262, not of #1259, so the 41 commits under tests/pyathena/filesystem/ include #1259 itself (19 GH-1239 commits) besides #1228 (3), #1229 (2), #1230 (6), #1208 (3, plus the merge d36a1f82), #1209 (6) and #1309 (1). #1281 changed no filesystem test. The range adds 22 and changes 66 test functions and helpers in seven files, and removes 81 (inventory by AST).

   Method. Every added or changed test was traced to the contract it names, compared with the tests that could protect the same contract with the same kind of evidence (for the asyncio tests: whether the asyncio code is its own or delegates to the synchronous code), and checked by about 140 mutations of pyathena/filesystem/ in scratch worktrees. Each mutation was run against the whole offline filesystem suite: --noconftest, every AWS endpoint pointed at a closed local port, 1,048 tests passing at baseline. The 95 tests that need S3 were read instead of run; their rows say "AWS-only". The rest of the suite was swept for the patterns of scope 2 and 3, and the contracts of the #1327 report and #1332-#1342 were checked for missing tests. A removal below always names the other tests that failed under the same mutation.

   cmu_ abbreviates test_copy_object_with_multipart_upload_.

   1. Tests changed since 59e00fbf

   test_s3_async.py

   Test	Verdict	Contract / evidence
   _RecordingCallback, _serve_keys, _multipart_copy (helpers)	keep	Used by the delegation and copy tests. _serve_keys and 13 sites in lines 1-1460 assign sync_fs._call, which nothing calls; only _core.call takes effect (see "Not in this issue").
   release_blocked_waits (fixture)	keep	Without it, a cleanup regression fails the tests in 21 s and keeps the process alive until killed (400 s). It patches pyathena.filesystem.s3_async.wait, so it silently stops working if #1332 or #1340 moves the wait to another module; re-check it there.
   cmu_small_head_object_size	keep	Single-request branch of the asyncio driver; only test failing when fits_single_request is ignored.
   cmu_copies_source	keep	Requests of the asyncio driver, annotations from the completed version.
   cmu_failed_part	keep	Only asyncio test of the abort request parameters.
   cmu_failed_annotation	keep	A failed annotation is raised and the completed destination kept.
   cmu_waits_for_running_parts	remove	Same except BaseException path as cmu_cancelled: every mutation that fails it (no shielded abort, no withdrawal, abort not waiting, no stop) also fails cmu_cancelled, cmu_loop_shutdown[part] or cmu_failed_part_with_threads_taken. Its ordering check depends on time.sleep(0.05).
   cmu_max_workers	keep	Direct test of the copy's own bound.
   test_delegations_send_the_requests_of_the_sync_methods	keep	Each parameter is the only test of its delegation (six mutations, each fails exactly its case). Extended in section 2.
   test_max_workers_none	keep	Only test of the asyncio None resolution.
   cmu_cancelled[1,2]	keep	[2] is the only test of the shielded cleanup under a repeated cancellation.
   test_close_releases_threads	keep	close(), with, async with each fail their own case.
   test_close_during_multipart_copy	keep	close() does not wait for a running part (shutdown instead of release fails it).
   cmu_cancelled_completion	keep	Waits for a started completion; the sleep(0.1) backs only a negative check.
   cmu_cancelled_creation	keep	Waits for the creation and aborts a created upload.
   cmu_cancelled_queued_completion	rewrite	await task (line 1021) is unbounded: use await_bounded. Its spy on submit_to makes it fail for an unrelated change of how the completion is submitted; assert the requests instead.
   cmu_interrupted_while_scheduling_completion	keep	Only test failing when the completion future is created by submit() (the shape before #1230).
   cmu_failed_part_with_threads_taken	keep	Only test of begin_complete() before queueing; carries the asyncio stop path.
   cmu_failed_part_stops_running_parts	remove	The stop is session code: the mutation fails the sync test of the same name and cmu_failed_part_with_threads_taken. The two asyncio-only mutations of that path (no begin_abort, no settled() wait) do not fail this test.
   cmu_cancelled_creation_stops_retries	remove	Passes with the whole cleanup (begin_abort and the shielded abort) removed, because operation.calls == 1 is checked before the retries end; cmu_cancelled_creation fails under that mutation. The session stop is covered by TestAioS3StreamedFile::test_cancelled_creation_stops_retries and TestS3File::test_interrupted_creation_stops_retries.
   cmu_failed_annotation_stops_running_copies, cmu_failed_annotation_skips_queued_copies	keep	Asyncio-only parallel annotations; each the only detector of its mutation.
   cmu_cancelled_queued_parts	keep	Queued part copies are cancelled before running ones are waited for.
   cmu_loop_shutdown	keep	Only test of the documented asyncio.run() shutdown guarantee, which task.cancel() does not reproduce.
   test_rm_requests	keep	Only test of the call's parameters on DeleteObjects.
   test_rm_maxdepth	keep	Only test of maxdepth in _rm.
   test_put_file_in_transaction_open_parameters	keep	Asyncio copy of the transaction code (goes with #1340's shared transaction).
   test_expand_path_glob_lists_stem_prefix	keep	Only offline test of the stem-prefix listing and of _find(detail=True).
   test_get_version	keep	Asyncio copy of the version preamble (#1340).
   test_cp_file_multipart_parameters	rewrite	Only test of max_workers reaching the copy from _cp_file, but max(concurrency) == 1 relies on two parts overlapping during time.sleep(0.01): use overlapping_calls.
   test_multipart_copy_uses_creation_algorithm	remove	The behavior is in S3Core: breaking it fails the sync test and TestS3Core::test_complete_multipart_upload_uses_creation_algorithm too; the asyncio-only parameter paths are caught by the other copy tests, not by this one.
   TestAioS3FileSystem::fs (fixture)	rewrite	Class-scoped and never closed; since #1209 the filesystem owns threads, so yield it and close().
   test_write_transaction	keep	AWS-only. Only real-S3 multipart write through AioS3File; revisit with #1332.
   test_find, test_rm_recursive, test_upload_cp_file	remove	AWS-only. The asyncio methods delegate to the synchronous code that the sync S3 tests cover; the forwarding is caught offline by the delegation test, the rm tests and test_cp_file_multipart_parameters. Saves about six PutObject, listings, a copy and reads per run. The implementing PR confirms on S3 that the sync tests fail under the same mutations.
   TestAioS3File::test_open_parts_in_flight, test_open_failed_part_while_waiting	remove	AioS3File is S3File, and the session enforces both: allowing max_workers + 1 parts fails these with TestS3File::test_write_parts_in_flight (5 cases) and test_write_failed_part_while_waiting (4 cases). They do not check S3AioExecutor's own bound either (removing it leaves them passing).
   TestAioS3File::test_open_lookup_parameters	keep	Only test of _open forwarding its parameters.
   TestAioS3StreamedFile::test_write_parts_in_flight	keep	Only test that the wait does not block the event loop.
   test_open_async_cancelled	keep	A file opened after a cancelled open_async() is closed, without a ResourceWarning.
   test_append_failed_part_copy	keep	Own driver of AioS3StreamedFile.
   test_commit_failed_part_first	keep	Only test of a failure that arrives while the commit waits (the sync commit() raises through check() before it waits).
   test_write_after_failed_part	keep
   test_close_cancelled	merge	No single mutation fails it (only two combined). Make it a close() case of test_flush_cancelled_then_exit_with_exception.
   test_flush_cancelled_while_queued_then_exit_with_exception	keep	Bound the final f._pool.shutdown(True).
   test_close_interrupted_while_queuing_commit, test_flush_cancelled_then_exit_joins_abort, test_close_small, test_cancelled_read_frees_ranges	keep	Each the only detector of its mutation.

   test_s3.py

   Test	Verdict	Contract / evidence
   cmu_unknown_parameter	keep	A parameter that CopyObject does not accept reaches CreateMultipartUpload.
   cmu_part_sizes	keep	Only test of the driver's default part size (GH-951).
   cmu_interrupted_creation	keep	With the removals below, the only test of the driver's except BaseException.
   cmu_interrupted_part_scheduling	remove	Catching only Exception fails cmu_interrupted_creation; an abort that does not wait for running parts fails TestS3File::test_part_scheduling_interrupted[write/append] (the [append] case schedules a part copy through the same submit_copy), test_discard_waits_for_running_parts and test_commit_stops_at_failed_part. Synchronizes by replacing the private session._changed.
   cmu_repeated_creation_interrupt	remove	An unshielded creation wait fails TestS3File::test_repeated_creation_interrupt[wb/ab]; catching only Exception fails cmu_interrupted_creation.
   cmu_interrupted_cleanup	rewrite	#1229 changed it from an interrupt while the cleanup waits for running part copies to an interrupted abort. The first case is documented (docs/filesystem.md:150-153) and now unprotected: suppressing KeyboardInterrupt in _S3WriteSession.abort()'s wait for the parts fails no offline test. Add it back as a second case.

   test_s3_core.py

   Test	Verdict	Contract / evidence
   test_request_kwargs_is_a_copy, test_call_translates_errors, test_get_object, test_list_objects, test_complete_multipart_upload, test_upload_part_uses_creation_algorithm_and_identity, test_replace_object_metadata, test_generate_presigned_url	keep	Each the only detector of its mutation (copy, __cause__, precedence of call parameters, storage_class, given parts, per-call fields, retained encryption, unsigned request_kwargs).
   test_abort_multipart_upload_in_stopped_scope	keep	Moved from test_s3.py by #1230; the only test of the abort's unstoppable_retries().
   test_multipart_upload_requires_identity, test_acl_validation, test_object_operations_reject_paths, TestS3DeleteBatch::test_from_paths	rewrite	Drop match= on undocumented messages; the exception type alone fails every mutation (without the checks, Stubber raises UnStubbedResponseError or nothing is raised).

   test_s3_executor.py

   Test	Verdict	Contract / evidence
   TestS3AioThreadPool::test_default_max_workers, test_run_cancelled_after_return, test_run_cancelled_before_start, test_release_after_calls, test_kept_future_keeps_no_pool, test_release_after_interrupted_submit, test_fork	keep	Each the only detector of its mutation; waits bounded.
   TestS3AioThreadPool::test_shutdown	rewrite	Removing the pool's own shutdown check leaves it passing, because the stdlib pool raises the same RuntimeError. Also submit to a shut-down pool that has no threads (never used, or after release()).
   TestS3Executor::test_submit_to_loop_shutdown	keep
   TestS3Executor::test_submit_to_executor_shut_down, test_submit_to_cancelled_executor_shut_down	keep	Reach their branch only through the private executor._pool.shutdown(), because S3AioExecutor.shutdown() orders the pool shutdown after the dispatches; decide with #1332. Drop match="after shutdown".

   test_s3_errors.py, test_s3_path_pairing.py, test_s3_path.py

   Test	Verdict	Contract / evidence
   TestS3ClientError::test_os_error_by_error_code, _errno, _by_http_status_code	keep	Each table entry fails its case. The generic 405 → EPERM case stays valid with #1338: #1345 recognizes the delete-marker header in test_s3_core.py and does not change this file.
   TestS3PathPairing::test_move_pairs_null_version_bucket_state	keep	Unit evidence for the pairing.
   test_s3_path.py	—	One removal only (GH-1239).

   Tests removed by #1228, #1229, #1230, #1208, #1209, #1309

   Removed test	Now protected by
   TestS3FileSystem::test_abort_multipart_upload_in_stopped_scope (#1230)	TestS3Core::test_abort_multipart_upload_in_stopped_scope (fails under the mutation)
   TestS3AioThreadPool::test_submit_during_release (added and replaced within #1286)	test_release_after_calls
   asyncio cmu_interrupted_while_queuing_completion (#1230)	renamed to cmu_interrupted_while_scheduling_completion
   the interrupt-during-part-wait case of cmu_interrupted_cleanup (#1229)	nothing; restored by the rewrite above

   2. Contracts without a test, and tests without a contract

   Missing tests that this issue adds (the current behavior is the contract)

   Each mutation left the whole offline suite passing, and no AWS test asserts the contract.

   Contract	Missing test	Where
   The asyncio methods that call the synchronous method pass their arguments and return its result: _mkdir, _makedirs, _rmdir/rmdir, sign, getxattr, setxattr, list_multipart_uploads, clear_multipart_uploads, created, modified, invalidate_cache, touch	Parameters of test_delegations_send_the_requests_of_the_sync_methods, with _serve_keys extended for the multipart listing, abort and ACL. Uncaught today: setxattr dropping copy_kwargs, sign dropping expiration, invalidate_cache doing nothing, list_multipart_uploads returning [], _mkdir dropping **kwargs. Before #1340, this protects its refactor.	test_s3_async.py
   sign(expiration=N) expires after N seconds	test_sign (AWS) passes a 3,600 s URL; add an upper bound, or check offline (signing sends no request); drop its sleep(1)	test_s3.py
   modified()/created() return LastModified, None for directories	Offline; no test calls them	test_s3.py
   touch() of a version path raises ValueError	A case of an existing touch test	test_s3.py
   A file opened without cache_type uses the documented "bytes"	An unset case of test_read_with_cache_type	test_s3.py
   The asyncio _expand_path rejects maxdepth < 1	A case of an asyncio _expand_path test, unless #1337 delegates _expand_path first	test_s3_async.py
   An interrupt while the copy cleanup waits for running parts stops the cleanup and logs the upload ID	The cmu_interrupted_cleanup rewrite in section 1	test_s3.py

   Tests without a contract, unsynchronized waits, and AWS cost (rest of the suite)

   File	Test	Verdict	Reason
   test_s3_async.py	register_async_filesystem (fixture)	remove	Registers by string, which lands in known_implementations, while import pyathena.pandas/pyathena.polars puts S3FileSystem in _registry, which fsspec checks first: after that import, get_filesystem_class("s3") is S3FileSystem. No teardown; with it disabled, all 139 offline tests of the file pass.
   	test_pandas_read_csv	remove	Its only dependent; in a suite run it reads through S3FileSystem and repeats the sync test against S3.
   	test_glob	remove	AWS-only. fsspec's _glob runs over the delegated _find; the only asyncio-specific branch (detail) fails test_expand_path_glob_lists_stem_prefix.
   	test_put_file_mode	remove	Delegated to put_file; dropping mode= fails the delegation test; the sync test protects the translation.
   	test_pipe_file_compression	rewrite	Keep intrans=True; the False cases run the sync code (breaking it fails them and 14 sync cases).
   	test_touch_sync_wrapper	merge	Into the delegation test as touch rows; renaming _touch away leaves it passing.
   	test_transaction_pipe_put_file_create_existing	rewrite	Drop exists.call_count == 2; the contract is FileExistsError without a request.
   	test_copy_version	rewrite	Assert the CopyObject request through _serve_keys instead of the awaited _cp_file arguments.
   	test_flush_cancelled_then_exit_with_exception	rewrite	Drop asyncio.sleep(0.1) (__aexit__ awaits the discard; the mutation still fails without it), bound release.wait(), and take the close() case above.
   	test_write_cancelled	rewrite	release.wait() has no timeout: blocking the loop in _wait_ready hangs the test instead of failing it.
   	test_mv, test_mv_copy_failure	keep	Replace the private _copy_file; rewrite through the client once #1340 settles the mv failure policy.
   	test_rm_request_error_keeps_errors, TestAioS3File::test_failed_read_frees_other_ranges, cmu_shares_threads, TestAioS3File::test_open_parallel_requests	keep	Checked: each has its own code or synchronizes the bound it checks.
   test_s3.py	test_write, test_append, test_pipe_cat, test_put, test_put_with_callback, test_upload_cp_file	rewrite	AWS cost: the (1, 2**20) case takes the same single-request path as (1, 2**10); drop it and the commented-out cases.
   	test_open_multipart_with_checksum[None-None], test_append_multipart_with_checksum[None-None]	rewrite	AWS cost: the plain multipart write and part-copy append that other tests check; drop the case unless a checksum case needs it as a control.
   test_s3_executor.py	TestS3Executor::test_submit_to	remove	Both outcomes fail other tests (test_submit_to_failure_frees_arguments, test_failed_part_is_released, test_submit_context, four S3AioExecutor tests).
   test_reader.py	test_close_waits_for_running_reads, test_interrupted_close, test_close_with_queued_reads	rewrite	join(0.2) plus is_alive() passes a regression whenever the closer has not reached close() in 0.2 s; assert the order of the gated read's end and the close.
   test_init.py	test_register_s3_filesystem_overwrites_existing_registration	rewrite	Matches the undocumented log text; assert a WARNING record from pyathena.filesystem.
   test_s3_object.py	model test_inits	keep	Look literal, but breaking four documented getters fails only them; rewrite with #1341.

   3. Not in this issue

   • Defects. Their tests belong to the fixing PRs, and these existing tests pin the defective behavior: aio test_mv asserts DeleteObjects without the given RequestPayer (Pass the source's SSE-C and RequestPayer parameters to the lookups of copies and setxattr #1333); test_open_version_id and test_open_version_aware_pins_version_in_path index f.__reduce__()[1][1] (Keep the caller's parameters and the open-time pin in S3File #1334); the three recursive-chmod tests mock fs.find and bypass the defect (Fix recursive chmod, empty key segments, and version paths in listings #1335); tests/pyathena/polars/test_result_set.py patches S3FileSystem while the whole-CSV read goes through fsspec.open() (Fix the filesystem's integration with PolarsCursor, the fsspec registry, pickling, and connection= #1339). Make files usable after fork with either filesystem #1336, Align AioS3FileSystem with S3FileSystem for cancellation, recursive put, expand_path, and arguments #1337 and Translate delete-marker versions to FileNotFoundError and fix two S3Core edge cases #1338 only add tests.
   • Design decisions. Remove the duplication between S3FileSystem and AioS3FileSystem #1340: the fs._call/fs._client seams, which nothing in the package calls, are used by 209 of 281 tests in test_s3.py and 30 of 91 in test_s3_async.py (58 more reach into _sync_fs); rewrite them onto fs.core in Remove the duplication between S3FileSystem and AioS3FileSystem #1340's PR, so they are rewritten once. Settle the response models, S3Object, and the listing cache #1341: fs.dircache is inspected by 37 sync and 3 asyncio tests, test_touch_put_object asserts ["_etag"], test_s3_object.py depends on the mapping. Consolidate the filesystem executors before parallel batch operations #1332: S3AioExecutor has no test of its own max_workers bound, and S3AioExecutor, S3ThreadPoolExecutor and S3AioThreadPool are named by 17, 18 and 18 tests. Decide request-parameter coverage, error translation, and S3 retry codes #1342: the retry-stop tests (8 sync, 11 asyncio, 1 core, 4 executor) fake S3 throttling with Athena's ThrottlingException, which S3 does not send.
   • Run fsspec's conformance tests against the S3 filesystems #1331. After it merges, its real-S3 copy, get, put, pipe and open conformance classes may repeat the contracts of sync integration tests in test_s3.py; compare them then.

   Implementation

   One PR per file group, as #1240 and #1241 did:

   1. test_s3_async.py: the asyncio rows of sections 1 and 2, including the delegation-test extension and the fixture removal. No open PR touches the file.
   2. test_s3_core.py, test_s3_errors.py, test_s3_executor.py, test_reader.py, test_init.py: after Translate delete-marker versions to FileNotFoundError and fix two S3Core edge cases (GH-1338) #1345 (Translate delete-marker versions to FileNotFoundError and fix two S3Core edge cases #1338) merges, which edits test_s3_core.py next to test_call_translates_errors and test_part_ranges.
   3. test_s3.py: after Run the batch operations of S3FileSystem concurrently (GH-1322) #1344 (Run the batch operations of S3FileSystem concurrently #1322) merges, which edits _make_fs and inserts tests after test_mv_nothing_within_maxdepth. The rows here do not overlap those lines, and the fs._call rewrite stays with Remove the duplication between S3FileSystem and AioS3FileSystem #1340.

   Every removal keeps the mutation evidence above in its PR; the five AWS-only removals are confirmed on S3 there.
4. laughingman7743

   added 3 commits that reference this issue on Oct 26, 2026

   Record the signing arguments instead of comparing signed URLs (GH-1328)

   ...

   5004add

   Check the asyncio find details, bound two waits, and enable the listi…

   ...

   f35cb2d

   Fail the cancellation tests whose gate is released by its timeout (GH…

   ...

   beb30b1