-
Notifications
You must be signed in to change notification settings - Fork 3
Description
Currently AIBridge tests suffer from large amount of copy-paste and not well fitting helper functions that needs to be extended for new tests.
This leads to tests being less and less readable / understandable / harder to write. Such complexity slows down development and can even lead to bugs (#72)
For example most tests follows the same setup steps, setting up mock server based on fixture files + streaming option, executes request from fixture then verify results. Server setup and performing request could be generalized.
Access to data that needs verification could be better structured. Instead of having multiple recorders/accumulators one place providing this data would be much simpler to use.
Examples of tech debt / code becoming too complicated by gluing more stuff:
callAccumulatoradded in fix: openai blocking requests do not call injected tools correctly #72proxiesreturn value added in https://github.com/coder/aibridge/pull/63/files#diff-1e57ed58f04a78014fda5dbda9beb8f9890538161e6d494550bbeffad89c03fcR349-R361
Now that more tests were implemented there are clear patterns that could be simplified.
Some examples (more thorough investigation is needed):
- create dedicated structs based on fixture for:
- upstream server (provider)
- could provide request creation instead of
createOpenAIChatCompletionsReq/createAnthropicMessagesReq
- could provide request creation instead of
- MCP server
- AIB server?
- upstream server (provider)
- clean up too many return values from helper functions
- move
callAccumulatorinto mock MCP server struct - move
setupMCPServerProxiesForTestand return values into MCP server struct - More to be found
- move
- Create testing package containing all testing helpers, maybe it could also be used in https://github.com/coder/coder/blob/pb/aibridge-otel-trace/enterprise/aibridged/aibridged_integration_test.go