Chore: Add testify-based HTTP mocking infrastructure and pilot migration

[Copilot]

Closes #1492

Description

Implements Phase 1 and begins Phase 2 of migrating test infrastructure from migueleliasweb/go-github-mock to stretchr/testify-based HTTP mocking. Consolidates testing dependencies while establishing patterns for incremental migration of remaining test files.

Migration Status: 2 of 16 test files migrated (454 lines), 14 files remaining (~14,210 lines).

Changes

New HTTP Mocking Infrastructure (pkg/github/helper_test.go)

• MockHTTPClientWithHandlers: Route-based HTTP client mocking with GitHub API path patterns

  // Before: go-github-mock
  mock.NewMockedHTTPClient(
      mock.WithRequestMatch(mock.GetReposByOwnerByRepo, mockRepo),
  )
  
  // After: testify-based with local constants
  MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
      GetReposByOwnerByRepo: mockResponse(t, http.StatusOK, mockRepo),
  })

• Path pattern matching: Supports {owner}, {repo}, {tree} wildcards for flexible endpoint matching
• executeHandler: Eliminates response creation duplication across transport implementations
• Local endpoint pattern constants: Defines GitHub API endpoint patterns as constants in helper_test.go:
  • GetReposByOwnerByRepo
  • GetReposGitTreesByOwnerByRepoByTree
  • GetReposCodeScanningAlertsByOwnerByRepo
  • GetReposCodeScanningAlertsByOwnerByRepoByAlertNumber

Migrated Test Files

• pkg/github/git_test.go (196 lines) - pilot migration, simplest file
• pkg/github/code_scanning_test.go (258 lines) - demonstrates query parameter validation

Both test files now use locally-defined endpoint pattern constants instead of hardcoded paths for better maintainability and consistency.

Dependencies

• Added github.com/stretchr/objx (required by testify/mock)
• Retained go-github-mock until full migration complete (will be removed in future PRs)

Tradeoffs

Partial migration over full replacement: Migrated 2 files to establish infrastructure and patterns rather than all 16 files. Reduces PR scope and risk while enabling future incremental work. Full migration deferred to subsequent PRs as outlined in issue.

HTTP-level mocking over interface-based: Maintains current architectural patterns (HTTP transport mocking) rather than introducing interface wrappers around GitHub client. Interface-based approach would require extensive production code changes; HTTP-level mocking achieves dependency consolidation goals with test-only changes.

Local constants over go-github-mock patterns: Defines endpoint pattern constants locally in this repo rather than reusing go-github-mock patterns. This enables eventual removal of the go-github-mock dependency while maintaining clean, maintainable test code.

No migration documentation: Migration patterns are documented inline in test files and helper functions rather than in a separate documentation file to keep the PR focused on code changes.

Alternatives

sed/automated migration: Considered batch migration via scripting, but endpoint patterns vary significantly across test files (query params, error scenarios, pagination). Manual migration ensures correctness and provides learning for establishing robust patterns.

Remove go-github-mock immediately: Cannot remove dependency until all test files migrated. Interim state maintains both approaches until migration complete.

Reuse mock.EndpointPattern constants: Initially considered reusing existing mock.EndpointPattern constants from go-github-mock, but defining local constants better supports the goal of removing the go-github-mock dependency entirely.

Original prompt

---

This section details on the original issue you should resolve

<issue_title>Chore: replace migueleliasweb/go-github-mock with stretchr/testify/mock</issue_title>
<issue_description>### Describe the feature or problem you'd like to solve

The current test suite uses migueleliasweb/go-github-mock for mocking GitHub REST API responses in unit tests. While this library has served us well, there are several reasons to consider replacing it with stretchr/testify/mock:

1. Dependency Consolidation: We already use stretchr/testify for assertions (assert and require). Using testify/mock would consolidate our testing dependencies.

2. Interface-based Mocking: testify/mock encourages interface-based mocking, which leads to better separation of concerns and more flexible test design.

3. Maintenance & Activity: stretchr/testify is one of the most widely used Go testing libraries with active maintenance.

4. Type Safety: Interface-based mocking provides compile-time type checking, whereas HTTP-level mocking relies on runtime matching.

5. Reduced Dependencies: One less external dependency to maintain

Proposed solution

Replace HTTP-level mocking with interface-based mocking using testify/mock:

Phase 1: Create Mock Interfaces

1. Define interfaces for GitHub client operations (if not already present)
2. Create mock implementations using testify/mock
3. Update the codebase to depend on interfaces rather than concrete clients

Phase 2: Migrate Tests Incrementally

1. Start with a single test file to establish patterns
2. Create helper functions for common mock setups
3. Migrate remaining test files one at a time
4. Remove go-github-mock dependency when complete

Example Migration

Before (HTTP-level mocking):

mockedClient := mock.NewMockedHTTPClient(
    mock.WithRequestMatch(
        mock.GetReposIssuesByOwnerByRepoByIssueNumber,
        mockIssue,
    ),
)
client := github.NewClient(mockedClient)

After (Interface-based mocking):

mockClient := new(MockGitHubClient)
mockClient.On("GetIssue", ctx, "owner", "repo", 42).Return(mockIssue, nil)

Related

• stretchr/testify documentation
• Current testing guidelines: docs/testing.md
  </issue_description>

<agent_instructions>You are working in the github/github-mcp-server repository.

1. Understand the issue first

Read issue #1492 carefully, including comments.

Identify:

• The expected behavior
• The current behavior
• Which MCP tools / code paths are affected
• If anything is ambiguous, infer the most conservative and backward-compatible fix.

2. Scope the change
   Keep the change minimal and focused on this issue.
   Avoid refactors unless strictly necessary.
   Do not change tool schemas or behavior unless required by the issue.

3. Implement the fix

Make the required code changes.
Follow existing patterns in the codebase.
Ensure behavior is safe for agent usage (pagination, filtering, limits, auth).

4. Run checks locally

Before opening the PR, run:

• ./script/lint
• ./script/test

If either command fails or cannot be run, explain why in the PR.

5. Open a pull request

Open a PR from your branch to main.

Use the repository PR template and fill all sections.

Keep answers short and concrete.

When filling the PR template:

Summary: 1–2 sentences describing the fix

Why: Reference issue #1492

What changed: Bullet list of code changes

MCP impact: Explicitly state whether tools or APIs changed

Security / limits: Confirm considerations

Lint & tests: Check boxes based on what you ran

6. Quality bar
   Code compiles
   Tests pass
   No unrelated changes
   PR description is complete and honest
   If something cannot be completed, clearly explain it in the PR description.</agent_instructions>

Comments on the Issue (you are @copilot in this section)

• Fixes Chore: replace migueleliasweb/go-github-mock with stretchr/testify/mock #1492

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[JoannaaKL]

@copilot remove this file - docs/testing-migration.md. We don't want to commit it.

[Copilot]

@copilot remove this file - docs/testing-migration.md. We don't want to commit it.

Done in f4f426b

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[SamMorrowDrums]

I like this, but is there value in defining github API route constances somewhere and sharing them versus writing them out per test?

Maybe, just a passing thought. The new pattern is otherwise clean and nice I feel.

[JoannaaKL]

@copilot don't add a path constant, reuse mock.GetReposCodeScanningAlertsByOwnerByRepoByAlertNumber.Pattern. Do it in other tests too

[Copilot]

Done in 9e403f5. Updated both code_scanning_test.go and git_test.go to reuse mock.EndpointPattern constants instead of hardcoded paths.

[JoannaaKL]

@copilot sorry for the confusion, revert previous changes. You need to define constants in this repo and not reuse them from migueleliasweb/go-github-mock. In the end the goal is to stop using this repo for mocks.

[Copilot]

Done in b96a791. I've reverted the previous changes and now define the endpoint path constants in helper_test.go within this repo instead of reusing them from go-github-mock. This prepares for the eventual removal of the go-github-mock dependency.