Skip to content

Commit c516044

Browse files
committed
Add tests for milestone search functionality
- Introduced new test cases for the SearchMilestones function, including wildcard matching and filtering by creator login. - Enhanced the SearchMilestones implementation to support wildcard queries and creator login matching. - Added a test to verify that lockdown mode correctly blocks milestones created by unsafe users. - Improved query handling by trimming whitespace and allowing for empty queries to match all milestones.
1 parent aa4af65 commit c516044

File tree

2 files changed

+200
-2
lines changed

2 files changed

+200
-2
lines changed

pkg/github/milestones.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,12 +155,25 @@ func SearchMilestones(getClient GetClientFn, cache *lockdown.RepoAccessCache, t
155155
milestones = filtered
156156
}
157157

158-
lowerQuery := strings.ToLower(query)
158+
lowerQuery := strings.ToLower(strings.TrimSpace(query))
159+
matchAll := lowerQuery == "" || lowerQuery == "*"
159160
result := make([]map[string]any, 0, len(milestones))
160161
for _, m := range milestones {
162+
if matchAll {
163+
result = append(result, milestoneSummary(m))
164+
continue
165+
}
166+
161167
title := strings.ToLower(m.GetTitle())
162168
description := strings.ToLower(m.GetDescription())
163-
if strings.Contains(title, lowerQuery) || strings.Contains(description, lowerQuery) {
169+
creatorLogin := ""
170+
if m.Creator != nil {
171+
creatorLogin = strings.ToLower(m.Creator.GetLogin())
172+
}
173+
174+
if strings.Contains(title, lowerQuery) ||
175+
strings.Contains(description, lowerQuery) ||
176+
(creatorLogin != "" && strings.Contains(creatorLogin, lowerQuery)) {
164177
result = append(result, milestoneSummary(m))
165178
}
166179
}

pkg/github/milestones_test.go

Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,132 @@ func TestSearchMilestones_Success(t *testing.T) {
344344
assert.Equal(t, "Alpha release", first["title"])
345345
}
346346

347+
func TestSearchMilestones_WildcardMatchesAll(t *testing.T) {
348+
t.Parallel()
349+
350+
mockedClient := mock.NewMockedHTTPClient(
351+
mock.WithRequestMatchHandler(
352+
mock.EndpointPattern{Pattern: "/repos/{owner}/{repo}/milestones", Method: http.MethodGet},
353+
expectQueryParams(t, map[string]string{
354+
"state": "closed",
355+
"page": "1",
356+
"per_page": "25",
357+
}).andThen(
358+
mockResponse(t, http.StatusOK, []map[string]any{
359+
{
360+
"id": 3,
361+
"number": 12,
362+
"title": "Should be closed",
363+
"state": "closed",
364+
"description": "no filter needed",
365+
"html_url": "https://example.com/3",
366+
"open_issues": 0,
367+
"closed_issues": 0,
368+
},
369+
}),
370+
),
371+
),
372+
)
373+
374+
client := github.NewClient(mockedClient)
375+
cache := stubRepoAccessCache(githubv4.NewClient(nil), 15*time.Minute)
376+
flags := stubFeatureFlags(map[string]bool{"lockdown-mode": false})
377+
_, handler := SearchMilestones(stubGetClientFn(client), cache, translations.NullTranslationHelper, flags)
378+
379+
args := map[string]any{
380+
"owner": "owner",
381+
"repo": "repo",
382+
"query": "*",
383+
"state": "closed",
384+
"per_page": float64(25),
385+
"page": float64(1),
386+
}
387+
388+
request := createMCPRequest(args)
389+
result, _, err := handler(context.Background(), &request, args)
390+
391+
require.NoError(t, err)
392+
require.NotNil(t, result)
393+
require.False(t, result.IsError)
394+
395+
text := getTextResult(t, result)
396+
var resp map[string]any
397+
require.NoError(t, json.Unmarshal([]byte(text.Text), &resp))
398+
399+
assert.Equal(t, float64(1), resp["count"])
400+
milestones, ok := resp["milestones"].([]any)
401+
require.True(t, ok)
402+
require.Len(t, milestones, 1)
403+
first, ok := milestones[0].(map[string]any)
404+
require.True(t, ok)
405+
assert.Equal(t, "Should be closed", first["title"])
406+
assert.Equal(t, "closed", first["state"])
407+
}
408+
409+
func TestSearchMilestones_MatchesCreatorLogin(t *testing.T) {
410+
t.Parallel()
411+
412+
mockedClient := mock.NewMockedHTTPClient(
413+
mock.WithRequestMatchHandler(
414+
mock.EndpointPattern{Pattern: "/repos/{owner}/{repo}/milestones", Method: http.MethodGet},
415+
mockResponse(t, http.StatusOK, []map[string]any{
416+
{
417+
"id": 4,
418+
"number": 13,
419+
"title": "Release tasks",
420+
"state": "open",
421+
"description": "tracking release work",
422+
"creator": map[string]any{
423+
"login": "alice-dev",
424+
},
425+
"html_url": "https://example.com/4",
426+
},
427+
{
428+
"id": 5,
429+
"number": 14,
430+
"title": "Other tasks",
431+
"state": "open",
432+
"description": "misc work",
433+
"creator": map[string]any{
434+
"login": "bob-dev",
435+
},
436+
"html_url": "https://example.com/5",
437+
},
438+
}),
439+
),
440+
)
441+
442+
client := github.NewClient(mockedClient)
443+
cache := stubRepoAccessCache(githubv4.NewClient(nil), 15*time.Minute)
444+
flags := stubFeatureFlags(map[string]bool{"lockdown-mode": false})
445+
_, handler := SearchMilestones(stubGetClientFn(client), cache, translations.NullTranslationHelper, flags)
446+
447+
args := map[string]any{
448+
"owner": "owner",
449+
"repo": "repo",
450+
"query": "alice",
451+
}
452+
453+
request := createMCPRequest(args)
454+
result, _, err := handler(context.Background(), &request, args)
455+
456+
require.NoError(t, err)
457+
require.NotNil(t, result)
458+
require.False(t, result.IsError)
459+
460+
text := getTextResult(t, result)
461+
var resp map[string]any
462+
require.NoError(t, json.Unmarshal([]byte(text.Text), &resp))
463+
464+
assert.Equal(t, float64(1), resp["count"])
465+
milestones, ok := resp["milestones"].([]any)
466+
require.True(t, ok)
467+
require.Len(t, milestones, 1)
468+
first, ok := milestones[0].(map[string]any)
469+
require.True(t, ok)
470+
assert.Equal(t, "Release tasks", first["title"])
471+
}
472+
347473
func TestSearchMilestones_Validation(t *testing.T) {
348474
t.Parallel()
349475

@@ -430,6 +556,65 @@ func TestSearchMilestones_Lockdown(t *testing.T) {
430556
assert.Equal(t, "Safe alpha", first["title"])
431557
}
432558

559+
func TestSearchMilestones_LockdownBlocksUnsafeCreatorMatches(t *testing.T) {
560+
t.Parallel()
561+
562+
mockedClient := mock.NewMockedHTTPClient(
563+
mock.WithRequestMatchHandler(
564+
mock.EndpointPattern{Pattern: "/repos/{owner}/{repo}/milestones", Method: http.MethodGet},
565+
mockResponse(t, http.StatusOK, []map[string]any{
566+
{
567+
"id": 1,
568+
"number": 10,
569+
"title": "Match me",
570+
"description": "created by unsafe user",
571+
"creator": map[string]any{
572+
"login": "testuser",
573+
},
574+
"html_url": "https://example.com/1",
575+
},
576+
{
577+
"id": 2,
578+
"number": 11,
579+
"title": "Other milestone",
580+
"description": "safe creator unrelated",
581+
"creator": map[string]any{
582+
"login": "testuser2",
583+
},
584+
"html_url": "https://example.com/2",
585+
},
586+
}),
587+
),
588+
)
589+
590+
client := github.NewClient(mockedClient)
591+
gqlClient := githubv4.NewClient(newRepoAccessHTTPClient())
592+
cache := stubRepoAccessCache(gqlClient, 15*time.Minute)
593+
flags := stubFeatureFlags(map[string]bool{"lockdown-mode": true})
594+
_, handler := SearchMilestones(stubGetClientFn(client), cache, translations.NullTranslationHelper, flags)
595+
596+
args := map[string]any{
597+
"owner": "owner",
598+
"repo": "repo",
599+
"query": "match",
600+
}
601+
602+
request := createMCPRequest(args)
603+
result, _, err := handler(context.Background(), &request, args)
604+
605+
require.NoError(t, err)
606+
require.False(t, result.IsError)
607+
608+
text := getTextResult(t, result)
609+
var resp map[string]any
610+
require.NoError(t, json.Unmarshal([]byte(text.Text), &resp))
611+
612+
assert.Equal(t, float64(0), resp["count"])
613+
milestones, ok := resp["milestones"].([]any)
614+
require.True(t, ok)
615+
assert.Len(t, milestones, 0)
616+
}
617+
433618
func TestSearchMilestones_ApiError(t *testing.T) {
434619
t.Parallel()
435620

0 commit comments

Comments
 (0)