Skip to content

test: fix flaky tests_temp teardown cleanup#28

Merged
knep merged 1 commit into
masterfrom
fix/flaky-tests-temp-cleanup
Jun 24, 2026
Merged

test: fix flaky tests_temp teardown cleanup#28
knep merged 1 commit into
masterfrom
fix/flaky-tests-temp-cleanup

Conversation

@knep

@knep knep commented Jun 24, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes the recurring CI flake in StreamingFormReaderTest (it broke PRs #21, #26 and #27, each needing a re-run). The failure was always at teardown:

OSError: [Errno 39] Directory not empty: 'tests_temp'
IsADirectoryError: [Errno 21] Is a directory: 'tests_temp'

Root cause — two bugs in test_utils._rmtree

  1. Handler mishandled directories. The on_rm_error handler always called os.remove(path). But shutil.rmtree also invokes the handler for directories (when os.rmdir fails), and os.remove on a directory raises IsADirectoryError. Fixed by re-running the operation that actually failed — func(path) (os.unlink for files, os.rmdir for directories) — which is the pattern from the Python docs.

  2. No tolerance for the race. Some tests upload files via background threads that are still writing into tests_temp when teardown starts; a file appearing between rmtree's scandir and rmdir leaves the directory non-empty. Added a retry loop (up to ~1s) so those threads can finish before giving up.

Testing

  • New regression tests (tests/test_utils_rmtree_test.py): plain tree, read-only file (exercises the handler), and a concurrent writer (reproduces the race). 8/8 stable.
  • Flaky test run 15× locally: 15/15 pass.
  • Full backend suite: 1740 passed (×2 runs).
  • No onerror DeprecationWarning on Python 3.14 (onerror kept for 3.10–3.11 compatibility; onexc is 3.12+).

🤖 Generated with Claude Code

The shared _rmtree() helper failed intermittently in CI
(StreamingFormReaderTest), with "IsADirectoryError: Is a directory: tests_temp".
Two bugs:

1. The on-error handler always called os.remove(path). shutil.rmtree also
   invokes the handler for directories (when os.rmdir fails), where os.remove
   raises IsADirectoryError. Fixed by re-running the failed operation func(path)
   (os.unlink for files, os.rmdir for directories) — the documented pattern.

2. No tolerance for the underlying race: upload threads in some tests are still
   writing into the temp folder during teardown, so a file appears between
   rmtree's scandir and rmdir and leaves the directory non-empty. Added a retry
   loop (up to ~1s) that lets those threads finish before giving up.

Adds regression tests covering a plain tree, a read-only file (exercises the
handler), and a concurrent writer (reproduces the race).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@knep knep merged commit d4a9b98 into master Jun 24, 2026
8 checks passed
@knep knep deleted the fix/flaky-tests-temp-cleanup branch June 24, 2026 20:10
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants