From 07dfdf97aeb357606c1daed37736ffefd9beb585 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Thu, 8 Jan 2026 14:26:21 -0700 Subject: [PATCH] Update edit tests Updated test mocks and logic to consistently use lowercase 'monalisa' for login names and display names for user assignees. Improved handling of dynamic assignee fetching in interactive flows by relying on searchFunc and metadata population, and clarified logic in FetchOptions to fetch assignees only when necessary. These changes ensure more accurate simulation of interactive assignment and better test coverage for actor assignee features. --- internal/prompter/accessible_prompter_test.go | 42 +++++++++---------- pkg/cmd/issue/edit/edit_test.go | 2 +- pkg/cmd/pr/edit/edit_test.go | 37 ++++++++-------- pkg/cmd/pr/shared/editable.go | 10 ++++- 4 files changed, 50 insertions(+), 41 deletions(-) diff --git a/internal/prompter/accessible_prompter_test.go b/internal/prompter/accessible_prompter_test.go index 20ce5d844..c944ef6cd 100644 --- a/internal/prompter/accessible_prompter_test.go +++ b/internal/prompter/accessible_prompter_test.go @@ -228,15 +228,27 @@ func TestAccessiblePrompter(t *testing.T) { console := newTestVirtualTerminal(t) p := newTestAccessiblePrompter(t, console) persistentOptions := []string{"persistent-option-1"} - searchFunc := func(input string) prompter.MultiSelectSearchResult { - var searchResultKeys []string - var searchResultLabels []string + searchFunc := func(input string) prompter.MultiSelectSearchResult { + var searchResultKeys []string + var searchResultLabels []string - // Initial search with no input - if input == "" { - moreResults := 2 - searchResultKeys = []string{"initial-result-1", "initial-result-2"} - searchResultLabels = []string{"Initial Result Label 1", "Initial Result Label 2"} + // Initial search with no input + if input == "" { + moreResults := 2 + searchResultKeys = []string{"initial-result-1", "initial-result-2"} + searchResultLabels = []string{"Initial Result Label 1", "Initial Result Label 2"} + return prompter.MultiSelectSearchResult{ + Keys: searchResultKeys, + Labels: searchResultLabels, + MoreResults: moreResults, + Err: nil, + } + } + + // Subsequent search with input + moreResults := 0 + searchResultKeys = []string{"search-result-1", "search-result-2"} + searchResultLabels = []string{"Search Result Label 1", "Search Result Label 2"} return prompter.MultiSelectSearchResult{ Keys: searchResultKeys, Labels: searchResultLabels, @@ -245,19 +257,7 @@ func TestAccessiblePrompter(t *testing.T) { } } - // Subsequent search with input - moreResults := 0 - searchResultKeys = []string{"search-result-1", "search-result-2"} - searchResultLabels = []string{"Search Result Label 1", "Search Result Label 2"} - return prompter.MultiSelectSearchResult{ - Keys: searchResultKeys, - Labels: searchResultLabels, - MoreResults: moreResults, - Err: nil, - } - } - - go func() { + go func() { // Wait for prompt to appear _, err := console.ExpectString("Select an option \r\n") require.NoError(t, err) diff --git a/pkg/cmd/issue/edit/edit_test.go b/pkg/cmd/issue/edit/edit_test.go index 4261efda7..2fcb85c56 100644 --- a/pkg/cmd/issue/edit/edit_test.go +++ b/pkg/cmd/issue/edit/edit_test.go @@ -845,7 +845,7 @@ func mockRepoMetadata(_ *testing.T, reg *httpmock.Registry) { { "data": { "repository": { "suggestedActors": { "nodes": [ { "login": "hubot", "id": "HUBOTID", "__typename": "Bot" }, - { "login": "MonaLisa", "id": "MONAID", "name": "Mona Display Name", "__typename": "User" } + { "login": "monalisa", "id": "MONAID", "name": "Mona Display Name", "__typename": "User" } ], "pageInfo": { "hasNextPage": false } } } } } diff --git a/pkg/cmd/pr/edit/edit_test.go b/pkg/cmd/pr/edit/edit_test.go index cd26c1a0a..2bd2497f4 100644 --- a/pkg/cmd/pr/edit/edit_test.go +++ b/pkg/cmd/pr/edit/edit_test.go @@ -714,7 +714,13 @@ func Test_editRun(t *testing.T) { editFields: func(e *shared.Editable, _ string) error { e.Title.Value = "new title" e.Body.Value = "new body" - e.Assignees.Value = []string{"monalisa", "hubot"} + // When ActorAssignees is enabled, the interactive flow returns display names (or logins for non-users) + e.Assignees.Value = []string{"monalisa (Mona Display Name)", "hubot"} + // Populate metadata to simulate what searchFunc would do during prompting + e.Metadata.AssignableActors = []api.AssignableActor{ + api.NewAssignableBot("HUBOTID", "hubot"), + api.NewAssignableUser("MONAID", "monalisa", "Mona Display Name"), + } e.Labels.Value = []string{"feature", "TODO", "bug"} e.Labels.Add = []string{"feature", "TODO", "bug"} e.Labels.Remove = []string{"docs"} @@ -728,7 +734,8 @@ func Test_editRun(t *testing.T) { }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { // interactive but reviewers not chosen; need everything except reviewers/teams - mockRepoMetadata(reg, mockRepoMetadataOptions{assignees: true, labels: true, projects: true, milestones: true}) + // assignees: false because searchFunc handles dynamic fetching (metadata populated in test mock) + mockRepoMetadata(reg, mockRepoMetadataOptions{assignees: false, labels: true, projects: true, milestones: true}) mockPullRequestUpdate(reg) mockPullRequestUpdateActorAssignees(reg) mockPullRequestUpdateLabels(reg) @@ -822,8 +829,13 @@ func Test_editRun(t *testing.T) { require.Equal(t, []string{"hubot"}, e.Assignees.Default) require.Equal(t, []string{"hubot"}, e.Assignees.DefaultLogins) - // Adding MonaLisa as PR assignee, should preserve hubot. - e.Assignees.Value = []string{"hubot", "MonaLisa (Mona Display Name)"} + // Adding monalisa as PR assignee, should preserve hubot. + e.Assignees.Value = []string{"hubot", "monalisa (Mona Display Name)"} + // Populate metadata to simulate what searchFunc would do during prompting + e.Metadata.AssignableActors = []api.AssignableActor{ + api.NewAssignableBot("HUBOTID", "hubot"), + api.NewAssignableUser("MONAID", "monalisa", "Mona Display Name"), + } return nil }, }, @@ -831,17 +843,8 @@ func Test_editRun(t *testing.T) { EditorRetriever: testEditorRetriever{}, }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { - reg.Register( - httpmock.GraphQL(`query RepositoryAssignableActors\b`), - httpmock.StringResponse(` - { "data": { "repository": { "suggestedActors": { - "nodes": [ - { "login": "hubot", "id": "HUBOTID", "__typename": "Bot" }, - { "login": "MonaLisa", "id": "MONAID", "name": "Mona Display Name", "__typename": "User" } - ], - "pageInfo": { "hasNextPage": false } - } } } } - `)) + // No RepositoryAssignableActors query needed - searchFunc handles dynamic fetching + // (metadata populated in test mock) mockPullRequestUpdate(reg) reg.Register( httpmock.GraphQL(`mutation ReplaceActorsForAssignable\b`), @@ -886,7 +889,7 @@ func Test_editRun(t *testing.T) { { "data": { "repository": { "assignableUsers": { "nodes": [ { "login": "hubot", "id": "HUBOTID" }, - { "login": "MonaLisa", "id": "MONAID" } + { "login": "monalisa", "id": "MONAID" } ], "pageInfo": { "hasNextPage": false } } } } } @@ -1001,7 +1004,7 @@ func mockRepoMetadata(reg *httpmock.Registry, opt mockRepoMetadataOptions) { { "data": { "repository": { "suggestedActors": { "nodes": [ { "login": "hubot", "id": "HUBOTID", "__typename": "Bot" }, - { "login": "MonaLisa", "id": "MONAID", "name": "Mona Display Name", "__typename": "User" } + { "login": "monalisa", "id": "MONAID", "name": "Mona Display Name", "__typename": "User" } ], "pageInfo": { "hasNextPage": false } } } } } diff --git a/pkg/cmd/pr/shared/editable.go b/pkg/cmd/pr/shared/editable.go index 7cfe49d17..7e6e77219 100644 --- a/pkg/cmd/pr/shared/editable.go +++ b/pkg/cmd/pr/shared/editable.go @@ -430,11 +430,17 @@ func FetchOptions(client *api.Client, repo ghrepo.Interface, editable *Editable, fetchAssignees := false if editable.Assignees.Edited { // Similar as above, this is likely an interactive flow if no Add/Remove slices are set. - // The addition here is that we also check for an assignee search func because - // if that is set, the prompter will handle dynamic fetching of assignees. + // The addition here is that we also check for an assignee search func. + // If we have a search func, we don't need to fetch assignees since we + // assume that will be done dynamically in the prompting flow. if len(editable.Assignees.Add) == 0 && len(editable.Assignees.Remove) == 0 && editable.AssigneeSearchFunc == nil { fetchAssignees = true } + // However, if we have Add/Remove operations (non-interactive flow), + // we do need to fetch the assignees. + if len(editable.Assignees.Add) > 0 || len(editable.Assignees.Remove) > 0 { + fetchAssignees = true + } } input := api.RepoMetadataInput{