Skip to content

Commit db34a0c

Browse files
omgitsadsomgitsads
committed
Fix the broken tests after our HTTP stack refactor
1 parent 32d2342 commit db34a0c

File tree

5 files changed

+117
-118
lines changed

5 files changed

+117
-118
lines changed

‎internal/ghmcp/server.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ func NewStdioMCPServer(cfg github.MCPServerConfig) (*mcp.Server, error) {
9999
cfg.ContentWindowSize,
100100
)
101101

102-
ghServer, err := github.NewMcpServer(&cfg, deps)
102+
ghServer, err := github.NewMCPServer(&cfg, deps)
103103
if err != nil {
104104
return nil, fmt.Errorf("failed to create GitHub MCP server: %w", err)
105105
}

‎internal/ghmcp/server_test.go‎

Lines changed: 0 additions & 111 deletions
Original file line numberDiff line numberDiff line change
@@ -1,112 +1 @@
11
package ghmcp
2-
3-
import (
4-
"testing"
5-
6-
"github.com/github/github-mcp-server/pkg/translations"
7-
"github.com/stretchr/testify/assert"
8-
"github.com/stretchr/testify/require"
9-
)
10-
11-
// TestNewMCPServer_CreatesSuccessfully verifies that the server can be created
12-
// with the deps injection middleware properly configured.
13-
func TestNewMCPServer_CreatesSuccessfully(t *testing.T) {
14-
t.Parallel()
15-
16-
// Create a minimal server configuration
17-
cfg := MCPServerConfig{
18-
Version: "test",
19-
Host: "", // defaults to github.com
20-
Token: "test-token",
21-
EnabledToolsets: []string{"context"},
22-
ReadOnly: false,
23-
Translator: translations.NullTranslationHelper,
24-
ContentWindowSize: 5000,
25-
LockdownMode: false,
26-
}
27-
28-
// Create the server
29-
server, err := NewMCPServer(cfg)
30-
require.NoError(t, err, "expected server creation to succeed")
31-
require.NotNil(t, server, "expected server to be non-nil")
32-
33-
// The fact that the server was created successfully indicates that:
34-
// 1. The deps injection middleware is properly added
35-
// 2. Tools can be registered without panicking
36-
//
37-
// If the middleware wasn't properly added, tool calls would panic with
38-
// "ToolDependencies not found in context" when executed.
39-
//
40-
// The actual middleware functionality and tool execution with ContextWithDeps
41-
// is already tested in pkg/github/*_test.go.
42-
}
43-
44-
// TestResolveEnabledToolsets verifies the toolset resolution logic.
45-
func TestResolveEnabledToolsets(t *testing.T) {
46-
t.Parallel()
47-
48-
tests := []struct {
49-
name string
50-
cfg MCPServerConfig
51-
expectedResult []string
52-
}{
53-
{
54-
name: "nil toolsets without dynamic mode and no tools - use defaults",
55-
cfg: MCPServerConfig{
56-
EnabledToolsets: nil,
57-
DynamicToolsets: false,
58-
EnabledTools: nil,
59-
},
60-
expectedResult: nil, // nil means "use defaults"
61-
},
62-
{
63-
name: "nil toolsets with dynamic mode - start empty",
64-
cfg: MCPServerConfig{
65-
EnabledToolsets: nil,
66-
DynamicToolsets: true,
67-
EnabledTools: nil,
68-
},
69-
expectedResult: []string{}, // empty slice means no toolsets
70-
},
71-
{
72-
name: "explicit toolsets",
73-
cfg: MCPServerConfig{
74-
EnabledToolsets: []string{"repos", "issues"},
75-
DynamicToolsets: false,
76-
},
77-
expectedResult: []string{"repos", "issues"},
78-
},
79-
{
80-
name: "empty toolsets - disable all",
81-
cfg: MCPServerConfig{
82-
EnabledToolsets: []string{},
83-
DynamicToolsets: false,
84-
},
85-
expectedResult: []string{}, // empty slice means no toolsets
86-
},
87-
{
88-
name: "specific tools without toolsets - no default toolsets",
89-
cfg: MCPServerConfig{
90-
EnabledToolsets: nil,
91-
DynamicToolsets: false,
92-
EnabledTools: []string{"get_me"},
93-
},
94-
expectedResult: []string{}, // empty slice when tools specified but no toolsets
95-
},
96-
{
97-
name: "dynamic mode with explicit toolsets removes all and default",
98-
cfg: MCPServerConfig{
99-
EnabledToolsets: []string{"all", "repos"},
100-
DynamicToolsets: true,
101-
},
102-
expectedResult: []string{"repos"}, // "all" is removed in dynamic mode
103-
},
104-
}
105-
106-
for _, tc := range tests {
107-
t.Run(tc.name, func(t *testing.T) {
108-
result := resolveEnabledToolsets(tc.cfg)
109-
assert.Equal(t, tc.expectedResult, result)
110-
})
111-
}
112-
}

‎pkg/github/server.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ type MCPServerConfig struct {
6666
TokenScopes []string
6767
}
6868

69-
func NewMcpServer(cfg *MCPServerConfig, deps ToolDependencies) (*mcp.Server, error) {
69+
func NewMCPServer(cfg *MCPServerConfig, deps ToolDependencies) (*mcp.Server, error) {
7070
enabledToolsets := resolveEnabledToolsets(cfg)
7171

7272
// For instruction generation, we need actual toolset names (not nil).

‎pkg/github/server_test.go‎

Lines changed: 114 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,16 @@ import (
66
"errors"
77
"fmt"
88
"net/http"
9+
"testing"
910
"time"
1011

1112
"github.com/github/github-mcp-server/pkg/lockdown"
1213
"github.com/github/github-mcp-server/pkg/raw"
1314
"github.com/github/github-mcp-server/pkg/translations"
1415
"github.com/google/go-github/v79/github"
1516
"github.com/shurcooL/githubv4"
17+
"github.com/stretchr/testify/assert"
18+
"github.com/stretchr/testify/require"
1619
)
1720

1821
// stubDeps is a test helper that implements ToolDependencies with configurable behavior.
@@ -49,10 +52,12 @@ func (s stubDeps) GetRawClient(ctx context.Context) (*raw.Client, error) {
4952
return nil, nil
5053
}
5154

52-
func (s stubDeps) GetRepoAccessCache() *lockdown.RepoAccessCache { return s.repoAccessCache }
53-
func (s stubDeps) GetT() translations.TranslationHelperFunc { return s.t }
54-
func (s stubDeps) GetFlags() FeatureFlags { return s.flags }
55-
func (s stubDeps) GetContentWindowSize() int { return s.contentWindowSize }
55+
func (s stubDeps) GetRepoAccessCache(ctx context.Context) (*lockdown.RepoAccessCache, error) {
56+
return s.repoAccessCache, nil
57+
}
58+
func (s stubDeps) GetT() translations.TranslationHelperFunc { return s.t }
59+
func (s stubDeps) GetFlags() FeatureFlags { return s.flags }
60+
func (s stubDeps) GetContentWindowSize() int { return s.contentWindowSize }
5661

5762
// Helper functions to create stub client functions for error testing
5863
func stubClientFnFromHTTP(httpClient *http.Client) func(context.Context) (*github.Client, error) {
@@ -98,3 +103,108 @@ func badRequestHandler(msg string) http.HandlerFunc {
98103
http.Error(w, string(b), http.StatusBadRequest)
99104
}
100105
}
106+
107+
// TestNewMCPServer_CreatesSuccessfully verifies that the server can be created
108+
// with the deps injection middleware properly configured.
109+
func TestNewMCPServer_CreatesSuccessfully(t *testing.T) {
110+
t.Parallel()
111+
112+
// Create a minimal server configuration
113+
cfg := MCPServerConfig{
114+
Version: "test",
115+
Host: "", // defaults to github.com
116+
Token: "test-token",
117+
EnabledToolsets: []string{"context"},
118+
ReadOnly: false,
119+
Translator: translations.NullTranslationHelper,
120+
ContentWindowSize: 5000,
121+
LockdownMode: false,
122+
}
123+
124+
deps := stubDeps{}
125+
126+
// Create the server
127+
server, err := NewMCPServer(&cfg, deps)
128+
require.NoError(t, err, "expected server creation to succeed")
129+
require.NotNil(t, server, "expected server to be non-nil")
130+
131+
// The fact that the server was created successfully indicates that:
132+
// 1. The deps injection middleware is properly added
133+
// 2. Tools can be registered without panicking
134+
//
135+
// If the middleware wasn't properly added, tool calls would panic with
136+
// "ToolDependencies not found in context" when executed.
137+
//
138+
// The actual middleware functionality and tool execution with ContextWithDeps
139+
// is already tested in pkg/github/*_test.go.
140+
}
141+
142+
// TestResolveEnabledToolsets verifies the toolset resolution logic.
143+
func TestResolveEnabledToolsets(t *testing.T) {
144+
t.Parallel()
145+
146+
tests := []struct {
147+
name string
148+
cfg MCPServerConfig
149+
expectedResult []string
150+
}{
151+
{
152+
name: "nil toolsets without dynamic mode and no tools - use defaults",
153+
cfg: MCPServerConfig{
154+
EnabledToolsets: nil,
155+
DynamicToolsets: false,
156+
EnabledTools: nil,
157+
},
158+
expectedResult: nil, // nil means "use defaults"
159+
},
160+
{
161+
name: "nil toolsets with dynamic mode - start empty",
162+
cfg: MCPServerConfig{
163+
EnabledToolsets: nil,
164+
DynamicToolsets: true,
165+
EnabledTools: nil,
166+
},
167+
expectedResult: []string{}, // empty slice means no toolsets
168+
},
169+
{
170+
name: "explicit toolsets",
171+
cfg: MCPServerConfig{
172+
EnabledToolsets: []string{"repos", "issues"},
173+
DynamicToolsets: false,
174+
},
175+
expectedResult: []string{"repos", "issues"},
176+
},
177+
{
178+
name: "empty toolsets - disable all",
179+
cfg: MCPServerConfig{
180+
EnabledToolsets: []string{},
181+
DynamicToolsets: false,
182+
},
183+
expectedResult: []string{}, // empty slice means no toolsets
184+
},
185+
{
186+
name: "specific tools without toolsets - no default toolsets",
187+
cfg: MCPServerConfig{
188+
EnabledToolsets: nil,
189+
DynamicToolsets: false,
190+
EnabledTools: []string{"get_me"},
191+
},
192+
expectedResult: []string{}, // empty slice when tools specified but no toolsets
193+
},
194+
{
195+
name: "dynamic mode with explicit toolsets removes all and default",
196+
cfg: MCPServerConfig{
197+
EnabledToolsets: []string{"all", "repos"},
198+
DynamicToolsets: true,
199+
},
200+
expectedResult: []string{"repos"}, // "all" is removed in dynamic mode
201+
},
202+
}
203+
204+
for _, tc := range tests {
205+
t.Run(tc.name, func(t *testing.T) {
206+
result := resolveEnabledToolsets(&tc.cfg)
207+
assert.Equal(t, tc.expectedResult, result)
208+
})
209+
}
210+
}

‎pkg/http/handler.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func (s *HttpMcpHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
4848
s.config.ContentWindowSize,
4949
)
5050

51-
ghServer, err := github.NewMcpServer(&github.MCPServerConfig{
51+
ghServer, err := github.NewMCPServer(&github.MCPServerConfig{
5252
Version: s.config.Version,
5353
Host: s.config.Host,
5454
EnabledToolsets: s.config.EnabledToolsets,

0 commit comments

Comments
 (0)