STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Issue develop early arg parsing

Browse source
This commit is contained in:
William Martin 2025-04-16 15:51:46 +02:00
parent 6129b26f9f
commit 7744e0564f
2 changed files with 96 additions and 105 deletions
Download patch file Download diff file Expand all files Collapse all files

45
pkg/cmd/issue/develop/develop.go
View file

@ -24,12 +24,12 @@ type DevelopOptions struct {
BaseRepo func() (ghrepo.Interface, error)
Remotes func() (context.Remotes, error)
IssueSelector string
Name string
BranchRepo string
BaseBranch string
Checkout bool
List bool
IssueNumber int
Name string
BranchRepo string
BaseBranch string
Checkout bool
List bool
}
func NewCmdDevelop(f *cmdutil.Factory, runF func(*DevelopOptions) error) *cobra.Command {
@ -89,9 +89,23 @@ func NewCmdDevelop(f *cmdutil.Factory, runF func(*DevelopOptions) error) *cobra.
return nil
},
RunE: func(cmd *cobra.Command, args []string) error {
// support `-R, --repo` override
opts.BaseRepo = f.BaseRepo
opts.IssueSelector = args[0]
issueNumber, baseRepo, err := shared.ParseIssueFromArg(args[0])
if err != nil {
return err
}
// If the args provided the base repo then use that directly.
if baseRepo, present := baseRepo.Value(); present {
opts.BaseRepo = func() (ghrepo.Interface, error) {
return baseRepo, nil
}
} else {
// support `-R, --repo` override
opts.BaseRepo = f.BaseRepo
}
opts.IssueNumber = issueNumber
if err := cmdutil.MutuallyExclusive("specify only one of `--list` or `--branch-repo`", opts.List, opts.BranchRepo != ""); err != nil {
return err
}
@ -131,8 +145,13 @@ func developRun(opts *DevelopOptions) error {
return err
}
baseRepo, err := opts.BaseRepo()
if err != nil {
return err
}
opts.IO.StartProgressIndicator()
issue, issueRepo, err := shared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.IssueSelector, []string{"id", "number"})
issue, err := shared.FindIssueOrPR(httpClient, baseRepo, opts.IssueNumber, []string{"id", "number"})
opts.IO.StopProgressIndicator()
if err != nil {
return err
@ -141,16 +160,16 @@ func developRun(opts *DevelopOptions) error {
apiClient := api.NewClientFromHTTP(httpClient)
opts.IO.StartProgressIndicator()
err = api.CheckLinkedBranchFeature(apiClient, issueRepo.RepoHost())
err = api.CheckLinkedBranchFeature(apiClient, baseRepo.RepoHost())
opts.IO.StopProgressIndicator()
if err != nil {
return err
}
if opts.List {
return developRunList(opts, apiClient, issueRepo, issue)
return developRunList(opts, apiClient, baseRepo, issue)
}
return developRunCreate(opts, apiClient, issueRepo, issue)
return developRunCreate(opts, apiClient, baseRepo, issue)
}
func developRunCreate(opts *DevelopOptions, apiClient *api.Client, issueRepo ghrepo.Interface, issue *api.Issue) error {

156
pkg/cmd/issue/develop/develop_test.go
View file

@ -11,89 +11,74 @@ import (
"github.com/cli/cli/v2/git"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/run"
"github.com/cli/cli/v2/pkg/cmd/issue/argparsetest"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/httpmock"
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/google/shlex"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestNewCmdDevelop(t *testing.T) {
// Test shared parsing of issue number / URL.
argparsetest.TestArgParsing(t, NewCmdDevelop)
tests := []struct {
name string
input string
output DevelopOptions
wantStdout string
wantStderr string
wantErr bool
errMsg string
name string
input string
output DevelopOptions
expectedBaseRepo ghrepo.Interface
wantStdout string
wantStderr string
wantErr bool
errMsg string
}{
{
name: "no argument",
input: "",
output: DevelopOptions{},
wantErr: true,
errMsg: "issue number or url is required",
},
{
name: "issue number",
input: "1",
output: DevelopOptions{
IssueSelector: "1",
},
},
{
name: "issue url",
input: "https://github.com/cli/cli/issues/1",
output: DevelopOptions{
IssueSelector: "https://github.com/cli/cli/issues/1",
},
},
{
name: "branch-repo flag",
input: "1 --branch-repo owner/repo",
output: DevelopOptions{
IssueSelector: "1",
BranchRepo: "owner/repo",
IssueNumber: 1,
BranchRepo: "owner/repo",
},
},
{
name: "base flag",
input: "1 --base feature",
output: DevelopOptions{
IssueSelector: "1",
BaseBranch: "feature",
IssueNumber: 1,
BaseBranch: "feature",
},
},
{
name: "checkout flag",
input: "1 --checkout",
output: DevelopOptions{
IssueSelector: "1",
Checkout: true,
IssueNumber: 1,
Checkout: true,
},
},
{
name: "list flag",
input: "1 --list",
output: DevelopOptions{
IssueSelector: "1",
List: true,
IssueNumber: 1,
List: true,
},
},
{
name: "name flag",
input: "1 --name feature",
output: DevelopOptions{
IssueSelector: "1",
Name: "feature",
IssueNumber: 1,
Name: "feature",
},
},
{
name: "issue-repo flag",
input: "1 --issue-repo cli/cli",
output: DevelopOptions{
IssueSelector: "1",
IssueNumber: 1,
},
wantStdout: "Flag --issue-repo has been deprecated, use `--repo` instead\n",
},
@ -143,18 +128,27 @@ func TestNewCmdDevelop(t *testing.T) {
_, err = cmd.ExecuteC()
if tt.wantErr {
assert.EqualError(t, err, tt.errMsg)
require.EqualError(t, err, tt.errMsg)
return
}
assert.NoError(t, err)
assert.Equal(t, tt.output.IssueSelector, gotOpts.IssueSelector)
require.NoError(t, err)
assert.Equal(t, tt.output.IssueNumber, gotOpts.IssueNumber)
assert.Equal(t, tt.output.Name, gotOpts.Name)
assert.Equal(t, tt.output.BaseBranch, gotOpts.BaseBranch)
assert.Equal(t, tt.output.Checkout, gotOpts.Checkout)
assert.Equal(t, tt.output.List, gotOpts.List)
assert.Equal(t, tt.wantStdout, stdOut.String())
assert.Equal(t, tt.wantStderr, stdErr.String())
if tt.expectedBaseRepo != nil {
baseRepo, err := gotOpts.BaseRepo()
require.NoError(t, err)
require.True(
t,
ghrepo.IsSame(tt.expectedBaseRepo, baseRepo),
"expected base repo %+v, got %+v", tt.expectedBaseRepo, baseRepo,
)
}
})
}
}
@ -178,8 +172,8 @@ func TestDevelopRun(t *testing.T) {
{
name: "returns an error when the feature is not supported by the API",
opts: &DevelopOptions{
IssueSelector: "42",
List: true,
IssueNumber: 42,
List: true,
},
httpStubs: func(reg *httpmock.Registry, t *testing.T) {
reg.Register(
@ -196,8 +190,8 @@ func TestDevelopRun(t *testing.T) {
{
name: "list branches for an issue",
opts: &DevelopOptions{
IssueSelector: "42",
List: true,
IssueNumber: 42,
List: true,
},
httpStubs: func(reg *httpmock.Registry, t *testing.T) {
reg.Register(
@ -223,8 +217,8 @@ func TestDevelopRun(t *testing.T) {
{
name: "list branches for an issue in tty",
opts: &DevelopOptions{
IssueSelector: "42",
List: true,
IssueNumber: 42,
List: true,
},
tty: true,
httpStubs: func(reg *httpmock.Registry, t *testing.T) {
@ -255,37 +249,10 @@ func TestDevelopRun(t *testing.T) {
bar https://github.com/OWNER/OTHER-REPO/tree/bar
`),
},
{
name: "list branches for an issue providing an issue url",
opts: &DevelopOptions{
IssueSelector: "https://github.com/cli/cli/issues/42",
List: true,
},
httpStubs: func(reg *httpmock.Registry, t *testing.T) {
reg.Register(
httpmock.GraphQL(`query IssueByNumber\b`),
httpmock.StringResponse(`{"data":{"repository":{"hasIssuesEnabled":true,"issue":{"id":"SOMEID","number":42}}}}`),
)
reg.Register(
httpmock.GraphQL(`query LinkedBranchFeature\b`),
httpmock.StringResponse(featureEnabledPayload),
)
reg.Register(
httpmock.GraphQL(`query ListLinkedBranches\b`),
httpmock.GraphQLQuery(`
{"data":{"repository":{"issue":{"linkedBranches":{"nodes":[{"ref":{"name":"foo","repository":{"url":"https://github.com/OWNER/REPO"}}},{"ref":{"name":"bar","repository":{"url":"https://github.com/OWNER/OTHER-REPO"}}}]}}}}}
`, func(query string, inputs map[string]interface{}) {
assert.Equal(t, float64(42), inputs["number"])
assert.Equal(t, "cli", inputs["owner"])
assert.Equal(t, "cli", inputs["name"])
}))
},
expectedOut: "foo\thttps://github.com/OWNER/REPO/tree/foo\nbar\thttps://github.com/OWNER/OTHER-REPO/tree/bar\n",
},
{
name: "develop new branch",
opts: &DevelopOptions{
IssueSelector: "123",
IssueNumber: 123,
},
remotes: map[string]string{
"origin": "OWNER/REPO",
@ -321,8 +288,8 @@ func TestDevelopRun(t *testing.T) {
{
name: "develop new branch in different repo than issue",
opts: &DevelopOptions{
IssueSelector: "123",
BranchRepo: "OWNER2/REPO",
IssueNumber: 123,
BranchRepo: "OWNER2/REPO",
},
remotes: map[string]string{
"origin": "OWNER2/REPO",
@ -367,9 +334,9 @@ func TestDevelopRun(t *testing.T) {
{
name: "develop new branch with name and base specified",
opts: &DevelopOptions{
Name: "my-branch",
BaseBranch: "main",
IssueSelector: "123",
Name: "my-branch",
BaseBranch: "main",
IssueNumber: 123,
},
remotes: map[string]string{
"origin": "OWNER/REPO",
@ -406,7 +373,10 @@ func TestDevelopRun(t *testing.T) {
{
name: "develop new branch outside of local git repo",
opts: &DevelopOptions{
IssueSelector: "https://github.com/cli/cli/issues/123",
IssueNumber: 123,
BaseRepo: func() (ghrepo.Interface, error) {
return ghrepo.New("cli", "cli"), nil
},
},
httpStubs: func(reg *httpmock.Registry, t *testing.T) {
reg.Register(
@ -436,9 +406,9 @@ func TestDevelopRun(t *testing.T) {
{
name: "develop new branch with checkout when local branch exists",
opts: &DevelopOptions{
Name: "my-branch",
IssueSelector: "123",
Checkout: true,
Name: "my-branch",
IssueNumber: 123,
Checkout: true,
},
remotes: map[string]string{
"origin": "OWNER/REPO",
@ -478,9 +448,9 @@ func TestDevelopRun(t *testing.T) {
{
name: "develop new branch with checkout when local branch does not exist",
opts: &DevelopOptions{
Name: "my-branch",
IssueSelector: "123",
Checkout: true,
Name: "my-branch",
IssueNumber: 123,
Checkout: true,
},
remotes: map[string]string{
"origin": "OWNER/REPO",
@ -519,8 +489,8 @@ func TestDevelopRun(t *testing.T) {
{
name: "develop with base branch which does not exist",
opts: &DevelopOptions{
IssueSelector: "123",
BaseBranch: "does-not-exist-branch",
IssueNumber: 123,
BaseBranch: "does-not-exist-branch",
},
remotes: map[string]string{
"origin": "OWNER/REPO",
@ -561,8 +531,10 @@ func TestDevelopRun(t *testing.T) {
ios.SetStderrTTY(tt.tty)
opts.IO = ios
opts.BaseRepo = func() (ghrepo.Interface, error) {
return ghrepo.New("OWNER", "REPO"), nil
if opts.BaseRepo == nil {
opts.BaseRepo = func() (ghrepo.Interface, error) {
return ghrepo.New("OWNER", "REPO"), nil
}
}
opts.Remotes = func() (context.Remotes, error) {