From b4213ac136aee9d7d2605c3ffe3bae2c45bc23aa Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Sun, 31 Aug 2025 21:11:55 +0100 Subject: [PATCH] test(issue/pr list): assert integration with advanced issue search Signed-off-by: Babak K. Shandiz --- pkg/cmd/issue/list/http_test.go | 47 ++++++++++++ pkg/cmd/issue/list/list_test.go | 124 +++++++++++++++++++------------- pkg/cmd/pr/list/http_test.go | 69 ++++++++++++++---- pkg/cmd/pr/list/list_test.go | 28 ++++---- 4 files changed, 192 insertions(+), 76 deletions(-) diff --git a/pkg/cmd/issue/list/http_test.go b/pkg/cmd/issue/list/http_test.go index a929746d1..8c73b7eac 100644 --- a/pkg/cmd/issue/list/http_test.go +++ b/pkg/cmd/issue/list/http_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/cli/cli/v2/api" + fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/ghrepo" prShared "github.com/cli/cli/v2/pkg/cmd/pr/shared" "github.com/cli/cli/v2/pkg/httpmock" @@ -165,3 +166,49 @@ func TestIssueList_pagination(t *testing.T) { assert.Equal(t, []string{"enhancement"}, getLabels(res.Issues[1])) assert.Equal(t, []string{"user2"}, getAssignees(res.Issues[1])) } + +func TestSearchIssuesAndAdvancedSearch(t *testing.T) { + tests := []struct { + name string + detector fd.Detector + wantSearchType string + }{ + { + name: "advanced issue search not supported", + detector: fd.AdvancedIssueSearchUnsupported(), + wantSearchType: "ISSUE", + }, + { + name: "advanced issue search supported as opt-in", + detector: fd.AdvancedIssueSearchSupportedAsOptIn(), + wantSearchType: "ISSUE_ADVANCED", + }, + { + name: "advanced issue search supported as only backend", + detector: fd.AdvancedIssueSearchSupportedAsOnlyBackend(), + wantSearchType: "ISSUE", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + + reg.Register( + httpmock.GraphQL(`query IssueSearch\b`), + httpmock.GraphQLQuery(`{"data":{}}`, func(query string, vars map[string]interface{}) { + assert.Equal(t, tt.wantSearchType, vars["type"]) + // Since no repeated usage of special search qualifiers is possible + // with our current implementation, we can assert against the same + // query for both search backend (i.e. legacy and advanced issue search). + assert.Equal(t, "repo:OWNER/REPO state:open type:issue", vars["query"]) + })) + + httpClient := &http.Client{Transport: reg} + client := api.NewClientFromHTTP(httpClient) + + searchIssues(client, tt.detector, ghrepo.New("OWNER", "REPO"), prShared.FilterOptions{State: "open"}, 30) + }) + } +} diff --git a/pkg/cmd/issue/list/list_test.go b/pkg/cmd/issue/list/list_test.go index b88414d43..040c38e8a 100644 --- a/pkg/cmd/issue/list/list_test.go +++ b/pkg/cmd/issue/list/list_test.go @@ -191,43 +191,69 @@ func TestIssueList_disabledIssues(t *testing.T) { } func TestIssueList_web(t *testing.T) { - ios, _, stdout, stderr := iostreams.Test() - ios.SetStdoutTTY(true) - ios.SetStderrTTY(true) - browser := &browser.Stub{} - - reg := &httpmock.Registry{} - defer reg.Verify(t) - - _, cmdTeardown := run.Stub() - defer cmdTeardown(t) - - err := listRun(&ListOptions{ - IO: ios, - Browser: browser, - HttpClient: func() (*http.Client, error) { - return &http.Client{Transport: reg}, nil + tests := []struct { + name string + detector fd.Detector + }{ + { + name: "advanced issue search not supported", + detector: fd.AdvancedIssueSearchUnsupported(), }, - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil + { + name: "advanced issue search supported as opt-in", + detector: fd.AdvancedIssueSearchSupportedAsOptIn(), + }, + { + name: "advanced issue search supported as only backend", + detector: fd.AdvancedIssueSearchSupportedAsOnlyBackend(), }, - Detector: fd.AdvancedIssueSearchUnsupported(), - WebMode: true, - State: "all", - Assignee: "peter", - Author: "john", - Labels: []string{"bug", "docs"}, - Mention: "frank", - Milestone: "v1.1", - LimitResults: 10, - }) - if err != nil { - t.Errorf("error running command `issue list` with `--web` flag: %v", err) } - assert.Equal(t, "", stdout.String()) - assert.Equal(t, "Opening https://github.com/OWNER/REPO/issues in your browser.\n", stderr.String()) - browser.Verify(t, "https://github.com/OWNER/REPO/issues?q=assignee%3Apeter+author%3Ajohn+label%3Abug+label%3Adocs+mentions%3Afrank+milestone%3Av1.1+type%3Aissue") + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ios, _, stdout, stderr := iostreams.Test() + ios.SetStdoutTTY(true) + ios.SetStderrTTY(true) + browser := &browser.Stub{} + + reg := &httpmock.Registry{} + defer reg.Verify(t) + + _, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + opts := &ListOptions{ + IO: ios, + Browser: browser, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + }, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + Detector: tt.detector, + WebMode: true, + State: "all", + Assignee: "peter", + Author: "john", + Labels: []string{"bug", "docs"}, + Mention: "frank", + Milestone: "v1.1", + LimitResults: 10, + } + + err := listRun(opts) + require.NoError(t, err) + + assert.Equal(t, "", stdout.String()) + assert.Equal(t, "Opening https://github.com/OWNER/REPO/issues in your browser.\n", stderr.String()) + + // Since no repeated usage of special search qualifiers is possible + // with our current implementation, we can assert against the same + // URL for both search backend (i.e. legacy and advanced issue search). + browser.Verify(t, "https://github.com/OWNER/REPO/issues?q=assignee%3Apeter+author%3Ajohn+label%3Abug+label%3Adocs+mentions%3Afrank+milestone%3Av1.1+type%3Aissue") + }) + } } func Test_issueList(t *testing.T) { @@ -246,9 +272,8 @@ func Test_issueList(t *testing.T) { { name: "default", args: args{ - detector: fd.AdvancedIssueSearchUnsupported(), - limit: 30, - repo: ghrepo.New("OWNER", "REPO"), + limit: 30, + repo: ghrepo.New("OWNER", "REPO"), filters: prShared.FilterOptions{ Entity: "issue", State: "open", @@ -274,7 +299,7 @@ func Test_issueList(t *testing.T) { { name: "milestone by number", args: args{ - detector: fd.AdvancedIssueSearchUnsupported(), + detector: fd.AdvancedIssueSearchSupportedAsOptIn(), limit: 30, repo: ghrepo.New("OWNER", "REPO"), filters: prShared.FilterOptions{ @@ -306,7 +331,7 @@ func Test_issueList(t *testing.T) { "repo": "REPO", "limit": float64(30), "query": "milestone:1.x repo:OWNER/REPO state:open type:issue", - "type": "ISSUE", + "type": "ISSUE_ADVANCED", }, params) })) }, @@ -314,7 +339,7 @@ func Test_issueList(t *testing.T) { { name: "milestone by title", args: args{ - detector: fd.AdvancedIssueSearchUnsupported(), + detector: fd.AdvancedIssueSearchSupportedAsOptIn(), limit: 30, repo: ghrepo.New("OWNER", "REPO"), filters: prShared.FilterOptions{ @@ -339,7 +364,7 @@ func Test_issueList(t *testing.T) { "repo": "REPO", "limit": float64(30), "query": "milestone:1.x repo:OWNER/REPO state:open type:issue", - "type": "ISSUE", + "type": "ISSUE_ADVANCED", }, params) })) }, @@ -347,9 +372,8 @@ func Test_issueList(t *testing.T) { { name: "@me syntax", args: args{ - detector: fd.AdvancedIssueSearchUnsupported(), - limit: 30, - repo: ghrepo.New("OWNER", "REPO"), + limit: 30, + repo: ghrepo.New("OWNER", "REPO"), filters: prShared.FilterOptions{ Entity: "issue", State: "open", @@ -384,7 +408,7 @@ func Test_issueList(t *testing.T) { { name: "@me with search", args: args{ - detector: fd.AdvancedIssueSearchUnsupported(), + detector: fd.AdvancedIssueSearchSupportedAsOptIn(), limit: 30, repo: ghrepo.New("OWNER", "REPO"), filters: prShared.FilterOptions{ @@ -412,7 +436,7 @@ func Test_issueList(t *testing.T) { "repo": "REPO", "limit": float64(30), "query": "auth bug assignee:@me author:@me mentions:@me repo:OWNER/REPO state:open type:issue", - "type": "ISSUE", + "type": "ISSUE_ADVANCED", }, params) })) }, @@ -420,7 +444,7 @@ func Test_issueList(t *testing.T) { { name: "with labels", args: args{ - detector: fd.AdvancedIssueSearchUnsupported(), + detector: fd.AdvancedIssueSearchSupportedAsOptIn(), limit: 30, repo: ghrepo.New("OWNER", "REPO"), filters: prShared.FilterOptions{ @@ -445,7 +469,7 @@ func Test_issueList(t *testing.T) { "repo": "REPO", "limit": float64(30), "query": `label:"one world" label:hello repo:OWNER/REPO state:open type:issue`, - "type": "ISSUE", + "type": "ISSUE_ADVANCED", }, params) })) }, @@ -516,7 +540,7 @@ func TestIssueList_withProjectItems(t *testing.T) { client := &http.Client{Transport: reg} issuesAndTotalCount, err := issueList( client, - fd.AdvancedIssueSearchUnsupported(), + nil, ghrepo.New("OWNER", "REPO"), prShared.FilterOptions{ Entity: "issue", @@ -582,7 +606,7 @@ func TestIssueList_Search_withProjectItems(t *testing.T) { require.Equal(t, map[string]interface{}{ "owner": "OWNER", "repo": "REPO", - "type": "ISSUE", + "type": "ISSUE_ADVANCED", "limit": float64(30), "query": "just used to force the search API branch repo:OWNER/REPO type:issue", }, params) @@ -591,7 +615,7 @@ func TestIssueList_Search_withProjectItems(t *testing.T) { client := &http.Client{Transport: reg} issuesAndTotalCount, err := issueList( client, - fd.AdvancedIssueSearchUnsupported(), + fd.AdvancedIssueSearchSupportedAsOptIn(), ghrepo.New("OWNER", "REPO"), prShared.FilterOptions{ Entity: "issue", diff --git a/pkg/cmd/pr/list/http_test.go b/pkg/cmd/pr/list/http_test.go index 4f796906a..e3b8cad5d 100644 --- a/pkg/cmd/pr/list/http_test.go +++ b/pkg/cmd/pr/list/http_test.go @@ -9,6 +9,7 @@ import ( "github.com/cli/cli/v2/internal/ghrepo" prShared "github.com/cli/cli/v2/pkg/cmd/pr/shared" "github.com/cli/cli/v2/pkg/httpmock" + "github.com/stretchr/testify/assert" ) func Test_ListPullRequests(t *testing.T) { @@ -27,9 +28,8 @@ func Test_ListPullRequests(t *testing.T) { { name: "default", args: args{ - detector: fd.AdvancedIssueSearchUnsupported(), - repo: ghrepo.New("OWNER", "REPO"), - limit: 30, + repo: ghrepo.New("OWNER", "REPO"), + limit: 30, filters: prShared.FilterOptions{ State: "open", }, @@ -53,9 +53,8 @@ func Test_ListPullRequests(t *testing.T) { { name: "closed", args: args{ - detector: fd.AdvancedIssueSearchUnsupported(), - repo: ghrepo.New("OWNER", "REPO"), - limit: 30, + repo: ghrepo.New("OWNER", "REPO"), + limit: 30, filters: prShared.FilterOptions{ State: "closed", }, @@ -79,7 +78,7 @@ func Test_ListPullRequests(t *testing.T) { { name: "with labels", args: args{ - detector: fd.AdvancedIssueSearchUnsupported(), + detector: fd.AdvancedIssueSearchSupportedAsOptIn(), repo: ghrepo.New("OWNER", "REPO"), limit: 30, filters: prShared.FilterOptions{ @@ -93,7 +92,7 @@ func Test_ListPullRequests(t *testing.T) { httpmock.GraphQLQuery(`{"data":{}}`, func(query string, vars map[string]interface{}) { want := map[string]interface{}{ "q": `label:"one world" label:hello repo:OWNER/REPO state:open type:pr`, - "type": "ISSUE", + "type": "ISSUE_ADVANCED", "limit": float64(30), } if !reflect.DeepEqual(vars, want) { @@ -105,7 +104,7 @@ func Test_ListPullRequests(t *testing.T) { { name: "with author", args: args{ - detector: fd.AdvancedIssueSearchUnsupported(), + detector: fd.AdvancedIssueSearchSupportedAsOptIn(), repo: ghrepo.New("OWNER", "REPO"), limit: 30, filters: prShared.FilterOptions{ @@ -119,7 +118,7 @@ func Test_ListPullRequests(t *testing.T) { httpmock.GraphQLQuery(`{"data":{}}`, func(query string, vars map[string]interface{}) { want := map[string]interface{}{ "q": "author:monalisa repo:OWNER/REPO state:open type:pr", - "type": "ISSUE", + "type": "ISSUE_ADVANCED", "limit": float64(30), } if !reflect.DeepEqual(vars, want) { @@ -131,7 +130,7 @@ func Test_ListPullRequests(t *testing.T) { { name: "with search", args: args{ - detector: fd.AdvancedIssueSearchUnsupported(), + detector: fd.AdvancedIssueSearchSupportedAsOptIn(), repo: ghrepo.New("OWNER", "REPO"), limit: 30, filters: prShared.FilterOptions{ @@ -145,7 +144,7 @@ func Test_ListPullRequests(t *testing.T) { httpmock.GraphQLQuery(`{"data":{}}`, func(query string, vars map[string]interface{}) { want := map[string]interface{}{ "q": "one world in:title repo:OWNER/REPO state:open type:pr", - "type": "ISSUE", + "type": "ISSUE_ADVANCED", "limit": float64(30), } if !reflect.DeepEqual(vars, want) { @@ -171,3 +170,49 @@ func Test_ListPullRequests(t *testing.T) { }) } } + +func TestSearchPullRequestsAndAdvancedSearch(t *testing.T) { + tests := []struct { + name string + detector fd.Detector + wantSearchType string + }{ + { + name: "advanced issue search not supported", + detector: fd.AdvancedIssueSearchUnsupported(), + wantSearchType: "ISSUE", + }, + { + name: "advanced issue search supported as opt-in", + detector: fd.AdvancedIssueSearchSupportedAsOptIn(), + wantSearchType: "ISSUE_ADVANCED", + }, + { + name: "advanced issue search supported as only backend", + detector: fd.AdvancedIssueSearchSupportedAsOnlyBackend(), + wantSearchType: "ISSUE", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + + reg.Register( + httpmock.GraphQL(`query PullRequestSearch\b`), + httpmock.GraphQLQuery(`{"data":{}}`, func(query string, vars map[string]interface{}) { + assert.Equal(t, tt.wantSearchType, vars["type"]) + + // Since no repeated usage of special search qualifiers is possible + // with our current implementation, we can assert against the same + // query for both search backend (i.e. legacy and advanced issue search). + assert.Equal(t, "repo:OWNER/REPO state:open type:pr", vars["q"]) + })) + + httpClient := &http.Client{Transport: reg} + + searchPullRequests(httpClient, tt.detector, ghrepo.New("OWNER", "REPO"), prShared.FilterOptions{State: "open"}, 30) + }) + } +} diff --git a/pkg/cmd/pr/list/list_test.go b/pkg/cmd/pr/list/list_test.go index 6a7c2815a..3df8721e6 100644 --- a/pkg/cmd/pr/list/list_test.go +++ b/pkg/cmd/pr/list/list_test.go @@ -80,7 +80,7 @@ func TestPRList(t *testing.T) { http.Register(httpmock.GraphQL(`query PullRequestList\b`), httpmock.FileResponse("./fixtures/prList.json")) - output, err := runCommand(http, fd.AdvancedIssueSearchUnsupported(), true, "") + output, err := runCommand(http, nil, true, "") if err != nil { t.Fatal(err) } @@ -103,7 +103,7 @@ func TestPRList_nontty(t *testing.T) { http.Register(httpmock.GraphQL(`query PullRequestList\b`), httpmock.FileResponse("./fixtures/prList.json")) - output, err := runCommand(http, fd.AdvancedIssueSearchUnsupported(), false, "") + output, err := runCommand(http, nil, false, "") if err != nil { t.Fatal(err) } @@ -126,7 +126,7 @@ func TestPRList_filtering(t *testing.T) { assert.Equal(t, []interface{}{"OPEN", "CLOSED", "MERGED"}, params["state"].([]interface{})) })) - output, err := runCommand(http, fd.AdvancedIssueSearchUnsupported(), true, `-s all`) + output, err := runCommand(http, nil, true, `-s all`) assert.Error(t, err) assert.Equal(t, "", output.String()) @@ -141,7 +141,7 @@ func TestPRList_filteringRemoveDuplicate(t *testing.T) { httpmock.GraphQL(`query PullRequestList\b`), httpmock.FileResponse("./fixtures/prListWithDuplicates.json")) - output, err := runCommand(http, fd.AdvancedIssueSearchUnsupported(), true, "") + output, err := runCommand(http, nil, true, "") if err != nil { t.Fatal(err) } @@ -164,7 +164,7 @@ func TestPRList_filteringClosed(t *testing.T) { assert.Equal(t, []interface{}{"CLOSED", "MERGED"}, params["state"].([]interface{})) })) - _, err := runCommand(http, fd.AdvancedIssueSearchUnsupported(), true, `-s closed`) + _, err := runCommand(http, nil, true, `-s closed`) assert.Error(t, err) } @@ -178,7 +178,7 @@ func TestPRList_filteringHeadBranch(t *testing.T) { assert.Equal(t, interface{}("bug-fix"), params["headBranch"]) })) - _, err := runCommand(http, fd.AdvancedIssueSearchUnsupported(), true, `-H bug-fix`) + _, err := runCommand(http, nil, true, `-H bug-fix`) assert.Error(t, err) } @@ -192,7 +192,7 @@ func TestPRList_filteringAssignee(t *testing.T) { assert.Equal(t, `assignee:hubot base:develop is:merged label:"needs tests" repo:OWNER/REPO type:pr`, params["q"].(string)) })) - _, err := runCommand(http, fd.AdvancedIssueSearchUnsupported(), true, `-s merged -l "needs tests" -a hubot -B develop`) + _, err := runCommand(http, fd.AdvancedIssueSearchSupportedAsOptIn(), true, `-s merged -l "needs tests" -a hubot -B develop`) assert.Error(t, err) } @@ -225,7 +225,7 @@ func TestPRList_filteringDraft(t *testing.T) { assert.Equal(t, test.expectedQuery, params["q"].(string)) })) - _, err := runCommand(http, fd.AdvancedIssueSearchUnsupported(), true, test.cli) + _, err := runCommand(http, fd.AdvancedIssueSearchSupportedAsOptIn(), true, test.cli) assert.Error(t, err) }) } @@ -270,7 +270,7 @@ func TestPRList_filteringAuthor(t *testing.T) { assert.Equal(t, test.expectedQuery, params["q"].(string)) })) - _, err := runCommand(http, fd.AdvancedIssueSearchUnsupported(), true, test.cli) + _, err := runCommand(http, fd.AdvancedIssueSearchSupportedAsOptIn(), true, test.cli) assert.Error(t, err) }) } @@ -279,7 +279,7 @@ func TestPRList_filteringAuthor(t *testing.T) { func TestPRList_withInvalidLimitFlag(t *testing.T) { http := initFakeHTTP() defer http.Verify(t) - _, err := runCommand(http, fd.AdvancedIssueSearchUnsupported(), true, `--limit=0`) + _, err := runCommand(http, nil, true, `--limit=0`) assert.EqualError(t, err, "invalid value for --limit: 0") } @@ -314,7 +314,7 @@ func TestPRList_web(t *testing.T) { _, cmdTeardown := run.Stub() defer cmdTeardown(t) - output, err := runCommand(http, fd.AdvancedIssueSearchUnsupported(), true, "--web "+test.cli) + output, err := runCommand(http, nil, true, "--web "+test.cli) if err != nil { t.Errorf("error running command `pr list` with `--web` flag: %v", err) } @@ -372,7 +372,7 @@ func TestPRList_withProjectItems(t *testing.T) { client := &http.Client{Transport: reg} prsAndTotalCount, err := listPullRequests( client, - fd.AdvancedIssueSearchUnsupported(), + nil, ghrepo.New("OWNER", "REPO"), prShared.FilterOptions{ Entity: "pr", @@ -436,14 +436,14 @@ func TestPRList_Search_withProjectItems(t *testing.T) { require.Equal(t, map[string]interface{}{ "limit": float64(30), "q": "just used to force the search API branch repo:OWNER/REPO state:open type:pr", - "type": "ISSUE", + "type": "ISSUE_ADVANCED", }, params) })) client := &http.Client{Transport: reg} prsAndTotalCount, err := listPullRequests( client, - fd.AdvancedIssueSearchUnsupported(), + fd.AdvancedIssueSearchSupportedAsOptIn(), ghrepo.New("OWNER", "REPO"), prShared.FilterOptions{ Entity: "pr",