-
Notifications
You must be signed in to change notification settings - Fork 270
Feat: Mock LLM servers for test #899
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
Conversation
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
There was a problem hiding this 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 introduces a comprehensive mock LLM server framework for testing AI agent interactions in end-to-end tests. The mock server simulates OpenAI and Anthropic LLM APIs, enabling deterministic testing without external API dependencies.
- Implements a configurable mock server supporting both OpenAI and Anthropic APIs with streaming capabilities
- Provides type-safe configuration using official SDK types for request/response mocking
- Updates existing E2E tests to use the mock server instead of real external LLM services
Reviewed Changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 6 comments.
Show a summary per file
File | Description |
---|---|
go/test/mockllm/types.go | Defines core types and configuration structures for mock LLM server |
go/test/mockllm/server.go | Implements HTTP server with health checks and routing for mock endpoints |
go/test/mockllm/openai.go | OpenAI provider implementation with request matching and response handling |
go/test/mockllm/anthropic.go | Anthropic provider implementation with header validation and streaming support |
go/test/mockllm/server_test.go | Comprehensive test suite for both OpenAI and Anthropic mock functionality |
go/test/e2e/invoke_api_test.go | Updated E2E tests to use mock server with dynamic configuration |
go/test/mockllm/DESIGN.md | Technical design documentation for the mock server architecture |
go/go.mod | Added dependencies for OpenAI and Anthropic SDK integration |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
go/test/e2e/invoke_api_test.go
Outdated
splitted := strings.Split(baseURL, ":") | ||
port := splitted[len(splitted)-1] | ||
|
||
localHost := "172.17.0.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you want to do if linux/mac here? i think this only works on linux
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^ +1. we probably should
i tried 127.0.0.1
on mac and hit an issue where i got a timeout in the test. host.docker.internal
work, since this url used so the agent within the cluster communicates with the locally-running mock llm.
}, | ||
{ | ||
"name": "k8s_get_resources_response", | ||
"match": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this a type of assertion? or scenario matching logic? maybe explain in readme?
go/test/mockllm/DESIGN.md
Outdated
### Configuration | ||
Two ways to configure scenarios: | ||
1) In-code builder (for brevity in tests): | ||
- Fluent API: `NewScenario("name").Expect(OpenAI()).WithModel("gpt-4o").WithMessages(...).Then().RespondWithText("hello").AsStream(...).Build()` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
??
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
modelCfg := generateModelCfg(baseURL + "/v1") | ||
err = cli.Create(t.Context(), modelCfg) | ||
require.NoError(t, err) | ||
agent := generateAgent() | ||
err = cli.Create(t.Context(), agent) | ||
require.NoError(t, err) | ||
|
||
defer func() { | ||
cli.Delete(t.Context(), modelCfg) //nolint:errcheck | ||
cli.Delete(t.Context(), agent) //nolint:errcheck | ||
}() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: having the defer
down here would mean if the agent failed to be created, we'll fail and end the test before deleting the model config.
super nit: I believe go tests are safer using t.Cleanup()
for post-test cleanups, at least true for parallel testing. We're not doing parallel tests here so nbd, but may be worth using it to err on the safer side of things.
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
No description provided.