Skip to content

Add dependency injection to ChatCompletionStream for improved testability #1016

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rjcorwin
Copy link

Describe the change
This PR refactors the ChatCompletionStream to use dependency injection by introducing a ChatStreamReader interface. This allows for injecting custom stream readers, primarily for testing purposes, making the streaming functionality more testable and maintainable.

Describe your solution
The changes include:

  • Added a ChatStreamReader interface that defines the contract for reading chat completion streams
  • Refactored ChatCompletionStream to use composition with a ChatStreamReader instead of embedding streamReader
  • Added NewChatCompletionStream() constructor function to enable dependency injection
  • Implemented explicit delegation methods (Recv(), Close(), Header(), GetRateLimitHeaders()) on ChatCompletionStream
  • Added interface compliance check via var _ ChatStreamReader = (*streamReader[ChatCompletionStreamResponse])(nil)

This approach maintains backward compatibility while enabling easier mocking and testing of streaming functionality.

Tests
Added comprehensive tests demonstrating the new functionality:

  • TestChatCompletionStream_MockInjection: Tests basic mock injection with the new constructor
  • mock_streaming_demo_test.go: A complete demonstration file showing how to create mock clients and stream readers for testing, including:
    • MockOpenAIStreamClient: Full mock client implementation
    • mockStreamReader: Custom stream reader for controlled test responses
    • TestMockOpenAIStreamClient_Demo: Demonstrates assembling multiple stream chunks
    • TestMockOpenAIStreamClient_ErrorHandling: Shows error handling patterns

Additional context
This refactoring improves the testability of code that depends on go-openai streaming without introducing breaking changes. The existing public API remains unchanged, but now supports dependency injection for testing scenarios. The new demo test file serves as documentation for users who want to mock streaming responses in their own tests.

…lity

**Describe the change**
This PR refactors the `ChatCompletionStream` to use dependency injection by introducing a `ChatStreamReader` interface. This allows for injecting custom stream readers, primarily for testing purposes, making the streaming functionality more testable and maintainable.

**Provide OpenAI documentation link**
https://platform.openai.com/docs/api-reference/chat/create

**Describe your solution**
The changes include:
- Added a `ChatStreamReader` interface that defines the contract for reading chat completion streams
- Refactored `ChatCompletionStream` to use composition with a `ChatStreamReader` instead of embedding `streamReader`
- Added `NewChatCompletionStream()` constructor function to enable dependency injection
- Implemented explicit delegation methods (`Recv()`, `Close()`, `Header()`, `GetRateLimitHeaders()`) on `ChatCompletionStream`
- Added interface compliance check via `var _ ChatStreamReader = (*streamReader[ChatCompletionStreamResponse])(nil)`

This approach maintains backward compatibility while enabling easier mocking and testing of streaming functionality.

**Tests**
Added comprehensive tests demonstrating the new functionality:
- `TestChatCompletionStream_MockInjection`: Tests basic mock injection with the new constructor
- `mock_streaming_demo_test.go`: A complete demonstration file showing how to create mock clients and stream readers for testing, including:
  - `MockOpenAIStreamClient`: Full mock client implementation
  - `mockStreamReader`: Custom stream reader for controlled test responses
  - `TestMockOpenAIStreamClient_Demo`: Demonstrates assembling multiple stream chunks
  - `TestMockOpenAIStreamClient_ErrorHandling`: Shows error handling patterns

**Additional context**
This refactoring improves the testability of code that depends on go-openai streaming without introducing breaking changes. The existing public API remains unchanged, but now supports dependency injection for testing scenarios. The new demo test file serves as documentation for users who want to mock streaming responses in their own tests.

Lint fix
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the ChatCompletionStream to use a ChatStreamReader interface for dependency injection, improving testability and maintainability without breaking existing APIs.

  • Introduced ChatStreamReader interface and compliance check
  • Updated ChatCompletionStream to use composition and added NewChatCompletionStream
  • Added delegation methods (Recv, Close, Header, GetRateLimitHeaders) and demo/mock tests

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
stream_reader.go Added interface compliance assertion for streamReader
chat_stream.go Defined ChatStreamReader, refactored stream composition, added constructor and delegation methods
mock_streaming_demo_test.go Added a full demo of mock client and controlled stream reader
chat_stream_test.go Added test verifying injection via NewChatCompletionStream
Comments suppressed due to low confidence (4)

chat_stream.go:119

  • Consider using the NewChatCompletionStream constructor here for consistency and to centralize any future initialization logic.
stream = &ChatCompletionStream{

chat_stream.go:125

  • Add doc comments for the Recv, Close, Header, and GetRateLimitHeaders methods to explain their behavior and any guarantees.
func (s *ChatCompletionStream) Recv() (ChatCompletionStreamResponse, error) {

chat_stream.go:133

  • There are no tests exercising the new Header and GetRateLimitHeaders methods; consider adding unit tests to validate their outputs when the underlying reader implements these extensions.
func (s *ChatCompletionStream) Header() http.Header {

mock_streaming_demo_test.go:9

  • [nitpick] Verify that the import path matches your module declaration; using the module's canonical path (e.g., the project’s own path) helps avoid import errors in tests.
"github.com/sashabaranov/go-openai"

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