Merge pull request #10549 from cli/10548-v2680-failed-to-run-secret-commands

Fix secret command panic when base repo from cwd
This commit is contained in:
William Martin 2025-03-06 12:01:40 +01:00 committed by GitHub
commit fc19ff321a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 90 additions and 30 deletions

View file

@ -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)

View file

@ -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
},
}

View file

@ -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)

View file

@ -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
},
}

View file

@ -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)

View file

@ -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
},
}