Skip to content

Commit b615d81

Browse files
CopilotSamMorrowDrums
Copilot
and
SamMorrowDrums
committed
Complete cursor-based pagination test fixes - all tests passing
Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com>
1 parent 2936eef commit b615d81

4 files changed

Lines changed: 55 additions & 41 deletions

File tree

‎pkg/github/discussions_test.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -212,22 +212,22 @@ func Test_ListDiscussions(t *testing.T) {
212212
varsListAll := map[string]interface{}{
213213
"owner": "owner",
214214
"repo": "repo",
215-
"first": float64(30),
215+
"first": float64(10),
216216
"after": (*string)(nil),
217217
}
218218

219219
varsRepoNotFound := map[string]interface{}{
220220
"owner": "owner",
221221
"repo": "nonexistent-repo",
222-
"first": float64(30),
222+
"first": float64(10),
223223
"after": (*string)(nil),
224224
}
225225

226226
varsDiscussionsFiltered := map[string]interface{}{
227227
"owner": "owner",
228228
"repo": "repo",
229229
"categoryId": "DIC_kwDOABC123",
230-
"first": float64(30),
230+
"first": float64(10),
231231
"after": (*string)(nil),
232232
}
233233

@@ -236,7 +236,7 @@ func Test_ListDiscussions(t *testing.T) {
236236
"repo": "repo",
237237
"orderByField": "CREATED_AT",
238238
"orderByDirection": "ASC",
239-
"first": float64(30),
239+
"first": float64(10),
240240
"after": (*string)(nil),
241241
}
242242

@@ -245,7 +245,7 @@ func Test_ListDiscussions(t *testing.T) {
245245
"repo": "repo",
246246
"orderByField": "UPDATED_AT",
247247
"orderByDirection": "DESC",
248-
"first": float64(30),
248+
"first": float64(10),
249249
"after": (*string)(nil),
250250
}
251251

@@ -255,14 +255,14 @@ func Test_ListDiscussions(t *testing.T) {
255255
"categoryId": "DIC_kwDOABC123",
256256
"orderByField": "CREATED_AT",
257257
"orderByDirection": "DESC",
258-
"first": float64(30),
258+
"first": float64(10),
259259
"after": (*string)(nil),
260260
}
261261

262262
varsOrgLevel := map[string]interface{}{
263263
"owner": "owner",
264264
"repo": ".github", // This is what gets set when repo is not provided
265-
"first": float64(30),
265+
"first": float64(10),
266266
"after": (*string)(nil),
267267
}
268268

@@ -578,7 +578,7 @@ func Test_GetDiscussionComments(t *testing.T) {
578578
"owner": "owner",
579579
"repo": "repo",
580580
"discussionNumber": float64(1),
581-
"first": float64(30),
581+
"first": float64(10),
582582
"after": (*string)(nil),
583583
}
584584

‎pkg/github/issues_test.go‎

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -315,11 +315,9 @@ func Test_SearchIssues(t *testing.T) {
315315
),
316316
),
317317
requestArgs: map[string]interface{}{
318-
"query": "repo:owner/repo is:open",
319-
"sort": "created",
320-
"order": "desc",
321-
"page": float64(1),
322-
"perPage": float64(30),
318+
"query": "repo:owner/repo is:open",
319+
"sort": "created",
320+
"order": "desc",
323321
},
324322
expectError: false,
325323
expectedResult: mockSearchResult,
@@ -553,8 +551,15 @@ func Test_SearchIssues(t *testing.T) {
553551
textContent := getTextResult(t, result)
554552

555553
// Unmarshal and verify the result
554+
var paginatedResponse PaginatedResponse
555+
err = json.Unmarshal([]byte(textContent.Text), &paginatedResponse)
556+
require.NoError(t, err)
557+
558+
// The data field contains the search result
559+
dataBytes, err := json.Marshal(paginatedResponse.Data)
560+
require.NoError(t, err)
556561
var returnedResult github.IssuesSearchResult
557-
err = json.Unmarshal([]byte(textContent.Text), &returnedResult)
562+
err = json.Unmarshal(dataBytes, &returnedResult)
558563
require.NoError(t, err)
559564
assert.Equal(t, *tc.expectedResult.Total, *returnedResult.Total)
560565
assert.Equal(t, *tc.expectedResult.IncompleteResults, *returnedResult.IncompleteResults)
@@ -747,7 +752,7 @@ func Test_ListIssues(t *testing.T) {
747752
assert.Contains(t, tool.InputSchema.Properties, "orderBy")
748753
assert.Contains(t, tool.InputSchema.Properties, "direction")
749754
assert.Contains(t, tool.InputSchema.Properties, "since")
750-
assert.Contains(t, tool.InputSchema.Properties, "after")
755+
assert.Contains(t, tool.InputSchema.Properties, "cursor")
751756
assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo"})
752757

753758
// Mock issues data
@@ -865,7 +870,7 @@ func Test_ListIssues(t *testing.T) {
865870
"states": []interface{}{"OPEN", "CLOSED"},
866871
"orderBy": "CREATED_AT",
867872
"direction": "DESC",
868-
"first": float64(30),
873+
"first": float64(10),
869874
"after": (*string)(nil),
870875
}
871876

@@ -875,7 +880,7 @@ func Test_ListIssues(t *testing.T) {
875880
"states": []interface{}{"OPEN"},
876881
"orderBy": "CREATED_AT",
877882
"direction": "DESC",
878-
"first": float64(30),
883+
"first": float64(10),
879884
"after": (*string)(nil),
880885
}
881886

@@ -885,7 +890,7 @@ func Test_ListIssues(t *testing.T) {
885890
"states": []interface{}{"CLOSED"},
886891
"orderBy": "CREATED_AT",
887892
"direction": "DESC",
888-
"first": float64(30),
893+
"first": float64(10),
889894
"after": (*string)(nil),
890895
}
891896

@@ -896,7 +901,7 @@ func Test_ListIssues(t *testing.T) {
896901
"labels": []interface{}{"bug", "enhancement"},
897902
"orderBy": "CREATED_AT",
898903
"direction": "DESC",
899-
"first": float64(30),
904+
"first": float64(10),
900905
"after": (*string)(nil),
901906
}
902907

@@ -906,7 +911,7 @@ func Test_ListIssues(t *testing.T) {
906911
"states": []interface{}{"OPEN", "CLOSED"},
907912
"orderBy": "CREATED_AT",
908913
"direction": "DESC",
909-
"first": float64(30),
914+
"first": float64(10),
910915
"after": (*string)(nil),
911916
}
912917

@@ -2580,7 +2585,7 @@ func Test_GetSubIssues(t *testing.T) {
25802585
mock.GetReposIssuesSubIssuesByOwnerByRepoByIssueNumber,
25812586
expectQueryParams(t, map[string]string{
25822587
"page": "2",
2583-
"per_page": "10",
2588+
"per_page": "11",
25842589
}).andThen(
25852590
mockResponse(t, http.StatusOK, mockSubIssues),
25862591
),
@@ -2591,8 +2596,7 @@ func Test_GetSubIssues(t *testing.T) {
25912596
"owner": "owner",
25922597
"repo": "repo",
25932598
"issue_number": float64(42),
2594-
"page": float64(2),
2595-
"perPage": float64(10),
2599+
"cursor": "page=2;perPage=10",
25962600
},
25972601
expectError: false,
25982602
expectedSubIssues: mockSubIssues,

‎pkg/github/pullrequests_test.go‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -900,11 +900,9 @@ func Test_SearchPullRequests(t *testing.T) {
900900
),
901901
),
902902
requestArgs: map[string]interface{}{
903-
"query": "repo:owner/repo is:open",
904-
"sort": "created",
905-
"order": "desc",
906-
"page": float64(1),
907-
"perPage": float64(30),
903+
"query": "repo:owner/repo is:open",
904+
"sort": "created",
905+
"order": "desc",
908906
},
909907
expectError: false,
910908
expectedResult: mockSearchResult,
@@ -1115,8 +1113,15 @@ func Test_SearchPullRequests(t *testing.T) {
11151113
textContent := getTextResult(t, result)
11161114

11171115
// Unmarshal and verify the result
1116+
var paginatedResponse PaginatedResponse
1117+
err = json.Unmarshal([]byte(textContent.Text), &paginatedResponse)
1118+
require.NoError(t, err)
1119+
1120+
// The data field contains the search result
1121+
dataBytes, err := json.Marshal(paginatedResponse.Data)
1122+
require.NoError(t, err)
11181123
var returnedResult github.IssuesSearchResult
1119-
err = json.Unmarshal([]byte(textContent.Text), &returnedResult)
1124+
err = json.Unmarshal(dataBytes, &returnedResult)
11201125
require.NoError(t, err)
11211126
assert.Equal(t, *tc.expectedResult.Total, *returnedResult.Total)
11221127
assert.Equal(t, *tc.expectedResult.IncompleteResults, *returnedResult.IncompleteResults)
@@ -1206,8 +1211,7 @@ func Test_GetPullRequestFiles(t *testing.T) {
12061211
"owner": "owner",
12071212
"repo": "repo",
12081213
"pullNumber": float64(42),
1209-
"page": float64(2),
1210-
"perPage": float64(10),
1214+
"cursor": "page=2;perPage=10",
12111215
},
12121216
expectError: false,
12131217
expectedFiles: mockFiles,

‎pkg/github/search_utils.go‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package github
22

33
import (
44
"context"
5-
"encoding/json"
65
"fmt"
76
"io"
87
"net/http"
@@ -73,18 +72,20 @@ func searchHandler(
7372
if err != nil {
7473
return mcp.NewToolResultError(err.Error()), nil
7574
}
76-
pagination, err := OptionalPaginationParams(request)
75+
76+
// Get cursor-based pagination parameters
77+
cursorParams, err := GetCursorBasedParams(request)
7778
if err != nil {
78-
return mcp.NewToolResultError(err.Error()), nil
79+
return nil, err
7980
}
8081

8182
opts := &github.SearchOptions{
8283
// Default to "created" if no sort is provided, as it's a common use case.
8384
Sort: sort,
8485
Order: order,
8586
ListOptions: github.ListOptions{
86-
Page: pagination.Page,
87-
PerPage: pagination.PerPage,
87+
Page: cursorParams.Page,
88+
PerPage: cursorParams.PerPage + 1, // Request one extra to check for more results
8889
},
8990
}
9091

@@ -106,10 +107,15 @@ func searchHandler(
106107
return mcp.NewToolResultError(fmt.Sprintf("%s: %s", errorPrefix, string(body))), nil
107108
}
108109

109-
r, err := json.Marshal(result)
110-
if err != nil {
111-
return nil, fmt.Errorf("%s: failed to marshal response: %w", errorPrefix, err)
110+
// Check if there are more results
111+
hasMore := len(result.Issues) > cursorParams.PerPage
112+
113+
// Trim to requested page size
114+
if hasMore {
115+
result.Issues = result.Issues[:cursorParams.PerPage]
112116
}
113-
114-
return mcp.NewToolResultText(string(r)), nil
117+
118+
// Create paginated response
119+
paginatedResp := NewPaginatedRESTResponse(result, cursorParams.Page, cursorParams.PerPage, hasMore)
120+
return MarshalPaginatedResponse(paginatedResp), nil
115121
}

0 commit comments

Comments
 (0)