test(s3): stop the logger tests leaking s3_callback_params on failure

[yuneng-berri]

TLDR

Problem this solves:

• Ten tests set litellm.s3_callback_params by hand
• Four reset it on a line that only runs when they pass
• A failure leaves bucket, endpoint and keys set

How it solves it:

• monkeypatch.setattr restores on failure too
• The six try/finally blocks come out with it

User Flow

No end-user behavior changes. A proxy admin logging spend to S3 gets the same
objects written the same way; this is the test file around that logger, not the
logger

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: clear the global, run one test with one line forced to fail, read
the global back in the same interpreter. A passing run is clean on both sides,
so the forced failure is the only state where they differ.

uv run python -c '
import litellm, pytest
litellm.s3_callback_params = None
pytest.main(["tests/test_litellm/integrations/test_s3_v2.py::test_s3_verify_false_handling", "-q", "-p", "no:randomly"])
print("litellm.s3_callback_params after the run:", litellm.s3_callback_params)
'

Before (ff02d5c)

1. Add raise AssertionError("forced") above the first assert of
   test_s3_verify_false_handling, then run the command above

litellm.s3_callback_params after the run: {'s3_bucket_name': 'test-bucket', 's3_endpoint_url': 'https://localhost:443', 's3_aws_access_key_id': 'minioadmin', 's3_aws_secret_access_key': 'minioadmin', 's3_region_name': 'us-east-1', 's3_verify': False, 's3_use_ssl': False}

2. uv run python scripts/check_test_quality.py tests/test_litellm/integrations/test_s3_v2.py | grep -oE "TQ00[0-9]" | sort | uniq -c

   5 TQ002
  28 TQ005

After (073fe21)

1. Same injection, same command

litellm.s3_callback_params after the run: None

2. uv run python scripts/check_test_quality.py tests/test_litellm/integrations/test_s3_v2.py | grep -oE "TQ00[0-9]" | sort | uniq -c

   5 TQ002

3. uv run pytest tests/test_litellm/integrations/ -q -p no:randomly -n 4

2823 passed, 3 skipped, 153 warnings, 2 subtests passed in 24.62s

Type

🐛 Bug Fix

✅ Test

Caveats (if any)

• The leak only shows on failure, which is when it matters
• Six of the ten already had try/finally and only lose scaffolding
• The five TQ002 mock-echo tests here are untouched and still weak
• Same defect class as test(guardrails): stop the onyx and prompt security tests leaking env vars on failure #37828, different global and directory
• Touches test-quality-budget.json, as do test: drop the cwd-relative sys.path.insert calls from the test suite #37802, test: unwind environment writes in tests/test_litellm with monkeypatch #37806, test(cost-calc): stop 182 global writes leaking out of the cost-calc suites #37815, test(mcp): retire the last file of the dead tests/litellm mirror #37824, test(callbacks): unwind the callbacks global the policy engine and realtime tests scaffold around #37826 and test(guardrails): stop the onyx and prompt security tests leaking env vars on failure #37828

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 Summary

The PR replaces manual mutation and cleanup of litellm.s3_callback_params in S3 logger tests with pytest's automatically restored monkeypatch fixture.

• Removes failure-sensitive cleanup scaffolding from ten S3 tests.
• Lowers the TQ005 test-quality budget to reflect the removed direct global mutations.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
tests/test_litellm/integrations/test_s3_v2.py	Replaces direct global assignments and manual cleanup with automatically restored monkeypatch state without changing test assertions or production behavior.
test-quality-budget.json	Reduces the TQ005 allowance from 2832 to 2804 to account for the eliminated test-quality violations.

_{Reviews (2): Last reviewed commit: "test(s3): stop the logger tests leaking ..." | Re-trigger Greptile}

[greptile-apps]

Changed lines exceed length limit

The changed test declaration exceeds the repository's 120-character Python line limit, and the same pattern appears in several other changed setup and assertion lines, making this test harder to scan and maintain.

Context Used: CLAUDE.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[yuneng-berri]

The longest line this PR adds is 104 characters and the limit here is 120, so I read that one as a false positive

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[yuneng-berri]

@greptileai