test: give mocked chat completion clients a real payload so the no-choices guard does not trip - #40517
test: give mocked chat completion clients a real payload so the no-choices guard does not trip#40517mateo-berri wants to merge 1 commit into
Conversation
…oices guard does not trip The four tests only assert on the kwargs the SDK client's create was called with, but since #40294 a bare MagicMock response raises the new no-choices APIError before those assertions run. Each mock now returns a minimal chat completion with one assistant message.
Greptile SummaryThis PR gives four mocked chat completion tests structurally valid parsed responses so the no-choices guard no longer prevents their request assertions from running
Confidence Score: 5/5The test-only changes appear safe to merge and preserve the existing request behavior checks The new payloads satisfy the production response guard without changing assertions, production code, or provider request behavior
|
| Filename | Overview |
|---|---|
| tests/llm_translation/conftest.py | Adds a valid, reusable OpenAI ChatCompletion fixture for mocked translation tests |
| tests/llm_translation/test_azure_o_series.py | Configures the mocked Azure o-series client to return a parsed completion before checking max_retries |
| tests/llm_translation/test_azure_openai.py | Gives the Azure raw-response mock a valid parse result while preserving response_format assertions |
| tests/llm_translation/test_openai.py | Configures the mocked OpenAI client to return a parsed completion before checking max_retries |
| tests/local_testing/test_completion.py | Reuses the existing realistic raw response helper in the Novita request argument test |
Reviews (1): Last reviewed commit: "test: give mocked chat completion client..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 74a9b42. Configure here.
|
Superseded by #40638, which landed on litellm_internal_staging and repairs the same four mocked completion fixtures, so closing this one |
Pull request was closed
TLDR
Problem this solves:
MagicMockresponses now trip the new no-choicesAPIErrorlocal_testing_part1andllm_translation_testingare red on every PR carrying that merge baseHow it solves it:
tests/llm_translationtests share onechat_completion_responseconftest fixturetest_completion.py's existing_openai_mock_responsecreatecall kwargs it was written forUser Flow
Before: a contributor opens a PR against
litellm_internal_stagingand two CircleCI jobs come back red on tests their change never touchedlitellm_internal_stagingci/circleci: local_testing_part1andci/circleci: llm_translation_testingshow as failedtest_completion_novita_ai,test_openai_max_retries_0,test_azure_gpt_4o_with_tool_call_and_response_format[2024-10-21], andtest_openai_o_series_max_retries_0failing withlitellm.APIError: LiteLLM: provider returned a response with no 'choices'. Raw keys: []After: the same PR gets green
local_testing_part1andllm_translation_testingjobs unless the contributor's own change breaks somethinglitellm_internal_stagingci/circleci: local_testing_part1andci/circleci: llm_translation_testingshow as passedRelevant issues
Follow-up to #40294
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
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@greptileaito re-request a review after pushing changes)Screenshots / Proof of Fix
This PR changes only test code, so there is no proxy or end-user behavior to drive: the observable behavior is the four tests themselves and the two CircleCI jobs that run them. Both legs run the same four node ids from a fresh worktree with
AZURE_API_KEYandAZURE_API_BASEset the way CircleCI injects them (the o-series test builds an Azure client before it reaches the mock, and it needs those two variables to get that far)Shared setup:
Before (2662634, the merge base on litellm_internal_staging)
After (74a9b42, PR tip)
Type
✅ Test
Caveats (if any)
Final Attestation
Note
Low Risk
Test-only changes to mock return values; no runtime or production code paths are modified.
Overview
Fixes four CI failures where mocked OpenAI/Azure completion tests started erroring with
provider returned a response with no 'choices'after the stricter response validation from #40294.Adds a shared
chat_completion_responsepytest fixture intests/llm_translation/conftest.py(minimalChatCompletionwith one assistant choice). Threellm_translationtests wire it into mockedwith_raw_response.create→.parsereturn values so completion parsing succeeds while assertions onmax_retries, tool/response_format kwargs, etc. stay the same.In
test_completion_novita_ai, the bareMagicMockoncreateis replaced withMagicMock(return_value=_openai_mock_response()), reusing the file’s existing helper.Test-only — no production/SDK behavior changes.
Reviewed by Cursor Bugbot for commit 74a9b42. Bugbot is set up for automated code reviews on this repo. Configure here.