Skip to content

test(proxy): stop monkeypatch.undo re-planting fixture-mocked prisma_client - #36872

Merged
mateo-berri merged 1 commit into
litellm_internal_stagingfrom
litellm_restore_prisma_client_global
Aug 14, 2026
Merged

test(proxy): stop monkeypatch.undo re-planting fixture-mocked prisma_client#36872
mateo-berri merged 1 commit into
litellm_internal_stagingfrom
litellm_restore_prisma_client_global

Conversation

@mateo-berri

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

  • Required proxy-endpoints CI check flakes on unrelated PRs
  • Three MCP management tests die on "MagicMock can't be used in 'await'"
  • Failures survive the job's per-test reruns and full job reruns

How it solves it:

  • Moves proxy_server global isolation from an autouse fixture to runtest hooks
  • Hooks restore after every fixture finalizer, so monkeypatch.undo can't re-leak mocks
  • Pins the three victim MCP tests to the no-database path
  • Adds a regression pair that fails on the old conftest

The mechanism: the root conftest's autouse fixtures request monkeypatch, so monkeypatch's undo stack unwinds after every other fixture finalizer. test_team_endpoints.py has an autouse fixture that patches litellm.proxy.proxy_server.prisma_client with a MagicMock, and its delete_team tests monkeypatch that same global, recording the fixture's mock as the "original". At teardown the patch exits, the old isolation fixture restores the real value, and then monkeypatch.undo runs last and re-plants the MagicMock. Every later test's snapshot faithfully preserves it, so the worker is poisoned for the rest of the session and any later no-database MCP test that reads the global takes the database path and fails awaiting a MagicMock. With xdist loadscope this repeats whenever the team and MCP modules share a worker, which is why reruns fail identically

User Flow

Before: a contributor's PR that never touches proxy code fails the required proxy-endpoints check, and re-running fails the same way

  1. They open a PR touching only a lint script and its test, e.g. fix(scripts): end make check with a ran/skipped summary and verdict #36864
  2. The required check Unit Tests: Proxy API Endpoints / proxy-endpoints / Run tests turns red after about 14 minutes
  3. The run log at https://github.com/BerriAI/litellm/actions/runs/31763090709 shows three MCP server management tests they never touched failing with TypeError: object MagicMock can't be used in 'await' expression, having already survived the job's two automatic per-test retries
  4. They re-run the failed job and the same three tests fail again, so the PR stays blocked

After: the same PR passes the proxy-endpoints check on the first run

  1. They open the same PR touching only a lint script and its test
  2. The required check Unit Tests: Proxy API Endpoints / proxy-endpoints / Run tests turns green
  3. The PR is mergeable with no rerun roulette

Relevant issues

Unblocks the required check failing on #36864

Linear ticket

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have added meaningful tests
  • My PR passes all CI/CD checks (e.g., lint, format, unit tests)
  • 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 (Greptile reviews automatically once the PR is opened; only comment @greptileai to re-request a review after pushing changes)

Delays in PR merge?

If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).

Screenshots / Proof of Fix

This PR changes only test isolation, so the end-to-end surface is the CI job itself rather than a live proxy route. The failing check is deterministic CI evidence: https://github.com/BerriAI/litellm/actions/runs/31763090709 failed on the same three tests in the original run and again on a full rerun, on a PR touching only scripts/pre_commit_lint.sh and its test

Red-green on the regression pair, before at 86f2f16 (staging tip conftest plus only the new test file):

$ pytest tests/test_litellm/proxy/test_conftest.py -q
FAILED tests/test_litellm/proxy/test_conftest.py::test_prisma_client_did_not_leak_from_previous_test
1 failed, 1 passed, 1 warning in 1.41s

After at a36ba05:

$ pytest tests/test_litellm/proxy/test_conftest.py -q
2 passed, 1 warning in 1.58s

Full tests/test_litellm/proxy sweep on the same machine went from 7 failed, 7296 passed at the base to 4 failed, 7299 passed with the fix. The three recovered tests are exactly the three CI victims. The residual failures also fail identically at the base and are local environment artifacts: a machine-local sso settings failure, two tests needing the optional detect_secrets package, and one test reading a PROXY_BASE_URL left in the local .env

Type

✅ Test

Caveats (if any)

  • Snapshot still covers only master_key and prisma_client, as before

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

@greptile-apps

greptile-apps Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR replaces fixture-based proxy-global isolation with setup and teardown hooks so restoration occurs after fixture finalizers

  • Stores each test's master_key and prisma_client snapshot in the pytest item stash
  • Forces three MCP tests through their intended no-database path
  • Adds a regression pair covering fixture and monkeypatch teardown ordering

Confidence Score: 5/5

The PR appears safe to merge

No blocking failure remains

Important Files Changed

Filename Overview
tests/test_litellm/proxy/conftest.py Moves proxy-global snapshot and restoration into pytest hookwrappers so monkeypatch teardown cannot reintroduce fixture mocks
tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py Pins three MCP management tests to the no-database branch by mocking the Prisma accessor
tests/test_litellm/proxy/test_conftest.py Adds a focused regression pair proving a fixture-planted Prisma mock does not leak into the next test

Reviews (2): Last reviewed commit: "test(proxy): stop monkeypatch.undo re-pl..." | Re-trigger Greptile

Comment thread tests/test_litellm/proxy/conftest.py
Comment thread tests/test_litellm/proxy/conftest.py
@codecov

codecov Bot commented Aug 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@mateo-berri

Copy link
Copy Markdown
Contributor Author

@greptileai

@shin-berri shin-berri left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This blocks CI. Self-approving safe test-only change

@mateo-berri
mateo-berri merged commit c278455 into litellm_internal_staging Aug 14, 2026
77 checks passed
@mateo-berri
mateo-berri deleted the litellm_restore_prisma_client_global branch August 14, 2026 03:52
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