Skip to content

Commit 4ee4594

Browse files
ppofficeSamMorrowDrumsCopilot
authored
Fix e2e harness compilation against go-github v89 and go-sdk v1.7 (#3187)
* Fix e2e harness compilation against go-github v89 and go-sdk v1.7 The e2e test package no longer compiled under --tags e2e because it lagged behind two dependency migrations: - go-github v89: NewClient now returns (*Client, error) and WithEnterpriseURLs moved from a *Client method to a ClientOptionsFunc. Update getRESTClient and the inline cleanup call sites, and drop the trailing nil options arg from ListReviewers. - go-sdk v1.7: ghmcp.NewMCPServer(MCPServerConfig) was replaced by ghmcp.NewStdioMCPServer(ctx, github.MCPServerConfig). Update the in-process setupMCPClient branch accordingly, supplying the required Logger and Version fields. * fix(e2e): align client host resolution Reuse the server API host resolver so GHEC REST and upload clients target the correct subdomains. Add token-free coverage for host URL resolution and the in-process stdio server lifecycle. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: ppoffice <8849362+ppoffice@users.noreply.github.com> Co-authored-by: Sam Morrow <sammorrowdrums@github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent b8f107b commit 4ee4594

1 file changed

Lines changed: 114 additions & 15 deletions

File tree

e2e/e2e_test.go

Lines changed: 114 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"context"
77
"encoding/json"
88
"fmt"
9+
"log/slog"
910
"net/http"
1011
"os"
1112
"os/exec"
@@ -18,6 +19,7 @@ import (
1819
"github.com/github/github-mcp-server/internal/ghmcp"
1920
"github.com/github/github-mcp-server/pkg/github"
2021
"github.com/github/github-mcp-server/pkg/translations"
22+
"github.com/github/github-mcp-server/pkg/utils"
2123
gogithub "github.com/google/go-github/v89/github"
2224
"github.com/modelcontextprotocol/go-sdk/mcp"
2325
"github.com/stretchr/testify/require"
@@ -62,21 +64,116 @@ func getE2EHost() string {
6264
}
6365

6466
func getRESTClient(t *testing.T) *gogithub.Client {
65-
// Get token and ensure Docker image is built
66-
token := getE2EToken(t)
67+
ghClient, err := newRESTClient(getE2EToken(t), getE2EHost())
68+
require.NoError(t, err, "expected to create GitHub client successfully")
6769

68-
// Create a new GitHub client with the token
69-
ghClient := gogithub.NewClient(nil).WithAuthToken(token)
70+
return ghClient
71+
}
7072

71-
if host := getE2EHost(); host != "" && host != "https://github.com" {
72-
var err error
73-
// Currently this works for GHEC because the API is exposed at the api subdomain and the path prefix
74-
// but it would be preferable to extract the host parsing from the main server logic, and use it here.
75-
ghClient, err = ghClient.WithEnterpriseURLs(host, host)
76-
require.NoError(t, err, "expected to create GitHub client with host")
73+
func newRESTClient(token, host string) (*gogithub.Client, error) {
74+
apiHost, err := utils.NewAPIHost(host)
75+
if err != nil {
76+
return nil, fmt.Errorf("failed to parse API host: %w", err)
7777
}
7878

79-
return ghClient
79+
restURL, err := apiHost.BaseRESTURL(context.Background())
80+
if err != nil {
81+
return nil, fmt.Errorf("failed to get base REST URL: %w", err)
82+
}
83+
84+
uploadURL, err := apiHost.UploadURL(context.Background())
85+
if err != nil {
86+
return nil, fmt.Errorf("failed to get upload URL: %w", err)
87+
}
88+
89+
return gogithub.NewClient(
90+
gogithub.WithAuthToken(token),
91+
gogithub.WithEnterpriseURLs(restURL.String(), uploadURL.String()),
92+
)
93+
}
94+
95+
func TestRESTClientURLs(t *testing.T) {
96+
t.Parallel()
97+
98+
tests := []struct {
99+
name string
100+
host string
101+
wantBaseURL string
102+
wantUploadURL string
103+
}{
104+
{
105+
name: "dotcom default",
106+
wantBaseURL: "https://api.github.com/",
107+
wantUploadURL: "https://uploads.github.com/",
108+
},
109+
{
110+
name: "dotcom explicit",
111+
host: "https://github.com",
112+
wantBaseURL: "https://api.github.com/",
113+
wantUploadURL: "https://uploads.github.com/",
114+
},
115+
{
116+
name: "GHEC",
117+
host: "https://example.ghe.com",
118+
wantBaseURL: "https://api.example.ghe.com/",
119+
wantUploadURL: "https://uploads.example.ghe.com/",
120+
},
121+
}
122+
123+
for _, tt := range tests {
124+
t.Run(tt.name, func(t *testing.T) {
125+
t.Parallel()
126+
127+
client, err := newRESTClient("test-token", tt.host)
128+
require.NoError(t, err)
129+
require.Equal(t, tt.wantBaseURL, client.BaseURL())
130+
require.Equal(t, tt.wantUploadURL, client.UploadURL())
131+
})
132+
}
133+
}
134+
135+
func TestInProcessStdioServer(t *testing.T) {
136+
t.Parallel()
137+
138+
ctx, cancel := context.WithCancel(context.Background())
139+
t.Cleanup(cancel)
140+
141+
server, err := ghmcp.NewStdioMCPServer(ctx, github.MCPServerConfig{
142+
Version: "e2e-test",
143+
Token: "test-token",
144+
EnabledToolsets: []string{"context"},
145+
Translator: translations.NullTranslationHelper,
146+
Logger: slog.New(slog.DiscardHandler),
147+
})
148+
require.NoError(t, err)
149+
150+
serverTransport, clientTransport := mcp.NewInMemoryTransports()
151+
serverErr := make(chan error, 1)
152+
go func() {
153+
serverErr <- server.Run(ctx, serverTransport)
154+
}()
155+
156+
client := mcp.NewClient(&mcp.Implementation{
157+
Name: "e2e-test-client",
158+
Version: "0.0.1",
159+
}, nil)
160+
session, err := client.Connect(ctx, clientTransport, nil)
161+
require.NoError(t, err)
162+
t.Cleanup(func() { _ = session.Close() })
163+
164+
tools, err := session.ListTools(ctx, nil)
165+
require.NoError(t, err)
166+
require.True(t, slices.ContainsFunc(tools.Tools, func(tool *mcp.Tool) bool {
167+
return tool.Name == "get_me"
168+
}))
169+
170+
require.NoError(t, session.Close())
171+
select {
172+
case err := <-serverErr:
173+
require.NoError(t, err)
174+
case <-time.After(time.Second):
175+
t.Fatal("timed out waiting for the in-process MCP server to stop")
176+
}
80177
}
81178

82179
// waitForRateLimit checks the current rate limit and waits if necessary.
@@ -221,11 +318,13 @@ func setupMCPClient(t *testing.T, options ...clientOption) *mcp.ClientSession {
221318
enabledToolsets = github.GetDefaultToolsetIDs()
222319
}
223320

224-
ghServer, err := ghmcp.NewMCPServer(ghmcp.MCPServerConfig{
321+
ghServer, err := ghmcp.NewStdioMCPServer(ctx, github.MCPServerConfig{
322+
Version: "e2e-test",
225323
Token: token,
226324
EnabledToolsets: enabledToolsets,
227325
Host: getE2EHost(),
228326
Translator: translations.NullTranslationHelper,
327+
Logger: slog.New(slog.DiscardHandler),
229328
})
230329
require.NoError(t, err, "expected to construct MCP server successfully")
231330

@@ -968,7 +1067,7 @@ func TestRequestCopilotReview(t *testing.T) {
9681067
// Cleanup the repository after the test
9691068
t.Cleanup(func() {
9701069
// MCP Server doesn't support deletions, but we can use the GitHub Client
971-
ghClient := gogithub.NewClient(nil).WithAuthToken(getE2EToken(t))
1070+
ghClient := getRESTClient(t)
9721071
t.Logf("Deleting repository %s/%s...", currentOwner, repoName)
9731072
_, err := ghClient.Repositories.Delete(context.Background(), currentOwner, repoName)
9741073
require.NoError(t, err, "expected to delete repository successfully")
@@ -1063,9 +1162,9 @@ func TestRequestCopilotReview(t *testing.T) {
10631162

10641163
// Finally, get requested reviews and see copilot is in there
10651164
// MCP Server doesn't support requesting reviews yet, but we can use the GitHub Client
1066-
ghClient := gogithub.NewClient(nil).WithAuthToken(getE2EToken(t))
1165+
ghClient := getRESTClient(t)
10671166
t.Logf("Getting reviews for pull request in %s/%s...", currentOwner, repoName)
1068-
reviewRequests, _, err := ghClient.PullRequests.ListReviewers(context.Background(), currentOwner, repoName, 1, nil)
1167+
reviewRequests, _, err := ghClient.PullRequests.ListReviewers(context.Background(), currentOwner, repoName, 1)
10691168
require.NoError(t, err, "expected to get review requests successfully")
10701169

10711170
// Check if Copilot was added as a reviewer - skip if not available

0 commit comments

Comments
 (0)