Skip to content

[Fix] Update Outdated test_responses_background_cost Assertions - #23589

Merged
yuneng-jiang merged 1 commit into
litellm_internal_dev_03_12_2026from
litellm_fix_responses_background_cost_tests
Mar 13, 2026
Merged

[Fix] Update Outdated test_responses_background_cost Assertions#23589
yuneng-jiang merged 1 commit into
litellm_internal_dev_03_12_2026from
litellm_fix_responses_background_cost_tests

Conversation

@yuneng-jiang

Copy link
Copy Markdown
Contributor

Relevant issues

Related to #23472

Summary

Problem

PR #23472 added pagination (take/order params) to find_many and stale-row cleanup (update_many for stale_expired) in CheckResponsesCost, but did not update 4 pre-existing tests that assert the old call signatures.

Fix

Updated test assertions to account for the new pagination params on find_many and the additional update_many call from stale cleanup. Tests now filter update_many calls to distinguish stale cleanup from completion updates.

Testing

All 4 previously failing tests now pass with the updated assertions.

Type

✅ Test

…ation and stale cleanup

Tests were outdated after #23472 added pagination (take/order) to find_many
and stale-row cleanup via update_many. Updated assertions to match new call
signatures.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vercel

vercel Bot commented Mar 13, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
litellm Building Building Preview, Comment Mar 13, 2026 11:18pm

Request Review

@greptile-apps

greptile-apps Bot commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR updates 4 pre-existing unit tests in TestCheckResponsesCost to align with the new behavior introduced in #23472, which added pagination (take/order) to find_many and a stale-row cleanup path (_cleanup_stale_managed_objects) that unconditionally calls update_many on every poll cycle.

Changes made:

  • Added take=MAX_OBJECTS_PER_POLL_CYCLE and order={"created_at": "asc"} to the find_many assertion in test_check_responses_cost_no_jobs, matching the new call signature.
  • Replaced update_many.assert_called_once() / update_many.assert_not_called() assertions with a filtered list (completion_calls) that isolates completion update_many calls (identified by the presence of "id" in the where dict) from stale-cleanup calls (which use file_purpose/status/created_at keys instead).
  • The filter logic correctly mirrors the actual implementation: stale cleanup uses where={"file_purpose": ..., "status": ..., "created_at": ...}, while completion updates use where={"id": {"in": [...]}}.
  • test_check_responses_cost_with_failed_job now checks len(completion_calls) == 1 but does not assert the specific where/data arguments, unlike the analogous completed-job test — a minor consistency gap.

Confidence Score: 5/5

  • This PR is safe to merge — it contains only test updates with no changes to production code.
  • The changes are test-only, correctly aligned with the updated implementation in fix(proxy): prevent OOM/Prisma connection loss from unbounded managed-object poll #23472, and the filter logic accurately distinguishes between the two distinct update_many call sites. The one minor gap (missing argument assertions in the failed-job test) is a pre-existing pattern and does not risk false passes.
  • No files require special attention.

Important Files Changed

Filename Overview
tests/test_litellm/integrations/test_responses_background_cost.py Updated 4 tests in TestCheckResponsesCost to account for pagination params on find_many and the extra update_many call from _cleanup_stale_managed_objects; filter logic correctly distinguishes stale-cleanup calls (no "id" key in where) from completion calls (has "id" key in where).

Sequence Diagram

sequenceDiagram
    participant Test
    participant CheckResponsesCost
    participant DB as litellm_managedobjecttable
    participant LiteLLM

    Test->>CheckResponsesCost: check_responses_cost()

    Note over CheckResponsesCost,DB: Stale cleanup (always runs)
    CheckResponsesCost->>DB: update_many(where={file_purpose, status, created_at}, data={status: stale_expired})
    DB-->>CheckResponsesCost: count

    Note over CheckResponsesCost,DB: Paginated fetch
    CheckResponsesCost->>DB: find_many(where={status: [queued, in_progress], file_purpose: response}, take=MAX_OBJECTS_PER_POLL_CYCLE, order={created_at: asc})
    DB-->>CheckResponsesCost: jobs[]

    loop For each job
        CheckResponsesCost->>LiteLLM: aget_responses(response_id)
        LiteLLM-->>CheckResponsesCost: response (status: completed/failed/in_progress/...)
    end

    alt Terminal jobs exist (completed/failed/cancelled)
        Note over CheckResponsesCost,DB: Completion update (id-keyed)
        CheckResponsesCost->>DB: update_many(where={id: {in: [job_ids]}}, data={status: completed})
    end
Loading

Last reviewed commit: 3aeca22

Comment on lines 451 to +458
# Verify job was marked as completed even though it failed
mock_prisma_client.db.litellm_managedobjecttable.update_many.assert_called_once()
# (stale cleanup also calls update_many, so check the specific completion call)
update_many_calls = mock_prisma_client.db.litellm_managedobjecttable.update_many.call_args_list
completion_calls = [
c for c in update_many_calls
if c.kwargs.get("where", {}).get("id") is not None
]
assert len(completion_calls) == 1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing argument assertions for failed-job completion call

test_check_responses_cost_with_failed_job now only verifies that exactly one completion update_many call occurred (len(completion_calls) == 1), but does not assert what the call was made with. The parallel test for a completed job (test_check_responses_cost_with_completed_job) was updated to check both where.id.in and data.status. For consistency and to guard against regressions, consider adding the same argument checks here:

Suggested change
# Verify job was marked as completed even though it failed
mock_prisma_client.db.litellm_managedobjecttable.update_many.assert_called_once()
# (stale cleanup also calls update_many, so check the specific completion call)
update_many_calls = mock_prisma_client.db.litellm_managedobjecttable.update_many.call_args_list
completion_calls = [
c for c in update_many_calls
if c.kwargs.get("where", {}).get("id") is not None
]
assert len(completion_calls) == 1
assert len(completion_calls) == 1
assert completion_calls[0].kwargs["where"]["id"]["in"] == ["job-456"]
assert completion_calls[0].kwargs["data"]["status"] == "completed"

@yuneng-jiang
yuneng-jiang merged commit 85e5ef4 into litellm_internal_dev_03_12_2026 Mar 13, 2026
33 of 60 checks passed
@ishaan-berri
ishaan-berri deleted the litellm_fix_responses_background_cost_tests branch March 26, 2026 22:29
fzowl pushed a commit to fzowl/litellm that referenced this pull request Jun 24, 2026
…ackground_cost_tests

[Fix] Update Outdated test_responses_background_cost Assertions
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.

1 participant