Skip to content

Partial migration from go-github-mock to testify-based mocks #1629

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.

Sign up for GitHub

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

Merged
SamMorrowDrums merged 16 commits into from
Dec 18, 2025
+783 −917
Merged

Partial migration from go-github-mock to testify-based mocks #1629

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
35cc9ba
Initial plan
Copilot Dec 17, 2025
637de7a
Add endpoint constants and migrate raw package tests
Copilot Dec 17, 2025
f4a8dba
Migrate security_advisories_test.go to testify mocks
Copilot Dec 17, 2025
070e14e
Migrate actions, dependabot, and secret_scanning tests to testify
Copilot Dec 17, 2025
3beeef9
Fix linting issues in helper constants
Copilot Dec 17, 2025
acf0be5
Migrate context_tools_test and repository_resource_test to testify
Copilot Dec 17, 2025
6d01033
Fix wildcard path matching for raw content endpoints
Copilot Dec 17, 2025
0e32603
Migrate gists_test.go to testify mocks
Copilot Dec 17, 2025
2998841
Add comprehensive migration guide for remaining files
Copilot Dec 17, 2025
7cea3b5
Add completion script and enhanced migration guide
Copilot Dec 17, 2025
e359be5
Migrate notifications_test.go to testify mocks
Copilot Dec 17, 2025
0e050f1
Remove migration guide and completion script
Copilot Dec 17, 2025
669a77f
Update pkg/github/helper_test.go
JoannaaKL Dec 17, 2025
ab31d17
Merge branch 'main' into copilot/complete-go-github-mock-migration
JoannaaKL Dec 17, 2025
d1f18dc
Merge branch 'main' into copilot/complete-go-github-mock-migration
JoannaaKL Dec 18, 2025
27edb48
Merge branch 'main' into copilot/complete-go-github-mock-migration
JoannaaKL Dec 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
547 changes: 240 additions & 307 deletions pkg/github/actions_test.go
View file
Open in desktop

Large diffs are not rendered by default.

46 changes: 15 additions & 31 deletions pkg/github/context_tools_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"github.com/github/github-mcp-server/internal/toolsnaps"
"github.com/github/github-mcp-server/pkg/translations"
"github.com/google/go-github/v79/github"
"github.com/migueleliasweb/go-github-mock/src/mock"
"github.com/shurcooL/githubv4"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -57,24 +56,18 @@ func Test_GetMe(t *testing.T) {
}{
{
name: "successful get user",
mockedClient: mock.NewMockedHTTPClient(
mock.WithRequestMatch(
mock.GetUser,
mockUser,
),
),
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetUser: mockResponse(t, http.StatusOK, mockUser),
}),
requestArgs: map[string]any{},
expectToolError: false,
expectedUser: mockUser,
},
{
name: "successful get user with reason",
mockedClient: mock.NewMockedHTTPClient(
mock.WithRequestMatch(
mock.GetUser,
mockUser,
),
),
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetUser: mockResponse(t, http.StatusOK, mockUser),
}),
requestArgs: map[string]any{
"reason": "Testing API",
},
Expand All @@ -90,12 +83,9 @@ func Test_GetMe(t *testing.T) {
},
{
name: "get user fails",
mockedClient: mock.NewMockedHTTPClient(
mock.WithRequestMatchHandler(
mock.GetUser,
badRequestHandler("expected test failure"),
),
),
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetUser: badRequestHandler("expected test failure"),
}),
requestArgs: map[string]any{},
expectToolError: true,
expectedToolErrMsg: "expected test failure",
Expand Down Expand Up @@ -255,21 +245,15 @@ func Test_GetTeams(t *testing.T) {

// Factory function for mock HTTP clients with user response
httpClientWithUser := func() *http.Client {
return mock.NewMockedHTTPClient(
mock.WithRequestMatch(
mock.GetUser,
mockUser,
),
)
return MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetUser: mockResponse(t, http.StatusOK, mockUser),
})
}

httpClientUserFails := func() *http.Client {
return mock.NewMockedHTTPClient(
mock.WithRequestMatchHandler(
mock.GetUser,
badRequestHandler("expected test failure"),
),
)
return MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetUser: badRequestHandler("expected test failure"),
})
}

tests := []struct {
Expand Down
81 changes: 31 additions & 50 deletions pkg/github/dependabot_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"github.com/github/github-mcp-server/internal/toolsnaps"
"github.com/github/github-mcp-server/pkg/translations"
"github.com/google/go-github/v79/github"
"github.com/migueleliasweb/go-github-mock/src/mock"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -42,12 +41,9 @@ func Test_GetDependabotAlert(t *testing.T) {
}{
{
name: "successful alert fetch",
mockedClient: mock.NewMockedHTTPClient(
mock.WithRequestMatch(
mock.GetReposDependabotAlertsByOwnerByRepoByAlertNumber,
mockAlert,
),
),
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposDependabotAlertsByOwnerByRepoByAlertNumber: mockResponse(t, http.StatusOK, mockAlert),
}),
requestArgs: map[string]interface{}{
"owner": "owner",
"repo": "repo",
Expand All @@ -58,15 +54,12 @@ func Test_GetDependabotAlert(t *testing.T) {
},
{
name: "alert fetch fails",
mockedClient: mock.NewMockedHTTPClient(
mock.WithRequestMatchHandler(
mock.GetReposDependabotAlertsByOwnerByRepoByAlertNumber,
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusNotFound)
_, _ = w.Write([]byte(`{"message": "Not Found"}`))
}),
),
),
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposDependabotAlertsByOwnerByRepoByAlertNumber: 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",
Expand Down Expand Up @@ -154,16 +147,13 @@ func Test_ListDependabotAlerts(t *testing.T) {
}{
{
name: "successful open alerts listing",
mockedClient: mock.NewMockedHTTPClient(
mock.WithRequestMatchHandler(
mock.GetReposDependabotAlertsByOwnerByRepo,
expectQueryParams(t, map[string]string{
"state": "open",
}).andThen(
mockResponse(t, http.StatusOK, []*github.DependabotAlert{&criticalAlert}),
),
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposDependabotAlertsByOwnerByRepo: expectQueryParams(t, map[string]string{
"state": "open",
}).andThen(
mockResponse(t, http.StatusOK, []*github.DependabotAlert{&criticalAlert}),
),
),
}),
requestArgs: map[string]interface{}{
"owner": "owner",
"repo": "repo",
Expand All @@ -174,16 +164,13 @@ func Test_ListDependabotAlerts(t *testing.T) {
},
{
name: "successful severity filtered listing",
mockedClient: mock.NewMockedHTTPClient(
mock.WithRequestMatchHandler(
mock.GetReposDependabotAlertsByOwnerByRepo,
expectQueryParams(t, map[string]string{
"severity": "high",
}).andThen(
mockResponse(t, http.StatusOK, []*github.DependabotAlert{&highSeverityAlert}),
),
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposDependabotAlertsByOwnerByRepo: expectQueryParams(t, map[string]string{
"severity": "high",
}).andThen(
mockResponse(t, http.StatusOK, []*github.DependabotAlert{&highSeverityAlert}),
),
),
}),
requestArgs: map[string]interface{}{
"owner": "owner",
"repo": "repo",
Expand All @@ -194,14 +181,11 @@ func Test_ListDependabotAlerts(t *testing.T) {
},
{
name: "successful all alerts listing",
mockedClient: mock.NewMockedHTTPClient(
mock.WithRequestMatchHandler(
mock.GetReposDependabotAlertsByOwnerByRepo,
expectQueryParams(t, map[string]string{}).andThen(
mockResponse(t, http.StatusOK, []*github.DependabotAlert{&criticalAlert, &highSeverityAlert}),
),
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposDependabotAlertsByOwnerByRepo: expectQueryParams(t, map[string]string{}).andThen(
mockResponse(t, http.StatusOK, []*github.DependabotAlert{&criticalAlert, &highSeverityAlert}),
),
),
}),
requestArgs: map[string]interface{}{
"owner": "owner",
"repo": "repo",
Expand All @@ -211,15 +195,12 @@ func Test_ListDependabotAlerts(t *testing.T) {
},
{
name: "alerts listing fails",
mockedClient: mock.NewMockedHTTPClient(
mock.WithRequestMatchHandler(
mock.GetReposDependabotAlertsByOwnerByRepo,
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusUnauthorized)
_, _ = w.Write([]byte(`{"message": "Unauthorized access"}`))
}),
),
),
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposDependabotAlertsByOwnerByRepo: 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",
Expand Down
Loading