diff --git a/pkg/cmd/root/official_extension.go b/pkg/cmd/root/official_extension.go index cde6bb6da..bc1f21893 100644 --- a/pkg/cmd/root/official_extension.go +++ b/pkg/cmd/root/official_extension.go @@ -5,6 +5,7 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/prompter" + "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/extensions" "github.com/cli/cli/v2/pkg/iostreams" "github.com/spf13/cobra" @@ -16,14 +17,11 @@ import ( // immediately. After a successful install, the extension is dispatched with // the original arguments. func NewCmdOfficialExtension(io *iostreams.IOStreams, p prompter.Prompter, em extensions.ExtensionManager, ext *extensions.OfficialExtension) *cobra.Command { - return &cobra.Command{ + cmd := &cobra.Command{ Use: ext.Name, Short: fmt.Sprintf("Install the official %s extension", ext.Name), Hidden: true, GroupID: "extension", - Annotations: map[string]string{ - "skipAuthCheck": "true", - }, // Accept any args/flags the user may have passed so we don't get // cobra validation errors before reaching RunE. DisableFlagParsing: true, @@ -31,6 +29,10 @@ func NewCmdOfficialExtension(io *iostreams.IOStreams, p prompter.Prompter, em ex return officialExtensionRun(io, p, em, ext, args) }, } + + cmdutil.DisableAuthCheck(cmd) + + return cmd } func officialExtensionRun(io *iostreams.IOStreams, p prompter.Prompter, em extensions.ExtensionManager, ext *extensions.OfficialExtension, args []string) error { diff --git a/pkg/cmd/root/official_extension_test.go b/pkg/cmd/root/official_extension_test.go index 8b6aaa3ae..6986c4cb5 100644 --- a/pkg/cmd/root/official_extension_test.go +++ b/pkg/cmd/root/official_extension_test.go @@ -13,156 +13,137 @@ import ( "github.com/stretchr/testify/require" ) -func TestOfficialExtensionRun_NonTTY(t *testing.T) { - ios, _, _, stderr := iostreams.Test() - // non-TTY by default +func TestOfficialExtensionRun(t *testing.T) { + ext := &extensions.OfficialExtension{Name: "cool", Owner: "github", Repo: "gh-cool"} - ext := &extensions.OfficialExtension{Name: "stack", Owner: "github", Repo: "gh-stack"} - em := &extensions.ExtensionManagerMock{} - p := &prompter.PrompterMock{} - - err := officialExtensionRun(ios, p, em, ext, nil) - require.NoError(t, err) - - assert.Contains(t, stderr.String(), "gh stack") - assert.Contains(t, stderr.String(), "gh extension install github/gh-stack") -} - -func TestOfficialExtensionRun_TTY_Confirmed(t *testing.T) { - ios, _, _, stderr := iostreams.Test() - ios.SetStdinTTY(true) - ios.SetStdoutTTY(true) - ios.SetStderrTTY(true) - - ext := &extensions.OfficialExtension{Name: "stack", Owner: "github", Repo: "gh-stack"} - var installedRepo ghrepo.Interface - var dispatchedArgs []string - em := &extensions.ExtensionManagerMock{ - InstallFunc: func(repo ghrepo.Interface, pin string) error { - installedRepo = repo - return nil + tests := []struct { + name string + isTTY bool + confirmResult bool + confirmErr error + installErr error + dispatchErr error + args []string + wantErr string + wantStderr string + wantInstalled bool + wantDispatched bool + wantDispArgs []string + }{ + { + name: "non-TTY prints install instructions", + isTTY: false, + wantStderr: "gh extension install github/gh-cool", }, - DispatchFunc: func(args []string, stdin io.Reader, stdout, stderr io.Writer) (bool, error) { - dispatchedArgs = args - return true, nil + { + name: "TTY confirmed installs and dispatches", + isTTY: true, + confirmResult: true, + args: []string{"--help"}, + wantStderr: "Successfully installed github/gh-cool", + wantInstalled: true, + wantDispatched: true, + wantDispArgs: []string{"cool", "--help"}, }, - } - p := &prompter.PrompterMock{ - ConfirmFunc: func(_ string, _ bool) (bool, error) { - return true, nil + { + name: "TTY declined does not install", + isTTY: true, + confirmResult: false, + }, + { + name: "TTY prompt error is propagated", + isTTY: true, + confirmErr: fmt.Errorf("prompt interrupted"), + wantErr: "prompt interrupted", + }, + { + name: "TTY install error is propagated", + isTTY: true, + confirmResult: true, + installErr: fmt.Errorf("network error"), + wantErr: "network error", + wantInstalled: true, + }, + { + name: "TTY dispatch error is propagated", + isTTY: true, + confirmResult: true, + dispatchErr: fmt.Errorf("dispatch failed"), + wantErr: "dispatch failed", + wantInstalled: true, + wantDispatched: true, }, } - err := officialExtensionRun(ios, p, em, ext, []string{"--help"}) - require.NoError(t, err) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ios, _, _, stderr := iostreams.Test() + if tt.isTTY { + ios.SetStdinTTY(true) + ios.SetStdoutTTY(true) + ios.SetStderrTTY(true) + } - require.NotNil(t, installedRepo) - assert.Equal(t, "github", installedRepo.RepoOwner()) - assert.Equal(t, "gh-stack", installedRepo.RepoName()) - assert.Equal(t, "github.com", installedRepo.RepoHost()) - assert.Contains(t, stderr.String(), "Successfully installed github/gh-stack") - assert.Equal(t, []string{"stack", "--help"}, dispatchedArgs) -} + em := &extensions.ExtensionManagerMock{ + InstallFunc: func(_ ghrepo.Interface, _ string) error { + return tt.installErr + }, + DispatchFunc: func(_ []string, _ io.Reader, _, _ io.Writer) (bool, error) { + if tt.dispatchErr != nil { + return false, tt.dispatchErr + } + return true, nil + }, + } + p := &prompter.PrompterMock{ + ConfirmFunc: func(_ string, _ bool) (bool, error) { + return tt.confirmResult, tt.confirmErr + }, + } -func TestOfficialExtensionRun_TTY_Declined(t *testing.T) { - ios, _, _, _ := iostreams.Test() - ios.SetStdinTTY(true) - ios.SetStdoutTTY(true) - ios.SetStderrTTY(true) + err := officialExtensionRun(ios, p, em, ext, tt.args) - ext := &extensions.OfficialExtension{Name: "stack", Owner: "github", Repo: "gh-stack"} - em := &extensions.ExtensionManagerMock{} - p := &prompter.PrompterMock{ - ConfirmFunc: func(_ string, _ bool) (bool, error) { - return false, nil - }, + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + } else { + require.NoError(t, err) + } + + if tt.wantStderr != "" { + assert.Contains(t, stderr.String(), tt.wantStderr) + } + + if tt.wantInstalled { + require.NotEmpty(t, em.InstallCalls()) + repo := em.InstallCalls()[0].InterfaceMoqParam + assert.Equal(t, "github", repo.RepoOwner()) + assert.Equal(t, "gh-cool", repo.RepoName()) + assert.Equal(t, "github.com", repo.RepoHost()) + } else if tt.isTTY && !tt.confirmResult && tt.confirmErr == nil { + assert.Empty(t, em.InstallCalls()) + } + + if tt.wantDispatched { + require.NotEmpty(t, em.DispatchCalls()) + if tt.wantDispArgs != nil { + assert.Equal(t, tt.wantDispArgs, em.DispatchCalls()[0].Args) + } + } + }) } - - err := officialExtensionRun(ios, p, em, ext, nil) - require.NoError(t, err) - - assert.Empty(t, em.InstallCalls()) -} - -func TestOfficialExtensionRun_TTY_PromptError(t *testing.T) { - ios, _, _, _ := iostreams.Test() - ios.SetStdinTTY(true) - ios.SetStdoutTTY(true) - ios.SetStderrTTY(true) - - ext := &extensions.OfficialExtension{Name: "stack", Owner: "github", Repo: "gh-stack"} - em := &extensions.ExtensionManagerMock{} - p := &prompter.PrompterMock{ - ConfirmFunc: func(_ string, _ bool) (bool, error) { - return false, fmt.Errorf("prompt interrupted") - }, - } - - err := officialExtensionRun(ios, p, em, ext, nil) - require.Error(t, err) - assert.Contains(t, err.Error(), "prompt interrupted") -} - -func TestOfficialExtensionRun_TTY_InstallError(t *testing.T) { - ios, _, _, _ := iostreams.Test() - ios.SetStdinTTY(true) - ios.SetStdoutTTY(true) - ios.SetStderrTTY(true) - - ext := &extensions.OfficialExtension{Name: "stack", Owner: "github", Repo: "gh-stack"} - em := &extensions.ExtensionManagerMock{ - InstallFunc: func(_ ghrepo.Interface, _ string) error { - return fmt.Errorf("network error") - }, - } - p := &prompter.PrompterMock{ - ConfirmFunc: func(_ string, _ bool) (bool, error) { - return true, nil - }, - } - - err := officialExtensionRun(ios, p, em, ext, nil) - require.Error(t, err) - assert.Contains(t, err.Error(), "network error") -} - -func TestOfficialExtensionRun_TTY_DispatchError(t *testing.T) { - ios, _, _, _ := iostreams.Test() - ios.SetStdinTTY(true) - ios.SetStdoutTTY(true) - ios.SetStderrTTY(true) - - ext := &extensions.OfficialExtension{Name: "stack", Owner: "github", Repo: "gh-stack"} - em := &extensions.ExtensionManagerMock{ - InstallFunc: func(_ ghrepo.Interface, _ string) error { - return nil - }, - DispatchFunc: func(_ []string, _ io.Reader, _, _ io.Writer) (bool, error) { - return false, fmt.Errorf("dispatch failed") - }, - } - p := &prompter.PrompterMock{ - ConfirmFunc: func(_ string, _ bool) (bool, error) { - return true, nil - }, - } - - err := officialExtensionRun(ios, p, em, ext, nil) - require.Error(t, err) - assert.Contains(t, err.Error(), "dispatch failed") } func TestNewCmdOfficialExtension_Properties(t *testing.T) { ios, _, _, _ := iostreams.Test() - ext := &extensions.OfficialExtension{Name: "stack", Owner: "github", Repo: "gh-stack"} + ext := &extensions.OfficialExtension{Name: "cool", Owner: "github", Repo: "gh-cool"} em := &extensions.ExtensionManagerMock{} p := &prompter.PrompterMock{} cmd := NewCmdOfficialExtension(ios, p, em, ext) - assert.Equal(t, "stack", cmd.Use) + assert.Equal(t, "cool", cmd.Use) assert.True(t, cmd.Hidden) assert.Equal(t, "extension", cmd.GroupID) assert.True(t, cmd.DisableFlagParsing) - assert.Equal(t, "true", cmd.Annotations["skipAuthCheck"]) }