Skip to content

test: fix stale and flaky tests across CircleCI, GHA and Buildkite - #45530

Merged
yuneng-berri merged 23 commits into
mainfrom
litellm_ci_upkeep_1009
Oct 9, 2026
Merged

yuneng-berri merged 23 commits into
mainfrom
litellm_ci_upkeep_1009

Conversation

@yuneng-berri

@yuneng-berri yuneng-berri commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

  • Stale, flaky and broken tests keep CircleCI, GHA and Buildkite red
  • People stop trusting red CI and merge past real failures

How it solves it:

  • 20 test-only fixes, one commit each, no product code touched
  • Product bugs found on the way are reported, not masked

Which tests, and why they flaked

Each entry names the test, the evidence that it was flaky or broken rather than a product bug, and the fix

CircleCI

  • tests/integration/database/test_partition_transactions.py::test_real_partition_ddl_survives_witnessed_lock_and_is_idempotent
    • Failed in both integration-database jobs on one PR run and passed on main at the same code
    • The lock-witness poll ran a blocking psycopg connect and query on the test's event loop, so the DDL task under test only progressed in 20ms gaps and missed its 3s deadline
    • Poll through asyncio.to_thread, deadline 10s. Passes against a local Postgres
  • tests/llm_translation/test_xai.py::TestXAIReasoningEffort (2 tests)
    • Red on every scheduled run. Called directly, grok-4.7 now returns reasoning tokens but no reasoning text, so reasoning_content is never set
    • Moved to grok-4.6 with a step-by-step arithmetic prompt: 6 of 6 direct calls returned reasoning text, a bare "Hello" only some of the time. Both tests passed 4 times live
  • tests/integration/observability/test_passthrough_upstream_error_chaos.py::test_passthrough_worker_sigkill_leaves_sibling_serving_and_logging and tests/integration/routing/test_audit_any_sweep_chaos.py::test_proxy_survives_worker_kill_mid_burst
    • Passed in 3 of 5 runs, the failures were the 90s pytest timeout firing while the owned proxy was stopping
    • About 50s of burst plus up to graceful_stop_seconds() (70s) of stop does not fit in 90s. They now get 2 * graceful_stop_seconds() + 120, like other owned-proxy tests
  • tests/integration/spend/test_vertex_image_regional_uplift.py::test_identical_regional_generations_each_bill_the_uplift_once
    • The two response bodies differed only in created (1791533128 vs 1791533129), a second-boundary race
    • Compare the bodies without created; both spend rows are still asserted
  • tests/integration/providers/test_s3_wire.py::test_s3_sync_and_async_uploads_pass_independent_wire_verification
    • Counted every asyncio task created while the logger starts, which also caught client close finalizers left by earlier tests, so the count ranged from 1 to 8 across runs
    • Count only the CustomBatchLogger.periodic_flush task the test owns. Passes locally twice
  • tests/integration/observability/test_guardrail_timeout_all_providers.py::test_litellm_params_timeout_bounds_outbound_call
    • A different set of providers failed with "sink saw no request" on every run
    • All 44 probes hit a two-worker proxy at once, so some guardrails used up their whole 1s timeout before their request left the proxy. Probes now go eight at a time
  • tests/integration/observability/test_arize_otel_v2_openinference_chaos.py::test_arize_otel_v2_f3_one_proxy_worker_can_die
    • Failed waiting 10s for the killed worker to stop running, then overran the 90s timeout while stopping
    • psutil reports an unreaped zombie as running. The check now accepts a zombie or a missing process, and the test gets the owned-proxy time budget
  • tests/integration/providers/test_decisions_wire.py::test_test_connection_evaluation_mode_uses_typesafe_decisions_path
    • Broken since it landed in feat(ui): add the evaluation mode to the Add Model form #45481, never green: it sent the proxy alias as litellm_params.model, which replaced the real typesafe/ model, so the check failed with "LLM Provider NOT provided"
    • Send the provider model, api_base and api_key directly. Passes locally
  • tests/integration/routing/test_usage_based_routing_redis_reads.py::test_proxy_usage_routing_reads_cooldown_tpm_then_rpm_from_redis (3 params)
    • Red in 5 of 5 runs because the owned proxy inherited store_model_in_db and the shared database, so a model left by another test added its cooldown key to the exact MGET comparison
    • Reproduced locally with one /model/new model present (3 failed). With model loading from the database off: 3 passed

GHA unit

  • tests/unit/test_router_silent_experiment.py::test_multiple_shadow_targets_fan_out_async
    • Needed a rerun to pass on 5 of 59 main runs
    • It waited for the 2 shadow success events, then asserted the primary one, which the logging worker sometimes had not delivered yet. It now waits for all 3
  • tests/unit/proxy/test_spend_log_cleanup.py::test_run_budget_stops_the_loop_and_leaves_the_backlog_for_the_next_run
    • The only red main run in the window: rows_deleted=0 because the 0.25s budget expired before the first batch
    • 1s budget; the 50-batch, 5s loop still has to stop on the deadline
  • tests/unit/proxy/test_proxy_utils.py::test_during_call_hook_parallel_execution
    • Failed on several unrelated PRs with "Parallel execution took 1.09s, expected < 0.2s"
    • Removed the wall-clock check; "all three hooks start before the first one ends" already proves they run in parallel
  • tests/unit/llms/vertex_ai/files/test_vertex_ai_files_streaming.py, the two peak-memory tests
    • tracemalloc ratios crossed their limits on unrelated PRs (0.77 vs 0.6, 0.43 vs 0.3). Peak memory on a shared xdist worker includes other threads' allocations and garbage from earlier tests
    • Replaced with deterministic laziness checks of the same property: the upload stream parses and maps a row only when it is pulled, and a Path source reflects a row rewritten on disk after the upload started. Making the parse, the output or the file read eager fails them
  • tests/unit/proxy/hooks/test_autorouter_baseline_cache.py::test_native_count_finishing_after_quarter_worker_budget_keeps_plan_and_spend
    • The slow token count was released at 2.3s against a 3s planner timeout, and on a loaded runner it timed out
    • Released at 2.05s, still past the 2s quarter of the worker budget the test is about. This one widens the slack rather than removing the timing, so it is the least certain fix here

Buildkite e2e

  • tests/e2e/llm_translation/test_responses_e2e.py::TestResponsesOpenAIHostedFeatures::test_reasoning_items_replay_into_structured_output_after_tool_call
    • Failed on 4 scheduled builds; a rerun at the same SHA passed. Whether the model emits a reasoning item next to a forced function call is up to the model
    • The first turn retries up to 3 times until a reasoning item appears; the replay assertions are unchanged
  • tests/e2e/claude_code/prompt_caching_5m/test_bedrock_converse.py::test_prompt_caching_5m_bedrock_converse
    • Opus on Bedrock Converse reported zero cache tokens on 4 of 8 builds; a rerun at the same SHA passed
    • A model that comes back uncached is asked once more inside the 5 minute window
  • tests/e2e/llm_translation/test_responses_retrieve_e2e.py, both delete tests
    • Azure kept serving a just-deleted response for a moment, so one retrieve did not raise; a rerun at the same SHA passed
    • Poll retrieve until it errors, and still require a 4xx
  • tests/e2e/llm_translation/test_together_ai_e2e.py::TestTogetherChatCompletions::test_cache_read_tokens_bill_at_the_cache_read_rate
    • Together's prefix cache is best-effort; 3 primed attempts got no cached tokens once, and a rerun at the same SHA passed
    • 5 attempts
  • tests/e2e/router/test_reliability_memory_e2e.py::TestReliabilityMemory::test_workers_idle_under_rss_budget_before_traffic
    • A loaded router replica took longer than the 10s session-start memory read, which pytest reruns cannot recover
    • 30s read timeout; the RSS budget is unchanged
  • tests/e2e/ui/tests/mcp/mcpSubmissionRules.spec.ts "Submission Rules panel shows saved rules..."
    • The register call timed out at 15s, which leaked a submitted server, so the retries matched two "3 passing, 1 failing" cards
    • Registration gets 60s and the check reads only the submitted server's card

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have added meaningful tests
  • The handful of test files covering my change pass locally, e.g. uv run pytest tests/unit/<your_test_file>.py -v. Leave the suites (make test-unit-*, make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more
  • My PR passes all required CI/CD checks (e.g., lint, schema.d.ts sync check, etc.)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review (Greptile reviews automatically once the PR is opened; only comment @greptileai to re-request a review after pushing changes)

Delays in PR merge?

If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).

Screenshots / Proof of Fix

The proof is CI itself: the same jobs before and after, with every CircleCI job forced to run on this branch by the temporary ci: commit

Before (145d382 and fb8e988)

  1. Main's scheduled CircleCI pipeline 5ad9cbd6 at 145d382 and this PR's first full run db09a739 at fb8e988 had every CircleCI test above failing in at least one of them
  2. Buildkite litellm-e2e builds 380 to 387 had the six e2e failures above, each with a passing rerun at the same SHA in litellm-e2e-pr
  3. GHA unit on main needed reruns or failed for the three main-side unit tests above, and the other two failed on unrelated PRs

After (724d304, plus 261d488 for the Vertex unit tests)

  1. CircleCI pipeline 8ee35d0c at 724d304: every CircleCI test fixed above passed, including llm_translation_testing (xAI) and integration-database (partition)
  2. The jobs still red there fail on tests this PR does not touch, listed under Caveats
  3. The Vertex laziness tests: 38 passed for the whole file locally, and three eager variants of the stream code each fail them

Type

✅ Test
🚄 Infrastructure

Caveats (if any)

High

  • The temporary ci: temporarily run main-only CircleCI jobs on this branch commit must be dropped before merge
  • Product bugs found here still keep CircleCI red, and are not masked:

Medium

  • Not triaged yet, red in the After run:
  • CircleCI has not run at the current tip 261d488 yet. The After run predates only the Vertex unit-test swap, which CircleCI does not run
  • Killed proxy workers sometimes take more than 70s to come back on CircleCI, against 3 to 7s locally; no wait was raised for that

Low

  • The xAI fix depends on grok-4.6 returning reasoning text, as measured on 2026-10-09
  • The autorouter fix widens a timing margin rather than removing it

QA runbook

  • tests/e2e/llm_translation/test_responses_e2e.py::TestResponsesOpenAIHostedFeatures::test_reasoning_items_replay_into_structured_output_after_tool_call - a reasoning item from a forced tool call replays into a structured-output turn
    • POST /v1/responses with openai/gpt-5.4-mini, the get_today tool forced, reasoning: {"effort": "medium"} and a JSON schema text format; retry until the output has a reasoning item (at most 3 times)
    • Send a second request with the first output plus a function_call_output of 2025-01-15
    • Expect status completed and today in the parsed output to contain 2025-01-15
    • Sanity check: this test makes sense to add and is not hand-wavey (e.g., assert actual expected spend instead of just spend > 0) or potentially flaky
  • tests/e2e/claude_code/prompt_caching_5m/test_bedrock_converse.py::test_prompt_caching_5m_bedrock_converse - the claude CLI through Bedrock Converse gets non-zero cache tokens
    • Run the claude CLI against the proxy for each Bedrock Converse model with "Reply with the single word 'pong'"
    • Expect cache_creation_input_tokens or cache_read_input_tokens above 0, asking an uncached model once more
    • Sanity check: this test makes sense to add and is not hand-wavey (e.g., assert actual expected spend instead of just spend > 0) or potentially flaky
  • tests/e2e/llm_translation/test_responses_retrieve_e2e.py::TestStoredResponseLifecycle::test_streamed_response_can_be_deleted - a deleted stored response stops being retrievable
    • Create a stored streamed response, then DELETE /v1/responses/{id}
    • GET /v1/responses/{id} until it errors, and expect a 4xx
    • Sanity check: this test makes sense to add and is not hand-wavey (e.g., assert actual expected spend instead of just spend > 0) or potentially flaky
  • tests/e2e/llm_translation/test_together_ai_e2e.py::TestTogetherChatCompletions::test_cache_read_tokens_bill_at_the_cache_read_rate - Together cache reads bill at the cache-read rate
    • Prime a long prefix and ask again, up to 5 fresh prefixes, until cached_tokens is above 0
    • Expect the spend to use the cache-read rate for those tokens
    • Sanity check: this test makes sense to add and is not hand-wavey (e.g., assert actual expected spend instead of just spend > 0) or potentially flaky
  • tests/e2e/router/test_reliability_memory_e2e.py::TestReliabilityMemory::test_workers_idle_under_rss_budget_before_traffic - idle workers stay under the RSS budget
    • GET /debug/memory/summary on every replica at session start, allowing 30s per read
    • Expect every worker under the RSS budget
    • Sanity check: this test makes sense to add and is not hand-wavey (e.g., assert actual expected spend instead of just spend > 0) or potentially flaky
  • tests/e2e/ui/tests/mcp/mcpSubmissionRules.spec.ts "Submission Rules panel shows saved rules, preloads the editor, and save keeps existing rules" - saved submission rules show on a submitted server's card
    • Register an MCP server with a team key via POST /v1/mcp/server/register
    • Open http://localhost:4000/ui/?page=mcp-servers, Submitted MCPs tab, and turn on "Auth configured" in Submission Rules
    • Expect that server's card to read "3 passing, 1 failing"
    • Sanity check: this test makes sense to add and is not hand-wavey (e.g., assert actual expected spend instead of just spend > 0) or potentially flaky

Final Attestation

  • The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR

@codecov

codecov Bot commented Oct 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

The witness poll ran a blocking psycopg connect and query on the test's event loop, so the partition DDL task only progressed during the 20ms sleeps and missed the 3s deadline on loaded runners. Polling through asyncio.to_thread lets the DDL reach the held lock concurrently, and the deadline is 10s since it only bounds how long the DDL takes to start waiting
… item

Whether gpt-5.4-mini emits a reasoning item next to a forced function call is up to the model, and it skipped it on four scheduled runs while a same-SHA rerun passed. The first turn now retries up to three times before the unchanged reasoning-item assertion
…he tokens

Opus on Bedrock Converse reported zero cache tokens on half of the scheduled runs while a same-SHA rerun passed. A model that comes back uncached is asked once more inside the 5 minute window, and the non-zero cache token assertion is unchanged
Azure kept serving a just-deleted streamed response for a moment, so a single retrieve did not raise. Both delete tests now poll retrieve until it returns an error and still require a 4xx
Together documents its prefix cache as best-effort, and three primed attempts came back with zero cached tokens once while a same-SHA rerun passed
A loaded router replica took longer than 10s to answer the session-start memory read, which pytest reruns cannot recover. The RSS budget assertion is unchanged
The register call timed out at 15s on a loaded stack and leaked a submitted server, so the retries matched two "3 passing, 1 failing" cards. Registration gets 60s and the check reads only the submitted server's card
The shadow fan-out test waited only for the two shadow success events and then asserted exactly one primary success, which the logging worker sometimes had not delivered yet. It needed a rerun on 5 of 59 main runs
A 0.25s budget could expire before the first batch on a loaded xdist worker, leaving rows_deleted at 0. One second still stops the 50-batch, 5-second loop on the deadline, which is what the test proves
The count only has to finish after a quarter of the 8s worker budget, and the planner gives it 3s, so releasing at 2.3s left 0.7s of slack that a loaded runner used up. 2.05s is still past the quarter mark with almost 1s of slack
…hat needs reasoning

grok-4.7 now returns reasoning tokens without any reasoning text, so reasoning_content was never set and both tests failed on every scheduled run. Called directly, grok-4.6 returned reasoning text 6 of 6 times on a step-by-step arithmetic prompt but only some of the time on a bare greeting
…xy time budget

Each test spends about 50s on the burst and then up to graceful_stop_seconds() stopping its owned proxy, which overran the 90s default pytest timeout on loaded runners. They now use the same 2 * graceful_stop_seconds() + 120 budget as other owned-proxy tests
…ated timestamp

Two identical generations that straddle a second boundary differ only in created, which failed the byte-for-byte comparison. Every other field and both spend rows are still compared
The set of asyncio tasks created while the logger starts also picked up client close finalizers left by earlier tests, so the count ranged from 1 to 8. The test now counts the periodic_flush task it owns and cancels
All 44 probes went at once to a two-worker proxy, so on a loaded runner some guardrails used their whole 1s timeout before their request left the proxy, and the sink never saw them. A different set of providers failed on each run. Eight in flight still overlaps the waits
…is a zombie

psutil reports an unreaped zombie as still running, so the 10s death check failed whenever the supervisor was slow to reap the SIGKILLed worker, and the stop then overran the 90s default timeout. The test now accepts a zombie or a missing process and has the owned-proxy time budget
…nd endpoint

The test passed the proxy alias as litellm_params.model, and /health/test_connection lays the request over the stored deployment, so the alias replaced the real typesafe/ model and the check failed with "LLM Provider NOT provided" on every run since it landed in #45481. It now sends the provider model, api_base and api_key directly
…is read test

The owned proxy inherited store_model_in_db and the job's shared database, so a model another test left behind joined the router and added its cooldown key to the MGET the test compares exactly. Reproduced locally with one /model/new model present (3 failed), and green with model loading from the database turned off
…f peak memory

The two tracemalloc ratio tests flaked on unrelated PRs because peak memory on a shared xdist worker includes other threads' allocations and garbage from earlier tests. They are replaced by deterministic checks of the same property: the upload stream parses and maps a row only when it is pulled, so a body whose tail is not JSON yields its valid rows first, and a Path source reflects a row rewritten on disk after the upload started. Making the parse, the output, or the file read eager fails these tests
@yuneng-berri
yuneng-berri force-pushed the litellm_ci_upkeep_1009 branch from 261d488 to 3b9cc14 Compare October 9, 2026 17:24
@yuneng-berri
yuneng-berri marked this pull request as ready for review October 9, 2026 17:25
@greptile-apps

greptile-apps Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Low impact] This PR appears safe to merge; no new actionable issue or outstanding numbered finding remains

Summary

This PR reduces test flakiness with scoped retries, longer waits, better isolation, and deterministic upload checks

  • Provider tests retry when a response lacks the detail they check.
  • Delete tests wait until a response stops being retrievable.
  • The partition test checks for a database lock without pausing its async work.
  • Proxy worker-kill tests allow time for the owned proxy to stop.
  • Guardrail probes run eight at a time against the two-worker proxy.
  • Vertex upload tests check that rows are transformed only when pulled.
  • Unit tests wait for the events and time windows they need to check.
  • The evaluation test sends provider details directly to the connection check.
  • The Redis routing test turns off model storage in its owned proxy.
  • The S3 test counts only the logger task it starts.
  • The image test compares response bodies without their changing timestamp.
  • The submission-rules test gives registration more time and checks the right server card.

Reviews (4) · Last reviewed commit: "test(vertex_ai): prove pull-driven trans..." · Reviewed by Greptile

Comment thread tests/integration/providers/test_decisions_wire.py
Comment thread tests/unit/llms/vertex_ai/files/test_vertex_ai_files_streaming.py Outdated
Comment thread tests/unit/llms/vertex_ai/files/test_vertex_ai_files_streaming.py Outdated
@yuneng-berri

Copy link
Copy Markdown
Contributor Author

@greptileai please re-review: the alias call is a BUG skip again, the new helpers are typed, and the memory thread has a reply

@yuneng-berri

Copy link
Copy Markdown
Contributor Author

@greptileai please re-review: every new local added by this PR is now Final, and the new list values are tuples

@yuneng-berri

Copy link
Copy Markdown
Contributor Author

@greptileai please re-review: the counting mapper and its mutable list are gone, and no new test code mutates state now

@yuneng-berri yuneng-berri added run-ci and removed run-ci labels Oct 9, 2026
@yuneng-berri
yuneng-berri merged commit cddc7cd into main Oct 9, 2026
86 of 89 checks passed
@yuneng-berri
yuneng-berri deleted the litellm_ci_upkeep_1009 branch October 9, 2026 18:40

This branch is waiting to be deployed

1 waiting deployment
e2e-changed — a50d62d2 Waiting Oct 9, 2026 by yuneng-berri via oauth #4156
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants