[NA] [SDK] test: add streaming coverage for Groq integration#7387
Open
petrotiurin wants to merge 9 commits into
Open
[NA] [SDK] test: add streaming coverage for Groq integration#7387petrotiurin wants to merge 9 commits into
petrotiurin wants to merge 9 commits into
Conversation
…ad constant Adds sync/async tests exercising track_groq's stream=True path (chunk aggregation, x_groq usage parsing) which had no coverage after #7341 merged, and removes RESPONSE_KEYS_TO_LOG_AS_OUTPUT, an unused constant in groq_chat_completions_decorator.py. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
⏱️ pre-commit per-hook timing
⏭️ 35 skipped (no matching files changed)
|
usage=ANY_DICT didn't verify the x_groq.usage fallback in chat_completion_chunks_aggregator actually propagates the real token counts. Use ANY_DICT.containing() rather than an exact dict match, since try_build_opik_usage_or_log_error duplicates fields under original_usage.* — verified empirically that an exact-dict assertion would have failed against real output.
…private _post Replaces monkeypatching client.chat.completions._post (private) and manually constructing groq.Stream/AsyncStream with the SDK's documented http_client override plus httpx.MockTransport. The mock now only fakes the wire response; _post, header/request construction, and Stream/AsyncStream construction all run for real.
Adds sync/async tests that hit the actual Groq API (gated behind the existing ensure_groq_configured fixture, previously unused) to validate the streaming path against real response shapes rather than only our hand-built SSE fixtures — in particular whether x_groq.usage is really present on the terminal chunk as assumed. Mirrors the existing openai/anthropic real-API test pattern in this suite; local runs without GROQ_API_KEY fail loudly via the fixture, same as those providers. CI already injects the secret in lib-groq-tests.yml.
This reverts commit c2eaacf.
Contributor
Python SDK E2E Tests Results (Python 3.13)285 tests 282 ✅ 5m 23s ⏱️ Results for commit 89d9f3f. ♻️ This comment has been updated with latest results. |
Contributor
This reverts commit 8098856.
…into petrot/NA-groq-streaming-tests
The Groq credentials/billing issue blocking this test in CI since April is now resolved.
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Details
Follow-up from reviewing #7341 (Groq SDK integration): the merged PR shipped with no test coverage for the
stream=Truepath, left an unusedRESPONSE_KEYS_TO_LOG_AS_OUTPUTconstant ingroq_chat_completions_decorator.py, and (per review feedback) its streaming tests should exercise the SDK's publichttp_clienttesting hook rather than monkeypatching a private method. This PR:stream=Truepath (chunk aggregation, including the Groq-specificx_groq.usagefallback), built viahttpx.MockTransportplugged intohttp_client=— not by monkeypatchingclient.chat.completions._postor constructinggroq.Stream/AsyncStreamdirectly.ensure_groq_configuredfixture) to validate the streaming path against real response shapes rather than only our hand-built SSE fixtures.RESPONSE_KEYS_TO_LOG_AS_OUTPUTconstant.test_langchain__openai_llm_is_used__token_usage_is_logged__happy_flowintest_langchain_groq.py, skipped since 2026-04-14 due to a Groq credentials/billing issue in CI that is now resolved (confirmed via the e2e tests above passing against the live API).Change checklist
Issues
AI-WATERMARK
AI-WATERMARK: yes
ghbefore requesting merge; diff review pending before merging out of draft.Testing
pytest tests/library_integration/groq/ -vv— 6 offline tests pass (4 pre-existing + 2 new streaming tests); the 2 e2e tests fail loudly viaensure_groq_configuredwhen run locally withoutGROQ_API_KEY(expected, matches theopenai/anthropicconvention in this suite).pre-commiton all changed files — ruff, ruff-format, mypy all pass.SDK Library Integration Tests Runner(libs=groq) against this branch in CI (which has the real secret): all 5 Python versions (3.10–3.14) pass, including both e2e tests against the live Groq API — confirms thex_groq.usagefallback and response-shape assumptions hold for real.libs=langchain) to validate the un-skipped langchain test against the real API.Not covered here: the stream-error/exception path, and the cross-client concurrency issue tracked separately in OPIK_7261 — a single-stream test can't exercise that race.
No video evidence included (non-UI, internal contribution, low complexity).
Documentation
No documentation changes required.