Tweak error messages and add more tests for extension install name check

This commit is contained in:
Mislav Marohnić 2021-07-16 14:52:05 +02:00
parent 16d218508d
commit d6b0749ea2
2 changed files with 102 additions and 17 deletions

View file

@ -10,6 +10,7 @@ import (
"github.com/cli/cli/git"
"github.com/cli/cli/internal/ghrepo"
"github.com/cli/cli/pkg/cmdutil"
"github.com/cli/cli/pkg/extensions"
"github.com/cli/cli/utils"
"github.com/spf13/cobra"
)
@ -73,28 +74,14 @@ func NewCmdExtensions(f *cmdutil.Factory) *cobra.Command {
}
return m.InstallLocal(wd)
}
repo, err := ghrepo.FromFullName(args[0])
if err != nil {
return err
}
if !strings.HasPrefix(repo.RepoName(), "gh-") {
return errors.New("the repository name must start with `gh-`")
}
commandName := strings.TrimPrefix(repo.RepoName(), "gh-")
rootCmd := cmd.Root()
c, _, err := rootCmd.Traverse([]string{commandName})
if err != nil {
if err := checkValidExtension(cmd.Root(), m, repo.RepoName()); err != nil {
return err
}
if c != rootCmd {
return fmt.Errorf("could not install %s: %s is already a command", args[0], commandName)
}
for _, ext := range m.List() {
if ext.Name() == commandName {
return fmt.Errorf("could not install %s: %s is already an installed extension", args[0], commandName)
}
}
cfg, err := f.Config()
if err != nil {
@ -145,3 +132,24 @@ func NewCmdExtensions(f *cmdutil.Factory) *cobra.Command {
extCmd.Hidden = true
return &extCmd
}
func checkValidExtension(rootCmd *cobra.Command, m extensions.ExtensionManager, extName string) error {
if !strings.HasPrefix(extName, "gh-") {
return errors.New("extension repository name must start with `gh-`")
}
commandName := strings.TrimPrefix(extName, "gh-")
if c, _, err := rootCmd.Traverse([]string{commandName}); err != nil {
return err
} else if c != rootCmd {
return fmt.Errorf("%q matches the name of a built-in command", commandName)
}
for _, ext := range m.List() {
if ext.Name() == commandName {
return fmt.Errorf("there is already an installed extension that provides the %q command", commandName)
}
}
return nil
}

View file

@ -11,6 +11,7 @@ import (
"github.com/cli/cli/pkg/cmdutil"
"github.com/cli/cli/pkg/extensions"
"github.com/cli/cli/pkg/iostreams"
"github.com/spf13/cobra"
"github.com/stretchr/testify/assert"
)
@ -60,7 +61,7 @@ func TestNewCmdExtensions(t *testing.T) {
}
},
wantErr: true,
errMsg: "could not install owner/gh-existing-ext: existing-ext is already an installed extension",
errMsg: "there is already an installed extension that provides the \"existing-ext\" command",
},
{
name: "install local extension",
@ -166,3 +167,79 @@ func TestNewCmdExtensions(t *testing.T) {
func normalizeDir(d string) string {
return strings.TrimPrefix(d, "/private")
}
func Test_checkValidExtension(t *testing.T) {
rootCmd := &cobra.Command{}
rootCmd.AddCommand(&cobra.Command{Use: "help"})
rootCmd.AddCommand(&cobra.Command{Use: "auth"})
m := &extensions.ExtensionManagerMock{
ListFunc: func() []extensions.Extension {
return []extensions.Extension{
&extensions.ExtensionMock{
NameFunc: func() string { return "screensaver" },
},
&extensions.ExtensionMock{
NameFunc: func() string { return "triage" },
},
}
},
}
type args struct {
rootCmd *cobra.Command
manager extensions.ExtensionManager
extName string
}
tests := []struct {
name string
args args
wantError string
}{
{
name: "valid extension",
args: args{
rootCmd: rootCmd,
manager: m,
extName: "gh-hello",
},
},
{
name: "invalid extension name",
args: args{
rootCmd: rootCmd,
manager: m,
extName: "gherkins",
},
wantError: "extension repository name must start with `gh-`",
},
{
name: "clashes with built-in command",
args: args{
rootCmd: rootCmd,
manager: m,
extName: "gh-auth",
},
wantError: "\"auth\" matches the name of a built-in command",
},
{
name: "clashes with an installed extension",
args: args{
rootCmd: rootCmd,
manager: m,
extName: "gh-triage",
},
wantError: "there is already an installed extension that provides the \"triage\" command",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := checkValidExtension(tt.args.rootCmd, tt.args.manager, tt.args.extName)
if tt.wantError == "" {
assert.NoError(t, err)
} else {
assert.EqualError(t, err, tt.wantError)
}
})
}
}