Skip to content

test(mcp): retire the last file of the dead tests/litellm mirror - #37824

Merged
yuneng-berri merged 1 commit into
litellm_internal_stagingfrom
litellm_retire_dead_test_mirror
Aug 22, 2026
Merged

test(mcp): retire the last file of the dead tests/litellm mirror#37824
yuneng-berri merged 1 commit into
litellm_internal_stagingfrom
litellm_retire_dead_test_mirror

Conversation

@yuneng-berri

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

  • tests/litellm/ is a mirror no job ever ran
  • Its last file froze; 25 of 32 cases now fail
  • An allowlist entry asked someone to decide its fate

How it solves it:

  • Three of its five orphan behaviours are already covered better
  • The other two get ported onto live mock conventions
  • Delete the file, drop the allowlist entry

User Flow

No end-user behavior changes. A client hitting
https://litellm-domain/mcp/.well-known/oauth-protected-resource gets the same
response resolved the same way; this is test coverage around those endpoints,
not the endpoints

Relevant issues

Linear ticket

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 (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

Screenshots / Proof of Fix

Shared setup: is_request_from_trusted_proxy decides whether X-Forwarded-*
headers are believed. Cases 2 and 3 pin its two answers open in turn and ask the
suite whether it notices, which is the only way to tell ported coverage from
ported text.

G=litellm/proxy/auth/ip_address_utils.py
F=tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py
pin() { cp $G /tmp/gate.bak; python3 -c "
import pathlib;p=pathlib.Path('$G');s=p.read_text().splitlines(keepends=True);s.insert(141,'        return $1\n');p.write_text(''.join(s))"; }
unpin() { cp /tmp/gate.bak $G; }

Before (ff02d5c)

1. the second mirror against today's code

  1. uv run pytest tests/litellm/ -q -p no:randomly | tail -1
25 failed, 7 passed, 1 warning in 3.76s

2. the trust gate pinned to always-trust

  1. pin True; uv run pytest $F -q -p no:randomly -k validate_trusted_redirect_uri | tail -1; unpin
1 failed, 310 deselected, 2 warnings in 1.92s

3. the trust gate pinned to never-trust

  1. pin False; uv run pytest $F -q -p no:randomly -k validate_trusted_redirect_uri | tail -1; unpin
1 passed, 310 deselected, 2 warnings in 1.90s

4. the live file

  1. uv run pytest $F -q -p no:randomly | tail -1
311 passed, 6 warnings in 2.58s

After (13d4074)

1. the second mirror against today's code

  1. uv run pytest tests/litellm/ -q -p no:randomly | tail -1
no tests ran in 0.01s

2. the trust gate pinned to always-trust

  1. pin True; uv run pytest $F -q -p no:randomly -k validate_trusted_redirect_uri | tail -1; unpin
2 failed, 1 passed, 310 deselected, 2 warnings in 1.95s

3. the trust gate pinned to never-trust

  1. pin False; uv run pytest $F -q -p no:randomly -k validate_trusted_redirect_uri | tail -1; unpin
1 failed, 2 passed, 310 deselected, 2 warnings in 2.02s

4. the live file

  1. uv run pytest $F -q -p no:randomly | tail -1
313 passed, 6 warnings in 2.68s

Case 3 is the one that justifies porting rather than deleting. A gate that never
trusts anything survives the suite as it stands and is caught after

Type

🧹 Refactoring

✅ Test

Caveats (if any)

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

tests/litellm/ was a second mirror beside tests/test_litellm/ that no workflow,
Makefile target, or CircleCI job ever named. Its other 33 files were reconciled
during August 2026; this one stayed behind under a ci-coverage-allowlist entry
asking a later pass to decide which of its five orphan behaviours still hold.

They no longer hold as written: 25 of its 32 cases fail against today's code,
because the file froze on the day it stopped being collected and the endpoints
kept moving. Three of the five are already covered by the live twin, and better.
test_get_request_base_url_xff_trust_gate parametrizes the trust gate in both
directions, including the exact untrusted-caller case the orphan asserted, and
the standard and legacy protected-resource shapes are both exercised through
use_standard_pattern.

The other two were the only tests anywhere for validate_trusted_redirect_uri
under that same gate, so they are ported rather than dropped, rebuilt on the
live file's request-mock conventions. Both directions are load-bearing: forcing
is_request_from_trusted_proxy to True fails the untrusted case, forcing it to
False fails the trusted one.

313 tests pass in the live file, up from 311. Dropping the dead file clears one
zero-assert TQ001 violation, so its ceiling ratchets down with it.
@yuneng-berri
yuneng-berri requested a review from a team August 21, 2026 11:44
@greptile-apps

greptile-apps Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR retires the final unused tests/litellm/ mirror file and removes its coverage allowlist entry. It ports the remaining trusted-proxy redirect coverage into the live MCP test suite and ratchets the test-quality budget.

Confidence Score: 5/5

The PR appears safe to merge, with the retired mirror’s relevant coverage preserved in the active MCP test suite.

The added tests drive the production trusted-proxy decision in both directions using the exact settings and request fields read by the implementation, and no actionable regression was identified.

Important Files Changed

Filename Overview
tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py Adds two live test cases that exercise both acceptance and rejection through the real forwarded-header trust gate.
tests/litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py Deletes the obsolete mirror after its unique valid trust-gate behavior is ported to the active suite and its other relevant behaviors are already covered there.
.github/ci-coverage-allowlist.yml Removes the obsolete allowlist entry for the deleted mirror file.
test-quality-budget.json Ratchets the TQ001 ceiling alongside the test cleanup; no concrete budget-gate failure was established.

Reviews (1): Last reviewed commit: "test(mcp): retire the last file of the d..." | Re-trigger Greptile

@codecov

codecov Bot commented Aug 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@yuneng-berri
yuneng-berri merged commit 6d34de5 into litellm_internal_staging Aug 22, 2026
67 checks passed
@yuneng-berri
yuneng-berri deleted the litellm_retire_dead_test_mirror branch August 22, 2026 03:12
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