Skip to content

Comments

fix(tests): replace asyncio.sleep(1) with event-based wait to fix flaky metadata callback tests#21805

Merged
ishaan-jaff merged 1 commit intomainfrom
fix/flaky-metadata-callback-test-v3
Feb 21, 2026
Merged

fix(tests): replace asyncio.sleep(1) with event-based wait to fix flaky metadata callback tests#21805
ishaan-jaff merged 1 commit intomainfrom
fix/flaky-metadata-callback-test-v3

Conversation

@ishaan-jaff
Copy link
Member

Relevant issues

Fixes flaky test test_metadata_passed_via_litellm_metadata_responses_api (CI test ID: litellm_mapped_tests_core/1262471).

What changed

The two tests in test_metadata_codex_callback.py used await asyncio.sleep(1) to wait for the custom callback to fire. The callback is dispatched via asyncio.create_task() in utils.py, so the fixed 1-second sleep races against the event loop scheduler — causing intermittent failures.

Fix: add an asyncio.Event to MetadataCaptureCallback that gets .set() inside async_log_success_event. Tests now do asyncio.wait_for(callback.event.wait(), timeout=5.0) — deterministic, no timing dependency.

Pre-Submission checklist

  • Added at least 1 test (fixing existing tests to be reliable)
  • make test-unit passes locally

Type

  • New Feature
  • Bug Fix
  • Documentation
  • Other

Changes

  • tests/test_litellm/responses/test_metadata_codex_callback.py: replace asyncio.sleep(1) with asyncio.Event-based wait in both test functions

@vercel
Copy link

vercel bot commented Feb 21, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
litellm Ready Ready Preview, Comment Feb 21, 2026 8:33pm

Request Review

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 21, 2026

Greptile Summary

Fixes flaky metadata callback tests by replacing asyncio.sleep(1) with an asyncio.Event-based synchronization mechanism. The callback handler dispatches via an async logging worker queue, making fixed-time sleeps unreliable. The new approach uses asyncio.wait_for(callback.event.wait(), timeout=5.0) which is deterministic — it proceeds as soon as the callback fires, with a 5-second timeout as a safety net.

  • Adds asyncio.Event to MetadataCaptureCallback.__init__ and sets it in async_log_success_event
  • Replaces await asyncio.sleep(1) with await asyncio.wait_for(callback.event.wait(), timeout=5.0) in both test functions
  • No functional changes to production code; all changes are test-only

Confidence Score: 5/5

  • This PR is safe to merge — it only modifies test code with a well-understood synchronization pattern.
  • The change is test-only, minimal in scope, and replaces a known-flaky pattern (fixed sleep) with a deterministic event-based wait. The asyncio.Event + wait_for approach is the standard Python idiom for this kind of async synchronization. No production code is affected.
  • No files require special attention.

Important Files Changed

Filename Overview
tests/test_litellm/responses/test_metadata_codex_callback.py Replaces flaky asyncio.sleep(1) with event-based wait using asyncio.Event in both test functions. Clean, correct fix with no issues found.

Sequence Diagram

sequenceDiagram
    participant Test as Test Function
    participant LiteLLM as litellm.acompletion
    participant Worker as LoggingWorker
    participant CB as MetadataCaptureCallback

    Test->>LiteLLM: await acompletion(model, messages, metadata)
    LiteLLM->>Worker: enqueue(async_success_handler)
    LiteLLM-->>Test: return result
    Note over Test: Before: await asyncio.sleep(1) — race condition
    Note over Test: After: await asyncio.wait_for(event.wait(), 5s)
    Worker->>CB: await async_log_success_event(kwargs, ...)
    CB->>CB: captured_kwargs = kwargs
    CB->>CB: event.set()
    CB-->>Test: event unblocks wait_for
    Test->>Test: assert metadata preserved
Loading

Last reviewed commit: 61236c0

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

@ishaan-jaff ishaan-jaff merged commit 0267058 into main Feb 21, 2026
34 of 35 checks passed
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.

1 participant