From 9010a2e12c0e3e0fedaae5d03c1a6ccc159b8296 Mon Sep 17 00:00:00 2001 From: nate smith Date: Fri, 7 Apr 2023 14:57:09 -0700 Subject: [PATCH 01/12] use new prompter in run view --- pkg/cmd/run/shared/shared.go | 7 +- pkg/cmd/run/view/view.go | 18 ++-- pkg/cmd/run/view/view_test.go | 166 ++++++++++++++++++++++------------ 3 files changed, 119 insertions(+), 72 deletions(-) diff --git a/pkg/cmd/run/shared/shared.go b/pkg/cmd/run/shared/shared.go index 94a5590a3..8cdd42f87 100644 --- a/pkg/cmd/run/shared/shared.go +++ b/pkg/cmd/run/shared/shared.go @@ -12,12 +12,15 @@ import ( "github.com/AlecAivazis/survey/v2" "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/ghrepo" - "github.com/cli/cli/v2/internal/prompter" workflowShared "github.com/cli/cli/v2/pkg/cmd/workflow/shared" "github.com/cli/cli/v2/pkg/iostreams" "github.com/cli/cli/v2/pkg/prompt" ) +type Prompter interface { + Select(string, string, []string) (int, error) +} + const ( // Run statuses Queued Status = "queued" @@ -443,7 +446,7 @@ func GetJob(client *api.Client, repo ghrepo.Interface, jobID string) (*Job, erro } // SelectRun prompts the user to select a run from a list of runs by using the recommended prompter interface -func SelectRun(p prompter.Prompter, cs *iostreams.ColorScheme, runs []Run) (string, error) { +func SelectRun(p Prompter, cs *iostreams.ColorScheme, runs []Run) (string, error) { now := time.Now() candidates := []string{} diff --git a/pkg/cmd/run/view/view.go b/pkg/cmd/run/view/view.go index 34c5fcbc7..d62737595 100644 --- a/pkg/cmd/run/view/view.go +++ b/pkg/cmd/run/view/view.go @@ -14,7 +14,6 @@ import ( "strconv" "time" - "github.com/AlecAivazis/survey/v2" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/browser" @@ -24,7 +23,6 @@ import ( "github.com/cli/cli/v2/pkg/cmd/run/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/pkg/prompt" "github.com/spf13/cobra" ) @@ -65,6 +63,7 @@ type ViewOptions struct { IO *iostreams.IOStreams BaseRepo func() (ghrepo.Interface, error) Browser browser.Browser + Prompter shared.Prompter RunLogCache runLogCache RunID string @@ -86,6 +85,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman opts := &ViewOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, + Prompter: f.Prompter, Now: time.Now, Browser: f.Browser, RunLogCache: rlc{}, @@ -205,7 +205,7 @@ func runView(opts *ViewOptions) error { if err != nil { return fmt.Errorf("failed to get runs: %w", err) } - runID, err = shared.PromptForRun(cs, runs.WorkflowRuns) + runID, err = shared.SelectRun(opts.Prompter, cs, runs.WorkflowRuns) if err != nil { return err } @@ -228,7 +228,7 @@ func runView(opts *ViewOptions) error { } if opts.Prompt && len(jobs) > 1 { - selectedJob, err = promptForJob(cs, jobs) + selectedJob, err = promptForJob(opts.Prompter, cs, jobs) if err != nil { return err } @@ -459,20 +459,14 @@ func getRunLog(cache runLogCache, httpClient *http.Client, repo ghrepo.Interface return cache.Open(filepath) } -func promptForJob(cs *iostreams.ColorScheme, jobs []shared.Job) (*shared.Job, error) { +func promptForJob(prompter shared.Prompter, cs *iostreams.ColorScheme, jobs []shared.Job) (*shared.Job, error) { candidates := []string{"View all jobs in this run"} for _, job := range jobs { symbol, _ := shared.Symbol(cs, job.Status, job.Conclusion) candidates = append(candidates, fmt.Sprintf("%s %s", symbol, job.Name)) } - var selected int - //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter - err := prompt.SurveyAskOne(&survey.Select{ - Message: "View a specific job in this run?", - Options: candidates, - PageSize: 12, - }, &selected) + selected, err := prompter.Select("View a specific job in this run?", "", candidates) if err != nil { return nil, err } diff --git a/pkg/cmd/run/view/view_test.go b/pkg/cmd/run/view/view_test.go index 99a9ef30e..be9e0554d 100644 --- a/pkg/cmd/run/view/view_test.go +++ b/pkg/cmd/run/view/view_test.go @@ -12,12 +12,12 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/browser" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/pkg/cmd/run/shared" workflowShared "github.com/cli/cli/v2/pkg/cmd/workflow/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/pkg/prompt" "github.com/google/shlex" "github.com/stretchr/testify/assert" ) @@ -169,15 +169,15 @@ func TestNewCmdView(t *testing.T) { func TestViewRun(t *testing.T) { tests := []struct { - name string - httpStubs func(*httpmock.Registry) - askStubs func(*prompt.AskStubber) - opts *ViewOptions - tty bool - wantErr bool - wantOut string - browsedURL string - errMsg string + name string + httpStubs func(*httpmock.Registry) + promptStubs func(*prompter.MockPrompter) + opts *ViewOptions + tty bool + wantErr bool + wantOut string + browsedURL string + errMsg string }{ { name: "associate with PR", @@ -532,9 +532,12 @@ func TestViewRun(t *testing.T) { httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), httpmock.JSONResponse(shared.TestWorkflow)) }, - askStubs: func(as *prompt.AskStubber) { - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(2) + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterSelect("Select a workflow run", + []string{"X cool commit, CI (trunk) Feb 23, 2021", "* cool commit, CI (trunk) Feb 23, 2021", "✓ cool commit, CI (trunk) Feb 23, 2021", "X cool commit, CI (trunk) Feb 23, 2021", "X cool commit, CI (trunk) Feb 23, 2021", "- cool commit, CI (trunk) Feb 23, 2021", "- cool commit, CI (trunk) Feb 23, 2021", "* cool commit, CI (trunk) Feb 23, 2021", "* cool commit, CI (trunk) Feb 23, 2021", "X cool commit, CI (trunk) Feb 23, 2021"}, + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "✓ cool commit, CI (trunk) Feb 23, 2021") + }) }, opts: &ViewOptions{ Prompt: true, @@ -579,11 +582,17 @@ func TestViewRun(t *testing.T) { }, })) }, - askStubs: func(as *prompt.AskStubber) { - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(2) - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(1) + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterSelect("Select a workflow run", + []string{"X cool commit, CI (trunk) Feb 23, 2021", "* cool commit, CI (trunk) Feb 23, 2021", "✓ cool commit, CI (trunk) Feb 23, 2021", "X cool commit, CI (trunk) Feb 23, 2021", "X cool commit, CI (trunk) Feb 23, 2021", "- cool commit, CI (trunk) Feb 23, 2021", "- cool commit, CI (trunk) Feb 23, 2021", "* cool commit, CI (trunk) Feb 23, 2021", "* cool commit, CI (trunk) Feb 23, 2021", "X cool commit, CI (trunk) Feb 23, 2021"}, + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "✓ cool commit, CI (trunk) Feb 23, 2021") + }) + pm.RegisterSelect("View a specific job in this run?", + []string{"View all jobs in this run", "✓ cool job", "X sad job"}, + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "✓ cool job") + }) }, wantOut: coolJobRunLogOutput, }, @@ -626,11 +635,17 @@ func TestViewRun(t *testing.T) { }, })) }, - askStubs: func(as *prompt.AskStubber) { - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(2) - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(1) + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterSelect("Select a workflow run", + []string{"X cool commit, CI (trunk) Feb 23, 2021", "* cool commit, CI (trunk) Feb 23, 2021", "✓ cool commit, CI (trunk) Feb 23, 2021", "X cool commit, CI (trunk) Feb 23, 2021", "X cool commit, CI (trunk) Feb 23, 2021", "- cool commit, CI (trunk) Feb 23, 2021", "- cool commit, CI (trunk) Feb 23, 2021", "* cool commit, CI (trunk) Feb 23, 2021", "* cool commit, CI (trunk) Feb 23, 2021", "X cool commit, CI (trunk) Feb 23, 2021"}, + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "✓ cool commit, CI (trunk) Feb 23, 2021") + }) + pm.RegisterSelect("View a specific job in this run?", + []string{"View all jobs in this run", "✓ cool job", "X sad job"}, + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "✓ cool job") + }) }, wantOut: coolJobRunLogOutput, }, @@ -717,11 +732,17 @@ func TestViewRun(t *testing.T) { }, })) }, - askStubs: func(as *prompt.AskStubber) { - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(2) - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(0) + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterSelect("Select a workflow run", + []string{"X cool commit, CI (trunk) Feb 23, 2021", "* cool commit, CI (trunk) Feb 23, 2021", "✓ cool commit, CI (trunk) Feb 23, 2021", "X cool commit, CI (trunk) Feb 23, 2021", "X cool commit, CI (trunk) Feb 23, 2021", "- cool commit, CI (trunk) Feb 23, 2021", "- cool commit, CI (trunk) Feb 23, 2021", "* cool commit, CI (trunk) Feb 23, 2021", "* cool commit, CI (trunk) Feb 23, 2021", "X cool commit, CI (trunk) Feb 23, 2021"}, + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "✓ cool commit, CI (trunk) Feb 23, 2021") + }) + pm.RegisterSelect("View a specific job in this run?", + []string{"View all jobs in this run", "✓ cool job", "X sad job"}, + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "View all jobs in this run") + }) }, wantOut: expectedRunLogOutput, }, @@ -791,11 +812,17 @@ func TestViewRun(t *testing.T) { }, })) }, - askStubs: func(as *prompt.AskStubber) { - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(4) - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(2) + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterSelect("Select a workflow run", + []string{"X cool commit, CI (trunk) Feb 23, 2021", "* cool commit, CI (trunk) Feb 23, 2021", "✓ cool commit, CI (trunk) Feb 23, 2021", "X cool commit, CI (trunk) Feb 23, 2021", "X cool commit, CI (trunk) Feb 23, 2021", "- cool commit, CI (trunk) Feb 23, 2021", "- cool commit, CI (trunk) Feb 23, 2021", "* cool commit, CI (trunk) Feb 23, 2021", "* cool commit, CI (trunk) Feb 23, 2021", "X cool commit, CI (trunk) Feb 23, 2021"}, + func(_, _ string, opts []string) (int, error) { + return 4, nil + }) + pm.RegisterSelect("View a specific job in this run?", + []string{"View all jobs in this run", "✓ cool job", "X sad job"}, + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "X sad job") + }) }, wantOut: quuxTheBarfLogOutput, }, @@ -838,11 +865,17 @@ func TestViewRun(t *testing.T) { }, })) }, - askStubs: func(as *prompt.AskStubber) { - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(4) - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(2) + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterSelect("Select a workflow run", + []string{"X cool commit, CI (trunk) Feb 23, 2021", "* cool commit, CI (trunk) Feb 23, 2021", "✓ cool commit, CI (trunk) Feb 23, 2021", "X cool commit, CI (trunk) Feb 23, 2021", "X cool commit, CI (trunk) Feb 23, 2021", "- cool commit, CI (trunk) Feb 23, 2021", "- cool commit, CI (trunk) Feb 23, 2021", "* cool commit, CI (trunk) Feb 23, 2021", "* cool commit, CI (trunk) Feb 23, 2021", "X cool commit, CI (trunk) Feb 23, 2021"}, + func(_, _ string, opts []string) (int, error) { + return 4, nil + }) + pm.RegisterSelect("View a specific job in this run?", + []string{"View all jobs in this run", "✓ cool job", "X sad job"}, + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "X sad job") + }) }, wantOut: quuxTheBarfLogOutput, }, @@ -906,11 +939,17 @@ func TestViewRun(t *testing.T) { httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), httpmock.JSONResponse(shared.TestWorkflow)) }, - askStubs: func(as *prompt.AskStubber) { - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(4) - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(0) + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterSelect("Select a workflow run", + []string{"X cool commit, CI (trunk) Feb 23, 2021", "* cool commit, CI (trunk) Feb 23, 2021", "✓ cool commit, CI (trunk) Feb 23, 2021", "X cool commit, CI (trunk) Feb 23, 2021", "X cool commit, CI (trunk) Feb 23, 2021", "- cool commit, CI (trunk) Feb 23, 2021", "- cool commit, CI (trunk) Feb 23, 2021", "* cool commit, CI (trunk) Feb 23, 2021", "* cool commit, CI (trunk) Feb 23, 2021", "X cool commit, CI (trunk) Feb 23, 2021"}, + func(_, _ string, opts []string) (int, error) { + return 4, nil + }) + pm.RegisterSelect("View a specific job in this run?", + []string{"View all jobs in this run", "✓ cool job", "X sad job"}, + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "View all jobs in this run") + }) }, wantOut: quuxTheBarfLogOutput, }, @@ -1054,11 +1093,17 @@ func TestViewRun(t *testing.T) { httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), httpmock.JSONResponse(shared.TestWorkflow)) }, - askStubs: func(as *prompt.AskStubber) { - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(2) - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(0) + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterSelect("Select a workflow run", + []string{"X cool commit, CI (trunk) Feb 23, 2021", "* cool commit, CI (trunk) Feb 23, 2021", "✓ cool commit, CI (trunk) Feb 23, 2021", "X cool commit, CI (trunk) Feb 23, 2021", "X cool commit, CI (trunk) Feb 23, 2021", "- cool commit, CI (trunk) Feb 23, 2021", "- cool commit, CI (trunk) Feb 23, 2021", "* cool commit, CI (trunk) Feb 23, 2021", "* cool commit, CI (trunk) Feb 23, 2021", "X cool commit, CI (trunk) Feb 23, 2021"}, + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "✓ cool commit, CI (trunk) Feb 23, 2021") + }) + pm.RegisterSelect("View a specific job in this run?", + []string{"View all jobs in this run", "✓ cool job", "X sad job"}, + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "View all jobs in this run") + }) }, wantOut: "\n✓ trunk CI · 3\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\nX sad job in 4m34s (ID 20)\n ✓ barf the quux\n X quux the barf\n\nANNOTATIONS\nX the job is sad\nsad job: blaze.py#420\n\n\nFor more information about a job, try: gh run view --job=\nView this run on GitHub: https://github.com/runs/3\n", }, @@ -1099,11 +1144,17 @@ func TestViewRun(t *testing.T) { httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), httpmock.JSONResponse(shared.TestWorkflow)) }, - askStubs: func(as *prompt.AskStubber) { - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(2) - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(1) + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterSelect("Select a workflow run", + []string{"X cool commit, CI (trunk) Feb 23, 2021", "* cool commit, CI (trunk) Feb 23, 2021", "✓ cool commit, CI (trunk) Feb 23, 2021", "X cool commit, CI (trunk) Feb 23, 2021", "X cool commit, CI (trunk) Feb 23, 2021", "- cool commit, CI (trunk) Feb 23, 2021", "- cool commit, CI (trunk) Feb 23, 2021", "* cool commit, CI (trunk) Feb 23, 2021", "* cool commit, CI (trunk) Feb 23, 2021", "X cool commit, CI (trunk) Feb 23, 2021"}, + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "✓ cool commit, CI (trunk) Feb 23, 2021") + }) + pm.RegisterSelect("View a specific job in this run?", + []string{"View all jobs in this run", "✓ cool job", "X sad job"}, + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "✓ cool job") + }) }, wantOut: "\n✓ trunk CI · 3\nTriggered via push about 59 minutes ago\n\n✓ cool job in 4m34s (ID 10)\n ✓ fob the barz\n ✓ barz the fob\n\nTo see the full job log, try: gh run view --log --job=10\nView this run on GitHub: https://github.com/runs/3\n", }, @@ -1211,11 +1262,10 @@ func TestViewRun(t *testing.T) { return ghrepo.FromFullName("OWNER/REPO") } - //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber - as, teardown := prompt.InitAskStubber() - defer teardown() - if tt.askStubs != nil { - tt.askStubs(as) + pm := prompter.NewMockPrompter(t) + tt.opts.Prompter = pm + if tt.promptStubs != nil { + tt.promptStubs(pm) } browser := &browser.Stub{} From 415424de85226ab44bbd3ccd51fc1c8edd378692 Mon Sep 17 00:00:00 2001 From: nate smith Date: Mon, 10 Apr 2023 16:29:24 -0700 Subject: [PATCH 02/12] use new prompter in run cancel --- pkg/cmd/run/cancel/cancel.go | 4 +++- pkg/cmd/run/cancel/cancel_test.go | 34 ++++++++++++++++--------------- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/pkg/cmd/run/cancel/cancel.go b/pkg/cmd/run/cancel/cancel.go index ca4521b1d..73953bd67 100644 --- a/pkg/cmd/run/cancel/cancel.go +++ b/pkg/cmd/run/cancel/cancel.go @@ -17,6 +17,7 @@ type CancelOptions struct { HttpClient func() (*http.Client, error) IO *iostreams.IOStreams BaseRepo func() (ghrepo.Interface, error) + Prompter shared.Prompter Prompt bool @@ -27,6 +28,7 @@ func NewCmdCancel(f *cmdutil.Factory, runF func(*CancelOptions) error) *cobra.Co opts := &CancelOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, + Prompter: f.Prompter, } cmd := &cobra.Command{ @@ -83,7 +85,7 @@ func runCancel(opts *CancelOptions) error { if len(runs) == 0 { return fmt.Errorf("found no in progress runs to cancel") } - runID, err = shared.PromptForRun(cs, runs) + runID, err = shared.SelectRun(opts.Prompter, cs, runs) if err != nil { return err } diff --git a/pkg/cmd/run/cancel/cancel_test.go b/pkg/cmd/run/cancel/cancel_test.go index bcde453c2..fdfedfa0c 100644 --- a/pkg/cmd/run/cancel/cancel_test.go +++ b/pkg/cmd/run/cancel/cancel_test.go @@ -7,12 +7,12 @@ import ( "testing" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/pkg/cmd/run/shared" workflowShared "github.com/cli/cli/v2/pkg/cmd/workflow/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/pkg/prompt" "github.com/google/shlex" "github.com/stretchr/testify/assert" ) @@ -86,13 +86,13 @@ func TestRunCancel(t *testing.T) { inProgressRun := shared.TestRun(1234, shared.InProgress, "") completedRun := shared.TestRun(4567, shared.Completed, shared.Failure) tests := []struct { - name string - httpStubs func(*httpmock.Registry) - askStubs func(*prompt.AskStubber) - opts *CancelOptions - wantErr bool - wantOut string - errMsg string + name string + httpStubs func(*httpmock.Registry) + promptStubs func(*prompter.MockPrompter) + opts *CancelOptions + wantErr bool + wantOut string + errMsg string }{ { name: "cancel run", @@ -194,9 +194,12 @@ func TestRunCancel(t *testing.T) { httpmock.REST("POST", "repos/OWNER/REPO/actions/runs/1234/cancel"), httpmock.StatusStringResponse(202, "{}")) }, - askStubs: func(as *prompt.AskStubber) { - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(0) + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterSelect("Select a workflow run", + []string{"* cool commit, CI (trunk) Feb 23, 2021"}, + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "* cool commit, CI (trunk) Feb 23, 2021") + }) }, wantOut: "✓ Request to cancel workflow submitted.\n", }, @@ -217,11 +220,10 @@ func TestRunCancel(t *testing.T) { return ghrepo.FromFullName("OWNER/REPO") } - //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber - as, teardown := prompt.InitAskStubber() - defer teardown() - if tt.askStubs != nil { - tt.askStubs(as) + pm := prompter.NewMockPrompter(t) + tt.opts.Prompter = pm + if tt.promptStubs != nil { + tt.promptStubs(pm) } t.Run(tt.name, func(t *testing.T) { From 07776d23b19612de357d31463d416b4ef11af344 Mon Sep 17 00:00:00 2001 From: nate smith Date: Mon, 10 Apr 2023 16:35:00 -0700 Subject: [PATCH 03/12] use new prompter in run rerun --- pkg/cmd/run/rerun/rerun.go | 5 +++-- pkg/cmd/run/rerun/rerun_test.go | 38 +++++++++++++++++---------------- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/pkg/cmd/run/rerun/rerun.go b/pkg/cmd/run/rerun/rerun.go index 2522e632c..b640f41f7 100644 --- a/pkg/cmd/run/rerun/rerun.go +++ b/pkg/cmd/run/rerun/rerun.go @@ -20,6 +20,7 @@ type RerunOptions struct { HttpClient func() (*http.Client, error) IO *iostreams.IOStreams BaseRepo func() (ghrepo.Interface, error) + Prompter shared.Prompter RunID string OnlyFailed bool @@ -32,6 +33,7 @@ type RerunOptions struct { func NewCmdRerun(f *cmdutil.Factory, runF func(*RerunOptions) error) *cobra.Command { opts := &RerunOptions{ IO: f.IOStreams, + Prompter: f.Prompter, HttpClient: f.HttpClient, } @@ -114,8 +116,7 @@ func runRerun(opts *RerunOptions) error { if len(runs) == 0 { return errors.New("no recent runs have failed; please specify a specific ``") } - runID, err = shared.PromptForRun(cs, runs) - if err != nil { + if runID, err = shared.SelectRun(opts.Prompter, cs, runs); err != nil { return err } } diff --git a/pkg/cmd/run/rerun/rerun_test.go b/pkg/cmd/run/rerun/rerun_test.go index 730952f1f..991ab5f3a 100644 --- a/pkg/cmd/run/rerun/rerun_test.go +++ b/pkg/cmd/run/rerun/rerun_test.go @@ -8,12 +8,12 @@ import ( "testing" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/pkg/cmd/run/shared" workflowShared "github.com/cli/cli/v2/pkg/cmd/workflow/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/pkg/prompt" "github.com/google/shlex" "github.com/stretchr/testify/assert" ) @@ -161,15 +161,15 @@ func TestNewCmdRerun(t *testing.T) { func TestRerun(t *testing.T) { tests := []struct { - name string - httpStubs func(*httpmock.Registry) - askStubs func(*prompt.AskStubber) - opts *RerunOptions - tty bool - wantErr bool - errOut string - wantOut string - wantDebug bool + name string + httpStubs func(*httpmock.Registry) + promptStubs func(*prompter.MockPrompter) + opts *RerunOptions + tty bool + wantErr bool + errOut string + wantOut string + wantDebug bool }{ { name: "arg", @@ -336,9 +336,12 @@ func TestRerun(t *testing.T) { httpmock.REST("POST", "repos/OWNER/REPO/actions/runs/1234/rerun"), httpmock.StringResponse("{}")) }, - askStubs: func(as *prompt.AskStubber) { - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(2) + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterSelect("Select a workflow run", + []string{"X cool commit, CI (trunk) Feb 23, 2021", "X cool commit, CI (trunk) Feb 23, 2021", "X cool commit, CI (trunk) Feb 23, 2021", "- cool commit, CI (trunk) Feb 23, 2021", "- cool commit, CI (trunk) Feb 23, 2021", "X cool commit, CI (trunk) Feb 23, 2021"}, + func(_, _ string, opts []string) (int, error) { + return 2, nil + }) }, wantOut: "✓ Requested rerun of run 1234\n", }, @@ -408,11 +411,10 @@ func TestRerun(t *testing.T) { return ghrepo.FromFullName("OWNER/REPO") } - //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber - as, teardown := prompt.InitAskStubber() - defer teardown() - if tt.askStubs != nil { - tt.askStubs(as) + pm := prompter.NewMockPrompter(t) + tt.opts.Prompter = pm + if tt.promptStubs != nil { + tt.promptStubs(pm) } t.Run(tt.name, func(t *testing.T) { From 76fdfaa895271a63e2e842b0161332b7c80ccaac Mon Sep 17 00:00:00 2001 From: nate smith Date: Mon, 10 Apr 2023 16:42:35 -0700 Subject: [PATCH 04/12] use new prompter in run watch --- pkg/cmd/run/watch/watch.go | 7 ++--- pkg/cmd/run/watch/watch_test.go | 46 ++++++++++++++++++--------------- 2 files changed, 29 insertions(+), 24 deletions(-) diff --git a/pkg/cmd/run/watch/watch.go b/pkg/cmd/run/watch/watch.go index 05b207693..090737557 100644 --- a/pkg/cmd/run/watch/watch.go +++ b/pkg/cmd/run/watch/watch.go @@ -23,6 +23,7 @@ type WatchOptions struct { IO *iostreams.IOStreams HttpClient func() (*http.Client, error) BaseRepo func() (ghrepo.Interface, error) + Prompter shared.Prompter RunID string Interval int @@ -37,6 +38,7 @@ func NewCmdWatch(f *cmdutil.Factory, runF func(*WatchOptions) error) *cobra.Comm opts := &WatchOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, + Prompter: f.Prompter, Now: time.Now, } @@ -102,11 +104,10 @@ func watchRun(opts *WatchOptions) error { if len(runs) == 0 { return fmt.Errorf("found no in progress runs to watch") } - runID, err = shared.PromptForRun(cs, runs) - if err != nil { + if runID, err = shared.SelectRun(opts.Prompter, cs, runs); err != nil { return err } - // TODO silly stopgap until dust settles and PromptForRun can just return a run + // TODO silly stopgap until dust settles and SelectRun can just return a run for _, r := range runs { if fmt.Sprintf("%d", r.ID) == runID { run = &r diff --git a/pkg/cmd/run/watch/watch_test.go b/pkg/cmd/run/watch/watch_test.go index 89c75e72c..8ff77516b 100644 --- a/pkg/cmd/run/watch/watch_test.go +++ b/pkg/cmd/run/watch/watch_test.go @@ -8,12 +8,12 @@ import ( "time" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/pkg/cmd/run/shared" workflowShared "github.com/cli/cli/v2/pkg/cmd/workflow/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/pkg/prompt" "github.com/google/shlex" "github.com/stretchr/testify/assert" ) @@ -193,14 +193,14 @@ func TestWatchRun(t *testing.T) { } tests := []struct { - name string - httpStubs func(*httpmock.Registry) - askStubs func(*prompt.AskStubber) - opts *WatchOptions - tty bool - wantErr bool - errMsg string - wantOut string + name string + httpStubs func(*httpmock.Registry) + promptStubs func(*prompter.MockPrompter) + opts *WatchOptions + tty bool + wantErr bool + errMsg string + wantOut string }{ { name: "run ID provided run already completed", @@ -269,10 +269,12 @@ func TestWatchRun(t *testing.T) { Prompt: true, }, httpStubs: successfulRunStubs, - askStubs: func(as *prompt.AskStubber) { - as.StubPrompt("Select a workflow run"). - AssertOptions([]string{"* commit1, CI (trunk) Feb 23, 2021", "* commit2, CI (trunk) Feb 23, 2021"}). - AnswerWith("* commit2, CI (trunk) Feb 23, 2021") + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterSelect("Select a workflow run", + []string{"* commit1, CI (trunk) Feb 23, 2021", "* commit2, CI (trunk) Feb 23, 2021"}, + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "* commit2, CI (trunk) Feb 23, 2021") + }) }, wantOut: "\x1b[?1049h\x1b[0;0H\x1b[JRefreshing run status every 0 seconds. Press Ctrl+C to quit.\n\n* trunk CI · 2\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\n ✓ fob the barz\n ✓ barz the fob\n\x1b[?1049l✓ trunk CI · 2\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\n ✓ fob the barz\n ✓ barz the fob\n\n✓ Run CI (2) completed with 'success'\n", }, @@ -285,10 +287,12 @@ func TestWatchRun(t *testing.T) { ExitStatus: true, }, httpStubs: failedRunStubs, - askStubs: func(as *prompt.AskStubber) { - as.StubPrompt("Select a workflow run"). - AssertOptions([]string{"* commit1, CI (trunk) Feb 23, 2021", "* commit2, CI (trunk) Feb 23, 2021"}). - AnswerWith("* commit2, CI (trunk) Feb 23, 2021") + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterSelect("Select a workflow run", + []string{"* commit1, CI (trunk) Feb 23, 2021", "* commit2, CI (trunk) Feb 23, 2021"}, + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "* commit2, CI (trunk) Feb 23, 2021") + }) }, wantOut: "\x1b[?1049h\x1b[0;0H\x1b[JRefreshing run status every 0 seconds. Press Ctrl+C to quit.\n\n* trunk CI · 2\nTriggered via push about 59 minutes ago\n\n\x1b[?1049lX trunk CI · 2\nTriggered via push about 59 minutes ago\n\nJOBS\nX sad job in 4m34s (ID 20)\n ✓ barf the quux\n X quux the barf\n\nANNOTATIONS\nX the job is sad\nsad job: blaze.py#420\n\n\nX Run CI (2) completed with 'failure'\n", wantErr: true, @@ -317,10 +321,10 @@ func TestWatchRun(t *testing.T) { } t.Run(tt.name, func(t *testing.T) { - //nolint:staticcheck // SA1019: prompt.NewAskStubber is deprecated: use PrompterMock - as := prompt.NewAskStubber(t) - if tt.askStubs != nil { - tt.askStubs(as) + pm := prompter.NewMockPrompter(t) + tt.opts.Prompter = pm + if tt.promptStubs != nil { + tt.promptStubs(pm) } err := watchRun(tt.opts) From 400da5ac9457769e495d624c10ab90841cef2a0a Mon Sep 17 00:00:00 2001 From: nate smith Date: Mon, 10 Apr 2023 16:43:24 -0700 Subject: [PATCH 05/12] golf --- pkg/cmd/run/cancel/cancel.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/run/cancel/cancel.go b/pkg/cmd/run/cancel/cancel.go index 73953bd67..e5f77e016 100644 --- a/pkg/cmd/run/cancel/cancel.go +++ b/pkg/cmd/run/cancel/cancel.go @@ -85,11 +85,10 @@ func runCancel(opts *CancelOptions) error { if len(runs) == 0 { return fmt.Errorf("found no in progress runs to cancel") } - runID, err = shared.SelectRun(opts.Prompter, cs, runs) - if err != nil { + if runID, err = shared.SelectRun(opts.Prompter, cs, runs); err != nil { return err } - // TODO silly stopgap until dust settles and PromptForRun can just return a run + // TODO silly stopgap until dust settles and SelectRun can just return a run for _, r := range runs { if fmt.Sprintf("%d", r.ID) == runID { run = &r From 6fd17ec2d212d046c16f2f70763043bf095b1425 Mon Sep 17 00:00:00 2001 From: nate smith Date: Mon, 10 Apr 2023 16:43:31 -0700 Subject: [PATCH 06/12] delete PromptForRun --- pkg/cmd/run/shared/shared.go | 32 -------------------------------- 1 file changed, 32 deletions(-) diff --git a/pkg/cmd/run/shared/shared.go b/pkg/cmd/run/shared/shared.go index 8cdd42f87..0f3a8fca8 100644 --- a/pkg/cmd/run/shared/shared.go +++ b/pkg/cmd/run/shared/shared.go @@ -9,12 +9,10 @@ import ( "strings" "time" - "github.com/AlecAivazis/survey/v2" "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/ghrepo" workflowShared "github.com/cli/cli/v2/pkg/cmd/workflow/shared" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/pkg/prompt" ) type Prompter interface { @@ -467,36 +465,6 @@ func SelectRun(p Prompter, cs *iostreams.ColorScheme, runs []Run) (string, error return fmt.Sprintf("%d", runs[selected].ID), nil } -// TODO: this should be deprecated in favor of SelectRun -func PromptForRun(cs *iostreams.ColorScheme, runs []Run) (string, error) { - var selected int - now := time.Now() - - candidates := []string{} - - for _, run := range runs { - symbol, _ := Symbol(cs, run.Status, run.Conclusion) - candidates = append(candidates, - // TODO truncate commit message, long ones look terrible - fmt.Sprintf("%s %s, %s (%s) %s", symbol, run.Title(), run.WorkflowName(), run.HeadBranch, preciseAgo(now, run.StartedTime()))) - } - - // TODO consider custom filter so it's fuzzier. right now matches start anywhere in string but - // become contiguous - //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter - err := prompt.SurveyAskOne(&survey.Select{ - Message: "Select a workflow run", - Options: candidates, - PageSize: 10, - }, &selected) - - if err != nil { - return "", err - } - - return fmt.Sprintf("%d", runs[selected].ID), nil -} - func GetRun(client *api.Client, repo ghrepo.Interface, runID string, attempt uint64) (*Run, error) { var result Run From d4a8d15a8a3ed15e5e3eb417273c5b401013e320 Mon Sep 17 00:00:00 2001 From: nate smith Date: Mon, 10 Apr 2023 17:37:36 -0700 Subject: [PATCH 07/12] implement multi select support for prompter --- internal/prompter/prompter.go | 19 ++++++++++--- internal/prompter/prompter_mock.go | 44 +++++++++++++++--------------- internal/prompter/test.go | 10 +++---- 3 files changed, 42 insertions(+), 31 deletions(-) diff --git a/internal/prompter/prompter.go b/internal/prompter/prompter.go index a9cf77992..60c223cff 100644 --- a/internal/prompter/prompter.go +++ b/internal/prompter/prompter.go @@ -13,7 +13,7 @@ import ( //go:generate moq -rm -out prompter_mock.go . Prompter type Prompter interface { Select(string, string, []string) (int, error) - MultiSelect(string, string, []string) ([]string, error) + MultiSelect(string, []string, []string) ([]string, error) Input(string, string) (string, error) InputHostname() (string, error) Password(string) (string, error) @@ -72,15 +72,26 @@ func (p *surveyPrompter) Select(message, defaultValue string, options []string) return } -func (p *surveyPrompter) MultiSelect(message, defaultValue string, options []string) (result []string, err error) { +func (p *surveyPrompter) MultiSelect(message string, defaultValue, options []string) (result []string, err error) { q := &survey.MultiSelect{ Message: message, Options: options, PageSize: 20, } - if defaultValue != "" { - q.Default = defaultValue + // TODO will I regret this returning []string and not []int? + + if defaultValue != nil && len(defaultValue) > 0 { + // TODO I don't actually know that this is needed, just being extra cautious + validatedDefault := []string{} + for _, x := range defaultValue { + for _, y := range options { + if x == y { + validatedDefault = append(validatedDefault, x) + } + } + } + q.Default = validatedDefault } err = p.ask(q, &result) diff --git a/internal/prompter/prompter_mock.go b/internal/prompter/prompter_mock.go index 859832450..174375bfd 100644 --- a/internal/prompter/prompter_mock.go +++ b/internal/prompter/prompter_mock.go @@ -35,7 +35,7 @@ var _ Prompter = &PrompterMock{} // MarkdownEditorFunc: func(s1 string, s2 string, b bool) (string, error) { // panic("mock out the MarkdownEditor method") // }, -// MultiSelectFunc: func(s1 string, s2 string, strings []string) ([]string, error) { +// MultiSelectFunc: func(s string, strings1 []string, strings2 []string) ([]string, error) { // panic("mock out the MultiSelect method") // }, // PasswordFunc: func(s string) (string, error) { @@ -70,7 +70,7 @@ type PrompterMock struct { MarkdownEditorFunc func(s1 string, s2 string, b bool) (string, error) // MultiSelectFunc mocks the MultiSelect method. - MultiSelectFunc func(s1 string, s2 string, strings []string) ([]string, error) + MultiSelectFunc func(s string, strings1 []string, strings2 []string) ([]string, error) // PasswordFunc mocks the Password method. PasswordFunc func(s string) (string, error) @@ -116,12 +116,12 @@ type PrompterMock struct { } // MultiSelect holds details about calls to the MultiSelect method. MultiSelect []struct { - // S1 is the s1 argument value. - S1 string - // S2 is the s2 argument value. - S2 string - // Strings is the strings argument value. - Strings []string + // S is the s argument value. + S string + // Strings1 is the strings1 argument value. + Strings1 []string + // Strings2 is the strings2 argument value. + Strings2 []string } // Password holds details about calls to the Password method. Password []struct { @@ -348,23 +348,23 @@ func (mock *PrompterMock) MarkdownEditorCalls() []struct { } // MultiSelect calls MultiSelectFunc. -func (mock *PrompterMock) MultiSelect(s1 string, s2 string, strings []string) ([]string, error) { +func (mock *PrompterMock) MultiSelect(s string, strings1 []string, strings2 []string) ([]string, error) { if mock.MultiSelectFunc == nil { panic("PrompterMock.MultiSelectFunc: method is nil but Prompter.MultiSelect was just called") } callInfo := struct { - S1 string - S2 string - Strings []string + S string + Strings1 []string + Strings2 []string }{ - S1: s1, - S2: s2, - Strings: strings, + S: s, + Strings1: strings1, + Strings2: strings2, } mock.lockMultiSelect.Lock() mock.calls.MultiSelect = append(mock.calls.MultiSelect, callInfo) mock.lockMultiSelect.Unlock() - return mock.MultiSelectFunc(s1, s2, strings) + return mock.MultiSelectFunc(s, strings1, strings2) } // MultiSelectCalls gets all the calls that were made to MultiSelect. @@ -372,14 +372,14 @@ func (mock *PrompterMock) MultiSelect(s1 string, s2 string, strings []string) ([ // // len(mockedPrompter.MultiSelectCalls()) func (mock *PrompterMock) MultiSelectCalls() []struct { - S1 string - S2 string - Strings []string + S string + Strings1 []string + Strings2 []string } { var calls []struct { - S1 string - S2 string - Strings []string + S string + Strings1 []string + Strings2 []string } mock.lockMultiSelect.RLock() calls = mock.calls.MultiSelect diff --git a/internal/prompter/test.go b/internal/prompter/test.go index a2d0b83ee..a5cebacda 100644 --- a/internal/prompter/test.go +++ b/internal/prompter/test.go @@ -50,7 +50,7 @@ type ConfirmStub struct { type MultiSelectStub struct { Prompt string ExpectedOpts []string - Fn func(string, string, []string) ([]string, error) + Fn func(string, []string, []string) ([]string, error) } type InputHostnameStub struct { @@ -120,11 +120,11 @@ func NewMockPrompter(t *testing.T) *MockPrompter { return s.Fn(p, d, opts) } - m.MultiSelectFunc = func(p, d string, opts []string) ([]string, error) { + m.MultiSelectFunc = func(p string, d, opts []string) ([]string, error) { var s MultiSelectStub - if len(m.SelectStubs) > 0 { + if len(m.MultiSelectStubs) > 0 { s = m.MultiSelectStubs[0] - m.SelectStubs = m.SelectStubs[1:len(m.SelectStubs)] + m.MultiSelectStubs = m.MultiSelectStubs[1:len(m.MultiSelectStubs)] } else { return []string{}, NoSuchPromptErr(p) } @@ -240,7 +240,7 @@ func (m *MockPrompter) RegisterSelect(prompt string, opts []string, stub func(_, Fn: stub}) } -func (m *MockPrompter) RegisterMultiSelect(prompt string, opts []string, stub func(_, _ string, _ []string) ([]string, error)) { +func (m *MockPrompter) RegisterMultiSelect(prompt string, d, opts []string, stub func(_ string, _, _ []string) ([]string, error)) { m.MultiSelectStubs = append(m.MultiSelectStubs, MultiSelectStub{ Prompt: prompt, ExpectedOpts: opts, From c536114de0fd6308b215ed3eaec0c8474ffbc0c2 Mon Sep 17 00:00:00 2001 From: nate smith Date: Mon, 10 Apr 2023 17:38:30 -0700 Subject: [PATCH 08/12] use new prompter in run download --- pkg/cmd/run/download/download.go | 25 ++++--------- pkg/cmd/run/download/download_test.go | 51 +++++++++------------------ 2 files changed, 23 insertions(+), 53 deletions(-) diff --git a/pkg/cmd/run/download/download.go b/pkg/cmd/run/download/download.go index 25eec5d62..4c5322806 100644 --- a/pkg/cmd/run/download/download.go +++ b/pkg/cmd/run/download/download.go @@ -5,12 +5,10 @@ import ( "fmt" "path/filepath" - "github.com/AlecAivazis/survey/v2" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/pkg/cmd/run/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/pkg/prompt" "github.com/cli/cli/v2/pkg/set" "github.com/spf13/cobra" ) @@ -18,7 +16,7 @@ import ( type DownloadOptions struct { IO *iostreams.IOStreams Platform platform - Prompter prompter + Prompter iprompter DoPrompt bool RunID string @@ -31,13 +29,14 @@ type platform interface { List(runID string) ([]shared.Artifact, error) Download(url string, dir string) error } -type prompter interface { - Prompt(message string, options []string, result interface{}) error +type iprompter interface { + MultiSelect(string, []string, []string) ([]string, error) } func NewCmdDownload(f *cmdutil.Factory, runF func(*DownloadOptions) error) *cobra.Command { opts := &DownloadOptions{ - IO: f.IOStreams, + IO: f.IOStreams, + Prompter: f.Prompter, } cmd := &cobra.Command{ @@ -85,7 +84,6 @@ func NewCmdDownload(f *cmdutil.Factory, runF func(*DownloadOptions) error) *cobr client: httpClient, repo: baseRepo, } - opts.Prompter = &surveyPrompter{} if runF != nil { return runF(opts) @@ -133,8 +131,7 @@ func runDownload(opts *DownloadOptions) error { if len(options) > 10 { options = options[:10] } - err := opts.Prompter.Prompt("Select artifacts to download:", options, &wantNames) - if err != nil { + if wantNames, err = opts.Prompter.MultiSelect("Select artifacts to download:", nil, options); err != nil { return err } if len(wantNames) == 0 { @@ -194,13 +191,3 @@ func matchAnyPattern(patterns []string, name string) bool { } return false } - -type surveyPrompter struct{} - -func (sp *surveyPrompter) Prompt(message string, options []string, result interface{}) error { - //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter - return prompt.SurveyAskOne(&survey.MultiSelect{ - Message: message, - Options: options, - }, result) -} diff --git a/pkg/cmd/run/download/download_test.go b/pkg/cmd/run/download/download_test.go index 10c7bbe3c..3640f93a0 100644 --- a/pkg/cmd/run/download/download_test.go +++ b/pkg/cmd/run/download/download_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/pkg/cmd/run/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" @@ -144,11 +145,11 @@ func Test_NewCmdDownload(t *testing.T) { func Test_runDownload(t *testing.T) { tests := []struct { - name string - opts DownloadOptions - mockAPI func(*mockPlatform) - mockPrompt func(*mockPrompter) - wantErr string + name string + opts DownloadOptions + mockAPI func(*mockPlatform) + promptStubs func(*prompter.MockPrompter) + wantErr string }{ { name: "download non-expired", @@ -281,13 +282,11 @@ func Test_runDownload(t *testing.T) { }, nil) p.On("Download", "http://download.com/artifact2.zip", ".").Return(nil) }, - mockPrompt: func(p *mockPrompter) { - p.On("Prompt", "Select artifacts to download:", []string{"artifact-1", "artifact-2"}, mock.AnythingOfType("*[]string")). - Run(func(args mock.Arguments) { - result := args.Get(2).(*[]string) - *result = []string{"artifact-2"} - }). - Return(nil) + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterMultiSelect("Select artifacts to download:", nil, []string{"artifact-1", "artifact-2"}, + func(_ string, _, opts []string) ([]string, error) { + return []string{"artifact-2"}, nil + }) }, }, } @@ -297,7 +296,12 @@ func Test_runDownload(t *testing.T) { ios, _, stdout, stderr := iostreams.Test() opts.IO = ios opts.Platform = newMockPlatform(t, tt.mockAPI) - opts.Prompter = newMockPrompter(t, tt.mockPrompt) + + pm := prompter.NewMockPrompter(t) + opts.Prompter = pm + if tt.promptStubs != nil { + tt.promptStubs(pm) + } err := runDownload(opts) if tt.wantErr != "" { @@ -337,24 +341,3 @@ func (p *mockPlatform) Download(url string, dir string) error { args := p.Called(url, dir) return args.Error(0) } - -type mockPrompter struct { - mock.Mock -} - -func newMockPrompter(t *testing.T, config func(*mockPrompter)) *mockPrompter { - m := &mockPrompter{} - m.Test(t) - t.Cleanup(func() { - m.AssertExpectations(t) - }) - if config != nil { - config(m) - } - return m -} - -func (p *mockPrompter) Prompt(msg string, opts []string, res interface{}) error { - args := p.Called(msg, opts, res) - return args.Error(0) -} From cb0d2a86715b18ca6833222dee6b62351ca87397 Mon Sep 17 00:00:00 2001 From: nate smith Date: Mon, 10 Apr 2023 18:03:13 -0700 Subject: [PATCH 09/12] linter appeasement --- internal/prompter/prompter.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/prompter/prompter.go b/internal/prompter/prompter.go index 60c223cff..ada470d06 100644 --- a/internal/prompter/prompter.go +++ b/internal/prompter/prompter.go @@ -81,7 +81,7 @@ func (p *surveyPrompter) MultiSelect(message string, defaultValue, options []str // TODO will I regret this returning []string and not []int? - if defaultValue != nil && len(defaultValue) > 0 { + if len(defaultValue) > 0 { // TODO I don't actually know that this is needed, just being extra cautious validatedDefault := []string{} for _, x := range defaultValue { From 5597139df3c9aeedcf8b6ac3379bb59dda4a1778 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 23 May 2023 00:12:44 -0700 Subject: [PATCH 10/12] switch to []int return for multiselect --- internal/prompter/prompter.go | 6 ++---- internal/prompter/test.go | 12 +++++------- pkg/cmd/run/download/download.go | 9 +++++++-- pkg/cmd/run/download/download_test.go | 4 ++-- 4 files changed, 16 insertions(+), 15 deletions(-) diff --git a/internal/prompter/prompter.go b/internal/prompter/prompter.go index 9803f8a49..b91445c49 100644 --- a/internal/prompter/prompter.go +++ b/internal/prompter/prompter.go @@ -14,7 +14,7 @@ import ( //go:generate moq -rm -out prompter_mock.go . Prompter type Prompter interface { Select(string, string, []string) (int, error) - MultiSelect(string, []string, []string) ([]string, error) + MultiSelect(string, []string, []string) ([]int, error) Input(string, string) (string, error) InputHostname() (string, error) Password(string) (string, error) @@ -86,7 +86,7 @@ func (p *surveyPrompter) Select(message, defaultValue string, options []string) return } -func (p *surveyPrompter) MultiSelect(message string, defaultValue, options []string) (result []string, err error) { +func (p *surveyPrompter) MultiSelect(message string, defaultValue, options []string) (result []int, err error) { q := &survey.MultiSelect{ Message: message, Options: options, @@ -94,8 +94,6 @@ func (p *surveyPrompter) MultiSelect(message string, defaultValue, options []str Filter: LatinMatchingFilter, } - // TODO will I regret this returning []string and not []int? - if len(defaultValue) > 0 { // TODO I don't actually know that this is needed, just being extra cautious validatedDefault := []string{} diff --git a/internal/prompter/test.go b/internal/prompter/test.go index a5cebacda..c376e9c83 100644 --- a/internal/prompter/test.go +++ b/internal/prompter/test.go @@ -50,7 +50,7 @@ type ConfirmStub struct { type MultiSelectStub struct { Prompt string ExpectedOpts []string - Fn func(string, []string, []string) ([]string, error) + Fn func(string, []string, []string) ([]int, error) } type InputHostnameStub struct { @@ -84,8 +84,6 @@ type MockPrompter struct { ConfirmDeletionStubs []ConfirmDeletionStub } -// TODO thread safety - func NewMockPrompter(t *testing.T) *MockPrompter { m := &MockPrompter{ t: t, @@ -120,17 +118,17 @@ func NewMockPrompter(t *testing.T) *MockPrompter { return s.Fn(p, d, opts) } - m.MultiSelectFunc = func(p string, d, opts []string) ([]string, error) { + m.MultiSelectFunc = func(p string, d, opts []string) ([]int, error) { var s MultiSelectStub if len(m.MultiSelectStubs) > 0 { s = m.MultiSelectStubs[0] m.MultiSelectStubs = m.MultiSelectStubs[1:len(m.MultiSelectStubs)] } else { - return []string{}, NoSuchPromptErr(p) + return []int{}, NoSuchPromptErr(p) } if s.Prompt != p { - return []string{}, NoSuchPromptErr(p) + return []int{}, NoSuchPromptErr(p) } AssertOptions(m.t, s.ExpectedOpts, opts) @@ -240,7 +238,7 @@ func (m *MockPrompter) RegisterSelect(prompt string, opts []string, stub func(_, Fn: stub}) } -func (m *MockPrompter) RegisterMultiSelect(prompt string, d, opts []string, stub func(_ string, _, _ []string) ([]string, error)) { +func (m *MockPrompter) RegisterMultiSelect(prompt string, d, opts []string, stub func(_ string, _, _ []string) ([]int, error)) { m.MultiSelectStubs = append(m.MultiSelectStubs, MultiSelectStub{ Prompt: prompt, ExpectedOpts: opts, diff --git a/pkg/cmd/run/download/download.go b/pkg/cmd/run/download/download.go index 4c5322806..993e16e52 100644 --- a/pkg/cmd/run/download/download.go +++ b/pkg/cmd/run/download/download.go @@ -30,7 +30,7 @@ type platform interface { Download(url string, dir string) error } type iprompter interface { - MultiSelect(string, []string, []string) ([]string, error) + MultiSelect(string, []string, []string) ([]int, error) } func NewCmdDownload(f *cmdutil.Factory, runF func(*DownloadOptions) error) *cobra.Command { @@ -131,9 +131,14 @@ func runDownload(opts *DownloadOptions) error { if len(options) > 10 { options = options[:10] } - if wantNames, err = opts.Prompter.MultiSelect("Select artifacts to download:", nil, options); err != nil { + var selected []int + if selected, err = opts.Prompter.MultiSelect("Select artifacts to download:", nil, options); err != nil { return err } + wantNames = []string{} + for _, x := range selected { + wantNames = append(wantNames, options[x]) + } if len(wantNames) == 0 { return errors.New("no artifacts selected") } diff --git a/pkg/cmd/run/download/download_test.go b/pkg/cmd/run/download/download_test.go index 3640f93a0..3c1c8f2d8 100644 --- a/pkg/cmd/run/download/download_test.go +++ b/pkg/cmd/run/download/download_test.go @@ -284,8 +284,8 @@ func Test_runDownload(t *testing.T) { }, promptStubs: func(pm *prompter.MockPrompter) { pm.RegisterMultiSelect("Select artifacts to download:", nil, []string{"artifact-1", "artifact-2"}, - func(_ string, _, opts []string) ([]string, error) { - return []string{"artifact-2"}, nil + func(_ string, _, opts []string) ([]int, error) { + return []int{1}, nil }) }, }, From a15eb91a0c7b1392f8833f64b0cff24c553e6c80 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 23 May 2023 00:13:57 -0700 Subject: [PATCH 11/12] go generate --- internal/prompter/prompter_mock.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/prompter/prompter_mock.go b/internal/prompter/prompter_mock.go index 174375bfd..9a59a7bd1 100644 --- a/internal/prompter/prompter_mock.go +++ b/internal/prompter/prompter_mock.go @@ -35,7 +35,7 @@ var _ Prompter = &PrompterMock{} // MarkdownEditorFunc: func(s1 string, s2 string, b bool) (string, error) { // panic("mock out the MarkdownEditor method") // }, -// MultiSelectFunc: func(s string, strings1 []string, strings2 []string) ([]string, error) { +// MultiSelectFunc: func(s string, strings1 []string, strings2 []string) ([]int, error) { // panic("mock out the MultiSelect method") // }, // PasswordFunc: func(s string) (string, error) { @@ -70,7 +70,7 @@ type PrompterMock struct { MarkdownEditorFunc func(s1 string, s2 string, b bool) (string, error) // MultiSelectFunc mocks the MultiSelect method. - MultiSelectFunc func(s string, strings1 []string, strings2 []string) ([]string, error) + MultiSelectFunc func(s string, strings1 []string, strings2 []string) ([]int, error) // PasswordFunc mocks the Password method. PasswordFunc func(s string) (string, error) @@ -348,7 +348,7 @@ func (mock *PrompterMock) MarkdownEditorCalls() []struct { } // MultiSelect calls MultiSelectFunc. -func (mock *PrompterMock) MultiSelect(s string, strings1 []string, strings2 []string) ([]string, error) { +func (mock *PrompterMock) MultiSelect(s string, strings1 []string, strings2 []string) ([]int, error) { if mock.MultiSelectFunc == nil { panic("PrompterMock.MultiSelectFunc: method is nil but Prompter.MultiSelect was just called") } From 3bd1d6770cd73babd288ae7eeaf7d00393d6d522 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 24 May 2023 12:40:46 -0700 Subject: [PATCH 12/12] fix variable name --- internal/prompter/prompter.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/prompter/prompter.go b/internal/prompter/prompter.go index b91445c49..cd90fcd2d 100644 --- a/internal/prompter/prompter.go +++ b/internal/prompter/prompter.go @@ -86,7 +86,7 @@ func (p *surveyPrompter) Select(message, defaultValue string, options []string) return } -func (p *surveyPrompter) MultiSelect(message string, defaultValue, options []string) (result []int, err error) { +func (p *surveyPrompter) MultiSelect(message string, defaultValues, options []string) (result []int, err error) { q := &survey.MultiSelect{ Message: message, Options: options, @@ -94,10 +94,10 @@ func (p *surveyPrompter) MultiSelect(message string, defaultValue, options []str Filter: LatinMatchingFilter, } - if len(defaultValue) > 0 { + if len(defaultValues) > 0 { // TODO I don't actually know that this is needed, just being extra cautious validatedDefault := []string{} - for _, x := range defaultValue { + for _, x := range defaultValues { for _, y := range options { if x == y { validatedDefault = append(validatedDefault, x)