Skip to content

test: enforce PT011 and PT014 so a broad pytest.raises cannot pass on the wrong error - #37769

Merged
ryan-crabbe-berri merged 1 commit into
litellm_internal_stagingfrom
litellm_ruff_raises_match_required
Aug 21, 2026
Merged

test: enforce PT011 and PT014 so a broad pytest.raises cannot pass on the wrong error#37769
ryan-crabbe-berri merged 1 commit into
litellm_internal_stagingfrom
litellm_ruff_raises_match_required

Conversation

@ryan-crabbe-berri

@ryan-crabbe-berri ryan-crabbe-berri commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

  • pytest.raises(Exception) with no match= passes on any error
  • An unrelated crash reads as the rejection under test
  • 317 such sites, plus 4 parametrize cases listed twice

How it solves it:

  • Selects ruff PT011 and PT014 in ruff-tests.toml, already wired to CI
  • Pins each site to the message the code actually raised
  • Patterns come from running the sites and recording the real error
  • Narrows 6 sites to a concrete type instead

User Flow

Before: a proxy admin gives a key two models, and the 403 that names those models can quietly stop naming them

  1. They POST https://litellm-domain/key/generate with "models": ["gpt-5.5"] and get back a key
  2. Someone using that key sends POST https://litellm-domain/v1/chat/completions for claude-sonnet-4-5
  3. The gateway answers 403 saying the key is not allowed to access that model, and lists the models the key can access, so they know what to ask for
  4. A later change reworks that path and the same request now answers 403 with only Access denied for model claude-sonnet-4-5
  5. CI stays green: the test guarding that 403 accepts any error at all, so the caller loses the list and no one finds out until a customer asks why the error stopped telling them anything

After: that change turns CI red before it ships

  1. The same rework is made
  2. CI runs the same test file and it fails, printing the wording it expected and the message it got
  3. The same holds when a refactor makes a helper crash before it ever reaches its own validation: the crash no longer passes for the rejection it replaced
  4. A new test that catches a bare Exception without pinning the message is rejected at lint time, so the next one cannot go quiet either

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

This PR changes lint config and tests, so there is no proxy route to curl. The proof is three mutations: break a behavior a test exists to police, and show the old test does not notice while the new one does. Every command is identical on both sides; only the tests/ tree differs.

The three mutations, applied one at a time and reverted after each run:

filename_message_reworded            litellm/proxy/common_utils/path_utils.py
                                    a rejected upload filename stops saying it was empty
                                    or unsafe and just says "Invalid filename"

prefix_helper_crashes_first         litellm/proxy/_experimental/mcp_server/utils.py
                                    a refactor makes the short-prefix helper crash before
                                    it reaches its own empty-server_id check

model_denial_stops_naming_the_key   litellm/proxy/auth/auth_checks.py
                                    the 403 for a model a key cannot use stops listing the
                                    models it can use

Before (354f497)

The rejected filename stops saying why

  1. Apply filename_message_reworded, then run
pytest tests/test_litellm/proxy/common_utils/test_path_utils.py::TestSafeFilename::test_empty_rejected -q
  1. Output:
1 passed, 2 warnings in 1.54s

A helper crashes before reaching its own check

  1. Apply prefix_helper_crashes_first, then run
pytest tests/test_litellm/proxy/_experimental/mcp_server/test_short_mcp_tool_prefix.py::TestShortPrefixHelpers::test_short_prefix_requires_server_id -q
  1. Output:
1 passed, 2 warnings in 1.94s

The 403 stops listing the models the key can use

  1. Apply model_denial_stops_naming_the_key, then run
pytest tests/proxy_unit_tests/test_auth_checks.py::test_can_key_call_model -q
  1. Output:
2 passed, 1 warning in 0.07s

After (b76def0)

The rejected filename stops saying why

  1. Apply filename_message_reworded, then run
pytest tests/test_litellm/proxy/common_utils/test_path_utils.py::TestSafeFilename::test_empty_rejected -q
  1. Output:
1 failed, 2 warnings in 1.75s

A helper crashes before reaching its own check

  1. Apply prefix_helper_crashes_first, then run
pytest tests/test_litellm/proxy/_experimental/mcp_server/test_short_mcp_tool_prefix.py::TestShortPrefixHelpers::test_short_prefix_requires_server_id -q
  1. Output:
1 failed, 2 warnings in 1.76s

The 403 stops listing the models the key can use

  1. Apply model_denial_stops_naming_the_key, then run
pytest tests/proxy_unit_tests/test_auth_checks.py::test_can_key_call_model -q
  1. Output:
1 failed, 1 passed, 1 warning in 0.15s

Rule and regression numbers

ruff check --config ruff-tests.toml tests went from 317 PT011 and 4 PT014 findings to zero. Ruff has no autofix for either, so 291 sites took a pattern recorded by running them under a plugin that logged the concrete exception and message per call site, and the remaining 26 were resolved by hand: 18 took the message the test itself plants, 6 narrowed to a real type, and 2 kept a noqa because their exception carries an empty str().

The 144 touched test files were run in full at both hashes. 196 tests fail identically on each side, all of them live-provider tests with no Fireworks, Azure, Gemini or Bedrock credentials on this machine. Nothing is new and nothing disappeared.

Type

🧹 Refactoring
🚄 Infrastructure
✅ Test

Caveats (if any)

  • 6 sites narrowed to a real type instead of a match=
  • 2 sites keep a noqa: their exception carries an empty str()

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

…arametrize cases

`pytest.raises(Exception)` with no `match=` passes on any error that broad. A
TypeError from a refactor, a botched fixture, an import that moved: all of them
read as the rejection the test claims to police, so the test goes green for the
wrong reason and stays green after the behaviour it guards is gone.

PT011 closes that gap for the 317 sites B017 could not reach, because B017 only
fires on a single-statement body with no `as e` binding. Each pattern here is the
message the code actually raised, recorded by running the sites under a plugin
that logged the concrete type and text per call site, so the assertions describe
observed behaviour rather than a guess. Where a site raises more than one message
across its parametrize cases, the pattern is an alternation of what was seen;
where the exception carries an empty `str()` and puts the text on `.message`, the
site keeps a narrow `noqa` with the reason.

PT014 removes four parametrize cases that were listed twice. The duplicate re-runs
an assertion that already passed, and it usually marks a case someone meant to
vary and forgot to edit.
@greptile-apps

greptile-apps Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Too many files changed for review (145 files, 100 file limit).

Bypass the limit by tagging @greptile-apps to review.

@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!

@ryan-crabbe-berri
ryan-crabbe-berri merged commit b76def0 into litellm_internal_staging Aug 21, 2026
67 checks passed
@ryan-crabbe-berri
ryan-crabbe-berri deleted the litellm_ruff_raises_match_required branch August 21, 2026 03:24
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