This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-5246-968144bdfa9eb96df1024c3c4f5ba34afdcf8122 in repository https://gitbox.apache.org/repos/asf/texera.git
commit e27d4d05273624355537bab2897e92d7d22c647b Author: Matthew B. <[email protected]> AuthorDate: Tue May 26 23:53:56 2026 -0700 test: close stream in upload-error test to prevent finalizer leak across tests (#5246) ### What changes were proposed in this PR? The pyamber test TestCleanupFailedUpload::test_delete_object_failure_is_swallowed fails intermittently on CI. It is timing/GC-dependent, so it passes locally but trips on CI runners under load. The test expects delete_object to be called once after a simulated upload failure, but on the failing run it's called twice: - Wrap the body of `test_write_after_upload_error_raises_error` in `try/finally` and call `stream.close()` inside the `with patch.object(...)` scope so the failed-upload stream cannot survive as a zombie past the test boundary. - No production code change. `large_binary_output_stream.py` is unchanged. > [!WARNING] > This is a temporary fix to stop the intermittent CI failures until we can diagnose the underlying problem. It addresses the test-level symptom (the zombie stream surviving past the test boundary) rather than the root cause of why the failed-upload stream is being finalized late. > [!NOTE] > To reproduce deterministically locally, force GC between the two tests so the leaked stream finalizes while the next test's patches are active: ### Any related issues, documentation, or discussions? Related Issue: #5245 ### How was this PR tested? - Reproduced the CI failure deterministically on Python 3.12.13 by running the leaky test followed by `test_delete_object_failure_is_swallowed` with a forced `gc.collect()` between them, confirming the duplicate `delete_object` call. - Re-ran the same scenario after the fix: both tests pass, no duplicate `delete_object` call. - Ran the full `test_large_binary_output_stream.py` file on Python 3.12.13 and 3.13.11 (26 tests pass on each). - `ruff check` and `ruff format --check` clean over `src/main/python` and `src/test/python`. ### Was this PR authored or co-authored using generative AI tooling? Co-authored with Claude Opus 4.7 in compliance with ASF --- .../pytexera/storage/test_large_binary_output_stream.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/amber/src/test/python/pytexera/storage/test_large_binary_output_stream.py b/amber/src/test/python/pytexera/storage/test_large_binary_output_stream.py index ce9fc4e5d1..17725d9c66 100644 --- a/amber/src/test/python/pytexera/storage/test_large_binary_output_stream.py +++ b/amber/src/test/python/pytexera/storage/test_large_binary_output_stream.py @@ -215,13 +215,19 @@ class TestLargeBinaryOutputStream: mock_s3.upload_fileobj.side_effect = Exception("Upload failed") stream = LargeBinaryOutputStream(large_binary) - stream.write(b"test data") + try: + stream.write(b"test data") - # Wait a bit for the error to be set - time.sleep(0.1) + # Wait a bit for the error to be set + time.sleep(0.1) - with pytest.raises(IOError, match="Background upload failed"): - stream.write(b"more data") + with pytest.raises(IOError, match="Background upload failed"): + stream.write(b"more data") + finally: + # Close inside the patch scope so finalizer-driven cleanup + # doesn't fire against a later test's mocks. + with pytest.raises(IOError): + stream.close() def test_multiple_close_calls(self, large_binary): """Test that multiple close() calls are safe."""
