Skip to content

Commit baaab83

Browse files
tonytrgtonytrg
committed
add better test
1 parent 03f7909 commit baaab83

File tree

4 files changed

+89
-51
lines changed

4 files changed

+89
-51
lines changed

‎pkg/github/dependencies.go‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,6 @@ type ToolDependencies interface {
8181
GetContentWindowSize() int
8282

8383
// IsFeatureEnabled checks if a feature flag is enabled.
84-
// Returns false if checker is nil or flag is not enabled.
85-
// This allows tools to conditionally change behavior based on feature flags.
8684
IsFeatureEnabled(ctx context.Context, flagName string) bool
8785
}
8886

‎pkg/github/dependencies_test.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ func TestIsFeatureEnabled_EmptyFlagName(t *testing.T) {
6363
t.Parallel()
6464

6565
// Create a feature checker
66-
checker := func(_ context.Context, flagName string) (bool, error) {
66+
checker := func(_ context.Context, _ string) (bool, error) {
6767
return true, nil
6868
}
6969

@@ -87,7 +87,7 @@ func TestIsFeatureEnabled_CheckerError(t *testing.T) {
8787
t.Parallel()
8888

8989
// Create a feature checker that returns an error
90-
checker := func(_ context.Context, flagName string) (bool, error) {
90+
checker := func(_ context.Context, _ string) (bool, error) {
9191
return false, errors.New("checker error")
9292
}
9393

@@ -106,4 +106,3 @@ func TestIsFeatureEnabled_CheckerError(t *testing.T) {
106106
result := deps.IsFeatureEnabled(context.Background(), "error_flag")
107107
assert.False(t, result, "Expected false when checker returns error")
108108
}
109-

‎pkg/github/hello_test.go‎

Lines changed: 83 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@ package github_test
22

33
import (
44
"context"
5+
"encoding/json"
56
"testing"
67

78
"github.com/github/github-mcp-server/pkg/github"
89
"github.com/github/github-mcp-server/pkg/translations"
10+
"github.com/modelcontextprotocol/go-sdk/mcp"
911
"github.com/stretchr/testify/assert"
1012
"github.com/stretchr/testify/require"
1113
)
@@ -34,50 +36,89 @@ func TestHelloWorld_ToolDefinition(t *testing.T) {
3436
assert.Empty(t, tool.FeatureFlagDisable)
3537
}
3638

37-
func TestHelloWorld_IsFeatureEnabledIntegration(t *testing.T) {
39+
func TestHelloWorld_ConditionalBehavior(t *testing.T) {
3840
t.Parallel()
3941

40-
// This test verifies that the feature flag checking mechanism works
41-
// by testing deps.IsFeatureEnabled directly
42-
43-
// Test 1: With feature flag disabled
44-
checkerDisabled := func(ctx context.Context, flagName string) (bool, error) {
45-
return false, nil
46-
}
47-
depsDisabled := github.NewBaseDeps(
48-
nil, nil, nil, nil,
49-
translations.NullTranslationHelper,
50-
github.FeatureFlags{},
51-
0,
52-
checkerDisabled,
53-
)
54-
55-
result := depsDisabled.IsFeatureEnabled(context.Background(), github.RemoteMCPExperimental)
56-
assert.False(t, result, "Feature flag should be disabled")
57-
58-
// Test 2: With feature flag enabled
59-
checkerEnabled := func(ctx context.Context, flagName string) (bool, error) {
60-
return flagName == github.RemoteMCPExperimental, nil
42+
tests := []struct {
43+
name string
44+
featureFlagEnabled bool
45+
inputName string
46+
expectedGreeting string
47+
expectedExperimentalMode bool
48+
}{
49+
{
50+
name: "Feature flag disabled - default greeting",
51+
featureFlagEnabled: false,
52+
inputName: "Alice",
53+
expectedGreeting: "Hello, Alice!",
54+
expectedExperimentalMode: false,
55+
},
56+
{
57+
name: "Feature flag enabled - experimental greeting",
58+
featureFlagEnabled: true,
59+
inputName: "Alice",
60+
expectedGreeting: "🚀 Hello, Alice! Welcome to the EXPERIMENTAL future of MCP! 🎉",
61+
expectedExperimentalMode: true,
62+
},
6163
}
62-
depsEnabled := github.NewBaseDeps(
63-
nil, nil, nil, nil,
64-
translations.NullTranslationHelper,
65-
github.FeatureFlags{},
66-
0,
67-
checkerEnabled,
68-
)
69-
70-
result = depsEnabled.IsFeatureEnabled(context.Background(), github.RemoteMCPExperimental)
71-
assert.True(t, result, "Feature flag should be enabled")
72-
73-
result = depsEnabled.IsFeatureEnabled(context.Background(), "other_flag")
74-
assert.False(t, result, "Other flag should be disabled")
75-
}
7664

77-
func TestHelloWorld_FeatureFlagConstant(t *testing.T) {
78-
t.Parallel()
79-
80-
// Verify the constant exists and has the expected value
81-
assert.Equal(t, "remote_mcp_experimental", github.RemoteMCPExperimental)
82-
require.NotEmpty(t, github.RemoteMCPExperimental, "Feature flag constant should not be empty")
65+
for _, tt := range tests {
66+
t.Run(tt.name, func(t *testing.T) {
67+
t.Parallel()
68+
69+
// Create feature checker based on test case
70+
checker := func(_ context.Context, flagName string) (bool, error) {
71+
if flagName == github.RemoteMCPExperimental {
72+
return tt.featureFlagEnabled, nil
73+
}
74+
return false, nil
75+
}
76+
77+
// Create deps with the checker
78+
deps := github.NewBaseDeps(
79+
nil, nil, nil, nil,
80+
translations.NullTranslationHelper,
81+
github.FeatureFlags{},
82+
0,
83+
checker,
84+
)
85+
86+
// Get the tool and its handler
87+
tool := github.HelloWorld(translations.NullTranslationHelper)
88+
handler := tool.Handler(deps)
89+
90+
// Create request
91+
args := map[string]any{}
92+
if tt.inputName != "" {
93+
args["name"] = tt.inputName
94+
}
95+
argsJSON, err := json.Marshal(args)
96+
require.NoError(t, err)
97+
98+
request := mcp.CallToolRequest{
99+
Params: &mcp.CallToolParamsRaw{
100+
Arguments: json.RawMessage(argsJSON),
101+
},
102+
}
103+
104+
// Call the handler with deps in context
105+
ctx := github.ContextWithDeps(context.Background(), deps)
106+
result, err := handler(ctx, &request)
107+
require.NoError(t, err)
108+
require.NotNil(t, result)
109+
require.Len(t, result.Content, 1)
110+
111+
// Parse the response - should be TextContent
112+
textContent, ok := result.Content[0].(*mcp.TextContent)
113+
require.True(t, ok, "expected content to be TextContent")
114+
115+
var response map[string]any
116+
err = json.Unmarshal([]byte(textContent.Text), &response)
117+
require.NoError(t, err)
118+
119+
// Verify the greeting matches expected based on feature flag
120+
assert.Equal(t, tt.expectedGreeting, response["greeting"])
121+
assert.Equal(t, tt.expectedExperimentalMode, response["experimental_mode"])
122+
})
123+
}
83124
}

‎pkg/github/server_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,10 @@ func (s stubDeps) GetRawClient(ctx context.Context) (*raw.Client, error) {
5151
return nil, nil
5252
}
5353

54-
func (s stubDeps) GetRepoAccessCache() *lockdown.RepoAccessCache { return s.repoAccessCache }
55-
func (s stubDeps) GetT() translations.TranslationHelperFunc { return s.t }
56-
func (s stubDeps) GetFlags() FeatureFlags { return s.flags }
57-
func (s stubDeps) GetContentWindowSize() int { return s.contentWindowSize }
54+
func (s stubDeps) GetRepoAccessCache() *lockdown.RepoAccessCache { return s.repoAccessCache }
55+
func (s stubDeps) GetT() translations.TranslationHelperFunc { return s.t }
56+
func (s stubDeps) GetFlags() FeatureFlags { return s.flags }
57+
func (s stubDeps) GetContentWindowSize() int { return s.contentWindowSize }
5858
func (s stubDeps) IsFeatureEnabled(_ context.Context, _ string) bool { return false }
5959

6060
// Helper functions to create stub client functions for error testing

0 commit comments

Comments
 (0)