Skip to content

Commit 40705cb

Browse files
pelikhanpelikhan
authored
Merge pull request #189 from githubnext/test-improver/mcp-connection-tests-0a4394c8a4630e2b
2 parents d349989 + 39450ee commit 40705cb

1 file changed

Lines changed: 290 additions & 0 deletions

File tree

‎internal/mcp/connection_test.go‎

Lines changed: 290 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"encoding/json"
66
"net/http"
77
"net/http/httptest"
8+
"os"
89
"testing"
910
)
1011

@@ -140,3 +141,292 @@ func TestHTTPRequest_ConfiguredHeaders(t *testing.T) {
140141
t.Errorf("Expected Mcp-Session-Id header '%s', got '%s'", sessionID, receivedSessionID)
141142
}
142143
}
144+
145+
// TestExpandDockerEnvArgs tests the Docker environment variable expansion function
146+
func TestExpandDockerEnvArgs(t *testing.T) {
147+
tests := []struct {
148+
name string
149+
args []string
150+
envVars map[string]string
151+
expected []string
152+
}{
153+
{
154+
name: "no -e flags",
155+
args: []string{"run", "--rm", "image"},
156+
envVars: map[string]string{},
157+
expected: []string{"run", "--rm", "image"},
158+
},
159+
{
160+
name: "expand single env variable",
161+
args: []string{"run", "-e", "VAR_NAME", "image"},
162+
envVars: map[string]string{"VAR_NAME": "value1"},
163+
expected: []string{"run", "-e", "VAR_NAME=value1", "image"},
164+
},
165+
{
166+
name: "expand multiple env variables",
167+
args: []string{"run", "-e", "VAR1", "-e", "VAR2", "image"},
168+
envVars: map[string]string{"VAR1": "value1", "VAR2": "value2"},
169+
expected: []string{"run", "-e", "VAR1=value1", "-e", "VAR2=value2", "image"},
170+
},
171+
{
172+
name: "preserve existing key=value format",
173+
args: []string{"run", "-e", "VAR=predefined", "image"},
174+
envVars: map[string]string{},
175+
expected: []string{"run", "-e", "VAR=predefined", "image"},
176+
},
177+
{
178+
name: "mixed: expand and preserve",
179+
args: []string{"run", "-e", "VAR1", "-e", "VAR2=fixed", "image"},
180+
envVars: map[string]string{"VAR1": "value1"},
181+
expected: []string{"run", "-e", "VAR1=value1", "-e", "VAR2=fixed", "image"},
182+
},
183+
{
184+
name: "undefined env variable",
185+
args: []string{"run", "-e", "UNDEFINED_VAR", "image"},
186+
envVars: map[string]string{},
187+
expected: []string{"run", "-e", "UNDEFINED_VAR", "image"},
188+
},
189+
{
190+
name: "empty env variable value",
191+
args: []string{"run", "-e", "EMPTY_VAR", "image"},
192+
envVars: map[string]string{"EMPTY_VAR": ""},
193+
expected: []string{"run", "-e", "EMPTY_VAR=", "image"},
194+
},
195+
{
196+
name: "-e at end of args (no following arg)",
197+
args: []string{"run", "image", "-e"},
198+
envVars: map[string]string{},
199+
expected: []string{"run", "image", "-e"},
200+
},
201+
}
202+
203+
for _, tt := range tests {
204+
t.Run(tt.name, func(t *testing.T) {
205+
// Set up environment variables for test
206+
for k, v := range tt.envVars {
207+
os.Setenv(k, v)
208+
}
209+
// Clean up after test
210+
t.Cleanup(func() {
211+
for k := range tt.envVars {
212+
os.Unsetenv(k)
213+
}
214+
})
215+
216+
result := expandDockerEnvArgs(tt.args)
217+
218+
if len(result) != len(tt.expected) {
219+
t.Fatalf("Expected %d args, got %d: %v", len(tt.expected), len(result), result)
220+
}
221+
222+
for i := range result {
223+
if result[i] != tt.expected[i] {
224+
t.Errorf("Arg %d: expected '%s', got '%s'", i, tt.expected[i], result[i])
225+
}
226+
}
227+
})
228+
}
229+
}
230+
231+
// TestHTTPRequest_ErrorResponses tests handling of various error conditions
232+
func TestHTTPRequest_ErrorResponses(t *testing.T) {
233+
tests := []struct {
234+
name string
235+
statusCode int
236+
responseBody map[string]interface{}
237+
expectError bool
238+
errorSubstring string
239+
}{
240+
{
241+
name: "HTTP 200 success",
242+
statusCode: http.StatusOK,
243+
responseBody: map[string]interface{}{
244+
"jsonrpc": "2.0",
245+
"id": 1,
246+
"result": map[string]interface{}{
247+
"tools": []interface{}{},
248+
},
249+
},
250+
expectError: false,
251+
},
252+
{
253+
name: "HTTP 404 not found",
254+
statusCode: http.StatusNotFound,
255+
responseBody: map[string]interface{}{
256+
"error": "Not found",
257+
},
258+
expectError: true,
259+
errorSubstring: "status=404",
260+
},
261+
{
262+
name: "HTTP 500 server error",
263+
statusCode: http.StatusInternalServerError,
264+
responseBody: map[string]interface{}{
265+
"error": "Internal server error",
266+
},
267+
expectError: true,
268+
errorSubstring: "status=500",
269+
},
270+
{
271+
name: "JSON-RPC error response",
272+
statusCode: http.StatusOK,
273+
responseBody: map[string]interface{}{
274+
"jsonrpc": "2.0",
275+
"id": 1,
276+
"error": map[string]interface{}{
277+
"code": -32600,
278+
"message": "Invalid request",
279+
},
280+
},
281+
expectError: false, // JSON-RPC errors are returned as valid responses
282+
},
283+
}
284+
285+
for _, tt := range tests {
286+
t.Run(tt.name, func(t *testing.T) {
287+
// Create test server with specific response
288+
testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
289+
w.WriteHeader(tt.statusCode)
290+
w.Header().Set("Content-Type", "application/json")
291+
json.NewEncoder(w).Encode(tt.responseBody)
292+
}))
293+
defer testServer.Close()
294+
295+
// Create connection with custom headers to use plain JSON transport
296+
conn, err := NewHTTPConnection(context.Background(), testServer.URL, map[string]string{
297+
"Authorization": "test-token",
298+
})
299+
if err != nil && tt.expectError {
300+
// Error during initialization is expected for some error conditions
301+
if tt.errorSubstring != "" && !containsSubstring(err.Error(), tt.errorSubstring) {
302+
t.Errorf("Expected error to contain '%s', got: %v", tt.errorSubstring, err)
303+
}
304+
return
305+
}
306+
if err != nil {
307+
t.Fatalf("Failed to create connection: %v", err)
308+
}
309+
310+
// Send request
311+
_, err = conn.SendRequestWithServerID(context.Background(), "tools/list", nil, "test-server")
312+
313+
if tt.expectError {
314+
if err == nil {
315+
t.Error("Expected error but got none")
316+
} else if tt.errorSubstring != "" && !containsSubstring(err.Error(), tt.errorSubstring) {
317+
t.Errorf("Expected error to contain '%s', got: %v", tt.errorSubstring, err)
318+
}
319+
} else {
320+
if err != nil {
321+
t.Errorf("Expected no error but got: %v", err)
322+
}
323+
}
324+
})
325+
}
326+
}
327+
328+
// TestConnection_IsHTTP tests the IsHTTP, GetHTTPURL, and GetHTTPHeaders methods
329+
func TestConnection_IsHTTP(t *testing.T) {
330+
// Create a mock HTTP server
331+
testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
332+
response := map[string]interface{}{
333+
"jsonrpc": "2.0",
334+
"id": 1,
335+
"result": map[string]interface{}{
336+
"protocolVersion": "2024-11-05",
337+
"capabilities": map[string]interface{}{},
338+
},
339+
}
340+
w.Header().Set("Content-Type", "application/json")
341+
json.NewEncoder(w).Encode(response)
342+
}))
343+
defer testServer.Close()
344+
345+
headers := map[string]string{
346+
"Authorization": "test-auth",
347+
"X-Custom": "custom-value",
348+
}
349+
350+
conn, err := NewHTTPConnection(context.Background(), testServer.URL, headers)
351+
if err != nil {
352+
t.Fatalf("Failed to create HTTP connection: %v", err)
353+
}
354+
defer conn.Close()
355+
356+
// Test IsHTTP
357+
if !conn.IsHTTP() {
358+
t.Error("Expected IsHTTP() to return true for HTTP connection")
359+
}
360+
361+
// Test GetHTTPURL
362+
if conn.GetHTTPURL() != testServer.URL {
363+
t.Errorf("Expected URL '%s', got '%s'", testServer.URL, conn.GetHTTPURL())
364+
}
365+
366+
// Test GetHTTPHeaders
367+
returnedHeaders := conn.GetHTTPHeaders()
368+
if len(returnedHeaders) != len(headers) {
369+
t.Errorf("Expected %d headers, got %d", len(headers), len(returnedHeaders))
370+
}
371+
for k, v := range headers {
372+
if returnedHeaders[k] != v {
373+
t.Errorf("Expected header '%s' to be '%s', got '%s'", k, v, returnedHeaders[k])
374+
}
375+
}
376+
}
377+
378+
// TestHTTPConnection_InvalidURL tests error handling for invalid URLs
379+
func TestHTTPConnection_InvalidURL(t *testing.T) {
380+
tests := []struct {
381+
name string
382+
url string
383+
headers map[string]string
384+
expectError bool
385+
errorSubstring string
386+
}{
387+
{
388+
name: "valid URL with headers",
389+
url: "http://localhost:3000",
390+
headers: map[string]string{"Authorization": "test"},
391+
expectError: true, // Will fail to connect but URL is valid
392+
},
393+
{
394+
name: "empty URL",
395+
url: "",
396+
headers: map[string]string{"Authorization": "test"},
397+
expectError: true,
398+
},
399+
}
400+
401+
for _, tt := range tests {
402+
t.Run(tt.name, func(t *testing.T) {
403+
_, err := NewHTTPConnection(context.Background(), tt.url, tt.headers)
404+
405+
if tt.expectError {
406+
if err == nil {
407+
t.Error("Expected error but got none")
408+
} else if tt.errorSubstring != "" && !containsSubstring(err.Error(), tt.errorSubstring) {
409+
t.Errorf("Expected error to contain '%s', got: %v", tt.errorSubstring, err)
410+
}
411+
} else {
412+
if err != nil {
413+
t.Errorf("Expected no error but got: %v", err)
414+
}
415+
}
416+
})
417+
}
418+
}
419+
420+
// containsSubstring is a helper to check if a string contains a substring
421+
func containsSubstring(s, substr string) bool {
422+
return len(substr) > 0 && len(s) >= len(substr) && stringContains(s, substr)
423+
}
424+
425+
func stringContains(s, substr string) bool {
426+
for i := 0; i <= len(s)-len(substr); i++ {
427+
if s[i:i+len(substr)] == substr {
428+
return true
429+
}
430+
}
431+
return false
432+
}

0 commit comments

Comments
 (0)