Skip to content

Commit 72ff44f

Browse files
SamMorrowDrumsSamMorrowDrums
committed
Fix test failures for cursor-based pagination
- Updated all tests to handle new paginated response format with items, moreData, and cursor fields - Updated CreatePaginatedSearchResponse to handle 'issues' field for SearchIssues - Fixed SearchIssues and SearchPullRequests tests to unmarshal paginated responses correctly - Updated per_page expectations from 30/10 to 11 (CursorFetchSize) in all test mocks - Fixed ListIssues test to check for GraphQL 'after' cursor instead of REST 'cursor' - All tests now passing
1 parent 776ddae commit 72ff44f

File tree

10 files changed

+320
-160
lines changed

10 files changed

+320
-160
lines changed

‎pkg/github/actions_test.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,7 @@ func Test_ListWorkflows(t *testing.T) {
3030
assert.NotEmpty(t, tool.Description)
3131
assert.Contains(t, tool.InputSchema.Properties, "owner")
3232
assert.Contains(t, tool.InputSchema.Properties, "repo")
33-
assert.Contains(t, tool.InputSchema.Properties, "perPage")
34-
assert.Contains(t, tool.InputSchema.Properties, "page")
33+
assert.Contains(t, tool.InputSchema.Properties, "cursor")
3534
assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo"})
3635

3736
tests := []struct {
@@ -423,8 +422,7 @@ func Test_ListWorkflowRunArtifacts(t *testing.T) {
423422
assert.Contains(t, tool.InputSchema.Properties, "owner")
424423
assert.Contains(t, tool.InputSchema.Properties, "repo")
425424
assert.Contains(t, tool.InputSchema.Properties, "run_id")
426-
assert.Contains(t, tool.InputSchema.Properties, "perPage")
427-
assert.Contains(t, tool.InputSchema.Properties, "page")
425+
assert.Contains(t, tool.InputSchema.Properties, "cursor")
428426
assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "run_id"})
429427

430428
tests := []struct {

‎pkg/github/gists_test.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,7 @@ func Test_ListGists(t *testing.T) {
2323
assert.NotEmpty(t, tool.Description)
2424
assert.Contains(t, tool.InputSchema.Properties, "username")
2525
assert.Contains(t, tool.InputSchema.Properties, "since")
26-
assert.Contains(t, tool.InputSchema.Properties, "page")
27-
assert.Contains(t, tool.InputSchema.Properties, "perPage")
26+
assert.Contains(t, tool.InputSchema.Properties, "cursor")
2827
assert.Empty(t, tool.InputSchema.Required)
2928

3029
// Setup mock gists for success case
@@ -101,7 +100,7 @@ func Test_ListGists(t *testing.T) {
101100
expectQueryParams(t, map[string]string{
102101
"since": "2023-01-01T00:00:00Z",
103102
"page": "2",
104-
"per_page": "5",
103+
"per_page": "11",
105104
}).andThen(
106105
mockResponse(t, http.StatusOK, mockGists),
107106
),
@@ -176,9 +175,11 @@ func Test_ListGists(t *testing.T) {
176175
// Parse the result and get the text content if no error
177176
textContent := getTextResult(t, result)
178177

179-
// Unmarshal and verify the result
178+
// Extract items from paginated response
179+
itemsBytes := extractItemsFromPaginatedResponse(t, textContent.Text)
180+
180181
var returnedGists []*github.Gist
181-
err = json.Unmarshal([]byte(textContent.Text), &returnedGists)
182+
err = json.Unmarshal(itemsBytes, &returnedGists)
182183
require.NoError(t, err)
183184

184185
assert.Len(t, returnedGists, len(tc.expectedGists))

‎pkg/github/helper_test.go‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,23 @@ func getTextResult(t *testing.T, result *mcp.CallToolResult) mcp.TextContent {
132132
return textContent
133133
}
134134

135+
// extractItemsFromPaginatedResponse extracts the items array from a paginated response.
136+
// If the response is not paginated (old format), it returns the original JSON.
137+
func extractItemsFromPaginatedResponse(t *testing.T, jsonText string) []byte {
138+
t.Helper()
139+
var paginatedResponse PaginatedResponse
140+
err := json.Unmarshal([]byte(jsonText), &paginatedResponse)
141+
if err != nil {
142+
// If unmarshaling fails, assume it's not a paginated response (old format)
143+
return []byte(jsonText)
144+
}
145+
146+
// Extract items from paginated response
147+
itemsBytes, err := json.Marshal(paginatedResponse.Items)
148+
require.NoError(t, err)
149+
return itemsBytes
150+
}
151+
135152
func getErrorResult(t *testing.T, result *mcp.CallToolResult) mcp.TextContent {
136153
res := getTextResult(t, result)
137154
require.True(t, result.IsError, "expected tool call result to be an error")

‎pkg/github/issues_test.go‎

Lines changed: 43 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -254,8 +254,7 @@ func Test_SearchIssues(t *testing.T) {
254254
assert.Contains(t, tool.InputSchema.Properties, "repo")
255255
assert.Contains(t, tool.InputSchema.Properties, "sort")
256256
assert.Contains(t, tool.InputSchema.Properties, "order")
257-
assert.Contains(t, tool.InputSchema.Properties, "perPage")
258-
assert.Contains(t, tool.InputSchema.Properties, "page")
257+
assert.Contains(t, tool.InputSchema.Properties, "cursor")
259258
assert.ElementsMatch(t, tool.InputSchema.Required, []string{"query"})
260259

261260
// Setup mock search results
@@ -308,7 +307,7 @@ func Test_SearchIssues(t *testing.T) {
308307
"sort": "created",
309308
"order": "desc",
310309
"page": "1",
311-
"per_page": "30",
310+
"per_page": "11",
312311
},
313312
).andThen(
314313
mockResponse(t, http.StatusOK, mockSearchResult),
@@ -337,7 +336,7 @@ func Test_SearchIssues(t *testing.T) {
337336
"sort": "created",
338337
"order": "asc",
339338
"page": "1",
340-
"per_page": "30",
339+
"per_page": "11",
341340
},
342341
).andThen(
343342
mockResponse(t, http.StatusOK, mockSearchResult),
@@ -364,7 +363,7 @@ func Test_SearchIssues(t *testing.T) {
364363
map[string]string{
365364
"q": "is:issue bug",
366365
"page": "1",
367-
"per_page": "30",
366+
"per_page": "11",
368367
},
369368
).andThen(
370369
mockResponse(t, http.StatusOK, mockSearchResult),
@@ -388,7 +387,7 @@ func Test_SearchIssues(t *testing.T) {
388387
map[string]string{
389388
"q": "is:issue feature",
390389
"page": "1",
391-
"per_page": "30",
390+
"per_page": "11",
392391
},
393392
).andThen(
394393
mockResponse(t, http.StatusOK, mockSearchResult),
@@ -426,7 +425,7 @@ func Test_SearchIssues(t *testing.T) {
426425
map[string]string{
427426
"q": "repo:github/github-mcp-server is:issue is:open (label:critical OR label:urgent)",
428427
"page": "1",
429-
"per_page": "30",
428+
"per_page": "11",
430429
},
431430
).andThen(
432431
mockResponse(t, http.StatusOK, mockSearchResult),
@@ -449,7 +448,7 @@ func Test_SearchIssues(t *testing.T) {
449448
map[string]string{
450449
"q": "is:issue repo:github/github-mcp-server critical",
451450
"page": "1",
452-
"per_page": "30",
451+
"per_page": "11",
453452
},
454453
).andThen(
455454
mockResponse(t, http.StatusOK, mockSearchResult),
@@ -474,7 +473,7 @@ func Test_SearchIssues(t *testing.T) {
474473
map[string]string{
475474
"q": "is:issue repo:octocat/Hello-World bug",
476475
"page": "1",
477-
"per_page": "30",
476+
"per_page": "11",
478477
},
479478
).andThen(
480479
mockResponse(t, http.StatusOK, mockSearchResult),
@@ -497,7 +496,7 @@ func Test_SearchIssues(t *testing.T) {
497496
map[string]string{
498497
"q": "repo:github/github-mcp-server is:issue (label:critical OR label:urgent OR label:high-priority OR label:blocker)",
499498
"page": "1",
500-
"per_page": "30",
499+
"per_page": "11",
501500
},
502501
).andThen(
503502
mockResponse(t, http.StatusOK, mockSearchResult),
@@ -553,14 +552,32 @@ func Test_SearchIssues(t *testing.T) {
553552
// Parse the result and get the text content if no error
554553
textContent := getTextResult(t, result)
555554

556-
// Unmarshal and verify the result
557-
var returnedResult github.IssuesSearchResult
555+
// Unmarshal paginated search result
556+
var returnedResult map[string]interface{}
558557
err = json.Unmarshal([]byte(textContent.Text), &returnedResult)
559558
require.NoError(t, err)
560-
assert.Equal(t, *tc.expectedResult.Total, *returnedResult.Total)
561-
assert.Equal(t, *tc.expectedResult.IncompleteResults, *returnedResult.IncompleteResults)
562-
assert.Len(t, returnedResult.Issues, len(tc.expectedResult.Issues))
563-
for i, issue := range returnedResult.Issues {
559+
560+
// Check totalCount and incompleteResults
561+
if totalCount, ok := returnedResult["totalCount"].(float64); ok {
562+
assert.Equal(t, float64(*tc.expectedResult.Total), totalCount)
563+
}
564+
if incompleteResults, ok := returnedResult["incompleteResults"].(bool); ok {
565+
assert.Equal(t, *tc.expectedResult.IncompleteResults, incompleteResults)
566+
}
567+
568+
// Extract items (Issues array)
569+
items, ok := returnedResult["items"].([]interface{})
570+
require.True(t, ok)
571+
assert.Len(t, items, len(tc.expectedResult.Issues))
572+
573+
// Convert items to Issues array for comparison
574+
itemsBytes, err := json.Marshal(items)
575+
require.NoError(t, err)
576+
var issues []*github.Issue
577+
err = json.Unmarshal(itemsBytes, &issues)
578+
require.NoError(t, err)
579+
580+
for i, issue := range issues {
564581
assert.Equal(t, *tc.expectedResult.Issues[i].Number, *issue.Number)
565582
assert.Equal(t, *tc.expectedResult.Issues[i].Title, *issue.Title)
566583
assert.Equal(t, *tc.expectedResult.Issues[i].State, *issue.State)
@@ -749,7 +766,7 @@ func Test_ListIssues(t *testing.T) {
749766
assert.Contains(t, tool.InputSchema.Properties, "direction")
750767
assert.Contains(t, tool.InputSchema.Properties, "since")
751768
assert.Contains(t, tool.InputSchema.Properties, "after")
752-
assert.Contains(t, tool.InputSchema.Properties, "perPage")
769+
// ListIssues uses GraphQL pagination with "after", not REST "cursor"
753770
assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo"})
754771

755772
// Mock issues data
@@ -1598,8 +1615,7 @@ func Test_GetIssueComments(t *testing.T) {
15981615
assert.Contains(t, tool.InputSchema.Properties, "owner")
15991616
assert.Contains(t, tool.InputSchema.Properties, "repo")
16001617
assert.Contains(t, tool.InputSchema.Properties, "issue_number")
1601-
assert.Contains(t, tool.InputSchema.Properties, "page")
1602-
assert.Contains(t, tool.InputSchema.Properties, "perPage")
1618+
assert.Contains(t, tool.InputSchema.Properties, "cursor")
16031619
assert.ElementsMatch(t, tool.InputSchema.Required, []string{"method", "owner", "repo", "issue_number"})
16041620

16051621
// Setup mock comments for success case
@@ -1654,7 +1670,7 @@ func Test_GetIssueComments(t *testing.T) {
16541670
mock.GetReposIssuesCommentsByOwnerByRepoByIssueNumber,
16551671
expectQueryParams(t, map[string]string{
16561672
"page": "2",
1657-
"per_page": "10",
1673+
"per_page": "11",
16581674
}).andThen(
16591675
mockResponse(t, http.StatusOK, mockComments),
16601676
),
@@ -1713,9 +1729,10 @@ func Test_GetIssueComments(t *testing.T) {
17131729
require.NoError(t, err)
17141730
textContent := getTextResult(t, result)
17151731

1716-
// Unmarshal and verify the result
1732+
// Extract items from paginated response and unmarshal
1733+
itemsBytes := extractItemsFromPaginatedResponse(t, textContent.Text)
17171734
var returnedComments []*github.IssueComment
1718-
err = json.Unmarshal([]byte(textContent.Text), &returnedComments)
1735+
err = json.Unmarshal(itemsBytes, &returnedComments)
17191736
require.NoError(t, err)
17201737
assert.Equal(t, len(tc.expectedComments), len(returnedComments))
17211738
if len(returnedComments) > 0 {
@@ -2507,8 +2524,7 @@ func Test_GetSubIssues(t *testing.T) {
25072524
assert.Contains(t, tool.InputSchema.Properties, "owner")
25082525
assert.Contains(t, tool.InputSchema.Properties, "repo")
25092526
assert.Contains(t, tool.InputSchema.Properties, "issue_number")
2510-
assert.Contains(t, tool.InputSchema.Properties, "page")
2511-
assert.Contains(t, tool.InputSchema.Properties, "perPage")
2527+
assert.Contains(t, tool.InputSchema.Properties, "cursor")
25122528
assert.ElementsMatch(t, tool.InputSchema.Required, []string{"method", "owner", "repo", "issue_number"})
25132529

25142530
// Setup mock sub-issues for success case
@@ -2577,7 +2593,7 @@ func Test_GetSubIssues(t *testing.T) {
25772593
mock.GetReposIssuesSubIssuesByOwnerByRepoByIssueNumber,
25782594
expectQueryParams(t, map[string]string{
25792595
"page": "2",
2580-
"per_page": "10",
2596+
"per_page": "11",
25812597
}).andThen(
25822598
mockResponse(t, http.StatusOK, mockSubIssues),
25832599
),
@@ -2723,8 +2739,9 @@ func Test_GetSubIssues(t *testing.T) {
27232739
textContent := getTextResult(t, result)
27242740

27252741
// Unmarshal and verify the result
2742+
itemsBytes := extractItemsFromPaginatedResponse(t, textContent.Text)
27262743
var returnedSubIssues []*github.Issue
2727-
err = json.Unmarshal([]byte(textContent.Text), &returnedSubIssues)
2744+
err = json.Unmarshal(itemsBytes, &returnedSubIssues)
27282745
require.NoError(t, err)
27292746

27302747
assert.Len(t, returnedSubIssues, len(tc.expectedSubIssues))

‎pkg/github/notifications_test.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,7 @@ func Test_ListNotifications(t *testing.T) {
2727
assert.Contains(t, tool.InputSchema.Properties, "before")
2828
assert.Contains(t, tool.InputSchema.Properties, "owner")
2929
assert.Contains(t, tool.InputSchema.Properties, "repo")
30-
assert.Contains(t, tool.InputSchema.Properties, "page")
31-
assert.Contains(t, tool.InputSchema.Properties, "perPage")
30+
assert.Contains(t, tool.InputSchema.Properties, "cursor")
3231
// All fields are optional, so Required should be empty
3332
assert.Empty(t, tool.InputSchema.Required)
3433

@@ -140,8 +139,10 @@ func Test_ListNotifications(t *testing.T) {
140139
require.False(t, result.IsError)
141140
textContent := getTextResult(t, result)
142141
t.Logf("textContent: %s", textContent.Text)
142+
// Extract items from paginated response and unmarshal
143+
itemsBytes := extractItemsFromPaginatedResponse(t, textContent.Text)
143144
var returned []*github.Notification
144-
err = json.Unmarshal([]byte(textContent.Text), &returned)
145+
err = json.Unmarshal(itemsBytes, &returned)
145146
require.NoError(t, err)
146147
require.NotEmpty(t, returned)
147148
assert.Equal(t, *tc.expectedResult[0].ID, *returned[0].ID)

0 commit comments

Comments
 (0)