Skip to content

test(lint): clear the two PT011/PT012 violations left on the test tree - #37864

Merged
tin-berri merged 1 commit into
litellm_internal_stagingfrom
litellm_fix_test_tree_pt_lints
Aug 21, 2026
Merged

test(lint): clear the two PT011/PT012 violations left on the test tree#37864
tin-berri merged 1 commit into
litellm_internal_stagingfrom
litellm_fix_test_tree_pt_lints

Conversation

@tin-berri

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

  • The test-tree ruff gate is red on staging itself
  • Every PR rebased onto staging inherits the failing lint check

How it solves it:

  • Sets a match on the broad pytest.raises in test_user_api_key_auth
  • Hoists the branch out of the raises block in the budget limiter test

User Flow

Before: a contributor rebases any branch onto litellm_internal_staging and the lint check fails on files they never touched

  1. They push a rebased branch and open the PR checks page
  2. The lint job fails with PT011 at tests/proxy_unit_tests/test_user_api_key_auth.py:1360 and PT012 at tests/proxy_unit_tests/test_unit_test_max_model_budget_limiter.py:1213
  3. Both files are byte-identical to the base branch, so nothing in their diff can fix it

After: the same rebase gets a green lint check

  1. They push a rebased branch and open the PR checks page
  2. uv run ruff check --config ruff-tests.toml tests reports All checks passed
  3. Both edited tests still pass (3 and 11 relevant cases respectively)

Relevant issues

The rules landed in #37731; these two violations were left behind on the tree it gated

Linear ticket

Pre-Submission checklist

  • 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

Screenshots / Proof of Fix

Before (merge base 4307b34)

$ uv run ruff check --config ruff-tests.toml tests
PT012 pytest.raises() block should contain a single simple statement  (test_unit_test_max_model_budget_limiter.py:1213)
PT011 pytest.raises(Exception) is too broad  (test_user_api_key_auth.py:1360)
Found 2 errors.

After (PR tip 349d55c)

$ uv run ruff check --config ruff-tests.toml tests
All checks passed!
$ pytest tests/proxy_unit_tests/test_unit_test_max_model_budget_limiter.py -k "prefix or entity"  # 3 passed
$ pytest tests/proxy_unit_tests/test_user_api_key_auth.py -k "model_max_budget or user_model"     # 11 passed

The refusal test keeps asserting on the budget message via the match parameter and still checks the user id lands in the error

@tin-berri

Copy link
Copy Markdown
Contributor Author

@greptileai can you reply here with a score and why that score?

@greptile-apps

greptile-apps Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Score: 5/5

This is a clean, minimal, correct fix. Here's why:

PT011 fix (test_user_api_key_auth.py): Moving the budget assertion into pytest.raises(Exception, match=r"(?i)budget") is the correct resolution — it narrows the broad Exception catch and preserves the semantic check. The user_id in str(exc.value) assertion remains unchanged, so coverage is equivalent.

PT012 fix (test_unit_test_max_model_budget_limiter.py): The PT012 rule requires a single simple statement inside raises(). The fix correctly hoists the if/else branch out of the block and assigns the coroutine to budget_check, then awaits it as the single statement inside with pytest.raises(...). This is the idiomatic resolution for this pattern — it keeps the branch logic intact and the BudgetExceededError assertion still fires on the awaited call.

Both changes are behavior-preserving (as confirmed by the passing test counts in the PR description), minimal in scope, and directly address the stated lint violations with no unrelated modifications.

@greptile-apps

greptile-apps Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR resolves the remaining PT011/PT012 test-tree lint violations without changing the tested behavior.

  • Selects the appropriate budget-check coroutine before entering the single-statement pytest.raises block.
  • Moves the case-insensitive budget-message assertion into pytest.raises(match=...).

Confidence Score: 5/5

The PR appears safe to merge because both lint-only test refactors preserve their prior assertions and execution behavior.

The selected budget methods remain awaited exactly once inside the exception context, and the new regex performs the same case-insensitive budget substring check as the removed assertion.

Important Files Changed

Filename Overview
tests/proxy_unit_tests/test_unit_test_max_model_budget_limiter.py Refactors the parametrized budget-limit test so the raises block contains one awaited statement while preserving branch selection, arguments, and exception assertions.
tests/proxy_unit_tests/test_user_api_key_auth.py Expresses the existing case-insensitive budget-message check through pytest's match parameter while retaining the user-ID assertion.

Reviews (1): Last reviewed commit: "test(lint): clear the two PT011/PT012 vi..." | 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!

@tin-berri
tin-berri merged commit ae1eea1 into litellm_internal_staging Aug 21, 2026
67 checks passed
@tin-berri
tin-berri deleted the litellm_fix_test_tree_pt_lints branch August 21, 2026 18:15
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