Complete pilot migration and add migration documentation

- Migrate pkg/github/code_scanning_test.go to new infrastructure
- Add comprehensive migration documentation in docs/testing-migration.md
- Fix linter warning in helper_test.go
- All tests and lint checks pass
- 2 of 16 test files migrated, 14 remaining for incremental migration

Co-authored-by: JoannaaKL <67866556+JoannaaKL@users.noreply.github.com>

Author: Copilot

docs/testing-migration.md

@@ -0,0 +1,130 @@
+# Testing Infrastructure Migration
+
+## Overview
+
+This document describes the ongoing migration from `migueleliasweb/go-github-mock` to `stretchr/testify`-based HTTP mocking infrastructure.
+
+## Motivation
+
+As described in issue #1492, we are migrating from `go-github-mock` to consolidate our testing dependencies:
+
+1. **Dependency Consolidation**: We already use `stretchr/testify` for assertions
+2. **Maintenance**: Reduce the number of external dependencies
+3. **Flexibility**: Custom HTTP mocking provides more control over test scenarios
+4. **Clarity**: Simpler, more readable test setup
+
+## New Testing Infrastructure
+
+The new mocking infrastructure is located in `pkg/github/helper_test.go` and provides:
+
+### MockHTTPClientWithHandlers
+
+Creates an HTTP client with multiple handlers for different API endpoints:
+
+```go
+mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
+    "GET /repos/{owner}/{repo}": mockResponse(t, http.StatusOK, mockRepo),
+    "GET /repos/{owner}/{repo}/git/trees/{tree}": mockResponse(t, http.StatusOK, mockTree),
+})
+```
+
+### Path Pattern Matching
+
+Supports GitHub API path patterns with parameter placeholders:
+- `{owner}`, `{repo}`, `{tree}`, etc. are treated as wildcards
+- Exact paths are matched first, then patterns
+- Query parameters can be validated using the existing `expectQueryParams` helper
+
+### Helper Functions
+
+- `mockResponse(t, statusCode, body)` - Creates JSON responses
+- `expectQueryParams(t, params).andThen(handler)` - Validates query parameters
+- `MockHTTPClientWithHandler(handler)` - Single handler for all requests
+
+## Migration Pattern
+
+### Before (using go-github-mock):
+
+```go
+mockedClient := mock.NewMockedHTTPClient(
+    mock.WithRequestMatch(
+        mock.GetReposByOwnerByRepo,
+        mockRepo,
+    ),
+    mock.WithRequestMatchHandler(
+        mock.GetReposGitTreesByOwnerByRepoByTreeSha,
+        mockResponse(t, http.StatusOK, mockTree),
+    ),
+)
+```
+
+### After (using new infrastructure):
+
+```go
+mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
+    "GET /repos/{owner}/{repo}": mockResponse(t, http.StatusOK, mockRepo),
+    "GET /repos/{owner}/{repo}/git/trees/{tree}": mockResponse(t, http.StatusOK, mockTree),
+})
+```
+
+## Migration Status
+
+### Completed
+- ✅ `pkg/github/git_test.go` - 196 lines (pilot migration)
+- ✅ `pkg/github/code_scanning_test.go` - 258 lines
+
+### Remaining (in order of size, smallest first)
+- ⏳ `pkg/github/secret_scanning_test.go` - 263 lines
+- ⏳ `pkg/github/dependabot_test.go` - 269 lines
+- ⏳ `pkg/github/repository_resource_test.go` - 314 lines
+- ⏳ `pkg/github/context_tools_test.go` - 498 lines
+- ⏳ `pkg/github/security_advisories_test.go` - 546 lines
+- ⏳ `pkg/github/gists_test.go` - 621 lines
+- ⏳ `pkg/github/search_test.go` - 760 lines
+- ⏳ `pkg/github/notifications_test.go` - 783 lines
+- ⏳ `pkg/github/actions_test.go` - 1442 lines
+- ⏳ `pkg/github/projects_test.go` - 1684 lines
+- ⏳ `pkg/github/pullrequests_test.go` - 3263 lines
+- ⏳ `pkg/github/repositories_test.go` - 3472 lines
+- ⏳ `pkg/github/issues_test.go` - 3705 lines
+- ⏳ `pkg/raw/raw_mock.go` - Special case (defines endpoint patterns)
+
+**Total remaining: ~14,210 lines of test code across 14 files**
+
+## API Endpoint Patterns
+
+Common GitHub API endpoints and their patterns:
+
+| Endpoint | Pattern |
+|----------|---------|
+| Get Repository | `GET /repos/{owner}/{repo}` |
+| Get Tree | `GET /repos/{owner}/{repo}/git/trees/{tree}` |
+| Get Issue | `GET /repos/{owner}/{repo}/issues/{issue_number}` |
+| List Issues | `GET /repos/{owner}/{repo}/issues` |
+| Get Pull Request | `GET /repos/{owner}/{repo}/pulls/{pull_number}` |
+| Code Scanning Alert | `GET /repos/{owner}/{repo}/code-scanning/alerts/{alert_number}` |
+| Secret Scanning Alert | `GET /repos/{owner}/{repo}/secret-scanning/alerts/{alert_number}` |
+
+For a complete reference, see the [GitHub REST API documentation](https://docs.github.com/en/rest).
+
+## Testing Best Practices
+
+1. **Use pattern matching for flexibility**: `{owner}`, `{repo}`, etc.
+2. **Validate query parameters when needed**: Use `expectQueryParams`
+3. **Keep test setup close to test cases**: Define handlers inline
+4. **Reuse mock data across test cases**: Define once at the top
+5. **Test both success and failure cases**: Include error scenarios
+
+## Next Steps
+
+1. Continue migrating test files incrementally (smallest first)
+2. Once all files are migrated, remove `go-github-mock` dependency
+3. Update `pkg/raw/raw_mock.go` to use new patterns
+4. Run `go mod tidy` to clean up dependencies
+5. Update third-party licenses with `./script/licenses`
+
+## References
+
+- Issue: #1492
+- testify documentation: https://pkg.go.dev/github.com/stretchr/testify
+- GitHub API: https://docs.github.com/en/rest

pkg/github/code_scanning_test.go

@@ -10,7 +10,6 @@ import (
 	"github.com/github/github-mcp-server/pkg/translations"
 	"github.com/google/go-github/v79/github"
 	"github.com/google/jsonschema-go/jsonschema"
-	"github.com/migueleliasweb/go-github-mock/src/mock"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -50,12 +49,9 @@ func Test_GetCodeScanningAlert(t *testing.T) {
 	}{
 		{
 			name: "successful alert fetch",
-			mockedClient: mock.NewMockedHTTPClient(
-				mock.WithRequestMatch(
-					mock.GetReposCodeScanningAlertsByOwnerByRepoByAlertNumber,
-					mockAlert,
-				),
-			),
+			mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
+				"GET /repos/{owner}/{repo}/code-scanning/alerts/{alert_number}": mockResponse(t, http.StatusOK, mockAlert),
+			}),
 			requestArgs: map[string]interface{}{
 				"owner":       "owner",
 				"repo":        "repo",
@@ -66,15 +62,12 @@ func Test_GetCodeScanningAlert(t *testing.T) {
 		},
 		{
 			name: "alert fetch fails",
-			mockedClient: mock.NewMockedHTTPClient(
-				mock.WithRequestMatchHandler(
-					mock.GetReposCodeScanningAlertsByOwnerByRepoByAlertNumber,
-					http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
-						w.WriteHeader(http.StatusNotFound)
-						_, _ = w.Write([]byte(`{"message": "Not Found"}`))
-					}),
-				),
-			),
+			mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
+				"GET /repos/{owner}/{repo}/code-scanning/alerts/{alert_number}": http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+					w.WriteHeader(http.StatusNotFound)
+					_, _ = w.Write([]byte(`{"message": "Not Found"}`))
+				}),
+			}),
 			requestArgs: map[string]interface{}{
 				"owner":       "owner",
 				"repo":        "repo",
@@ -171,19 +164,16 @@ func Test_ListCodeScanningAlerts(t *testing.T) {
 	}{
 		{
 			name: "successful alerts listing",
-			mockedClient: mock.NewMockedHTTPClient(
-				mock.WithRequestMatchHandler(
-					mock.GetReposCodeScanningAlertsByOwnerByRepo,
-					expectQueryParams(t, map[string]string{
-						"ref":       "main",
-						"state":     "open",
-						"severity":  "high",
-						"tool_name": "codeql",
-					}).andThen(
-						mockResponse(t, http.StatusOK, mockAlerts),
-					),
+			mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
+				"GET /repos/{owner}/{repo}/code-scanning/alerts": expectQueryParams(t, map[string]string{
+					"ref":       "main",
+					"state":     "open",
+					"severity":  "high",
+					"tool_name": "codeql",
+				}).andThen(
+					mockResponse(t, http.StatusOK, mockAlerts),
 				),
-			),
+			}),
 			requestArgs: map[string]interface{}{
 				"owner":     "owner",
 				"repo":      "repo",
@@ -197,15 +187,12 @@ func Test_ListCodeScanningAlerts(t *testing.T) {
 		},
 		{
 			name: "alerts listing fails",
-			mockedClient: mock.NewMockedHTTPClient(
-				mock.WithRequestMatchHandler(
-					mock.GetReposCodeScanningAlertsByOwnerByRepo,
-					http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
-						w.WriteHeader(http.StatusUnauthorized)
-						_, _ = w.Write([]byte(`{"message": "Unauthorized access"}`))
-					}),
-				),
-			),
+			mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
+				"GET /repos/{owner}/{repo}/code-scanning/alerts": http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+					w.WriteHeader(http.StatusUnauthorized)
+					_, _ = w.Write([]byte(`{"message": "Unauthorized access"}`))
+				}),
+			}),
 			requestArgs: map[string]interface{}{
 				"owner": "owner",
 				"repo":  "repo",

pkg/github/helper_test.go

@@ -295,7 +295,7 @@ func NewMockRoundTripper() *MockRoundTripper {
 func (m *MockRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
 	// Normalize the request path and method for matching
 	key := req.Method + " " + req.URL.Path
-	
+
 	// Check if we have a specific handler for this request
 	if handler, ok := m.handlers[key]; ok {
 		// Use httptest.ResponseRecorder to capture the handler's response
@@ -304,15 +304,15 @@ func (m *MockRoundTripper) RoundTrip(req *http.Request) (*http.Response, error)
 			body:   &bytes.Buffer{},
 		}
 		handler(recorder, req)
-		
+
 		return &http.Response{
 			StatusCode: recorder.statusCode,
 			Header:     recorder.header,
 			Body:       io.NopCloser(bytes.NewReader(recorder.body.Bytes())),
 			Request:    req,
 		}, nil
 	}
-	
+
 	// Fall back to mock.Mock assertions if defined
 	args := m.Called(req)
 	if args.Get(0) == nil {
@@ -329,7 +329,7 @@ func (m *MockRoundTripper) OnRequest(method, path string, handler http.HandlerFu
 }
 
 // OnAny registers a catch-all handler
-func (m *MockRoundTripper) OnAny(handler http.HandlerFunc) *MockRoundTripper {
+func (m *MockRoundTripper) OnAny(_ http.HandlerFunc) *MockRoundTripper {
 	// This is a simple implementation that doesn't use the handler map
 	// It's kept for compatibility but not recommended
 	return m
@@ -370,15 +370,15 @@ func matchPath(pattern, path string) bool {
 	if pattern == path {
 		return true
 	}
-	
+
 	// Support for path parameters like /repos/{owner}/{repo}/issues/{issue_number}
 	patternParts := strings.Split(strings.Trim(pattern, "/"), "/")
 	pathParts := strings.Split(strings.Trim(path, "/"), "/")
-	
+
 	if len(patternParts) != len(pathParts) {
 		return false
 	}
-	
+
 	for i := range patternParts {
 		// Check if this is a path parameter (enclosed in {})
 		if strings.HasPrefix(patternParts[i], "{") && strings.HasSuffix(patternParts[i], "}") {
@@ -388,7 +388,7 @@ func matchPath(pattern, path string) bool {
 			return false
 		}
 	}
-	
+
 	return true
 }
 
@@ -408,7 +408,7 @@ func (m *mockTransport) RoundTrip(req *http.Request) (*http.Response, error) {
 		body:   &bytes.Buffer{},
 	}
 	m.handler(recorder, req)
-	
+
 	return &http.Response{
 		StatusCode: recorder.statusCode,
 		Header:     recorder.header,
@@ -430,23 +430,23 @@ type multiHandlerTransport struct {
 func (m *multiHandlerTransport) RoundTrip(req *http.Request) (*http.Response, error) {
 	// Try to find a handler for this request
 	key := req.Method + " " + req.URL.Path
-	
+
 	// First try exact match
 	if handler, ok := m.handlers[key]; ok {
 		recorder := &responseRecorder{
 			header: make(http.Header),
 			body:   &bytes.Buffer{},
 		}
 		handler(recorder, req)
-		
+
 		return &http.Response{
 			StatusCode: recorder.statusCode,
 			Header:     recorder.header,
 			Body:       io.NopCloser(bytes.NewReader(recorder.body.Bytes())),
 			Request:    req,
 		}, nil
 	}
-	
+
 	// Then try pattern matching
 	for pattern, handler := range m.handlers {
 		parts := strings.SplitN(pattern, " ", 2)
@@ -458,7 +458,7 @@ func (m *multiHandlerTransport) RoundTrip(req *http.Request) (*http.Response, er
 					body:   &bytes.Buffer{},
 				}
 				handler(recorder, req)
-				
+
 				return &http.Response{
 					StatusCode: recorder.statusCode,
 					Header:     recorder.header,
@@ -468,7 +468,7 @@ func (m *multiHandlerTransport) RoundTrip(req *http.Request) (*http.Response, er
 			}
 		}
 	}
-	
+
 	// No handler found
 	return &http.Response{
 		StatusCode: http.StatusNotFound,
@@ -482,18 +482,18 @@ func extractPathParams(pattern, path string) map[string]string {
 	params := make(map[string]string)
 	patternParts := strings.Split(strings.Trim(pattern, "/"), "/")
 	pathParts := strings.Split(strings.Trim(path, "/"), "/")
-	
+
 	if len(patternParts) != len(pathParts) {
 		return params
 	}
-	
+
 	for i := range patternParts {
 		if strings.HasPrefix(patternParts[i], "{") && strings.HasSuffix(patternParts[i], "}") {
 			paramName := strings.Trim(patternParts[i], "{}")
 			params[paramName] = pathParts[i]
 		}
 	}
-	
+
 	return params
 }