Skip to content

test(proxy): move auth, hooks, policy_engine and client tests into tests/unit/proxy - #43998

Merged
yuneng-berri merged 6 commits into
mainfrom
litellm_migrate_proxy_tests_p2
Oct 1, 2026
Merged

yuneng-berri merged 6 commits into
mainfrom
litellm_migrate_proxy_tests_p2

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

  • tests/test_litellm/proxy still holds the auth, hooks, policy_engine and client tests
  • They should live in the unit tier at tests/unit/proxy/<same path>

How it solves it:

  • git mv of all four directories (118 files, every one a pure rename)
  • Five colliding files renamed to say what they cover
  • Ports only the legacy conftest fixtures and hooks the moved tests need
  • Rewrites the proxy-auth and proxy-infra shard paths in test-unit.yml, shard names unchanged

PR 2 of 7, now based on main since #43996 merged. Legacy conftests stay until PR 7

Screenshots / Proof of Fix

Coverage ledger

Every node id collected from the 118 legacy files on the base is collected at its new path on the head

base origin/main: 5864 node ids in 118 legacy files
head litellm_migrate_proxy_tests_p2: 5864 node ids
renamed files: 118, deleted legacy files: 0
carried over after path mapping: 5864
missing explained: 0
missing unexplained: 0
head ids with no base counterpart: 0

Generated with python3 ledger.py origin/main litellm_migrate_proxy_tests_p2. It reads the renames from git diff -M --name-status, runs pytest --collect-only -q on each side in a throwaway worktree with an empty env, maps old paths to new ones, and exits 1 on any unexplained missing id

ledger.py
import json
import os
import pathlib
import subprocess
import sys
import tempfile
from typing import Final

REPO: Final = pathlib.Path(os.environ.get("LEDGER_REPO", "/home/ubuntu/repos/litellm"))
PYTHON: Final = os.environ.get("LEDGER_PYTHON", str(REPO / ".venv/bin/python"))
LEGACY: Final = "tests/test_litellm/proxy/"


def git(*args: str) -> str:
    return subprocess.run(["git", "-C", str(REPO), *args], check=True, capture_output=True, text=True).stdout


def moves(base: str, head: str) -> tuple[dict[str, str], list[str], list[str]]:
    renamed: dict[str, str] = {}
    deleted: list[str] = []
    added: list[str] = []
    for line in git("diff", "-M", "--name-status", f"{base}...{head}").splitlines():
        status, *paths = line.split("\t")
        if status.startswith("R") and paths[0].startswith(LEGACY):
            renamed[paths[0]] = paths[1]
        elif status == "D" and paths[0].startswith(LEGACY):
            deleted.append(paths[0])
        elif status == "A":
            added.append(paths[0])
    return renamed, deleted, added


def collect(ref: str, files: list[str]) -> set[str]:
    targets: Final = [f for f in files if pathlib.PurePosixPath(f).name.startswith("test_") and f.endswith(".py")]
    if not targets:
        return set()
    with tempfile.TemporaryDirectory() as scratch:
        tree: Final = pathlib.Path(scratch) / "tree"
        git("worktree", "add", "--detach", "-q", str(tree), ref)
        try:
            result: Final = subprocess.run(
                [PYTHON, "-m", "pytest", "--collect-only", "-q", "-p", "no:cacheprovider", "-o", "addopts=", *targets],
                cwd=tree,
                capture_output=True,
                text=True,
                env={"PATH": os.environ["PATH"], "HOME": os.environ["HOME"]},
            )
        finally:
            git("worktree", "remove", "--force", str(tree))
    ids: Final = {line for line in result.stdout.splitlines() if "::" in line}
    errors: Final = [line for line in result.stdout.splitlines() if line.startswith("ERROR")]
    if errors or result.returncode not in (0, 5):
        sys.stderr.write(f"collection problems on {ref} (exit {result.returncode}):\n" + "\n".join(errors[:50]) + "\n")
        sys.stderr.write(result.stdout[-4000:])
    return ids


def main() -> int:
    base, head = sys.argv[1], sys.argv[2]
    explained: Final = json.loads(pathlib.Path(sys.argv[3]).read_text()) if len(sys.argv) > 3 else {}
    renamed, deleted, added = moves(base, head)
    base_ids: Final = collect(base, list(renamed) + deleted)
    head_ids: Final = collect(head, list(renamed.values()) + [a for a in added if a.startswith("tests/")])
    mapped: Final = {}
    for node in base_ids:
        path, _, rest = node.partition("::")
        mapped[node] = f"{renamed[path]}::{rest}" if path in renamed else None
    missing: Final = sorted(old for old, new in mapped.items() if new is None or new not in head_ids)
    carried: Final = sum(1 for new in mapped.values() if new is not None and new in head_ids)
    unexplained: Final = [m for m in missing if m not in explained]
    new_only: Final = sorted(head_ids - {n for n in mapped.values() if n is not None})
    print(f"base {base}: {len(base_ids)} node ids in {len(renamed) + len(deleted)} legacy files")
    print(f"head {head}: {len(head_ids)} node ids")
    print(f"renamed files: {len(renamed)}, deleted legacy files: {len(deleted)}")
    print(f"carried over after path mapping: {carried}")
    print(f"missing explained: {len(missing) - len(unexplained)}")
    for m in missing:
        if m in explained:
            print(f"  {m} -> {explained[m]}")
    print(f"missing unexplained: {len(unexplained)}")
    for m in unexplained:
        print(f"  {m}")
    print(f"head ids with no base counterpart: {len(new_only)}")
    for n in new_only:
        print(f"  {n}")
    return 1 if unexplained else 0


if __name__ == "__main__":
    sys.exit(main())

Collisions

legacy file new name
test_proxy_server.py tests/unit/proxy/test_proxy_server_endpoints_and_startup.py
test_proxy_utils.py tests/unit/proxy/test_proxy_utils_model_creation_and_error_logging.py
auth/test_auth_checks.py tests/unit/proxy/auth/test_auth_checks_object_access_and_lookup.py
auth/test_user_api_key_auth.py tests/unit/proxy/auth/test_user_api_key_auth_request_flow.py
_experimental/mcp_server/test_mcp_server.py tests/unit/proxy/_experimental/mcp_server/test_mcp_server_tool_calls_and_headers.py
conftest.py merged into tests/unit/proxy/conftest.py
_experimental/mcp_server/conftest.py merged into tests/unit/proxy/_experimental/mcp_server/conftest.py

The three root and MCP renames move ahead of their directories so the collisions are settled in one place. Their node ids are in the ledger above

Non-rename edits (152 added, 12 removed)

file +/- reason
tests/unit/proxy/conftest.py 63/0 conftest port: the proxy_server globals snapshot hooks (master_key, prisma_client, llm_router), _reset_graceful_shutdown_state, secret_vault_factory for the client CLI tests, and a shared httpx_transport fixture that turns off the aiohttp transport so respx routes see the request. The existing isolation fixture and collection hook are untouched
tests/unit/proxy/_experimental/mcp_server/conftest.py 60/0 conftest merge: the legacy registry and SERVER_ROOT_PATH isolation fixtures and the _mcp_request_ctx factory, next to the destination's own fixtures. The registry fixture now empties registry, config_mcp_servers, the tool name mapping and the OAuth discovery slots before each test, then restores them. Before that, config servers that tests/mcp_tests/test_proxy_mcp_e2e.py loaded into the shared manager in the same xdist worker survived, so test_reload_resets_probe_cooldown's background tool listing task dialed them and hung event loop teardown past the 120s timeout in mcp-integration
tests/unit/proxy/auth/test_onboarding.py 2/2 network the unit tier blocks: the HIBP client uses the aiohttp transport, which respx cannot see, so the breached-password test was really calling HIBP. Both HIBP tests now request httpx_transport, so the existing respx routes serve them
tests/unit/proxy/auth/test_login_utils.py 7/1 network: test_db_user_login_still_works_when_disabled ran the real HIBP breach check. A respx route on the range/ prefix of its password now answers it through httpx_transport
tests/unit/proxy/client/__init__.py, hooks/litellm_skills/__init__.py 0/0 empty package markers so tests/unit/test_package_layout.py passes
tests/proxy_behavior/auth/test_auth_object_prefetch.py 1/1 docstring path to the moved unit test
.github/workflows/test-unit.yml 15/4 CI paths: proxy-auth runs the four new dirs and ignores the 9 files test-unit-proxy-db.yml already owns there, proxy-infra picks up the two renamed root files
.github/merge-smoke-tests.json 2/2 MODEL-ALLOW and MODEL-DENY follow auth/test_auth_checks.py
Makefile 2/2 test-unit-proxy-core and test-unit-proxy-misc paths

assert_ci_coverage.py --shards and plain mode both print OK without adding tests/unit/proxy to SHARDED_ROOTS, so it is unchanged. .circleci/scripts/unit_selection.sh is unchanged: its unit catch-all picks up the moved files and the renamed MCP file stays under mcp-integration through the existing tests/unit/proxy/_experimental/mcp_server entry

Type

✅ Test

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/d9bda95617da464e88d3619765819d0e
Open in Devin Desktop: https://app.devin.ai/desktop/session/d9bda95617da464e88d3619765819d0e?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

@greptile-apps

greptile-apps Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Low risk] Reorganizes test files into a new directory structure.

The PR appears safe to merge based on the current review.

Summary

This PR moves proxy auth, hooks, policy-engine, client, and selected MCP and root tests into tests/unit/proxy, updates test selection paths, and ports the fixtures needed at their new locations. No new actionable issue was identified since the previous review.

Reviews (2) · Last reviewed commit: "test(proxy): restore proxy globals witho..."

Comment thread tests/unit/proxy/conftest.py Outdated
@devin-ai-integration
devin-ai-integration Bot force-pushed the litellm_migrate_proxy_tests_p2 branch from 32fd263 to 20b524f Compare October 1, 2026 09:43
@codecov

codecov Bot commented Oct 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

@greptileai review

yuneng-berri and others added 6 commits October 1, 2026 16:27
…sts/unit/proxy

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…e HTTP boundary

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration
devin-ai-integration Bot force-pushed the litellm_migrate_proxy_tests_p2 branch from 676ca84 to b20d053 Compare October 1, 2026 16:28
@yuneng-berri
yuneng-berri merged commit 39e3195 into main Oct 1, 2026
100 of 103 checks passed
@yuneng-berri
yuneng-berri deleted the litellm_migrate_proxy_tests_p2 branch October 1, 2026 17:11
koladefaj added a commit to koladefaj/litellm that referenced this pull request Oct 1, 2026
Absorbs the test move in BerriAI#43998, so the four test files this PR touches now
live under tests/unit/proxy/auth and nothing references the deleted
tests/test_litellm paths. GitHub's merge was not following those renames and
reported the PR as conflicting.
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.

1 participant