-
Notifications
You must be signed in to change notification settings - Fork 176
test(mcp): add tests for WatchKubeConfig tools reload #449
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
c912484 to
e2feff5
Compare
Verifies that tools are added and **removed** when kubeconfig file changes. Signed-off-by: Marc Nuri <[email protected]>
e2feff5 to
8b14937
Compare
|
|
||
| s.Run("OpenShift tool is removed after kubeconfig change", func() { | ||
| // Reload Config without OpenShift | ||
| s.mockServer.ResetHandlers() |
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.
It took me a bit, to see that it actually "reset" the handlers (while the other one accepts one handler).
I guess no major issue
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.
I can try to make it clearer.
This was the easiest path I found to test the functionality.
I will probably improve the test with a custom provider to verify other behaviors such as the Provider close if exists:
kubernetes-mcp-server/pkg/mcp/mcp.go
Lines 107 to 110 in 6342379
| // close the old provider | |
| if s.p != nil { | |
| s.p.Close() | |
| } |
and so on.
Bu It don't want to spend too much time on this at the moment since it's blocking #385
| } | ||
|
|
||
| func (m *MockServer) ResetHandlers() { | ||
| m.restHandlers = make([]http.HandlerFunc, 0) |
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.
there is similar code above when creating the server
is this worth to call out as an issue on the repo, or was already a similar issue? |
|
LGTM |
Once I implement the workaround here I'll open a PR upstream so it's clear what's the problem. I was the one who actually implemented SetTools in the mark3labs repo mark3labs/mcp-go#24 in the first place :) |
Ensures #385 / #219 preserves current behavior.
Verifies that tools are added and removed when kubeconfig file changes.
Go SDK lacks a SetTools method, AddTool only upserts tools. This test verifies that internally we handle the case that no-longer applicable tools are removed from the list of tools.