Skip to content

test: take keys out of the legacy proxy, enterprise and mcp unit tests before moving them - #42901

Merged
yuneng-berri merged 8 commits into
mainfrom
litellm_ci_mig_2_key_cleanup
Sep 24, 2026
Merged

yuneng-berri merged 8 commits into
mainfrom
litellm_ci_mig_2_key_cleanup

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Stack 2/5 of the split of #42831 (reference PR, stays open until the stack lands). Below: #42900. Above: #42902. Carved from litellm_ci_migration_proxy_misc at c17f440 with git checkout <branch> -- <paths>, no rewrites

TLDR

Problem this solves:

  • unit tests at the legacy paths read real provider keys or need live credentials
  • test_proxy_pass_user_config.py::test_chat_completion needs Azure/OpenAI keys and fails keyless
  • test_vertex_ai_gemini_token_counting_with_contents is unconditionally skipped, never runs
  • two files mix keyless tests with provider-bound ones, blocking the later moves

How it solves it:

  • tests/integration/routing/test_user_config_routing.py replaces the unit test; unit file deleted
  • Gemini contents token counter test becomes tests/e2e/llm_translation/test_token_counter_gemini_contents_e2e.py
  • load_vertex_ai_credentials and the credential imports dropped from test_proxy_token_counter.py
  • synthetic literals in test_proxy_utils.py, test_prometheus_unit_tests.py, test_proxy_custom_auth.py
  • test_gemini_pass_through_endpoint split to test_proxy_server_gemini_pass_through.py, legacy selector line added
  • provider-bound MCP tests split to tests/mcp_tests/test_aresponses_api_with_mcp_providers.py

Everything stays at its legacy path. The moves happen in stacks 3 to 5

Type

🧹 Refactoring
✅ Test

Screenshots / Proof of Fix

litellm-tests on this head: https://app.circleci.com/pipelines/github/BerriAI/litellm/90271

The routing integration test boots an owned proxy with general_settings.allow_client_side_credentials: true against a scripted local upstream, sends a chat completion with a user_config pointing at that upstream with a user-supplied key, and asserts exactly one /v1/chat/completions reached the upstream with Authorization: Bearer <user key>. The second case boots without the opt-in and asserts 401. It is selected by the providers group through routing in run.py GROUPS and passes locally under run_integration.sh (real Postgres and Redis, no provider keys)

The Gemini e2e test runs through e2e-changed-tests (.github/workflows/test-e2e-changed.yml, select_tests.py picks changed tests/e2e/**/test_*.py) once the e2e-changed environment is approved. Locally it passed for gemini-2.5-flash and gemini-2.5-flash-vertex through /utils/token_counter against real credentials

Greptile follow-up in d2054d6: the two restating comments in test_proxy_server_gemini_pass_through.py are gone

Caveats (if any)

Low

  • test_proxy_token_counter.py has two failures inherited from main (test_factory_gpt4_endpoint_does_not_call_anthropic_counter, test_internal_token_counter_anthropic_provider_detection), reproduced on the base tree, untouched here

QA runbook

  • tests/e2e/llm_translation/test_token_counter_gemini_contents_e2e.py::TestGeminiContentsTokenCounting::test_contents_are_counted_by_the_provider_endpoint - Gemini contents through /utils/token_counter?call_endpoint=true is counted by Google's countTokens upstream for both the Gemini and Vertex deployments (needs GEMINI_API_KEY and Vertex credentials)
    • POST http://localhost:4000/utils/token_counter?call_endpoint=true with a scoped key, {"model": "gemini-2.5-flash", "contents": [{"parts": [{"text": "hello world"}]}]}
    • Expect 200 with request_model echoing the model, total_tokens > 0 equal to original_response.totalTokens, and every promptTokensDetails[].tokenCount > 0
    • Repeat with gemini-2.5-flash-vertex, expect the same shape
    • 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

Pre-Submission checklist

  • I have added meaningful tests
  • The handful of test files covering my change pass locally
  • My PR passes all required CI/CD checks
  • 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

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

Link to Devin session: https://app.devin.ai/sessions/d541aba7e5844703bab65085ef2d54b2
Open in Devin Desktop: https://app.devin.ai/desktop/session/d541aba7e5844703bab65085ef2d54b2?variant=devin

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@codecov

codecov Bot commented Sep 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@greptile-apps

greptile-apps Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge; no outstanding findings or new actionable issues remain.

Summary

This PR replaces credential-dependent legacy tests with keyless fixtures or moves them to integration and e2e coverage. It also splits provider-bound MCP and Gemini tests from unit-test files and updates the proxy test selection.

Reviews (3) · Last reviewed commit: "Merge branch 'litellm_ci_mig_1_ci_fix' i..."

Comment thread tests/e2e/llm_translation/test_token_counter_gemini_contents_e2e.py
Comment thread tests/proxy_unit_tests/test_proxy_server_gemini_pass_through.py Outdated
@mateo-berri

Copy link
Copy Markdown
Contributor

@greptileai

@mateo-berri

Copy link
Copy Markdown
Contributor

@greptileai

Base automatically changed from litellm_ci_mig_1_ci_fix to main September 24, 2026 21:49
@yuneng-berri
yuneng-berri requested a review from a team September 24, 2026 21:49
@yuneng-berri
yuneng-berri merged commit b72a030 into main Sep 24, 2026
110 of 158 checks passed
@yuneng-berri
yuneng-berri deleted the litellm_ci_mig_2_key_cleanup branch September 24, 2026 21:56
devin-ai-integration Bot added a commit that referenced this pull request Sep 24, 2026
…s before moving them (#42901)

* ci: fix the litellm-tests unit job with sysmon coverage, an env allowlist and coverage upload on failure

* test: replace key-dependent proxy, enterprise and mcp unit tests with synthetic values and integration and e2e coverage

* test: drop key reads at the legacy proxy, enterprise and mcp paths and wire the gemini pass-through split

* ci: fail the unit shard when circleci tests split errors

* test: drop restating comments from the gemini pass-through split

* ci: exit the unit shard cleanly when circleci tests split assigns it no files

---------

Co-authored-by: yuneng <yuneng@berri.ai>

This branch is waiting to be deployed

1 waiting deployment
e2e-changed — f2d11871 Waiting Sep 24, 2026 by devin-ai-integration[bot] via Run changed e2e tests against the stage-mirror stack #11155
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.

2 participants