From d8a276e83b4c41768beda22e62096592d16c470c Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 21 Sep 2022 16:59:33 -0700 Subject: [PATCH] add and use ConfirmDeletion in {label,repo} delete --- internal/prompter/prompter.go | 18 +++++++++ internal/prompter/prompter_mock.go | 59 ++++++++++++++++++++++++++---- pkg/cmd/label/delete.go | 25 ++++--------- pkg/cmd/label/delete_test.go | 34 ++++++++--------- pkg/cmd/repo/delete/delete.go | 14 +++---- pkg/cmd/repo/delete/delete_test.go | 12 +++--- 6 files changed, 106 insertions(+), 56 deletions(-) diff --git a/internal/prompter/prompter.go b/internal/prompter/prompter.go index b2c6925b7..25c50c8ff 100644 --- a/internal/prompter/prompter.go +++ b/internal/prompter/prompter.go @@ -3,6 +3,7 @@ package prompter import ( "fmt" "io" + "strings" "github.com/AlecAivazis/survey/v2" "github.com/cli/cli/v2/internal/ghinstance" @@ -18,6 +19,7 @@ type Prompter interface { Password(string) (string, error) AuthToken() (string, error) Confirm(string, bool) (bool, error) + ConfirmDeletion(string) error MarkdownEditor(string, string, bool) (string, error) } @@ -97,6 +99,22 @@ func (p *surveyPrompter) Input(prompt, defaultValue string) (result string, err return } +func (p *surveyPrompter) ConfirmDeletion(requiredValue string) error { + var result string + return p.ask( + &survey.Input{ + Message: fmt.Sprintf("Type %s to confirm deletion:", requiredValue), + }, + &result, + survey.WithValidator( + func(val interface{}) error { + if str := val.(string); !strings.EqualFold(str, requiredValue) { + return fmt.Errorf("You entered %s", str) + } + return nil + })) +} + func (p *surveyPrompter) InputHostname() (result string, err error) { err = p.ask( &survey.Input{ diff --git a/internal/prompter/prompter_mock.go b/internal/prompter/prompter_mock.go index eea5393b2..d9848483c 100644 --- a/internal/prompter/prompter_mock.go +++ b/internal/prompter/prompter_mock.go @@ -23,6 +23,9 @@ var _ Prompter = &PrompterMock{} // ConfirmFunc: func(s string, b bool) (bool, error) { // panic("mock out the Confirm method") // }, +// ConfirmDeletionFunc: func(s string) error { +// panic("mock out the ConfirmDeletion method") +// }, // InputFunc: func(s1 string, s2 string) (string, error) { // panic("mock out the Input method") // }, @@ -54,6 +57,9 @@ type PrompterMock struct { // ConfirmFunc mocks the Confirm method. ConfirmFunc func(s string, b bool) (bool, error) + // ConfirmDeletionFunc mocks the ConfirmDeletion method. + ConfirmDeletionFunc func(s string) error + // InputFunc mocks the Input method. InputFunc func(s1 string, s2 string) (string, error) @@ -84,6 +90,11 @@ type PrompterMock struct { // B is the b argument value. B bool } + // ConfirmDeletion holds details about calls to the ConfirmDeletion method. + ConfirmDeletion []struct { + // S is the s argument value. + S string + } // Input holds details about calls to the Input method. Input []struct { // S1 is the s1 argument value. @@ -127,14 +138,15 @@ type PrompterMock struct { Strings []string } } - lockAuthToken sync.RWMutex - lockConfirm sync.RWMutex - lockInput sync.RWMutex - lockInputHostname sync.RWMutex - lockMarkdownEditor sync.RWMutex - lockMultiSelect sync.RWMutex - lockPassword sync.RWMutex - lockSelect sync.RWMutex + lockAuthToken sync.RWMutex + lockConfirm sync.RWMutex + lockConfirmDeletion sync.RWMutex + lockInput sync.RWMutex + lockInputHostname sync.RWMutex + lockMarkdownEditor sync.RWMutex + lockMultiSelect sync.RWMutex + lockPassword sync.RWMutex + lockSelect sync.RWMutex } // AuthToken calls AuthTokenFunc. @@ -198,6 +210,37 @@ func (mock *PrompterMock) ConfirmCalls() []struct { return calls } +// ConfirmDeletion calls ConfirmDeletionFunc. +func (mock *PrompterMock) ConfirmDeletion(s string) error { + if mock.ConfirmDeletionFunc == nil { + panic("PrompterMock.ConfirmDeletionFunc: method is nil but Prompter.ConfirmDeletion was just called") + } + callInfo := struct { + S string + }{ + S: s, + } + mock.lockConfirmDeletion.Lock() + mock.calls.ConfirmDeletion = append(mock.calls.ConfirmDeletion, callInfo) + mock.lockConfirmDeletion.Unlock() + return mock.ConfirmDeletionFunc(s) +} + +// ConfirmDeletionCalls gets all the calls that were made to ConfirmDeletion. +// Check the length with: +// len(mockedPrompter.ConfirmDeletionCalls()) +func (mock *PrompterMock) ConfirmDeletionCalls() []struct { + S string +} { + var calls []struct { + S string + } + mock.lockConfirmDeletion.RLock() + calls = mock.calls.ConfirmDeletion + mock.lockConfirmDeletion.RUnlock() + return calls +} + // Input calls InputFunc. func (mock *PrompterMock) Input(s1 string, s2 string) (string, error) { if mock.InputFunc == nil { diff --git a/pkg/cmd/label/delete.go b/pkg/cmd/label/delete.go index 3d51cf4c4..8788b7215 100644 --- a/pkg/cmd/label/delete.go +++ b/pkg/cmd/label/delete.go @@ -3,21 +3,23 @@ package label import ( "fmt" "net/http" - "strings" - "github.com/AlecAivazis/survey/v2" "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/ghrepo" "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" ) +type iprompter interface { + ConfirmDeletion(string) error +} + type deleteOptions struct { BaseRepo func() (ghrepo.Interface, error) HttpClient func() (*http.Client, error) IO *iostreams.IOStreams + Prompter iprompter Name string Confirmed bool @@ -27,6 +29,7 @@ func newCmdDelete(f *cmdutil.Factory, runF func(*deleteOptions) error) *cobra.Co opts := deleteOptions{ HttpClient: f.HttpClient, IO: f.IOStreams, + Prompter: f.Prompter, } cmd := &cobra.Command{ @@ -66,20 +69,8 @@ func deleteRun(opts *deleteOptions) error { } if !opts.Confirmed { - var valid string - //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter - err := prompt.SurveyAskOne( - &survey.Input{Message: fmt.Sprintf("Type %s to confirm deletion:", opts.Name)}, - &valid, - survey.WithValidator( - func(val interface{}) error { - if str := val.(string); !strings.EqualFold(str, opts.Name) { - return fmt.Errorf("You entered %s", str) - } - return nil - })) - if err != nil { - return fmt.Errorf("could not prompt: %w", err) + if err := opts.Prompter.ConfirmDeletion(opts.Name); err != nil { + return err } } diff --git a/pkg/cmd/label/delete_test.go b/pkg/cmd/label/delete_test.go index d5d0ac195..3bee0d987 100644 --- a/pkg/cmd/label/delete_test.go +++ b/pkg/cmd/label/delete_test.go @@ -6,10 +6,10 @@ import ( "testing" "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/google/shlex" "github.com/stretchr/testify/assert" ) @@ -83,14 +83,14 @@ func TestNewCmdDelete(t *testing.T) { func TestDeleteRun(t *testing.T) { tests := []struct { - name string - tty bool - opts *deleteOptions - httpStubs func(*httpmock.Registry) - askStubs func(*prompt.AskStubber) - wantStdout string - wantErr bool - errMsg string + name string + tty bool + opts *deleteOptions + httpStubs func(*httpmock.Registry) + prompterStubs func(*prompter.PrompterMock) + wantStdout string + wantErr bool + errMsg string }{ { name: "deletes label", @@ -102,10 +102,10 @@ func TestDeleteRun(t *testing.T) { httpmock.StatusStringResponse(204, "{}"), ) }, - askStubs: func(as *prompt.AskStubber) { - // TODO: survey stubber doesn't have WithValidator support - // so this always passes regardless of prompt input - as.StubPrompt("Type test to confirm deletion:").AnswerWith("test") + prompterStubs: func(pm *prompter.PrompterMock) { + pm.ConfirmDeletionFunc = func(_ string) error { + return nil + } }, wantStdout: "✓ Label \"test\" deleted from OWNER/REPO\n", }, @@ -153,11 +153,11 @@ func TestDeleteRun(t *testing.T) { return &http.Client{Transport: reg}, nil } - //nolint:staticcheck // SA1019: prompt.NewAskStubber is deprecated: use PrompterMock - as := prompt.NewAskStubber(t) - if tt.askStubs != nil { - tt.askStubs(as) + pm := &prompter.PrompterMock{} + if tt.prompterStubs != nil { + tt.prompterStubs(pm) } + tt.opts.Prompter = pm io, _, stdout, _ := iostreams.Test() io.SetStdoutTTY(tt.tty) diff --git a/pkg/cmd/repo/delete/delete.go b/pkg/cmd/repo/delete/delete.go index 9e1d6d1d8..67e4fd298 100644 --- a/pkg/cmd/repo/delete/delete.go +++ b/pkg/cmd/repo/delete/delete.go @@ -8,17 +8,20 @@ import ( "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/ghinstance" "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/iostreams" "github.com/spf13/cobra" ) +type iprompter interface { + ConfirmDeletion(string) error +} + type DeleteOptions struct { HttpClient func() (*http.Client, error) BaseRepo func() (ghrepo.Interface, error) - Prompter prompter.Prompter + Prompter iprompter IO *iostreams.IOStreams RepoArg string Confirmed bool @@ -92,14 +95,9 @@ func deleteRun(opts *DeleteOptions) error { fullName := ghrepo.FullName(toDelete) if !opts.Confirmed { - result, err := opts.Prompter.Input( - fmt.Sprintf("Type %s to confirm deletion:", fullName), "") - if err != nil { + if err := opts.Prompter.ConfirmDeletion(fullName); err != nil { return err } - if !strings.EqualFold(result, fullName) { - return fmt.Errorf("You entered %s", result) - } } err = deleteRepo(httpClient, toDelete) diff --git a/pkg/cmd/repo/delete/delete_test.go b/pkg/cmd/repo/delete/delete_test.go index a49bbff4f..a08cf3473 100644 --- a/pkg/cmd/repo/delete/delete_test.go +++ b/pkg/cmd/repo/delete/delete_test.go @@ -92,8 +92,8 @@ func Test_deleteRun(t *testing.T) { opts: &DeleteOptions{RepoArg: "OWNER/REPO"}, wantStdout: "✓ Deleted repository OWNER/REPO\n", prompterStubs: func(p *prompter.PrompterMock) { - p.InputFunc = func(_, _ string) (string, error) { - return "OWNER/REPO", nil + p.ConfirmDeletionFunc = func(_ string) error { + return nil } }, httpStubs: func(reg *httpmock.Registry) { @@ -108,8 +108,8 @@ func Test_deleteRun(t *testing.T) { opts: &DeleteOptions{}, wantStdout: "✓ Deleted repository OWNER/REPO\n", prompterStubs: func(p *prompter.PrompterMock) { - p.InputFunc = func(_, _ string) (string, error) { - return "OWNER/REPO", nil + p.ConfirmDeletionFunc = func(_ string) error { + return nil } }, httpStubs: func(reg *httpmock.Registry) { @@ -136,8 +136,8 @@ func Test_deleteRun(t *testing.T) { wantStdout: "✓ Deleted repository OWNER/REPO\n", tty: true, prompterStubs: func(p *prompter.PrompterMock) { - p.InputFunc = func(_, _ string) (string, error) { - return "OWNER/REPO", nil + p.ConfirmDeletionFunc = func(_ string) error { + return nil } }, httpStubs: func(reg *httpmock.Registry) {