From 2ddfc3827def5430a21ff38a47870ddb6861568d Mon Sep 17 00:00:00 2001 From: Nate Smith Date: Wed, 16 Aug 2023 16:02:14 -0500 Subject: [PATCH 1/4] use new prompter in repo fork --- pkg/cmd/repo/fork/fork.go | 17 +++++---- pkg/cmd/repo/fork/fork_test.go | 70 ++++++++++++++++++---------------- 2 files changed, 47 insertions(+), 40 deletions(-) diff --git a/pkg/cmd/repo/fork/fork.go b/pkg/cmd/repo/fork/fork.go index 087d7219e..0d2d1da69 100644 --- a/pkg/cmd/repo/fork/fork.go +++ b/pkg/cmd/repo/fork/fork.go @@ -19,13 +19,16 @@ import ( "github.com/cli/cli/v2/pkg/cmd/repo/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" "github.com/spf13/pflag" ) const defaultRemoteName = "origin" +type iprompter interface { + Confirm(string, bool) (bool, error) +} + type ForkOptions struct { HttpClient func() (*http.Client, error) GitClient *git.Client @@ -35,6 +38,7 @@ type ForkOptions struct { Remotes func() (ghContext.Remotes, error) Since func(time.Time) time.Duration BackOff backoff.BackOff + Prompter iprompter GitArgs []string Repository string @@ -65,6 +69,7 @@ func NewCmdFork(f *cmdutil.Factory, runF func(*ForkOptions) error) *cobra.Comman Config: f.Config, BaseRepo: f.BaseRepo, Remotes: f.Remotes, + Prompter: f.Prompter, Since: time.Since, } @@ -273,10 +278,9 @@ func forkRun(opts *ForkOptions) error { remoteDesired := opts.Remote if opts.PromptRemote { - //nolint:staticcheck // SA1019: prompt.Confirm is deprecated: use Prompter - err = prompt.Confirm("Would you like to add a remote for the fork?", &remoteDesired) + remoteDesired, err = opts.Prompter.Confirm("Would you like to add a remote for the fork?", false) if err != nil { - return fmt.Errorf("failed to prompt: %w", err) + return err } } @@ -317,10 +321,9 @@ func forkRun(opts *ForkOptions) error { } else { cloneDesired := opts.Clone if opts.PromptClone { - //nolint:staticcheck // SA1019: prompt.Confirm is deprecated: use Prompter - err = prompt.Confirm("Would you like to clone the fork?", &cloneDesired) + cloneDesired, err = opts.Prompter.Confirm("Would you like to clone the fork?", false) if err != nil { - return fmt.Errorf("failed to prompt: %w", err) + return err } } if cloneDesired { diff --git a/pkg/cmd/repo/fork/fork_test.go b/pkg/cmd/repo/fork/fork_test.go index 5833acb5d..f59fba1b7 100644 --- a/pkg/cmd/repo/fork/fork_test.go +++ b/pkg/cmd/repo/fork/fork_test.go @@ -14,11 +14,11 @@ import ( "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/internal/run" "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" ) @@ -205,18 +205,18 @@ func TestRepoFork(t *testing.T) { } tests := []struct { - name string - opts *ForkOptions - tty bool - httpStubs func(*httpmock.Registry) - execStubs func(*run.CommandStubber) - askStubs func(*prompt.AskStubber) - cfgStubs func(*config.ConfigMock) - remotes []*context.Remote - wantOut string - wantErrOut string - wantErr bool - errMsg string + name string + opts *ForkOptions + tty bool + httpStubs func(*httpmock.Registry) + execStubs func(*run.CommandStubber) + promptStubs func(*prompter.MockPrompter) + cfgStubs func(*config.ConfigMock) + remotes []*context.Remote + wantOut string + wantErrOut string + wantErr bool + errMsg string }{ { name: "implicit match, configured protocol overrides provided", @@ -272,9 +272,10 @@ func TestRepoFork(t *testing.T) { RemoteName: defaultRemoteName, }, httpStubs: forkPost, - askStubs: func(as *prompt.AskStubber) { - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(false) + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterConfirm("Would you like to add a remote for the fork?", func(_ string, _ bool) (bool, error) { + return false, nil + }) }, wantErrOut: "āœ“ Created fork someone/REPO\n", }, @@ -291,9 +292,10 @@ func TestRepoFork(t *testing.T) { cs.Register("git remote rename origin upstream", 0, "") cs.Register(`git remote add origin https://github.com/someone/REPO.git`, 0, "") }, - askStubs: func(as *prompt.AskStubber) { - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(true) + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterConfirm("Would you like to add a remote for the fork?", func(_ string, _ bool) (bool, error) { + return true, nil + }) }, wantErrOut: "āœ“ Created fork someone/REPO\nāœ“ Added remote origin\n", }, @@ -497,9 +499,10 @@ func TestRepoFork(t *testing.T) { PromptClone: true, }, httpStubs: forkPost, - askStubs: func(as *prompt.AskStubber) { - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(false) + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterConfirm("Would you like to clone the fork?", func(_ string, _ bool) (bool, error) { + return false, nil + }) }, wantErrOut: "āœ“ Created fork someone/REPO\n", }, @@ -511,9 +514,10 @@ func TestRepoFork(t *testing.T) { PromptClone: true, }, httpStubs: forkPost, - askStubs: func(as *prompt.AskStubber) { - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(true) + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterConfirm("Would you like to clone the fork?", func(_ string, _ bool) (bool, error) { + return true, nil + }) }, execStubs: func(cs *run.CommandStubber) { cs.Register(`git clone https://github.com/someone/REPO\.git`, 0, "") @@ -533,9 +537,10 @@ func TestRepoFork(t *testing.T) { }, }, httpStubs: forkPost, - askStubs: func(as *prompt.AskStubber) { - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(true) + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterConfirm("Would you like to clone the fork?", func(_ string, _ bool) (bool, error) { + return true, nil + }) }, execStubs: func(cs *run.CommandStubber) { cs.Register(`git clone https://github.com/someone/REPO\.git`, 0, "") @@ -746,11 +751,10 @@ func TestRepoFork(t *testing.T) { GitPath: "some/path/git", } - //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) } cs, restoreRun := run.Stub() From f2e5ad6dcd9a692fe97badd0b3485f1177a7592e Mon Sep 17 00:00:00 2001 From: Nate Smith Date: Wed, 16 Aug 2023 21:26:37 -0500 Subject: [PATCH 2/4] fix RegisterPassword --- internal/prompter/test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/prompter/test.go b/internal/prompter/test.go index c376e9c83..0dea18e5a 100644 --- a/internal/prompter/test.go +++ b/internal/prompter/test.go @@ -258,7 +258,7 @@ func (m *MockPrompter) RegisterInputHostname(stub func() (string, error)) { } func (m *MockPrompter) RegisterPassword(prompt string, stub func(string) (string, error)) { - m.PasswordStubs = append(m.PasswordStubs, PasswordStub{Fn: stub}) + m.PasswordStubs = append(m.PasswordStubs, PasswordStub{Prompt: prompt, Fn: stub}) } func (m *MockPrompter) RegisterConfirmDeletion(prompt string, stub func(string) error) { From fc73c16fe82c925e72b8f017172315456922a9ce Mon Sep 17 00:00:00 2001 From: Nate Smith Date: Wed, 16 Aug 2023 21:26:57 -0500 Subject: [PATCH 3/4] use prompter in secret set --- pkg/cmd/secret/set/set.go | 14 +++++++------- pkg/cmd/secret/set/set_test.go | 12 +++++++----- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/pkg/cmd/secret/set/set.go b/pkg/cmd/secret/set/set.go index 86dd35258..95f80f8b1 100644 --- a/pkg/cmd/secret/set/set.go +++ b/pkg/cmd/secret/set/set.go @@ -9,7 +9,6 @@ import ( "os" "strings" - "github.com/AlecAivazis/survey/v2" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/config" @@ -17,7 +16,6 @@ import ( "github.com/cli/cli/v2/pkg/cmd/secret/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/pkg/prompt" "github.com/hashicorp/go-multierror" "github.com/joho/godotenv" "github.com/spf13/cobra" @@ -29,6 +27,7 @@ type SetOptions struct { IO *iostreams.IOStreams Config func() (config.Config, error) BaseRepo func() (ghrepo.Interface, error) + Prompter iprompter RandomOverride func() io.Reader @@ -44,11 +43,16 @@ type SetOptions struct { Application string } +type iprompter interface { + Password(string) (string, error) +} + func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command { opts := &SetOptions{ IO: f.IOStreams, Config: f.Config, HttpClient: f.HttpClient, + Prompter: f.Prompter, } cmd := &cobra.Command{ @@ -375,11 +379,7 @@ func getBody(opts *SetOptions) ([]byte, error) { } if opts.IO.CanPrompt() { - var bodyInput string - //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter - err := prompt.SurveyAskOne(&survey.Password{ - Message: "Paste your secret", - }, &bodyInput) + bodyInput, err := opts.Prompter.Password("Paste your secret") if err != nil { return nil, err } diff --git a/pkg/cmd/secret/set/set_test.go b/pkg/cmd/secret/set/set_test.go index 592527c5f..376ef2d53 100644 --- a/pkg/cmd/secret/set/set_test.go +++ b/pkg/cmd/secret/set/set_test.go @@ -12,11 +12,11 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/pkg/cmd/secret/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" ) @@ -595,12 +595,14 @@ func Test_getBodyPrompt(t *testing.T) { ios.SetStdinTTY(true) ios.SetStdoutTTY(true) - //nolint:staticcheck // SA1019: prompt.NewAskStubber is deprecated: use PrompterMock - as := prompt.NewAskStubber(t) - as.StubPrompt("Paste your secret").AnswerWith("cool secret") + pm := prompter.NewMockPrompter(t) + pm.RegisterPassword("Paste your secret", func(_ string) (string, error) { + return "cool secret", nil + }) body, err := getBody(&SetOptions{ - IO: ios, + IO: ios, + Prompter: pm, }) assert.NoError(t, err) assert.Equal(t, string(body), "cool secret") From 0d3b7db495d8082df3ade134e85ecb1541750efd Mon Sep 17 00:00:00 2001 From: Nate Smith Date: Wed, 16 Aug 2023 21:48:30 -0500 Subject: [PATCH 4/4] use prompter in issue delete --- pkg/cmd/issue/delete/delete.go | 25 +++++++----------- pkg/cmd/issue/delete/delete_test.go | 39 ++++++++++++++--------------- 2 files changed, 28 insertions(+), 36 deletions(-) diff --git a/pkg/cmd/issue/delete/delete.go b/pkg/cmd/issue/delete/delete.go index f0c68de42..cee367cb8 100644 --- a/pkg/cmd/issue/delete/delete.go +++ b/pkg/cmd/issue/delete/delete.go @@ -3,16 +3,13 @@ package delete import ( "fmt" "net/http" - "strconv" - "github.com/AlecAivazis/survey/v2" "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmd/issue/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/pkg/prompt" "github.com/shurcooL/githubv4" "github.com/spf13/cobra" ) @@ -22,16 +19,22 @@ type DeleteOptions struct { Config func() (config.Config, error) IO *iostreams.IOStreams BaseRepo func() (ghrepo.Interface, error) + Prompter iprompter SelectorArg string Confirmed bool } +type iprompter interface { + ConfirmDeletion(string) error +} + func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Command { opts := &DeleteOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, Config: f.Config, + Prompter: f.Prompter, } cmd := &cobra.Command{ @@ -79,22 +82,12 @@ func deleteRun(opts *DeleteOptions) error { // When executed in an interactive shell, require confirmation, unless // already provided. Otherwise skip confirmation. if opts.IO.CanPrompt() && !opts.Confirmed { - answer := "" - //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter - err = prompt.SurveyAskOne( - &survey.Input{ - Message: fmt.Sprintf("You're going to delete issue #%d. This action cannot be reversed. To confirm, type the issue number:", issue.Number), - }, - &answer, - ) + cs := opts.IO.ColorScheme() + fmt.Printf("%s Deleted issues cannot be recovered.\n", cs.WarningIcon()) + err := opts.Prompter.ConfirmDeletion(fmt.Sprintf("%d", issue.Number)) if err != nil { return err } - answerInt, err := strconv.Atoi(answer) - if err != nil || answerInt != issue.Number { - fmt.Fprintf(opts.IO.Out, "Issue #%d was not deleted.\n", issue.Number) - return nil - } } if err := apiDelete(httpClient, baseRepo, issue.ID); err != nil { diff --git a/pkg/cmd/issue/delete/delete_test.go b/pkg/cmd/issue/delete/delete_test.go index e3b2613ce..acc52af65 100644 --- a/pkg/cmd/issue/delete/delete_test.go +++ b/pkg/cmd/issue/delete/delete_test.go @@ -2,6 +2,7 @@ package delete import ( "bytes" + "errors" "io" "net/http" "regexp" @@ -9,16 +10,16 @@ import ( "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" "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/cli/cli/v2/test" "github.com/google/shlex" "github.com/stretchr/testify/assert" ) -func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, error) { +func runCommand(rt http.RoundTripper, pm *prompter.MockPrompter, isTTY bool, cli string) (*test.CmdOut, error) { ios, _, stdout, stderr := iostreams.Test() ios.SetStdoutTTY(isTTY) ios.SetStdinTTY(isTTY) @@ -26,6 +27,7 @@ func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, err factory := &cmdutil.Factory{ IOStreams: ios, + Prompter: pm, HttpClient: func() (*http.Client, error) { return &http.Client{Transport: rt}, nil }, @@ -76,11 +78,10 @@ func TestIssueDelete(t *testing.T) { }), ) - //nolint:staticcheck // SA1019: prompt.NewAskStubber is deprecated: use PrompterMock - as := prompt.NewAskStubber(t) - as.StubPrompt("You're going to delete issue #13. This action cannot be reversed. To confirm, type the issue number:").AnswerWith("13") + pm := prompter.NewMockPrompter(t) + pm.RegisterConfirmDeletion("13", func(_ string) error { return nil }) - output, err := runCommand(httpRegistry, true, "13") + output, err := runCommand(httpRegistry, pm, true, "13") if err != nil { t.Fatalf("error running command `issue delete`: %v", err) } @@ -112,7 +113,7 @@ func TestIssueDelete_confirm(t *testing.T) { }), ) - output, err := runCommand(httpRegistry, true, "13 --confirm") + output, err := runCommand(httpRegistry, nil, true, "13 --confirm") if err != nil { t.Fatalf("error running command `issue delete`: %v", err) } @@ -137,19 +138,17 @@ func TestIssueDelete_cancel(t *testing.T) { } } }`), ) - //nolint:staticcheck // SA1019: prompt.NewAskStubber is deprecated: use PrompterMock - as := prompt.NewAskStubber(t) - as.StubPrompt("You're going to delete issue #13. This action cannot be reversed. To confirm, type the issue number:").AnswerWith("14") + pm := prompter.NewMockPrompter(t) + pm.RegisterConfirmDeletion("13", func(_ string) error { + return errors.New("You entered 14") + }) - output, err := runCommand(httpRegistry, true, "13") - if err != nil { - t.Fatalf("error running command `issue delete`: %v", err) + _, err := runCommand(httpRegistry, pm, true, "13") + if err == nil { + t.Fatalf("expected error") } - - r := regexp.MustCompile(`Issue #13 was not deleted`) - - if !r.MatchString(output.String()) { - t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.String()) + if err.Error() != "You entered 14" { + t.Fatalf("got unexpected error '%s'", err) } } @@ -166,7 +165,7 @@ func TestIssueDelete_doesNotExist(t *testing.T) { `), ) - _, err := runCommand(httpRegistry, true, "13") + _, err := runCommand(httpRegistry, nil, true, "13") if err == nil || err.Error() != "GraphQL: Could not resolve to an Issue with the number of 13." { t.Errorf("error running command `issue delete`: %v", err) } @@ -199,7 +198,7 @@ func TestIssueDelete_issuesDisabled(t *testing.T) { }`), ) - _, err := runCommand(httpRegistry, true, "13") + _, err := runCommand(httpRegistry, nil, true, "13") if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" { t.Fatalf("got error: %v", err) }