-
Notifications
You must be signed in to change notification settings - Fork 4.3k
test: improve retries and timeouts coverage (sync + async) #2615
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: improve retries and timeouts coverage (sync + async) #2615
Conversation
Message for the OpenAI team
Hi team 👋,
I just wrapped up a PR that enhances our test coverage for timeouts and retries across both sync and async clients. The main goals were to make the suite more robust, easier to read, and safer against regressions:
Added async tests to ensure per-request timeouts correctly override client defaults.
Validated that Retry-After headers on 429 responses are fully respected, preventing unnecessary hammering of the service.
Improved docstrings and test structure using a clear given / when / then style for readability.
Patched OPENAI_API_KEY with a dummy value during tests to keep the suite self-contained.
Fixed minor pitfalls (e.g., removing inline type hints that could trigger UnboundLocalError).
These changes don’t alter production code — they just give us a safer safety net around behavior that directly impacts latency, reliability, and fairness under load.
Thanks for reviewing and approving 🙏. Looking forward to iterating further on resilience and DX.
— Lucas
Uh oh!
There was an error while loading. Please reload this page.
Note: This PR overlaps with #2615 / #2616.
If those land first, I’m happy to close this one.
The goal here is simply to improve async timeout/retry coverage and clarify per-request overrides.
Description
This PR strengthens our test suite around timeouts and retries:
✅ New async coverage (test_timeouts_async.py, test_retries.py)
Verifies per-request timeout override works for both sync and async clients.
Confirms smaller per-request timeout triggers TimeoutException.
✅ Retries with Retry-After
Ensures client respects Retry-After header for 429 responses.
Covers success-after-retry, no-retry-on-success, and max-retries-exceeded scenarios.
✅ DX improvements
Clearer comments and docstrings (given / when / then style).
OPENAI_API_KEY patched with dummy value during tests.
✅ Bugfixes
Removed inline type hints in async handlers (avoids UnboundLocalError).
Why:
Prevent regressions in retry/timeout handling across sync and async paths.
Make expected flows explicit and maintainable for future contributors.
Ensure correctness when Retry-After headers are returned by the API.
Notes:
Tests are designed to run offline with httpx.MockTransport.
No functional changes to production code.
Approved by @OscaeGTX on review.