mirror of
https://github.com/BerriAI/litellm.git
synced 2026-11-14 09:03:16 +00:00
test(e2e): assert tool discovery survives a budget-only key update
The key-grant test asserted the stored object_permission row but never that the key could still resolve the server's tools, which is the symptom the customer actually reported: a key that authenticated fine and listed nothing. Also adds the test-first working rule to CLAUDE.md.
This commit is contained in:
parent
7b721c852a
commit
eea99cd05c
2 changed files with 16 additions and 1 deletions
|
|
@ -23,6 +23,12 @@ When adding new features, add meaningful tests. Don't add tests that don't check
|
|||
|
||||
Same thing for bug fixes. The tests should make it so that this specific bug can never happen again without failing tests (i.e., regression)
|
||||
|
||||
Work test-first (TDD). Write the failing test before the fix or the feature, run it, and confirm it fails for the reason you expect. A test you have never seen fail is not evidence of anything: it may be asserting something that was already true, or asserting nothing at all. Only once it is red do you write the code that turns it green
|
||||
|
||||
Red means red for the right reason. An import error, a typo in a fixture, a 401, or a missing env var is not the failure you are looking for. Read the actual assertion message and check that it names the behavior under test
|
||||
|
||||
When the bug is already fixed on your branch and you cannot get a genuine red, prove the test's power another way and say in the PR which one you used: run it against a build from before the fix, or revert the fix locally, or mutate the product code the test claims to protect. If none of those makes it fail, the test does not cover the bug and should not be counted as covering it
|
||||
|
||||
`tests/test_litellm/` mirrors `litellm/` in a parallel path (see `tests/test_litellm/readme.md`). Name tests `test_<filename>.py`, but always match the existing test file in the directory you touch — many provider dirs use longer descriptive names (e.g. `test_anthropic_chat_transformation.py`) to avoid ambiguity across sibling folders. For bug fixes, extend the existing mapped test file rather than creating a new one. Only create a new test file for a new feature (provider, endpoint, or transformation module) that has no mapped test yet, following that directory's naming convention (or `test_<filename>.py` if you're the first test there). One focused regression test beats many shallow ones
|
||||
|
||||
End-to-end tests belong in `tests/e2e/` and must follow the harness conventions documented in that directory's `CLAUDE.md`
|
||||
|
|
|
|||
|
|
@ -15,7 +15,7 @@ from __future__ import annotations
|
|||
|
||||
import pytest
|
||||
|
||||
from datadog_mcp import register_datadog_mcp
|
||||
from datadog_mcp import SEARCH_LOGS_TOOL, register_datadog_mcp
|
||||
from e2e_config import unique_marker
|
||||
from e2e_http import NoBody, unwrap
|
||||
from lifecycle import ResourceManager
|
||||
|
|
@ -59,6 +59,7 @@ class TestKeyUpdatePreservesMcpGrants:
|
|||
client.await_registered(server_id)
|
||||
key = _mcp_key(client, resources, [server_id])
|
||||
assert _key_mcp_servers(client, key) == [server_id]
|
||||
tool_before = client.await_tool(key, server_id, SEARCH_LOGS_TOOL)
|
||||
|
||||
unwrap(
|
||||
client.proxy.transport.post(
|
||||
|
|
@ -84,6 +85,14 @@ class TestKeyUpdatePreservesMcpGrants:
|
|||
f"(PR #34452 regression): {info.object_permission}"
|
||||
)
|
||||
|
||||
# The stored row surviving is not the thing the customer noticed; they noticed a key
|
||||
# that still authenticated but listed no tools. Assert the grant still resolves.
|
||||
tools_after = unwrap(client.list_tools(key)).tool_names_for_server(server_id)
|
||||
assert tool_before in tools_after, (
|
||||
f"the key stopped seeing {tool_before!r} after a budget-only update; the stored "
|
||||
f"grant survived but tool discovery did not: {tools_after}"
|
||||
)
|
||||
|
||||
@pytest.mark.covers("mcp.key_update.api_key.stale_server_ids_dropped")
|
||||
def test_update_with_deleted_server_ids_succeeds_and_drops_them(
|
||||
self, client: McpClient, resources: ResourceManager
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue