Skip to content

test(bedrock): let monkeypatch own bedrock_request_metadata_fields - #37840

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

test(bedrock): let monkeypatch own bedrock_request_metadata_fields#37840
yuneng-berri merged 1 commit into
litellm_internal_stagingfrom
litellm_bedrock_request_metadata_test_globals

Conversation

@yuneng-berri

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

  • Twenty tests assign litellm.bedrock_request_metadata_fields directly
  • An autouse fixture exists only to put that global back
  • What a test sets and what undoes it live in different places

How it solves it:

  • monkeypatch.setattr sets and restores at the point of use
  • The autouse fixture goes away with nothing left to undo
  • 22 TQ005 violations in the file drop to zero

User Flow

This one is test-only scaffolding with no user-visible behavior, so there is no before and after flow to walk. The tests assert the same things on the same code, and the same 61 pass either way

Relevant issues

Linear ticket

Pre-Submission checklist

  • I have added meaningful tests
  • The handful of test files covering my change pass locally, e.g. uv run pytest tests/test_litellm/<your_test_file>.py -v. Leave the suites (make test-unit-*, make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more
  • 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 (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

To be plain about what this does and does not prove: the autouse fixture it removes already restored the global, so the run reads "restored" on both sides. That is the point. The bookkeeping was correct and sitting fifty lines from the code it covered, and monkeypatch does the same job where the value is set. What changes is the count the quality rules see.

Shared setup: run the file, read the global back in the same interpreter, then count what the quality rules see in it.

uv run python -c '
import litellm, pytest
A = "bedrock_request_metadata_fields"
before = getattr(litellm, A, None)
pytest.main(["tests/test_litellm/llms/bedrock/test_request_metadata.py", "-q", "-p", "no:randomly"])
print("litellm." + A + " after the run:", "restored" if getattr(litellm, A, None) == before else "LEAKED")
'
uv run python scripts/check_test_quality.py tests/test_litellm/llms/bedrock/test_request_metadata.py | grep -oE "TQ00[0-9]" | sort | uniq -c

Before (ff02d5c)

  1. The first command
61 passed, 1 warning in 0.23s
litellm.bedrock_request_metadata_fields after the run: restored
  1. The second command
   1 TQ003
  22 TQ005

After (2ac787e)

  1. The first command
61 passed, 1 warning in 0.21s
litellm.bedrock_request_metadata_fields after the run: restored
  1. The second command
   1 TQ003
  1. uv run python scripts/test_quality_gate.py --base origin/litellm_internal_staging
OK: every TQ rule is within its test-suite ceiling (base origin/litellm_internal_staging)

Type

🧹 Refactoring
✅ Test

Caveats (if any)

  • The file's one TQ003 is a sys.path.insert, left for its own change

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

Twenty tests in test_request_metadata.py assigned the global directly and
leaned on an autouse fixture to put it back afterwards. monkeypatch.setattr
does both jobs at the point of use, so each test now says what it sets and the
fixture that existed only to undo them goes away.
@greptile-apps

greptile-apps Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This test-only refactor moves cleanup of litellm.bedrock_request_metadata_fields from a distant autouse fixture to pytest's monkeypatch fixture at each mutation site.

  • Replaces direct global assignments with monkeypatch.setattr while preserving test parameters and assertions.
  • Removes the now-unnecessary autouse cleanup fixture.
  • Ratchets the TQ005 quality budget down by the 22 eliminated violations.

Confidence Score: 5/5

The PR appears safe to merge because it preserves test behavior while localizing and automating global-state cleanup.

Every changed mutation is restored by pytest's monkeypatch lifecycle, assertions and parameterization remain unchanged, and the quality-budget reduction matches the eliminated violations.

Important Files Changed

Filename Overview
tests/test_litellm/llms/bedrock/test_request_metadata.py All global-setting mutations are now monkeypatch-owned and automatically restored, with test behavior and coverage unchanged.
test-quality-budget.json Reduces the TQ005 ceiling by 22, matching the 22 direct global assignments eliminated from the test file.

Reviews (1): Last reviewed commit: "test(bedrock): let monkeypatch own bedro..." | 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 4a008b6 into litellm_internal_staging Aug 22, 2026
67 checks passed
@yuneng-berri
yuneng-berri deleted the litellm_bedrock_request_metadata_test_globals branch August 22, 2026 03:16
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