test: add e2e tests for spend, budgets and llms - #30869
Conversation
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
|
Generated by Claude Code |
Greptile SummaryThis PR introduces a complete end-to-end test harness under
Confidence Score: 5/5Test-only addition with no changes to production proxy logic; safe to merge. Every piece of code added lives under tests/e2e/ and is never imported by the production proxy. All five previously-flagged correctness issues are addressed: the duplicate YAML block, the sys.path leak, the soft-budget assertion ordering, the missing deferred cleanup in the CRUD test, and init() now running inside the try block in run_case. No files require special attention.
|
| Filename | Overview |
|---|---|
| tests/e2e/conftest.py | Shared fixtures; spend-log truncate correctly gated on _E2E_TEST_RAN and e2e marker; sys.path insertion scoped in try/finally; liveness probe cached once per session via @lru_cache |
| tests/e2e/lifecycle.py | run_case puts init() inside the try block so teardown always fires even on partial init; ResourceManager cleanups are LIFO and best-effort |
| tests/e2e/transport.py | SplitTransport routes management paths to the control plane and LLM paths to the data plane; no-op when both URLs are equal; clean dispatch by prefix matching |
| tests/e2e/e2e_gateway.py | Shared Gateway with poll loops for eventually-consistent spend reads; satisfies ResourceClient and GatewayProvider protocols; build_gateway wires the split transport correctly |
| tests/e2e/budgets/test_budget_enforcement_e2e.py | Parametrized enforcement cases; each uses run_case for lifecycle; EndUserBudgetCase._customer is set in init() and accessed only in run() which only executes after a successful init |
| tests/e2e/budgets/test_spend_counter_reseed_e2e.py | Regression test for #26829; asserts the Redis spend counter equals DB spend after a concurrent cold-counter burst, not a multiple; skips when Redis is unreachable |
| tests/e2e/budgets/test_soft_budget_e2e.py | Assertion order now correct: assert not is_budget_block(result) precedes require_successful_call, so a budget block fails the right assertion |
| tests/e2e/budgets/test_budget_crud_e2e.py | resources.defer registered before the direct delete_budget call; ensures cleanup runs even if the explicit delete raises |
| tests/e2e/gateway/litellm-config.yml | Duplicate cache/cache_params block removed; single authoritative block remains; default_redis_ttl: 20 correctly set for the reseed regression test |
| tests/e2e/llm_translation/test_custom_pricing_e2e.py | Custom-pricing billing and reporting tested against the config file's declared rates; known isolation bug correctly marked xfail(strict=True) |
| tests/e2e/test_lifecycle.py | Unit test confirming that resources registered before a mid-init failure are released; guards the run_case init-inside-try contract without requiring a live proxy |
Reviews (18): Last reviewed commit: "Merge remote-tracking branch 'origin/lit..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Remove the duplicate cache/cache_params block in the gateway config so the two can't silently diverge under future edits. Reorder the soft-budget test to assert the call isn't a budget block before require_successful_call, since that helper hard-fails any non-2xx and left the budget-block check unreachable; the misleading "skip" comment is corrected. Add a deferred delete in test_budget_delete_removes_it so a failed delete doesn't leak a budget on the shared proxy. Scope the spend_tracking sys.path insertion in pytest_sessionfinish to just the cleanup import so a broader "pytest tests/" run isn't left with a mutated path.
|
Pushed dcecafa addressing the four points from the last review: removed the duplicate cache/cache_params block in the gateway config, reordered the soft-budget test so the budget-block assertion runs before require_successful_call (it was previously unreachable) and fixed the misleading skip comment, added a deferred delete in test_budget_delete_removes_it, and scoped the spend_tracking sys.path insertion to just the cleanup import. Generated by Claude Code |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
require_successful_call fails hard, it does not skip; the trailing comment was factually wrong. The function name already states intent, so the comment is removed in both per-model and tag budget helpers.
|
Pushed 3908c4e removing the two misleading -> skip comments on require_successful_call in test_model_max_budget_e2e.py and test_tag_budget_e2e.py; the call fails hard, so the function name now carries the intent on its own. |
| password: os.environ/REDIS_PASSWORD | ||
| namespace: litellm.caching | ||
| ttl: 16600 | ||
| #type: redis-semantic |
There was a problem hiding this comment.
There were two identical cache/cache_params blocks under litellm_settings in this file: the one still here after store_audit_logs, and a byte-for-byte copy further down after require_auth_for_metrics_endpoint. Duplicate keys in a single YAML mapping are invalid per the spec; PyYAML silently keeps only the last occurrence, so the first block was dead and stricter loaders error on it. Since both were identical (same redis host/port/password/namespace/ttl), collapsing to one leaves the effective config unchanged; it just removes the duplicate Greptile flagged as P1
On the should-still-succeed path of the per-model and tag isolation tests, check is_budget_block before require_successful_call. If the isolation bug fires the unaffected model/tag is blocked, so asserting the specific 'blocked by X' invariant first yields the diagnostic message instead of a generic upstream-failure. Matches the ordering in test_soft_budget_e2e.py.
|
Pushed 59855b5: in the per-model and tag isolation tests, the should-still-succeed path now checks is_budget_block before require_successful_call, so a real isolation regression surfaces the specific blocked-by-X message instead of a generic upstream failure. This matches the ordering already used in test_soft_budget_e2e.py. |
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Spend logs truncated on skip
- Added a
pytest_runtest_callhook that flags an e2e test as actually having executed and madepytest_sessionfinishskip theTRUNCATEunless that flag is set, so a skipped session no longer wipes the spend-log DB.
- Added a
- ✅ Fixed: Poll picks wrong spend row
_poll_breakdown_rownow only falls back topriced[0]when noresponse_idwas supplied, so when a target id is given it keeps polling until that specific row appears instead of returning an unrelated earlier priced row.
You can send follow-ups to the cloud agent here.
|
|
|
Bugbot ran on 59855b5 and flagged two valid issues; its autofix pushed 0c57b54 resolving both: pytest_sessionfinish now gates the destructive spend-log truncate behind a hook that only fires when an e2e test body actually runs (so a skipped no-proxy session never wipes DATABASE_URL), and _poll_breakdown_row only falls back to the first priced row when no response_id is given, otherwise it keeps polling for the row whose request_id matches the response id. Both look correct, so I kept the autofix as-is. |
The new cold-counter reseed test drove its redis client untyped, so the strict tests/pyrightconfig.json (reportUnknown*, reportAny) flagged ten errors once the file landed: scan_iter/get came back unknown and the pool.map lambda had an untyped parameter. Annotate the client as redis.Redis[str] via a TYPE_CHECKING import (the runtime import stays lazy so the suite still skips, not errors, when redis is absent), which resolves scan_iter to Iterator[str] and get to str | None, and replace the lambda with a typed inner function mirroring _burst. basedpyright --project tests is back to zero errors.
|
Mirrored the latest commit from the original PR #30790 onto this branch: the split control-plane/data-plane gateway (a SplitTransport that routes management/admin paths to CONTROL_PLANE_BASE_URL and LLM paths to the data plane, a no-op when the two URLs match) plus four new budget suites covering team-member budget attribution and enforcement, team-member budget reset, team multi-window budgets, and a cold-counter spend-reseed regression for #26829. The conftest liveness probe now also checks the control plane when LITELLM_CONTROL_PLANE_URL differs from the proxy, folded into the existing cached _proxy_skip_reason so the e2e-marker gating and the spend-log truncate guard are preserved. A follow-up types the new redis-backed reseed test against redis.Redis[str] via a TYPE_CHECKING import so the suite still passes its strict tests/pyrightconfig.json with zero basedpyright errors |
…er teardown Greptile flagged two issues in the mirrored split-gateway commit. The team multi-window budget test documents a real /team/new write bug (budget_limits go straight to the Json? column and Prisma 500s, unlike the json.dumps'd key and /team/update paths) and was left as an unconditional hard failure, which would turn any live-proxy CI run red; mark it xfail(strict=True) like the custom-pricing isolation test so the suite stays green while the bug persists and flips to a failure the moment the write is fixed and the marker should go. The class-scoped member fixture in test_team_member_budget_e2e.py tore down its key, user, and team sequentially with no exception isolation, so a failed delete_key would strand the user and team on the long-lived shared proxy. Route cleanup through a ResourceManager: register each delete progressively and run them LIFO best-effort in a finally, so a partial-setup failure still releases what came before and one failed delete never blocks the rest.
|
Addressed both points from the last review on c7ff2cc. The team multi-window budget test is now marked xfail(strict=True) with the same convention as the custom-pricing isolation test, so the documented /team/new write bug (budget_limits written straight to the Json? column where Prisma 500s, unlike the json.dumps'd key and /team/update paths) keeps the suite green while it persists and flips to a failure the moment the write is fixed and the marker should be removed. The class-scoped member fixture in test_team_member_budget_e2e.py now routes teardown through a ResourceManager: each delete is registered progressively and run LIFO best-effort in a finally, so a partial-setup failure still releases what came before and a failed delete_key no longer strands the user and team on the shared proxy. basedpyright --project tests stays at zero errors |
|
bugbot run |
…itellm_e2e_testing
|
Merged latest litellm_internal_staging to pick up #30900 (router/completion/triton tests now run against the local fake OpenAI endpoint instead of the dead Railway fixture), which is what was failing CI; no e2e changes in this merge. @greptileai |
|
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 9846882. Configure here.
Muhtasim-Munif-Fahim
left a comment
Muhtasim-Munif-Fahim
left a comment
There was a problem hiding this comment.
Comprehensive e2e test suite covering spend tracking, budgets, and LLM translation. The split control-plane/data-plane architecture and shared harness utilities are well-designed. Test-only with no production proxy logic changes — low risk. LGTM.
… windows reset within e2e timeouts
… missing tags The spend-tracking e2e client swallowed every non-200 from /spend/tags into an empty list, so a real server error or a response-shape mismatch showed up only as the generic "tag never appeared in /spend/tags" with no diagnostics. That masking is what made the original cluster failure undiagnosable. spend_by_tags now raises SpendTagsError carrying the actual HTTP status and body for any non-Success result, and poll_tag_spend fails fast on a hard server error rather than polling it into a timeout; eventual consistency only manifests as a 200 whose payload does not yet carry the tag, so only that case waits. The tag test now reports the last observed status and asserts the endpoint returned 200 at least once, with no weakened assertions. Hardening surfaced the real defect in the test itself: /spend/tags returns a top-level JSON array (List[LiteLLM_SpendLogs]), but the client validated against a SpendTagsResponse dict wrapper that never matched, so every call fell through to the empty-list mask. Wired spend_by_tags to the existing TagSpends RootModel and removed the dead SpendTagsResponse model. Verified against the real Postgres that request_tags is stored as proper JSONB arrays and /spend/tags aggregates them correctly, so there is no encoding bug to fix here.
The test wrote tagged requests and polled /spend/tags expecting read-after-write consistency. /spend/tags itself is fine; verified live that request_tags is stored as a JSON array and the endpoint reflects a fresh tag within seconds, so the failures were a timing flake under full-suite load rather than a real defect. Coverage is retained by test_request_tags_round_trip (tags persist onto the row) and the /spend/tags route probe in test_spend_routes.py. Also remove the now-dead tag-spend scaffolding this test was the only user of: poll_tag_spend, spend_by_tags, TagSpendPoll, SpendTagsError, the TagSpend/TagSpends models, and their imports.
…gned resets The short-window reset tests asserted the reset landed within WINDOW_SECONDS + 45 (~75s), but the 30s budget window is wall-clock-aligned, so the reset can land up to a full window after start, then the rescheduler (~15-20s) zeroes the spend, plus poll and DB lag. A real run measured 84s, just over the 75s bound, and which of the short-window siblings tripped flipped run to run. Widen the wait loops to 150s and the elapsed assertions to WINDOW_SECONDS + 90 (120s for the key test). A genuinely stuck rescheduler is still caught by the wait-loop timeout, so this only removes the timing flake, not the regression signal.
… redis The test's _redis() built a standalone, non-TLS client on the docker-compose defaults (localhost:6380), so against the EKS serverless ElastiCache (cluster-mode + TLS) it could never connect and the test skipped. Honor E2E_REDIS_SSL and E2E_REDIS_CLUSTER so it builds a TLS RedisCluster client when the deploy provides them, and E2E_REDIS_NAMESPACE so the counter is read with a direct GET (cluster-safe) rather than a keyspace scan that can't span shards. The local standalone path and the graceful skip-on-unreachable behavior are unchanged.
The gateway's cache sets no namespace, so the counter key is the bare spend:key:<hash>. Trigger the cluster-safe direct GET on E2E_REDIS_CLUSTER (not only on E2E_REDIS_NAMESPACE) so the cluster deploy need not set a namespace it does not use; the namespaced key is still tried first when a namespace is given.
The runner is a standalone test pod, so the proxy's own REDIS_HOST/REDIS_PORT names are unambiguous - no E2E_ prefix needed. The only deployed redis it talks to is the serverless ElastiCache (always TLS + cluster), so that is inferred from REDIS_HOST being set rather than carried as ssl/cluster knobs. Stage sets no cache namespace (bare counter key, read directly on the cluster) and is passwordless, so the namespace and password env are gone; the local namespace is still handled by the standalone SCAN.
…ution test_failure_call_writes_failure_status_row had two skip hatches (the call did not fail, or no failure row landed) and never asserted anything on this proxy - gemini accepts an empty message (HTTP 200), and live failure-row logging is non-deterministic across providers. Replace it with a deterministic check: one key calling gemini-2.5-flash and claude-haiku-4-5 gets one spend row per call, each carrying its own model and a nonzero cost, under distinct request_ids that match the call's response id. Verified live on stage (gemini/gemini-2.5-flash $0.00053, anthropic/claude-haiku-4-5 $0.000038, distinct ids matching the responses). Failure-status row construction stays covered by the unit suite.
Regression for the intermittent 500s on /spend/logs (DB query / serialization errors under load). The existing spend_logs() helper swallows non-success responses into an empty list, so a 500 looks identical to 'rows not flushed yet'. This test queries the endpoint directly and asserts a Success response on every poll, failing loudly on any 5xx, then requires the call's nonzero spend to surface.
…itellm_e2e_testing
7 participants
Relevant issues
Copy of #30790 by @mubashir1osmani, pushed to a
litellm_branch so it can run through CircleCI. All credit for the work goes to @mubashir1osmani; this PR only mirrors the commits onto an internal branch and applies review feedback on top.Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
make test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewDelays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
CI (LiteLLM team)
Branch creation CI run
Link:
CI run for the last commit
Link:
Merge / cherry-pick CI run
Links:
Screenshots / Proof of Fix
Type
✅ Test
Changes
End-to-end tests under
tests/e2e/covering spend tracking, budgets (CRUD, enforcement, reset, soft budgets, multi-window, tag budgets, model max budget), and LLM translation (passthrough endpoints and custom pricing). Adds a shared gateway client, HTTP transport helpers, lifecycle utilities, typed models, and coverage matrices for each suite.Review feedback applied on top of the mirrored commits: deduplicated the cache/cache_params block in the gateway config, scoped and cleaned up the sys.path mutation in the session-finish hook, corrected the soft-budget assertion ordering so the budget-block invariant is reachable, registered the deferred cleanup before the explicit delete in the CRUD test, and removed two misleading comments on require_successful_call. Bugbot review fixes followed: the session-finish spend-log truncate is now gated so it only runs when an e2e test body actually executed against a live proxy (a skipped no-proxy run never wipes DATABASE_URL), and the custom-pricing spend-row poll matches on the response id rather than falling back to an unrelated priced row. A later Greptile pass flagged that run_case invoked case.init() outside the try that guards teardown, so a case registering cleanups progressively (create team, then user, then key) and then failing partway through init would leak the already-created entities on the long-lived shared proxy; init() now runs inside the try and a regression test in tests/e2e/test_lifecycle.py locks that contract. The intentionally-failing custom-pricing isolation test, which documents a real per-deployment pricing leak in the proxy, is now marked xfail(strict=True) so the suite gives a clean green/red signal and turns into a failure the moment the leak is fixed and the marker should be removed. A strict-typing pass then made the suite pass its own shipped basedpyright config (tests/pyrightconfig.json): the parametrize ids lambda became a typed _case_id function and the underscore-prefixed autouse live-proxy fixture was renamed so basedpyright no longer flags it as an unused private function, bringing basedpyright --project tests to zero errors. A second Bugbot pass then noted that every test under tests/e2e/ armed the spend-log truncate and was subject to the live-proxy skip, including test_lifecycle.py, a pure unit test of run_case that never touches the proxy; both the truncate guard and the skip now key off the e2e marker, so the live suites still gate on a reachable proxy while the harness unit test runs and gives signal regardless of whether one is up. The liveness probe moved into a pytest_runtest_setup hook with its result cached so it still fires once per session
Mirrored the latest commit from the original PR (#30790): a split control-plane/data-plane gateway where a SplitTransport routes management/admin paths (keys, users, teams, orgs, budgets, spend, model info) to CONTROL_PLANE_BASE_URL and LLM paths to the data plane, a no-op when the two URLs match, plus four new budget suites covering team-member budget attribution and enforcement, team-member budget reset, team multi-window budgets, and a cold-counter spend-reseed regression for #26829. The conftest liveness probe now also checks the control plane when LITELLM_CONTROL_PLANE_URL differs from the proxy, folded into the existing cached _proxy_skip_reason so the e2e-marker gating and the spend-log truncate guard carry over unchanged. A follow-up types the new redis-backed reseed test against redis.Redis[str] through a TYPE_CHECKING import (the runtime import stays lazy so the suite still skips rather than errors when redis is absent), keeping basedpyright --project tests at zero errors
Note
Low Risk
Test-only changes with no production proxy logic modified; the main caveat is optional session-end DB truncation of spend logs when live e2e tests run against a shared DATABASE_URL.
Overview
Introduces a
tests/e2e/live-proxy harness and three suites that exercise real requests against a long-lived gateway, with typed HTTP (e2e_http),Gateway+SplitTransport(management vs LLM paths when control plane URL differs),ResourceManager/run_casecleanup, and coverage matrices documenting what is implemented vs what these tests add.Spend tracking polls
SpendLogsand aggregates (chat/stream/embed, cache hits, tags, end-user, failure rows,/spend/calculate, breadth probes on spend routes). Budgets drivebudget_exceededfor keys, users, customers, orgs, team members, tags, and per-model caps; cover CRUD, soft budget non-block, key/window resets, team-member attribution, Redis cold-counter reseed (#26829), plusxfail(strict=True)for known/team/newbudget_limitsand custom-pricing isolation bugs. LLM translation asserts Gemini/Anthropic passthrough (stream + tools) cost rows and custom per-token billing from gateway config.Shared
conftest:e2emarker skips when proxy/control plane is down; session-end spend-log truncate runs only after at least one marked test body executed. Adds gatewaylitellm-config.yml(short Redis TTL for reseed tests) andtests/pyrightconfig.jsonstrict typing for the e2e tree.Reviewed by Cursor Bugbot for commit 9846882. Bugbot is set up for automated code reviews on this repo. Configure here.