Skip to content

test(key-management): unwind the global writes the key tests scaffold around - #37822

Merged
yuneng-berri merged 2 commits into
litellm_internal_stagingfrom
litellm_key_management_global_writes
Aug 22, 2026
Merged

test(key-management): unwind the global writes the key tests scaffold around#37822
yuneng-berri merged 2 commits into
litellm_internal_stagingfrom
litellm_key_management_global_writes

Conversation

@yuneng-berri

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

  • 17 tests hand-roll save, try, write, restore around a global
  • 4 more restore a flag to a hard-coded value, not the old one
  • Every one of them is a TQ005 violation

How it solves it:

  • monkeypatch.setattr, so the scaffolding comes out
  • The fixtures stop guessing what they restore to

User Flow

No end-user behavior changes. A proxy admin calling
https://litellm-domain/key/generate gets the same limits enforced the same way;
this is the test file around that endpoint, not the endpoint.

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: run the file, then read the three globals back in the same
interpreter, and count what the quality rules see.

uv run python -c '
import litellm, pytest
names = ("upperbound_key_generate_params", "default_key_generate_params", "enable_key_alias_format_validation")
before = {n: getattr(litellm, n) for n in names}
pytest.main(["tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py", "-q", "-p", "no:randomly"])
for n in names:
    print(f"litellm.{n} after the run:", "restored" if getattr(litellm, n) == before[n] else "LEAKED")
'

Before (ff02d5c)

  1. The command above
443 passed, 4 warnings in 2.66s
litellm.upperbound_key_generate_params after the run: restored
litellm.default_key_generate_params after the run: restored
litellm.enable_key_alias_format_validation after the run: restored
  1. uv run python scripts/check_test_quality.py tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py | grep -o 'TQ00[0-9]' | sort | uniq -c
  24 TQ001
  16 TQ002
   1 TQ003
  40 TQ005

After (1f682f8)

  1. The same command
443 passed, 4 warnings in 2.29s
litellm.upperbound_key_generate_params after the run: restored
litellm.default_key_generate_params after the run: restored
litellm.enable_key_alias_format_validation after the run: restored
  1. The same count
  24 TQ001
  16 TQ002
  1. uv run python scripts/test_quality_gate.py
OK: every TQ rule is within its test-suite ceiling (base origin/litellm_internal_staging)

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

… around

Seventeen tests in this file save a litellm module global, open a try, write
it, and restore it in a finally. Four more sit behind autouse fixtures that
reset the flag to a hard-coded False rather than to whatever it was.

monkeypatch.setattr does all of that, so the capture, the try and the finally
come out and the test body loses a level of indentation. The alias-format
fixtures stop guessing the value they are restoring to.

Also drops the sys.path.insert, whose argument resolves four levels above the
repo, so it was never what made the imports work.

TQ003 1077 -> 1076 and TQ005 2836 -> 2796, and the budget ceilings come down
with them. 443 tests pass either way; the conftest snapshot was already
catching these globals, so this is about not needing it.
@greptile-apps

greptile-apps Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This test-only refactor replaces manual writes and restoration of key-management globals with pytest monkeypatching and removes an ineffective path mutation

  • Restores key-generation defaults, upper bounds, and alias-validation flags to their actual prior values after each test
  • Lowers the TQ003 and TQ005 quality budgets to reflect removed violations
  • Leaves production key-management behavior and test assertions unchanged

Confidence Score: 5/5

The PR appears safe to merge because it only improves test isolation and ratchets matching quality budgets

The changed tests preserve their configured values and assertions while pytest now restores global state even when a test exits exceptionally

Important Files Changed

Filename Overview
tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py Replaces manual global mutation scaffolding with function-scoped monkeypatch restoration without weakening the existing assertions
test-quality-budget.json Ratchets TQ003 and TQ005 ceilings to the reported post-refactor counts

Reviews (1): Last reviewed commit: "test(key-management): unwind the global ..." | 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!

…itellm_key_management_global_writes

# Conflicts:
#	test-quality-budget.json
@yuneng-berri
yuneng-berri merged commit 7330707 into litellm_internal_staging Aug 22, 2026
68 checks passed
@yuneng-berri
yuneng-berri deleted the litellm_key_management_global_writes branch August 22, 2026 04:09
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