diff --git a/pkg/cmd/secret/delete/delete.go b/pkg/cmd/secret/delete/delete.go index 1cbfbca9d..b73b59848 100644 --- a/pkg/cmd/secret/delete/delete.go +++ b/pkg/cmd/secret/delete/delete.go @@ -49,9 +49,9 @@ func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Co RunE: func(cmd *cobra.Command, args []string) error { // If the user specified a repo directly, then we're using the OverrideBaseRepoFunc set by EnableRepoOverride // So there's no reason to use the specialised BaseRepoFunc that requires remote disambiguation. - if cmd.Flags().Changed("repo") || os.Getenv("GH_REPO") != "" { - opts.BaseRepo = f.BaseRepo - } else { + opts.BaseRepo = f.BaseRepo + isRepoUserProvided := cmd.Flags().Changed("repo") || os.Getenv("GH_REPO") != "" + if !isRepoUserProvided { // If they haven't specified a repo directly, then we will wrap the BaseRepoFunc in one that errors if // there might be multiple valid remotes. opts.BaseRepo = shared.RequireNoAmbiguityBaseRepoFunc(opts.BaseRepo, f.Remotes) diff --git a/pkg/cmd/secret/delete/delete_test.go b/pkg/cmd/secret/delete/delete_test.go index 2f6cad431..48200b881 100644 --- a/pkg/cmd/secret/delete/delete_test.go +++ b/pkg/cmd/secret/delete/delete_test.go @@ -126,7 +126,7 @@ func TestNewCmdDelete(t *testing.T) { } func TestNewCmdDeleteBaseRepoFuncs(t *testing.T) { - remotes := ghContext.Remotes{ + multipleRemotes := ghContext.Remotes{ &ghContext.Remote{ Remote: &git.Remote{ Name: "origin", @@ -141,10 +141,20 @@ func TestNewCmdDeleteBaseRepoFuncs(t *testing.T) { }, } + singleRemote := ghContext.Remotes{ + &ghContext.Remote{ + Remote: &git.Remote{ + Name: "origin", + }, + Repo: ghrepo.New("owner", "repo"), + }, + } + tests := []struct { name string args string env map[string]string + remotes ghContext.Remotes prompterStubs func(*prompter.MockPrompter) wantRepo ghrepo.Interface wantErr error @@ -152,6 +162,7 @@ func TestNewCmdDeleteBaseRepoFuncs(t *testing.T) { { name: "when there is a repo flag provided, the factory base repo func is used", args: "SECRET_NAME --repo owner/repo", + remotes: multipleRemotes, wantRepo: ghrepo.New("owner", "repo"), }, { @@ -160,18 +171,27 @@ func TestNewCmdDeleteBaseRepoFuncs(t *testing.T) { env: map[string]string{ "GH_REPO": "owner/repo", }, + remotes: multipleRemotes, wantRepo: ghrepo.New("owner", "repo"), }, { - name: "when there is no repo flag or GH_REPO env var provided, and no prompting, the base func requiring no ambiguity is used", - args: "SECRET_NAME", + name: "when there is no repo flag or GH_REPO env var provided, and no prompting, the base func requiring no ambiguity is used", + args: "SECRET_NAME", + remotes: multipleRemotes, wantErr: shared.AmbiguousBaseRepoError{ - Remotes: remotes, + Remotes: multipleRemotes, }, }, { - name: "when there is no repo flag or GH_REPO env var provided, and can prompt, the base func resolving ambiguity is used", - args: "SECRET_NAME", + name: "when there is no repo flag or GH_REPO env provided, and there is a single remote, the factory base repo func is used", + args: "SECRET_NAME", + remotes: singleRemote, + wantRepo: ghrepo.New("owner", "repo"), + }, + { + name: "when there is no repo flag or GH_REPO env var provided, and can prompt, the base func resolving ambiguity is used", + args: "SECRET_NAME", + remotes: multipleRemotes, prompterStubs: func(pm *prompter.MockPrompter) { pm.RegisterSelect( "Select a repo", @@ -204,7 +224,7 @@ func TestNewCmdDeleteBaseRepoFuncs(t *testing.T) { }, Prompter: pm, Remotes: func() (ghContext.Remotes, error) { - return remotes, nil + return tt.remotes, nil }, } diff --git a/pkg/cmd/secret/list/list.go b/pkg/cmd/secret/list/list.go index c976bd60e..06476a86d 100644 --- a/pkg/cmd/secret/list/list.go +++ b/pkg/cmd/secret/list/list.go @@ -70,9 +70,9 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman RunE: func(cmd *cobra.Command, args []string) error { // If the user specified a repo directly, then we're using the OverrideBaseRepoFunc set by EnableRepoOverride // So there's no reason to use the specialised BaseRepoFunc that requires remote disambiguation. - if cmd.Flags().Changed("repo") || os.Getenv("GH_REPO") != "" { - opts.BaseRepo = f.BaseRepo - } else { + opts.BaseRepo = f.BaseRepo + isRepoUserProvided := cmd.Flags().Changed("repo") || os.Getenv("GH_REPO") != "" + if !isRepoUserProvided { // If they haven't specified a repo directly, then we will wrap the BaseRepoFunc in one that errors if // there might be multiple valid remotes. opts.BaseRepo = shared.RequireNoAmbiguityBaseRepoFunc(opts.BaseRepo, f.Remotes) diff --git a/pkg/cmd/secret/list/list_test.go b/pkg/cmd/secret/list/list_test.go index f4ecde73e..5c4dd4874 100644 --- a/pkg/cmd/secret/list/list_test.go +++ b/pkg/cmd/secret/list/list_test.go @@ -106,7 +106,7 @@ func Test_NewCmdList(t *testing.T) { } func TestNewCmdListBaseRepoFuncs(t *testing.T) { - remotes := ghContext.Remotes{ + multipleRemotes := ghContext.Remotes{ &ghContext.Remote{ Remote: &git.Remote{ Name: "origin", @@ -121,10 +121,20 @@ func TestNewCmdListBaseRepoFuncs(t *testing.T) { }, } + singleRemote := ghContext.Remotes{ + &ghContext.Remote{ + Remote: &git.Remote{ + Name: "origin", + }, + Repo: ghrepo.New("owner", "repo"), + }, + } + tests := []struct { name string args string env map[string]string + remotes ghContext.Remotes prompterStubs func(*prompter.MockPrompter) wantRepo ghrepo.Interface wantErr error @@ -132,6 +142,7 @@ func TestNewCmdListBaseRepoFuncs(t *testing.T) { { name: "when there is a repo flag provided, the factory base repo func is used", args: "--repo owner/repo", + remotes: multipleRemotes, wantRepo: ghrepo.New("owner", "repo"), }, { @@ -139,18 +150,27 @@ func TestNewCmdListBaseRepoFuncs(t *testing.T) { env: map[string]string{ "GH_REPO": "owner/repo", }, + remotes: multipleRemotes, wantRepo: ghrepo.New("owner", "repo"), }, { - name: "when there is no repo flag or GH_REPO env var provided, and no prompting, the base func requiring no ambiguity is used", - args: "", + name: "when there is no repo flag or GH_REPO env var provided, and no prompting, the base func requiring no ambiguity is used", + args: "", + remotes: multipleRemotes, wantErr: shared.AmbiguousBaseRepoError{ - Remotes: remotes, + Remotes: multipleRemotes, }, }, { - name: "when there is no repo flag or GH_REPO env var provided, and can prompt, the base func resolving ambiguity is used", - args: "", + name: "when there is no repo flag or GH_REPO env provided, and there is a single remote, the factory base repo func is used", + args: "", + remotes: singleRemote, + wantRepo: ghrepo.New("owner", "repo"), + }, + { + name: "when there is no repo flag or GH_REPO env var provided, and can prompt, the base func resolving ambiguity is used", + args: "", + remotes: multipleRemotes, prompterStubs: func(pm *prompter.MockPrompter) { pm.RegisterSelect( "Select a repo", @@ -183,7 +203,7 @@ func TestNewCmdListBaseRepoFuncs(t *testing.T) { }, Prompter: pm, Remotes: func() (ghContext.Remotes, error) { - return remotes, nil + return tt.remotes, nil }, } diff --git a/pkg/cmd/secret/set/set.go b/pkg/cmd/secret/set/set.go index 89f11f1cf..0a6581559 100644 --- a/pkg/cmd/secret/set/set.go +++ b/pkg/cmd/secret/set/set.go @@ -106,9 +106,9 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command RunE: func(cmd *cobra.Command, args []string) error { // If the user specified a repo directly, then we're using the OverrideBaseRepoFunc set by EnableRepoOverride // So there's no reason to use the specialised BaseRepoFunc that requires remote disambiguation. - if cmd.Flags().Changed("repo") || os.Getenv("GH_REPO") != "" { - opts.BaseRepo = f.BaseRepo - } else { + opts.BaseRepo = f.BaseRepo + isRepoUserProvided := cmd.Flags().Changed("repo") || os.Getenv("GH_REPO") != "" + if !isRepoUserProvided { // If they haven't specified a repo directly, then we will wrap the BaseRepoFunc in one that errors if // there might be multiple valid remotes. opts.BaseRepo = shared.RequireNoAmbiguityBaseRepoFunc(opts.BaseRepo, f.Remotes) diff --git a/pkg/cmd/secret/set/set_test.go b/pkg/cmd/secret/set/set_test.go index 22c7aaab8..0b305eda6 100644 --- a/pkg/cmd/secret/set/set_test.go +++ b/pkg/cmd/secret/set/set_test.go @@ -225,7 +225,7 @@ func TestNewCmdSet(t *testing.T) { } func TestNewCmdSetBaseRepoFuncs(t *testing.T) { - remotes := ghContext.Remotes{ + multipleRemotes := ghContext.Remotes{ &ghContext.Remote{ Remote: &git.Remote{ Name: "origin", @@ -240,10 +240,20 @@ func TestNewCmdSetBaseRepoFuncs(t *testing.T) { }, } + singleRemote := ghContext.Remotes{ + &ghContext.Remote{ + Remote: &git.Remote{ + Name: "origin", + }, + Repo: ghrepo.New("owner", "repo"), + }, + } + tests := []struct { name string args string env map[string]string + remotes ghContext.Remotes prompterStubs func(*prompter.MockPrompter) wantRepo ghrepo.Interface wantErr error @@ -251,6 +261,7 @@ func TestNewCmdSetBaseRepoFuncs(t *testing.T) { { name: "when there is a repo flag provided, the factory base repo func is used", args: "SECRET_NAME --repo owner/repo", + remotes: multipleRemotes, wantRepo: ghrepo.New("owner", "repo"), }, { @@ -259,18 +270,27 @@ func TestNewCmdSetBaseRepoFuncs(t *testing.T) { env: map[string]string{ "GH_REPO": "owner/repo", }, + remotes: multipleRemotes, wantRepo: ghrepo.New("owner", "repo"), }, { - name: "when there is no repo flag or GH_REPO env var provided, and no prompting, the base func requiring no ambiguity is used", - args: "SECRET_NAME", + name: "when there is no repo flag or GH_REPO env var provided, and no prompting, the base func requiring no ambiguity is used", + args: "SECRET_NAME", + remotes: multipleRemotes, wantErr: shared.AmbiguousBaseRepoError{ - Remotes: remotes, + Remotes: multipleRemotes, }, }, { - name: "when there is no repo flag or GH_REPO env var provided, and can prompt, the base func resolving ambiguity is used", - args: "SECRET_NAME", + name: "when there is no repo flag or GH_REPO env provided, and there is a single remote, the factory base repo func is used", + args: "SECRET_NAME", + remotes: singleRemote, + wantRepo: ghrepo.New("owner", "repo"), + }, + { + name: "when there is no repo flag or GH_REPO env var provided, and can prompt, the base func resolving ambiguity is used", + args: "SECRET_NAME", + remotes: multipleRemotes, prompterStubs: func(pm *prompter.MockPrompter) { pm.RegisterSelect( "Select a repo", @@ -303,7 +323,7 @@ func TestNewCmdSetBaseRepoFuncs(t *testing.T) { }, Prompter: pm, Remotes: func() (ghContext.Remotes, error) { - return remotes, nil + return tt.remotes, nil }, }