Skip to content

[Fix] Deflake spend tracking tests - #26349

Merged
shin-berri merged 4 commits into
litellm_internal_stagingfrom
litellm_deflakeSpendTests
Apr 23, 2026
Merged

[Fix] Deflake spend tracking tests#26349
shin-berri merged 4 commits into
litellm_internal_stagingfrom
litellm_deflakeSpendTests

Conversation

@yuneng-berri

Copy link
Copy Markdown
Collaborator

Relevant issues

Summary

Two independent deflakes of spend tracking tests.

Fix

1. test_ui_view_spend_logs_unauthorized was intermittently returning 400 instead of 401/403 for a request with an invalid bearer token. Earlier tests in the same file could leave proxy-auth globals (prisma_client, master_key, user_custom_auth, general_settings, cached tokens) in a state where an invalid token slipped past user_api_key_auth and reached the endpoint's own start_date/end_date validation. Added an autouse fixture that pins those globals to their import-time defaults for every test in the file, and tightened the assertion to include the response body so any future regression surfaces the actual cause instead of a bare status code.

2. test_basic_spend_accuracy (CI job proxy_spend_accuracy_tests) depends on the Redis transaction buffer flushing spend from Redis to Postgres. The buffer uses a single global pod-lock key (cronjob_lock:db_spend_update_job) and a single global buffer list key (litellm_spend_update_buffer). The CI job was pointed at the shared remote Redis via $REDIS_HOST, so concurrent CI pipelines would contend for the same lock and could LPOP one another's spend entries and commit them to the wrong Postgres instance. Added a reusable start_redis CircleCI command that boots a per-job redis:7-alpine container (digest-pinned) and switched proxy_spend_accuracy_tests to REDIS_HOST=host.docker.internal:6379 so lock and buffer state are isolated per CI run. This keeps the Redis transaction buffer code path under test while eliminating cross-pipeline contention.

Testing

  • Ran tests/test_litellm/proxy/spend_tracking/test_spend_management_endpoints.py three times in a row locally — 61 passed each run.
  • CI behavior for test_basic_spend_accuracy can only be validated on CircleCI; the structural fix (per-job Redis) is observable via docker logs redis-cache and the usual job flow.

Type

🐛 Bug Fix
🚄 Infrastructure
✅ Test

Screenshots

[Infra] Promote interal staging to main
Two independent deflakes:

1. test_ui_view_spend_logs_unauthorized (unit) was returning 400 instead
   of 401/403 when earlier tests in the file left proxy-auth globals
   (prisma_client, master_key, user_custom_auth, general_settings,
   user_api_key_cache) in a state that let invalid tokens pass auth and
   fall through to the endpoint's own start_date/end_date validation.
   Add an autouse fixture that pins those globals to their import-time
   defaults for every test in the file. Harden the assertion to include
   response body so future flakes are diagnosable.

2. test_basic_spend_accuracy (CI job proxy_spend_accuracy_tests) depends
   on the Redis transaction buffer flushing spend to Postgres. The buffer
   uses a single global pod-lock key (cronjob_lock:db_spend_update_job)
   and a single global buffer list key. Pointing the proxy at the shared
   remote Redis means concurrent CI pipelines contend for the same lock
   and can drain each other's buffer into the wrong database. Add a
   start_redis reusable command that boots a per-job redis:7-alpine
   container (digest-pinned), and switch proxy_spend_accuracy_tests to
   REDIS_HOST=host.docker.internal:6379 so lock and buffer state are
   isolated per CI run.
@yuneng-berri
yuneng-berri changed the base branch from main to litellm_internal_staging April 23, 2026 21:15
@yuneng-berri
yuneng-berri requested a review from a team April 23, 2026 21:15
@veria-ai

veria-ai Bot commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

Low: CI and test-only changes

This PR deflakes spend tracking tests by spinning up a job-local Redis container in CI instead of sharing a remote one, and adds a test fixture to reset proxy auth globals between tests. Both changes are scoped entirely to CI configuration and test files with no impact on production runtime code.


Status: 0 open
Risk: 1/10

Posted by Veria AI · 2026-04-23T21:22:02.432Z

@codspeed-hq

codspeed-hq Bot commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 16 untouched benchmarks


Comparing litellm_deflakeSpendTests (e37d1b0) with litellm_internal_staging (6a25866)

Open in CodSpeed

@greptile-apps

greptile-apps Bot commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR deflakes two spend-tracking tests: it pins proxy auth globals (prisma_client, master_key, user_custom_auth, general_settings, in-memory token cache) to a clean baseline before each test via an autouse fixture, and isolates the proxy_spend_accuracy_tests CI job from shared remote Redis by booting a per-job redis:7-alpine container (digest-pinned) and pointing the proxy at host.docker.internal:6379. The cleanup step is now guarded with when: always to prevent stale container name conflicts on subsequent runs.

Confidence Score: 5/5

Safe to merge — changes are test/CI infrastructure only with no production code modifications.

Both fixes are well-reasoned and narrowly scoped: the autouse fixture correctly uses monkeypatch (auto-teardown) to reset proxy globals and directly clears the in-memory cache dict at test start; the CI fix uses a digest-pinned local Redis container and guards cleanup with when: always, addressing the concerns raised in prior review threads. No custom rules are violated and no production logic is touched.

No files require special attention.

Important Files Changed

Filename Overview
.circleci/config.yml Adds a reusable start_redis command (digest-pinned image) and switches proxy_spend_accuracy_tests to a job-local Redis, eliminating cross-pipeline contention on the global pod-lock and buffer keys; cleanup step now guards with when: always.
tests/test_litellm/proxy/spend_tracking/test_spend_management_endpoints.py Adds an autouse fixture that resets proxy auth globals and the in-memory token cache before every test, preventing global-state leakage between tests; assertion messages now include response.text for clearer failure output.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    subgraph CI Job: proxy_spend_accuracy_tests
        A[checkout] --> B[start_postgres\npostgres:14 on :5432]
        B --> C[start_redis\nredis:7-alpine on :6379]
        C --> D[docker run my-app\nREDIS_HOST=host.docker.internal\nREDIS_PORT=6379]
        D --> E[wait_for_service :4000]
        E --> F[pytest spend_tracking_tests]
        F --> G{when: always}
        G --> H[docker stop/rm my-app\ndocker stop/rm redis-cache]
    end

    subgraph Before: shared remote Redis
        X[Pipeline A] -->|cronjob_lock| Z[(Remote Redis)]
        Y[Pipeline B] -->|cronjob_lock| Z
        Z -->|contention / wrong DB flush| ERR[Flaky test]
    end

    subgraph After: per-job local Redis
        P1[Pipeline A] --> R1[(Local Redis A)]
        P2[Pipeline B] --> R2[(Local Redis B)]
        R1 --> OK1[Isolated flush]
        R2 --> OK2[Isolated flush]
    end
Loading

Reviews (3): Last reviewed commit: "[Fix] Drop orphan teardown step from Gre..." | Re-trigger Greptile

Comment thread .circleci/config.yml
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Comment thread .circleci/config.yml Outdated
Comment on lines 1845 to 1846
- run:
name: Stop and remove first container

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Dangling run step with no command

The first "Stop and remove first container" step at line 1845 has only a name key and no command. CircleCI validates that every run step includes a command; this config will fail validation (or produce a confusing empty-step error at runtime). The original step should have been modified in-place — with when: always and the updated commands — rather than leaving an empty copy behind and adding a second step.

Suggested change
- run:
name: Stop and remove first container
# Clean up first container
- run:
name: Stop and remove first container
when: always
command: |
docker stop my-app
docker rm my-app
docker stop redis-cache
docker rm redis-cache

Previous commit from greptile-apps added a new `when: always` teardown
step without removing the prior `name:`-only step, leaving a `- run`
block with no `command:` — CircleCI config validation rejects that.
Collapse back to a single teardown step that runs on success and
failure.
@yuneng-berri
yuneng-berri temporarily deployed to integration-postgres April 23, 2026 21:22 — with GitHub Actions Inactive
@yuneng-berri
yuneng-berri temporarily deployed to integration-postgres April 23, 2026 21:22 — with GitHub Actions Inactive
@yuneng-berri
yuneng-berri temporarily deployed to integration-postgres April 23, 2026 21:22 — with GitHub Actions Inactive
@yuneng-berri
yuneng-berri temporarily deployed to integration-postgres April 23, 2026 21:22 — with GitHub Actions Inactive
@yuneng-berri
yuneng-berri temporarily deployed to integration-postgres April 23, 2026 21:22 — with GitHub Actions Inactive
@shin-berri
shin-berri enabled auto-merge April 23, 2026 23:12
@shin-berri
shin-berri merged commit 7c69262 into litellm_internal_staging Apr 23, 2026
97 of 99 checks passed
@shin-berri
shin-berri deleted the litellm_deflakeSpendTests branch April 23, 2026 23:12
fzowl pushed a commit to fzowl/litellm that referenced this pull request Jun 24, 2026
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.

3 participants