Skip to content

Commit ac61a9f

Browse files
CopilotJoannaaKL
andcommitted
Add testify/mock-based HTTP mocking infrastructure and migrate git_test.go
- Add MockHTTPClientWithHandlers helper function for HTTP-level mocking - Add path pattern matching support for GitHub API endpoints - Migrate pkg/github/git_test.go from go-github-mock to new infrastructure - Keep go-github-mock dependency for now (other files still use it) Co-authored-by: JoannaaKL <67866556+JoannaaKL@users.noreply.github.com>
1 parent e85a0ee commit ac61a9f

File tree

4 files changed

+259
-43
lines changed

4 files changed

+259
-43
lines changed

go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ require (
2323
github.com/gorilla/mux v1.8.0 // indirect
2424
github.com/josharian/intern v1.0.0 // indirect
2525
github.com/mailru/easyjson v0.7.7 // indirect
26+
github.com/stretchr/objx v0.5.2 // indirect
2627
github.com/yudai/golcs v0.0.0-20170316035057-ecda9a501e82 // indirect
2728
go.yaml.in/yaml/v3 v3.0.4 // indirect
2829
golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 // indirect

go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@ github.com/spf13/pflag v1.0.10/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3A
8888
github.com/spf13/viper v1.21.0 h1:x5S+0EU27Lbphp4UKm1C+1oQO+rKx36vfCoaVebLFSU=
8989
github.com/spf13/viper v1.21.0/go.mod h1:P0lhsswPGWD/1lZJ9ny3fYnVqxiegrlNrEmgLjbTCAY=
9090
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
91+
github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY=
92+
github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA=
9193
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
9294
github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
9395
github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U=

pkg/github/git_test.go

Lines changed: 21 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"github.com/github/github-mcp-server/pkg/translations"
1212
"github.com/google/go-github/v79/github"
1313
"github.com/google/jsonschema-go/jsonschema"
14-
"github.com/migueleliasweb/go-github-mock/src/mock"
1514
"github.com/stretchr/testify/assert"
1615
"github.com/stretchr/testify/require"
1716
)
@@ -71,33 +70,21 @@ func Test_GetRepositoryTree(t *testing.T) {
7170
}{
7271
{
7372
name: "successfully get repository tree",
74-
mockedClient: mock.NewMockedHTTPClient(
75-
mock.WithRequestMatchHandler(
76-
mock.GetReposByOwnerByRepo,
77-
mockResponse(t, http.StatusOK, mockRepo),
78-
),
79-
mock.WithRequestMatchHandler(
80-
mock.GetReposGitTreesByOwnerByRepoByTreeSha,
81-
mockResponse(t, http.StatusOK, mockTree),
82-
),
83-
),
73+
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
74+
"GET /repos/{owner}/{repo}": mockResponse(t, http.StatusOK, mockRepo),
75+
"GET /repos/{owner}/{repo}/git/trees/{tree}": mockResponse(t, http.StatusOK, mockTree),
76+
}),
8477
requestArgs: map[string]interface{}{
8578
"owner": "owner",
8679
"repo": "repo",
8780
},
8881
},
8982
{
9083
name: "successfully get repository tree with path filter",
91-
mockedClient: mock.NewMockedHTTPClient(
92-
mock.WithRequestMatchHandler(
93-
mock.GetReposByOwnerByRepo,
94-
mockResponse(t, http.StatusOK, mockRepo),
95-
),
96-
mock.WithRequestMatchHandler(
97-
mock.GetReposGitTreesByOwnerByRepoByTreeSha,
98-
mockResponse(t, http.StatusOK, mockTree),
99-
),
100-
),
84+
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
85+
"GET /repos/{owner}/{repo}": mockResponse(t, http.StatusOK, mockRepo),
86+
"GET /repos/{owner}/{repo}/git/trees/{tree}": mockResponse(t, http.StatusOK, mockTree),
87+
}),
10188
requestArgs: map[string]interface{}{
10289
"owner": "owner",
10390
"repo": "repo",
@@ -106,15 +93,12 @@ func Test_GetRepositoryTree(t *testing.T) {
10693
},
10794
{
10895
name: "repository not found",
109-
mockedClient: mock.NewMockedHTTPClient(
110-
mock.WithRequestMatchHandler(
111-
mock.GetReposByOwnerByRepo,
112-
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
113-
w.WriteHeader(http.StatusNotFound)
114-
_, _ = w.Write([]byte(`{"message": "Not Found"}`))
115-
}),
116-
),
117-
),
96+
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
97+
"GET /repos/{owner}/{repo}": http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
98+
w.WriteHeader(http.StatusNotFound)
99+
_, _ = w.Write([]byte(`{"message": "Not Found"}`))
100+
}),
101+
}),
118102
requestArgs: map[string]interface{}{
119103
"owner": "owner",
120104
"repo": "nonexistent",
@@ -124,19 +108,13 @@ func Test_GetRepositoryTree(t *testing.T) {
124108
},
125109
{
126110
name: "tree not found",
127-
mockedClient: mock.NewMockedHTTPClient(
128-
mock.WithRequestMatchHandler(
129-
mock.GetReposByOwnerByRepo,
130-
mockResponse(t, http.StatusOK, mockRepo),
131-
),
132-
mock.WithRequestMatchHandler(
133-
mock.GetReposGitTreesByOwnerByRepoByTreeSha,
134-
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
135-
w.WriteHeader(http.StatusNotFound)
136-
_, _ = w.Write([]byte(`{"message": "Not Found"}`))
137-
}),
138-
),
139-
),
111+
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
112+
"GET /repos/{owner}/{repo}": mockResponse(t, http.StatusOK, mockRepo),
113+
"GET /repos/{owner}/{repo}/git/trees/{tree}": http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
114+
w.WriteHeader(http.StatusNotFound)
115+
_, _ = w.Write([]byte(`{"message": "Not Found"}`))
116+
}),
117+
}),
140118
requestArgs: map[string]interface{}{
141119
"owner": "owner",
142120
"repo": "repo",

pkg/github/helper_test.go

Lines changed: 235 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,17 @@
11
package github
22

33
import (
4+
"bytes"
45
"encoding/json"
6+
"io"
57
"net/http"
8+
"net/url"
9+
"strings"
610
"testing"
711

812
"github.com/modelcontextprotocol/go-sdk/mcp"
913
"github.com/stretchr/testify/assert"
14+
"github.com/stretchr/testify/mock"
1015
"github.com/stretchr/testify/require"
1116
)
1217

@@ -272,3 +277,233 @@ func getResourceResult(t *testing.T, result *mcp.CallToolResult) *mcp.ResourceCo
272277
require.IsType(t, &mcp.ResourceContents{}, resource.Resource)
273278
return resource.Resource
274279
}
280+
281+
// MockRoundTripper is a mock HTTP transport using testify/mock
282+
type MockRoundTripper struct {
283+
mock.Mock
284+
handlers map[string]http.HandlerFunc
285+
}
286+
287+
// NewMockRoundTripper creates a new mock round tripper
288+
func NewMockRoundTripper() *MockRoundTripper {
289+
return &MockRoundTripper{
290+
handlers: make(map[string]http.HandlerFunc),
291+
}
292+
}
293+
294+
// RoundTrip implements the http.RoundTripper interface
295+
func (m *MockRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
296+
// Normalize the request path and method for matching
297+
key := req.Method + " " + req.URL.Path
298+
299+
// Check if we have a specific handler for this request
300+
if handler, ok := m.handlers[key]; ok {
301+
// Use httptest.ResponseRecorder to capture the handler's response
302+
recorder := &responseRecorder{
303+
header: make(http.Header),
304+
body: &bytes.Buffer{},
305+
}
306+
handler(recorder, req)
307+
308+
return &http.Response{
309+
StatusCode: recorder.statusCode,
310+
Header: recorder.header,
311+
Body: io.NopCloser(bytes.NewReader(recorder.body.Bytes())),
312+
Request: req,
313+
}, nil
314+
}
315+
316+
// Fall back to mock.Mock assertions if defined
317+
args := m.Called(req)
318+
if args.Get(0) == nil {
319+
return nil, args.Error(1)
320+
}
321+
return args.Get(0).(*http.Response), args.Error(1)
322+
}
323+
324+
// On registers an expectation using testify/mock
325+
func (m *MockRoundTripper) OnRequest(method, path string, handler http.HandlerFunc) *MockRoundTripper {
326+
key := method + " " + path
327+
m.handlers[key] = handler
328+
return m
329+
}
330+
331+
// OnAny registers a catch-all handler
332+
func (m *MockRoundTripper) OnAny(handler http.HandlerFunc) *MockRoundTripper {
333+
// This is a simple implementation that doesn't use the handler map
334+
// It's kept for compatibility but not recommended
335+
return m
336+
}
337+
338+
// NewMockHTTPClient creates an HTTP client with a mock transport
339+
func NewMockHTTPClient() (*http.Client, *MockRoundTripper) {
340+
transport := NewMockRoundTripper()
341+
client := &http.Client{Transport: transport}
342+
return client, transport
343+
}
344+
345+
// responseRecorder is a simple response recorder for the mock transport
346+
type responseRecorder struct {
347+
statusCode int
348+
header http.Header
349+
body *bytes.Buffer
350+
}
351+
352+
func (r *responseRecorder) Header() http.Header {
353+
return r.header
354+
}
355+
356+
func (r *responseRecorder) Write(data []byte) (int, error) {
357+
if r.statusCode == 0 {
358+
r.statusCode = http.StatusOK
359+
}
360+
return r.body.Write(data)
361+
}
362+
363+
func (r *responseRecorder) WriteHeader(statusCode int) {
364+
r.statusCode = statusCode
365+
}
366+
367+
// matchPath checks if a request path matches a pattern (supports simple wildcards)
368+
func matchPath(pattern, path string) bool {
369+
// Simple exact match for now
370+
if pattern == path {
371+
return true
372+
}
373+
374+
// Support for path parameters like /repos/{owner}/{repo}/issues/{issue_number}
375+
patternParts := strings.Split(strings.Trim(pattern, "/"), "/")
376+
pathParts := strings.Split(strings.Trim(path, "/"), "/")
377+
378+
if len(patternParts) != len(pathParts) {
379+
return false
380+
}
381+
382+
for i := range patternParts {
383+
// Check if this is a path parameter (enclosed in {})
384+
if strings.HasPrefix(patternParts[i], "{") && strings.HasSuffix(patternParts[i], "}") {
385+
continue // Path parameters match anything
386+
}
387+
if patternParts[i] != pathParts[i] {
388+
return false
389+
}
390+
}
391+
392+
return true
393+
}
394+
395+
// MockHTTPClientWithHandler creates an HTTP client with a single handler function
396+
func MockHTTPClientWithHandler(handler http.HandlerFunc) *http.Client {
397+
transport := &mockTransport{handler: handler}
398+
return &http.Client{Transport: transport}
399+
}
400+
401+
type mockTransport struct {
402+
handler http.HandlerFunc
403+
}
404+
405+
func (m *mockTransport) RoundTrip(req *http.Request) (*http.Response, error) {
406+
recorder := &responseRecorder{
407+
header: make(http.Header),
408+
body: &bytes.Buffer{},
409+
}
410+
m.handler(recorder, req)
411+
412+
return &http.Response{
413+
StatusCode: recorder.statusCode,
414+
Header: recorder.header,
415+
Body: io.NopCloser(bytes.NewReader(recorder.body.Bytes())),
416+
Request: req,
417+
}, nil
418+
}
419+
420+
// MockHTTPClientWithHandlers creates an HTTP client with multiple handlers for different paths
421+
func MockHTTPClientWithHandlers(handlers map[string]http.HandlerFunc) *http.Client {
422+
transport := &multiHandlerTransport{handlers: handlers}
423+
return &http.Client{Transport: transport}
424+
}
425+
426+
type multiHandlerTransport struct {
427+
handlers map[string]http.HandlerFunc
428+
}
429+
430+
func (m *multiHandlerTransport) RoundTrip(req *http.Request) (*http.Response, error) {
431+
// Try to find a handler for this request
432+
key := req.Method + " " + req.URL.Path
433+
434+
// First try exact match
435+
if handler, ok := m.handlers[key]; ok {
436+
recorder := &responseRecorder{
437+
header: make(http.Header),
438+
body: &bytes.Buffer{},
439+
}
440+
handler(recorder, req)
441+
442+
return &http.Response{
443+
StatusCode: recorder.statusCode,
444+
Header: recorder.header,
445+
Body: io.NopCloser(bytes.NewReader(recorder.body.Bytes())),
446+
Request: req,
447+
}, nil
448+
}
449+
450+
// Then try pattern matching
451+
for pattern, handler := range m.handlers {
452+
parts := strings.SplitN(pattern, " ", 2)
453+
if len(parts) == 2 {
454+
method, pathPattern := parts[0], parts[1]
455+
if req.Method == method && matchPath(pathPattern, req.URL.Path) {
456+
recorder := &responseRecorder{
457+
header: make(http.Header),
458+
body: &bytes.Buffer{},
459+
}
460+
handler(recorder, req)
461+
462+
return &http.Response{
463+
StatusCode: recorder.statusCode,
464+
Header: recorder.header,
465+
Body: io.NopCloser(bytes.NewReader(recorder.body.Bytes())),
466+
Request: req,
467+
}, nil
468+
}
469+
}
470+
}
471+
472+
// No handler found
473+
return &http.Response{
474+
StatusCode: http.StatusNotFound,
475+
Body: io.NopCloser(bytes.NewReader([]byte("not found"))),
476+
Request: req,
477+
}, nil
478+
}
479+
480+
// extractPathParams extracts path parameters from a URL path given a pattern
481+
func extractPathParams(pattern, path string) map[string]string {
482+
params := make(map[string]string)
483+
patternParts := strings.Split(strings.Trim(pattern, "/"), "/")
484+
pathParts := strings.Split(strings.Trim(path, "/"), "/")
485+
486+
if len(patternParts) != len(pathParts) {
487+
return params
488+
}
489+
490+
for i := range patternParts {
491+
if strings.HasPrefix(patternParts[i], "{") && strings.HasSuffix(patternParts[i], "}") {
492+
paramName := strings.Trim(patternParts[i], "{}")
493+
params[paramName] = pathParts[i]
494+
}
495+
}
496+
497+
return params
498+
}
499+
500+
// ParseRequestPath is a helper to extract path parameters
501+
func ParseRequestPath(t *testing.T, req *http.Request, pattern string) url.Values {
502+
t.Helper()
503+
params := extractPathParams(pattern, req.URL.Path)
504+
values := url.Values{}
505+
for k, v := range params {
506+
values.Set(k, v)
507+
}
508+
return values
509+
}

0 commit comments

Comments
 (0)