diff --git a/pkg/cmd/issue/view/view_test.go b/pkg/cmd/issue/view/view_test.go index a1664aa07..a3a42bc93 100644 --- a/pkg/cmd/issue/view/view_test.go +++ b/pkg/cmd/issue/view/view_test.go @@ -8,7 +8,6 @@ import ( "os/exec" "testing" - "github.com/briandowns/spinner" "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/internal/run" @@ -16,7 +15,6 @@ import ( "github.com/cli/cli/pkg/httpmock" "github.com/cli/cli/pkg/iostreams" "github.com/cli/cli/test" - "github.com/cli/cli/utils" "github.com/google/shlex" "github.com/stretchr/testify/assert" ) @@ -404,7 +402,6 @@ func TestIssueView_tty_Comments(t *testing.T) { } for name, tc := range tests { t.Run(name, func(t *testing.T) { - stubSpinner() http := &httpmock.Registry{} defer http.Verify(t) for name, file := range tc.fixtures { @@ -496,8 +493,3 @@ func TestIssueView_nontty_Comments(t *testing.T) { }) } } - -func stubSpinner() { - utils.StartSpinner = func(_ *spinner.Spinner) {} - utils.StopSpinner = func(_ *spinner.Spinner) {} -} diff --git a/pkg/cmd/pr/view/view_test.go b/pkg/cmd/pr/view/view_test.go index ff2f255cf..ef983212c 100644 --- a/pkg/cmd/pr/view/view_test.go +++ b/pkg/cmd/pr/view/view_test.go @@ -9,7 +9,6 @@ import ( "strings" "testing" - "github.com/briandowns/spinner" "github.com/cli/cli/context" "github.com/cli/cli/git" "github.com/cli/cli/internal/config" @@ -19,7 +18,6 @@ import ( "github.com/cli/cli/pkg/httpmock" "github.com/cli/cli/pkg/iostreams" "github.com/cli/cli/test" - "github.com/cli/cli/utils" "github.com/google/shlex" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -864,7 +862,6 @@ func TestPRView_tty_Comments(t *testing.T) { } for name, tt := range tests { t.Run(name, func(t *testing.T) { - stubSpinner() http := &httpmock.Registry{} defer http.Verify(t) for name, file := range tt.fixtures { @@ -987,8 +984,3 @@ func TestPRView_nontty_Comments(t *testing.T) { }) } } - -func stubSpinner() { - utils.StartSpinner = func(_ *spinner.Spinner) {} - utils.StopSpinner = func(_ *spinner.Spinner) {} -} diff --git a/pkg/cmd/repo/fork/fork.go b/pkg/cmd/repo/fork/fork.go index 19341870d..72e15a1c8 100644 --- a/pkg/cmd/repo/fork/fork.go +++ b/pkg/cmd/repo/fork/fork.go @@ -128,18 +128,6 @@ func forkRun(opts *ForkOptions) error { cs := opts.IO.ColorScheme() stderr := opts.IO.ErrOut - s := utils.Spinner(stderr) - stopSpinner := func() {} - - if connectedToTerminal { - loading := cs.Gray("Forking ") + cs.Bold(cs.Gray(ghrepo.FullName(repoToFork))) + cs.Gray("...") - s.Suffix = " " + loading - s.FinalMSG = cs.Gray(fmt.Sprintf("- %s\n", loading)) - utils.StartSpinner(s) - stopSpinner = func() { - utils.StopSpinner(s) - } - } httpClient, err := opts.HttpClient() if err != nil { @@ -148,14 +136,13 @@ func forkRun(opts *ForkOptions) error { apiClient := api.NewClientFromHTTP(httpClient) + opts.IO.StartProgressIndicator() forkedRepo, err := api.ForkRepo(apiClient, repoToFork) + opts.IO.StopProgressIndicator() if err != nil { - stopSpinner() return fmt.Errorf("failed to fork: %w", err) } - stopSpinner() - // This is weird. There is not an efficient way to determine via the GitHub API whether or not a // given user has forked a given repo. We noticed, also, that the create fork API endpoint just // returns the fork repo data even if it already exists -- with no change in status code or diff --git a/pkg/cmd/repo/fork/fork_test.go b/pkg/cmd/repo/fork/fork_test.go index e63175425..3a06321a7 100644 --- a/pkg/cmd/repo/fork/fork_test.go +++ b/pkg/cmd/repo/fork/fork_test.go @@ -9,7 +9,6 @@ import ( "testing" "time" - "github.com/briandowns/spinner" "github.com/cli/cli/context" "github.com/cli/cli/git" "github.com/cli/cli/internal/config" @@ -20,7 +19,6 @@ import ( "github.com/cli/cli/pkg/iostreams" "github.com/cli/cli/pkg/prompt" "github.com/cli/cli/test" - "github.com/cli/cli/utils" "github.com/google/shlex" "github.com/stretchr/testify/assert" ) @@ -204,7 +202,6 @@ func TestRepoFork_outside_parent_nontty(t *testing.T) { } func TestRepoFork_already_forked(t *testing.T) { - stubSpinner() reg := &httpmock.Registry{} defer reg.StubWithFixturePath(200, "./forkResult.json")() httpClient := &http.Client{Transport: reg} @@ -229,7 +226,6 @@ func TestRepoFork_already_forked(t *testing.T) { } func TestRepoFork_reuseRemote(t *testing.T) { - stubSpinner() remotes := []*context.Remote{ { Remote: &git.Remote{Name: "origin", FetchURL: &url.URL{}}, @@ -257,7 +253,6 @@ func TestRepoFork_reuseRemote(t *testing.T) { } func TestRepoFork_in_parent(t *testing.T) { - stubSpinner() reg := &httpmock.Registry{} defer reg.StubWithFixturePath(200, "./forkResult.json")() httpClient := &http.Client{Transport: reg} @@ -283,7 +278,6 @@ func TestRepoFork_in_parent(t *testing.T) { } func TestRepoFork_outside(t *testing.T) { - stubSpinner() tests := []struct { name string args string @@ -322,7 +316,6 @@ func TestRepoFork_outside(t *testing.T) { } func TestRepoFork_in_parent_yes(t *testing.T) { - stubSpinner() defer stubSince(2 * time.Second)() reg := &httpmock.Registry{} defer reg.StubWithFixturePath(200, "./forkResult.json")() @@ -352,7 +345,6 @@ func TestRepoFork_in_parent_yes(t *testing.T) { } func TestRepoFork_outside_yes(t *testing.T) { - stubSpinner() defer stubSince(2 * time.Second)() reg := &httpmock.Registry{} defer reg.StubWithFixturePath(200, "./forkResult.json")() @@ -381,7 +373,6 @@ func TestRepoFork_outside_yes(t *testing.T) { } func TestRepoFork_outside_survey_yes(t *testing.T) { - stubSpinner() defer stubSince(2 * time.Second)() reg := &httpmock.Registry{} defer reg.StubWithFixturePath(200, "./forkResult.json")() @@ -412,7 +403,6 @@ func TestRepoFork_outside_survey_yes(t *testing.T) { } func TestRepoFork_outside_survey_no(t *testing.T) { - stubSpinner() defer stubSince(2 * time.Second)() reg := &httpmock.Registry{} defer reg.StubWithFixturePath(200, "./forkResult.json")() @@ -444,7 +434,6 @@ func TestRepoFork_outside_survey_no(t *testing.T) { } func TestRepoFork_in_parent_survey_yes(t *testing.T) { - stubSpinner() reg := &httpmock.Registry{} defer reg.StubWithFixturePath(200, "./forkResult.json")() httpClient := &http.Client{Transport: reg} @@ -476,7 +465,6 @@ func TestRepoFork_in_parent_survey_yes(t *testing.T) { } func TestRepoFork_in_parent_survey_no(t *testing.T) { - stubSpinner() reg := &httpmock.Registry{} defer reg.StubWithFixturePath(200, "./forkResult.json")() httpClient := &http.Client{Transport: reg} @@ -508,7 +496,6 @@ func TestRepoFork_in_parent_survey_no(t *testing.T) { } func TestRepoFork_in_parent_match_protocol(t *testing.T) { - stubSpinner() defer stubSince(2 * time.Second)() reg := &httpmock.Registry{} defer reg.StubWithFixturePath(200, "./forkResult.json")() @@ -546,14 +533,6 @@ func TestRepoFork_in_parent_match_protocol(t *testing.T) { reg.Verify(t) } -func stubSpinner() { - // not bothering with teardown since we never want spinners when doing tests - utils.StartSpinner = func(_ *spinner.Spinner) { - } - utils.StopSpinner = func(_ *spinner.Spinner) { - } -} - func stubSince(d time.Duration) func() { originalSince := Since Since = func(t time.Time) time.Duration { diff --git a/utils/utils.go b/utils/utils.go index 4b58508ef..57b39b5ba 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -2,12 +2,10 @@ package utils import ( "fmt" - "io" "net/url" "strings" "time" - "github.com/briandowns/spinner" "github.com/cli/cli/internal/run" "github.com/cli/cli/pkg/browser" ) @@ -88,20 +86,6 @@ func Humanize(s string) string { return strings.Map(h, s) } -// We do this so we can stub out the spinner in tests -- it made things really flakey. This is not -// an elegant solution. -var StartSpinner = func(s *spinner.Spinner) { - s.Start() -} - -var StopSpinner = func(s *spinner.Spinner) { - s.Stop() -} - -func Spinner(w io.Writer) *spinner.Spinner { - return spinner.New(spinner.CharSets[11], 400*time.Millisecond, spinner.WithWriter(w)) -} - func IsURL(s string) bool { return strings.HasPrefix(s, "http:/") || strings.HasPrefix(s, "https:/") }