Repository navigation
test: inject the HIBP client and the MCP loop clock so two backend tests stop flaking - #44007
Conversation
… test Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…lock Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
|
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 9b97b5f. Configure here.
TLDR
Problem this solves:
How it solves it:
hibp_clientparameter defaulting to today's behaviorUser Flow
Before: a contributor's unrelated PR goes red on the Unit Tests or Postgres Tests job
test_user_update_rejects_breached_passwordor one case oftest_optional_discovery_rejects_incomplete_walksAfter: the same commit gets a green job on the first attempt
Flake evidence (run date 2026-10-01)
tests/test_litellm/proxy/management_endpoints/test_internal_user_endpoints.py::test_user_update_rejects_breached_passwordFailed once on main, attempt 1: https://github.com/BerriAI/litellm/actions/runs/36805338275 (Postgres Tests at
c168199e33). The log showsBreached-password check skipped, HIBP lookup failed: Timeout on reading data from socketand thenTypeError: object MagicMock can't be used in 'await' expression. The same test passed on 18 other main runs of that job in the window, including https://github.com/BerriAI/litellm/actions/runs/36798373650 at the adjacent SHA6997223068Mechanism: real network call. The test mocked HIBP with
respx, but the HIBP client is built byget_async_httpx_client, whose transportrespxnever intercepts, so the route was never hit and the test really queried api.pwnedpasswords.com. When that lookup timed out the check failed open by design and the test fell through to the unmocked database calls. Locally, at the merge base, the test passes with network access and fails with the CI signature underunshare -rn(no network), and a probe confirmed therespxroute records zero callsFix:
_update_single_user_helperand_hash_password_in_dictaccept an optionalhibp_client, the same seambulk_update_processed_usersalready exposes. The test injects anAsyncHTTPHandleroverhttpx.MockTransportand now also asserts exactly oneGET https://api.pwnedpasswords.com/range/<prefix>lookup. Production callers pass nothing and keep using the shared HIBP client. Mutation check: skipping the breach check in_hash_password_in_dictmakes the test failtests/unit/experimental_mcp_client/test_mcp_client.py::test_optional_discovery_rejects_incomplete_walksFailed 5 times on main, all attempt 1, interleaved with passing runs of the same jobs at adjacent SHAs:
6997223068),[False-internal_error-resources/templates/list],assert 0 == 20c515ed7a8), same case and signatureef6aa4ad66),[True-method_not_found-resources/templates/list],TimeoutError6d7d183a80),[False-internal_error-resources/templates/list],assert 0 == 20980f756bd), same case and signatureIt also failed on two unrelated PR branches, https://github.com/BerriAI/litellm/actions/runs/36808597812 (
[True-repeat-resources/templates/list],TimeoutError) and https://github.com/BerriAI/litellm/actions/runs/36830884630 ([False-unauthorized-resources/list],assert 0 == 2)Mechanism: wall-clock dependence. Every case built the client with
timeout=0.2, which is the read timeout for initialize and for each page as well as the deadline for the whole page walk. Only thedeadlinecase is about time, but on a contended runner initialize or the first page of any case could take longer than 200ms of real time and time out before the behavior under test ran. Locally, with 8 busy loops pinned to the same CPU, the merge base failed 9 of 10 runs (36 failing cases, spread acrosscap,internal_error,unauthorizedand others)Fix: the test runs its coroutine on
_ManualClockLoop, an asyncio loop whosetime()only moves when the test callsadvance. anyio deadlines and the MCP SDK read timeouts schedule on that clock, so no case can time out from scheduler latency. Thedeadlinecase advances the clock on purpose: 0.1s while serving page one, then 0.15s when page two arrives and hangs, so the 0.2s walk deadline is due while page two's own read timeout (due at 0.3s) is not, and strict mode getsTimeoutErrordeterministically. Page two then yields to the loop at most 1,000 times and returns a 500 if nothing cancelled it, so a broken deadline fails in seconds instead of hanging (cancellation arrives after 10 yields today). The loop is driven withrun_until_complete, which keeps the test running on Python 3.10. Therepeatcase's page cap goes from 2 to 3: with a cap of 2 the cap error masked a missing repeated-cursor check, so that check was untested. Mutation checks: removing the repeated-cursor check fails all sixrepeatcases, and pushing the walk deadline out by 1s fails all sixdeadlinecases within 3 secondsNot flakes in the window
Real breakages on main, failing on every run until fixed or still failing:
tests/unit/interactions/test_openapi_compliance.py(4 tests, 51 failing runs),tests/test_litellm/proxy/batches_endpoints/test_endpoints.py::test_create__non_object_metadata_is_400(52 failing runs),tests/test_litellm/proxy/test_component_allowlists.py::test_gateway_plus_backend_covers_full_app(13 consecutive runs, then fixed), andtests/unit/test_utils.py::test_aaamodel_prices_and_context_window_json_is_valid(schema rejected new*_ultrafastcost keys). Infra: Codecov upload signature verification failures on Postgres Tests, dependency install timeouts, and the Code Quality undocumented env var check. None of these are touched hereOther open deflake PRs
The auto-router modal integration flake (
edit_auto_router_modal.integration.test.tsx, failing on 17 main UI Unit Tests runs in this window) is already covered by #42936 and #43437, and #41219 covers TeamSSOSettings. All three carry a human approval, so they are left untouched under the human-reviewed exception and this PR is the new rolling PRPre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
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@greptileaito re-request a review after pushing changes)Screenshots / Proof of Fix
There is no proxy or Admin UI surface for this change: the production edit only adds an optional parameter that every live caller leaves unset. The proof is the flake reproduction itself, run from the repo root at the merge base and at the tip
Before (0980f75)
Breached-password test without network
LITELLM_LOCAL_MODEL_COST_MAP=True unshare -rn .venv/bin/python -m pytest "tests/test_litellm/proxy/management_endpoints/test_internal_user_endpoints.py::test_user_update_rejects_breached_password" -qTypeError: object MagicMock can't be used in 'await' expressionafterHIBP lookup failed, the CI signatureMCP walk test under CPU contention
for i in $(seq 8); do taskset -c 0 sh -c 'while :; do :; done' & done, then 10 timesLITELLM_LOCAL_MODEL_COST_MAP=True taskset -c 0 uv run --no-sync pytest tests/unit/experimental_mcp_client/test_mcp_client.py -k test_optional_discovery_rejects_incomplete_walks -n 4 -q -p no:randomlyAfter (9b97b5f)
Breached-password test without network
1 passedMCP walk test under CPU contention
42 passedType
✅ Test
Caveats (if any)
Low
test_claim_token_rejects_breached_password_before_consuming_inviteuses the samerespxpattern on the onboarding pathFinal Attestation
Link to Devin session: https://app.devin.ai/sessions/b0ccd764fa784fda8926e026f0cec8c9
Open in Devin Desktop: https://app.devin.ai/desktop/session/b0ccd764fa784fda8926e026f0cec8c9?variant=devin
Note
Low Risk
Production change is an optional
hibp_clientdefaulting to existing HIBP behavior; remaining edits are test-only determinism fixes.Overview
Stabilizes two flaky backend tests with a small, backward-compatible production seam and deterministic test harnesses.
HIBP breach check:
_hash_password_in_dictand_update_single_user_helpernow accept an optionalhibp_client(wired intovalidate_password_not_breached), matching the bulk user-update path. The breached-password update test dropsrespxand injects a mockAsyncHTTPHandler, so it no longer hits the real Have I Been Pwned API when mocks miss the shared httpx client.MCP discovery timeouts:
test_optional_discovery_rejects_incomplete_walksruns on a new_ManualClockLoopwhose time advances only when the test callsadvance, so wall-clock scheduler contention cannot fire read/walk timeouts early. Therepeatparametrization raises the page cap to 3 so repeated-cursor rejection is actually exercised. Live callers omithibp_clientand behavior stays the same.Reviewed by Cursor Bugbot for commit 9b97b5f. Bugbot is set up for automated code reviews on this repo. Configure here.