Add test coverage for OpenAI completion provider#72
Add test coverage for OpenAI completion provider#72ps06756 wants to merge 2 commits intoFuristo:mainfrom
Conversation
|
Let me work on the failing checks and raise a new rev |
|
@Furisto Can you review this ? |
| } | ||
| } | ||
|
|
||
| func TestNewOpenAICompletionProviderWithService(t *testing.T) { |
There was a problem hiding this comment.
Can we remove this test? It does not provide much value.
| } | ||
| } | ||
|
|
||
| func TestOpenAICompletionProvider_InvokeModel_ModelProfileValidation(t *testing.T) { |
There was a problem hiding this comment.
Would prefer to write a separate test for the Validate function of the model profile and not intermingle it with the InvokeModel test.
| // Edge Cases Tests | ||
| // ============================================================================= | ||
|
|
||
| func TestOpenAICompletionProvider_TransformMessages_EdgeCases(t *testing.T) { |
There was a problem hiding this comment.
This should be part of the TransformMessages test instead of a separate test for edge cases.
| } | ||
| } | ||
|
|
||
| func TestOpenAICompletionProvider_TransformTools_ManyTools(t *testing.T) { |
There was a problem hiding this comment.
Let's drop this test. We already have tests that cover transformation of tools and testing that n+1 works when we already test n does not contribute much value.
| var wg sync.WaitGroup | ||
| errorChan := make(chan error, 100) | ||
|
|
||
| for i := 0; i < 100; i++ { |
There was a problem hiding this comment.
Validation of inputs is stateless. What scenario would this cover?
| return m.stream | ||
| } | ||
|
|
||
| func (m *mockChatCompletionService) getCallCount() int { |
| } | ||
| } | ||
|
|
||
| func TestOpenAICompletionProvider_ConcurrentTransformMessages(t *testing.T) { |
There was a problem hiding this comment.
Why do we need to test concurrent transformation of messages?
| } | ||
| } | ||
|
|
||
| // ============================================================================= |
There was a problem hiding this comment.
Nit: We do not need the comment. The test name is enough.
2 participants
Summary
Fixes #68
OpenAIChatCompletionServiceinterface to enable mocking in testsNewOpenAICompletionProviderWithService()constructor for test injectionTest Coverage
Test Plan
go test ./backend/model/...)go build ./backend/... ./api/go/... ./frontend/cli/... ./shared/...)