Skip to content

Commit 4dfc946

Browse files
kerobbikerobbi
committed
improve tests
1 parent 227f73e commit 4dfc946

File tree

3 files changed

+26
-47
lines changed

3 files changed

+26
-47
lines changed

‎pkg/github/issues_test.go‎

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313

1414
"github.com/github/github-mcp-server/internal/githubv4mock"
1515
"github.com/github/github-mcp-server/internal/toolsnaps"
16-
"github.com/github/github-mcp-server/pkg/lockdown"
1716
"github.com/github/github-mcp-server/pkg/translations"
1817
"github.com/google/go-github/v82/github"
1918
"github.com/google/jsonschema-go/jsonschema"
@@ -228,19 +227,16 @@ func Test_GetIssue(t *testing.T) {
228227
t.Run(tc.name, func(t *testing.T) {
229228
client := github.NewClient(tc.mockedClient)
230229

231-
gqlClient := defaultGQLClient
232-
var cache *lockdown.RepoAccessCache
230+
var restClient *github.Client
233231
if tc.restPermission != "" {
234-
restClient := mockRESTPermissionServer(t, tc.restPermission, nil)
235-
cache = stubLockdownCache(t, restClient, 15*time.Minute)
236-
} else {
237-
cache = stubRepoAccessCache(gqlClient, 15*time.Minute)
232+
restClient = mockRESTPermissionServer(t, tc.restPermission, nil)
238233
}
234+
cache := stubRepoAccessCache(restClient, 15*time.Minute)
239235

240236
flags := stubFeatureFlags(map[string]bool{"lockdown-mode": tc.lockdownEnabled})
241237
deps := BaseDeps{
242238
Client: client,
243-
GQLClient: gqlClient,
239+
GQLClient: defaultGQLClient,
244240
RepoAccessCache: cache,
245241
Flags: flags,
246242
}
@@ -2011,21 +2007,18 @@ func Test_GetIssueComments(t *testing.T) {
20112007
t.Run(tc.name, func(t *testing.T) {
20122008
// Setup client with mock
20132009
client := github.NewClient(tc.mockedClient)
2014-
gqlClient := defaultGQLClient
2015-
var cache *lockdown.RepoAccessCache
2010+
var restClient *github.Client
20162011
if tc.lockdownEnabled {
2017-
restClient := mockRESTPermissionServer(t, "read", map[string]string{
2012+
restClient = mockRESTPermissionServer(t, "read", map[string]string{
20182013
"maintainer": "write",
20192014
"testuser": "read",
20202015
})
2021-
cache = stubLockdownCache(t, restClient, 15*time.Minute)
2022-
} else {
2023-
cache = stubRepoAccessCache(gqlClient, 15*time.Minute)
20242016
}
2017+
cache := stubRepoAccessCache(restClient, 15*time.Minute)
20252018
flags := stubFeatureFlags(map[string]bool{"lockdown-mode": tc.lockdownEnabled})
20262019
deps := BaseDeps{
20272020
Client: client,
2028-
GQLClient: gqlClient,
2021+
GQLClient: defaultGQLClient,
20292022
RepoAccessCache: cache,
20302023
Flags: flags,
20312024
}
@@ -2146,7 +2139,7 @@ func Test_GetIssueLabels(t *testing.T) {
21462139
deps := BaseDeps{
21472140
Client: client,
21482141
GQLClient: gqlClient,
2149-
RepoAccessCache: stubRepoAccessCache(gqlClient, 15*time.Minute),
2142+
RepoAccessCache: stubRepoAccessCache(nil, 15*time.Minute),
21502143
Flags: stubFeatureFlags(map[string]bool{"lockdown-mode": false}),
21512144
}
21522145
handler := serverTool.Handler(deps)
@@ -2575,7 +2568,7 @@ func Test_GetSubIssues(t *testing.T) {
25752568
deps := BaseDeps{
25762569
Client: client,
25772570
GQLClient: gqlClient,
2578-
RepoAccessCache: stubRepoAccessCache(gqlClient, 15*time.Minute),
2571+
RepoAccessCache: stubRepoAccessCache(nil, 15*time.Minute),
25792572
Flags: stubFeatureFlags(map[string]bool{"lockdown-mode": false}),
25802573
}
25812574
handler := serverTool.Handler(deps)

‎pkg/github/pullrequests_test.go‎

Lines changed: 11 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99

1010
"github.com/github/github-mcp-server/internal/githubv4mock"
1111
"github.com/github/github-mcp-server/internal/toolsnaps"
12-
"github.com/github/github-mcp-server/pkg/lockdown"
1312
"github.com/github/github-mcp-server/pkg/translations"
1413
"github.com/google/go-github/v82/github"
1514
"github.com/google/jsonschema-go/jsonschema"
@@ -101,7 +100,7 @@ func Test_GetPullRequest(t *testing.T) {
101100
deps := BaseDeps{
102101
Client: client,
103102
GQLClient: gqlClient,
104-
RepoAccessCache: stubRepoAccessCache(gqlClient, 5*time.Minute),
103+
RepoAccessCache: stubRepoAccessCache(nil, 5*time.Minute),
105104
Flags: stubFeatureFlags(map[string]bool{"lockdown-mode": false}),
106105
}
107106
handler := serverTool.Handler(deps)
@@ -1202,7 +1201,7 @@ func Test_GetPullRequestFiles(t *testing.T) {
12021201
serverTool := PullRequestRead(translations.NullTranslationHelper)
12031202
deps := BaseDeps{
12041203
Client: client,
1205-
RepoAccessCache: stubRepoAccessCache(githubv4.NewClient(githubv4mock.NewMockedHTTPClient()), 5*time.Minute),
1204+
RepoAccessCache: stubRepoAccessCache(nil, 5*time.Minute),
12061205
Flags: stubFeatureFlags(map[string]bool{"lockdown-mode": false}),
12071206
}
12081207
handler := serverTool.Handler(deps)
@@ -1362,7 +1361,7 @@ func Test_GetPullRequestStatus(t *testing.T) {
13621361
serverTool := PullRequestRead(translations.NullTranslationHelper)
13631362
deps := BaseDeps{
13641363
Client: client,
1365-
RepoAccessCache: stubRepoAccessCache(githubv4.NewClient(nil), 5*time.Minute),
1364+
RepoAccessCache: stubRepoAccessCache(nil, 5*time.Minute),
13661365
Flags: stubFeatureFlags(map[string]bool{"lockdown-mode": false}),
13671366
}
13681367
handler := serverTool.Handler(deps)
@@ -1518,7 +1517,7 @@ func Test_GetPullRequestCheckRuns(t *testing.T) {
15181517
serverTool := PullRequestRead(translations.NullTranslationHelper)
15191518
deps := BaseDeps{
15201519
Client: client,
1521-
RepoAccessCache: stubRepoAccessCache(githubv4.NewClient(nil), 5*time.Minute),
1520+
RepoAccessCache: stubRepoAccessCache(nil, 5*time.Minute),
15221521
Flags: stubFeatureFlags(map[string]bool{"lockdown-mode": false}),
15231522
}
15241523
handler := serverTool.Handler(deps)
@@ -1937,17 +1936,15 @@ func Test_GetPullRequestComments(t *testing.T) {
19371936
}
19381937

19391938
// Setup cache for lockdown mode
1940-
var cache *lockdown.RepoAccessCache
1939+
var restClient *github.Client
19411940
if tc.lockdownEnabled {
1942-
restClient := mockRESTPermissionServer(t, "read", map[string]string{
1941+
restClient = mockRESTPermissionServer(t, "read", map[string]string{
19431942
"maintainer": "write",
19441943
"external-user": "read",
19451944
"testuser": "read",
19461945
})
1947-
cache = stubLockdownCache(t, restClient, 5*time.Minute)
1948-
} else {
1949-
cache = stubRepoAccessCache(gqlClient, 5*time.Minute)
19501946
}
1947+
cache := stubRepoAccessCache(restClient, 5*time.Minute)
19511948

19521949
flags := stubFeatureFlags(map[string]bool{"lockdown-mode": tc.lockdownEnabled})
19531950
serverTool := PullRequestRead(translations.NullTranslationHelper)
@@ -2111,20 +2108,14 @@ func Test_GetPullRequestReviews(t *testing.T) {
21112108
t.Run(tc.name, func(t *testing.T) {
21122109
// Setup client with mock
21132110
client := github.NewClient(tc.mockedClient)
2114-
gqlClient := defaultGQLClient
2115-
if tc.gqlHTTPClient != nil {
2116-
gqlClient = githubv4.NewClient(tc.gqlHTTPClient)
2117-
}
2118-
var cache *lockdown.RepoAccessCache
2111+
var restClient *github.Client
21192112
if tc.lockdownEnabled {
2120-
restClient := mockRESTPermissionServer(t, "read", map[string]string{
2113+
restClient = mockRESTPermissionServer(t, "read", map[string]string{
21212114
"maintainer": "write",
21222115
"testuser": "read",
21232116
})
2124-
cache = stubLockdownCache(t, restClient, 5*time.Minute)
2125-
} else {
2126-
cache = stubRepoAccessCache(gqlClient, 5*time.Minute)
21272117
}
2118+
cache := stubRepoAccessCache(restClient, 5*time.Minute)
21282119
flags := stubFeatureFlags(map[string]bool{"lockdown-mode": tc.lockdownEnabled})
21292120
serverTool := PullRequestRead(translations.NullTranslationHelper)
21302121
deps := BaseDeps{
@@ -3359,7 +3350,7 @@ index 5d6e7b2..8a4f5c3 100644
33593350
serverTool := PullRequestRead(translations.NullTranslationHelper)
33603351
deps := BaseDeps{
33613352
Client: client,
3362-
RepoAccessCache: stubRepoAccessCache(githubv4.NewClient(nil), 5*time.Minute),
3353+
RepoAccessCache: stubRepoAccessCache(nil, 5*time.Minute),
33633354
Flags: stubFeatureFlags(map[string]bool{"lockdown-mode": false}),
33643355
}
33653356
handler := serverTool.Handler(deps)

‎pkg/github/server_test.go‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -98,13 +98,7 @@ func stubGQLClientFnErr(errMsg string) func(context.Context) (*githubv4.Client,
9898
}
9999
}
100100

101-
func stubRepoAccessCache(client *githubv4.Client, ttl time.Duration) *lockdown.RepoAccessCache {
102-
cacheName := fmt.Sprintf("repo-access-cache-test-%d", time.Now().UnixNano())
103-
return lockdown.NewRepoAccessCache(client, nil, lockdown.WithTTL(ttl), lockdown.WithCacheName(cacheName))
104-
}
105-
106-
func stubLockdownCache(t *testing.T, restClient *gogithub.Client, ttl time.Duration) *lockdown.RepoAccessCache {
107-
t.Helper()
101+
func stubRepoAccessCache(restClient *gogithub.Client, ttl time.Duration) *lockdown.RepoAccessCache {
108102
cacheName := fmt.Sprintf("repo-access-cache-test-%d", time.Now().UnixNano())
109103
return lockdown.NewRepoAccessCache(
110104
githubv4.NewClient(newRepoAccessHTTPClient()),
@@ -124,10 +118,11 @@ func mockRESTPermissionServer(t *testing.T, defaultPerm string, overrides map[st
124118
break
125119
}
126120
}
127-
w.Header().Set("Content-Type", "application/json")
128-
_ = json.NewEncoder(w).Encode(gogithub.RepositoryPermissionLevel{
121+
resp := gogithub.RepositoryPermissionLevel{
129122
Permission: gogithub.Ptr(perm),
130-
})
123+
}
124+
w.Header().Set("Content-Type", "application/json")
125+
_ = json.NewEncoder(w).Encode(resp)
131126
}))
132127
}
133128

0 commit comments

Comments
 (0)