Address PR review feedback
- Use cmdutil.DisableAuthCheck instead of raw annotation map - Convert tests to table-driven format - Use generic extension name in tests (gh-cool) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
William Martin
parent
9596f99e56
commit
abc2a50b4c
2 changed files with 117 additions and 134 deletions
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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"])
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue