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

Merge branch 'trunk' into trunk

Browse source
This commit is contained in:
Barak Amar 2025-04-22 09:05:38 -04:00 • committed by GitHub
commit 66527cf75c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
24 changed files with 914 additions and 750 deletions
Download patch file Download diff file Expand all files Collapse all files

137
pkg/cmd/issue/argparsetest/argparsetest.go Normal file
View file

@ -0,0 +1,137 @@
package argparsetest
import (
"bytes"
"reflect"
"testing"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/google/shlex"
"github.com/spf13/cobra"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// newCmdFunc represents the typical function signature we use for creating commands e.g. `NewCmdView`.
//
// It is generic over `T` as each command construction has their own Options type e.g. `ViewOptions`
type newCmdFunc[T any] func(f *cmdutil.Factory, runF func(*T) error) *cobra.Command
// TestArgParsing is a test helper that verifies that issue commands correctly parse the `{issue number | url}`
// positional arg into an issue number and base repo.
//
// Looking through the existing tests, I noticed that the coverage was pretty smattered.
// Since nearly all issue commands only accept a single positional argument, we are able to reuse this test helper.
// Commands with no further flags or args can use this solely.
// Commands with extra flags use this and further table tests.
// Commands with extra required positional arguments (like `transfer`) cannot use this. They duplicate these cases inline.
func TestArgParsing[T any](t *testing.T, fn newCmdFunc[T]) {
tests := []struct {
name string
input string
expectedissueNumber int
expectedBaseRepo ghrepo.Interface
expectErr bool
}{
{
name: "no argument",
input: "",
expectErr: true,
},
{
name: "issue number argument",
input: "23 --repo owner/repo",
expectedissueNumber: 23,
expectedBaseRepo: ghrepo.New("owner", "repo"),
},
{
name: "argument is hash prefixed number",
// Escaping is required here to avoid what I think is shellex treating it as a comment.
input: "\\#23 --repo owner/repo",
expectedissueNumber: 23,
expectedBaseRepo: ghrepo.New("owner", "repo"),
},
{
name: "argument is a URL",
input: "https://github.com/cli/cli/issues/23",
expectedissueNumber: 23,
expectedBaseRepo: ghrepo.New("cli", "cli"),
},
{
name: "argument cannot be parsed to an issue",
input: "unparseable",
expectErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
f := &cmdutil.Factory{}
argv, err := shlex.Split(tt.input)
assert.NoError(t, err)
var gotOpts T
cmd := fn(f, func(opts *T) error {
gotOpts = *opts
return nil
})
cmdutil.EnableRepoOverride(cmd, f)
// TODO: remember why we do this
cmd.Flags().BoolP("help", "x", false, "")
cmd.SetArgs(argv)
cmd.SetIn(&bytes.Buffer{})
cmd.SetOut(&bytes.Buffer{})
cmd.SetErr(&bytes.Buffer{})
_, err = cmd.ExecuteC()
if tt.expectErr {
require.Error(t, err)
return
} else {
require.NoError(t, err)
}
actualIssueNumber := issueNumberFromOpts(t, gotOpts)
assert.Equal(t, tt.expectedissueNumber, actualIssueNumber)
actualBaseRepo := baseRepoFromOpts(t, gotOpts)
assert.True(
t,
ghrepo.IsSame(tt.expectedBaseRepo, actualBaseRepo),
"expected base repo %+v, got %+v", tt.expectedBaseRepo, actualBaseRepo,
)
})
}
}
func issueNumberFromOpts(t *testing.T, v any) int {
rv := reflect.ValueOf(v)
field := rv.FieldByName("IssueNumber")
if !field.IsValid() || field.Kind() != reflect.Int {
t.Fatalf("Type %T does not have IssueNumber int field", v)
}
return int(field.Int())
}
func baseRepoFromOpts(t *testing.T, v any) ghrepo.Interface {
rv := reflect.ValueOf(v)
field := rv.FieldByName("BaseRepo")
// check whether the field is valid and of type func() (ghrepo.Interface, error)
if !field.IsValid() || field.Kind() != reflect.Func {
t.Fatalf("Type %T does not have BaseRepo func field", v)
}
// call the function and check the return value
results := field.Call([]reflect.Value{})
if len(results) != 2 {
t.Fatalf("%T.BaseRepo() does not return two values", v)
}
if !results[1].IsNil() {
t.Fatalf("%T.BaseRepo() returned an error: %v", v, results[1].Interface())
}
return results[0].Interface().(ghrepo.Interface)
}

29
pkg/cmd/issue/close/close.go
View file

@ -21,7 +21,7 @@ type CloseOptions struct {
IO *iostreams.IOStreams
BaseRepo func() (ghrepo.Interface, error)
SelectorArg string
IssueNumber int
Comment string
Reason string
@ -39,13 +39,23 @@ func NewCmdClose(f *cmdutil.Factory, runF func(*CloseOptions) error) *cobra.Comm
Short: "Close issue",
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
// support `-R, --repo` override
opts.BaseRepo = f.BaseRepo
if len(args) > 0 {
opts.SelectorArg = 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 runF != nil {
return runF(opts)
}
@ -67,7 +77,12 @@ func closeRun(opts *CloseOptions) error {
return err
}
issue, baseRepo, err := shared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.SelectorArg, []string{"id", "number", "title", "state"})
baseRepo, err := opts.BaseRepo()
if err != nil {
return err
}
issue, err := shared.FindIssueOrPR(httpClient, baseRepo, opts.IssueNumber, []string{"id", "number", "title", "state"})
if err != nil {
return err
}

67
pkg/cmd/issue/close/close_test.go
View file

@ -7,46 +7,32 @@ import (
fd "github.com/cli/cli/v2/internal/featuredetection"
"github.com/cli/cli/v2/internal/ghrepo"
"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 TestNewCmdClose(t *testing.T) {
// Test shared parsing of issue number / URL.
argparsetest.TestArgParsing(t, NewCmdClose)
tests := []struct {
name string
input string
output CloseOptions
wantErr bool
errMsg string
name string
input string
output CloseOptions
expectedBaseRepo ghrepo.Interface
wantErr bool
errMsg string
}{
{
name: "no argument",
input: "",
wantErr: true,
errMsg: "accepts 1 arg(s), received 0",
},
{
name: "issue number",
input: "123",
output: CloseOptions{
SelectorArg: "123",
},
},
{
name: "issue url",
input: "https://github.com/cli/cli/3",
output: CloseOptions{
SelectorArg: "https://github.com/cli/cli/3",
},
},
{
name: "comment",
input: "123 --comment 'closing comment'",
output: CloseOptions{
SelectorArg: "123",
IssueNumber: 123,
Comment: "closing comment",
},
},
@ -54,7 +40,7 @@ func TestNewCmdClose(t *testing.T) {
name: "reason",
input: "123 --reason 'not planned'",
output: CloseOptions{
SelectorArg: "123",
IssueNumber: 123,
Reason: "not planned",
},
},
@ -79,15 +65,24 @@ func TestNewCmdClose(t *testing.T) {
_, err = cmd.ExecuteC()
if tt.wantErr {
assert.Error(t, err)
require.Error(t, err)
assert.Equal(t, tt.errMsg, err.Error())
return
}
assert.NoError(t, err)
assert.Equal(t, tt.output.SelectorArg, gotOpts.SelectorArg)
require.NoError(t, err)
assert.Equal(t, tt.output.IssueNumber, gotOpts.IssueNumber)
assert.Equal(t, tt.output.Comment, gotOpts.Comment)
assert.Equal(t, tt.output.Reason, gotOpts.Reason)
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,
)
}
})
}
}
@ -104,7 +99,7 @@ func TestCloseRun(t *testing.T) {
{
name: "close issue by number",
opts: &CloseOptions{
SelectorArg: "13",
IssueNumber: 13,
},
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
@ -128,7 +123,7 @@ func TestCloseRun(t *testing.T) {
{
name: "close issue with comment",
opts: &CloseOptions{
SelectorArg: "13",
IssueNumber: 13,
Comment: "closing comment",
},
httpStubs: func(reg *httpmock.Registry) {
@ -164,7 +159,7 @@ func TestCloseRun(t *testing.T) {
{
name: "close issue with reason",
opts: &CloseOptions{
SelectorArg: "13",
IssueNumber: 13,
Reason: "not planned",
Detector: &fd.EnabledDetectorMock{},
},
@ -192,7 +187,7 @@ func TestCloseRun(t *testing.T) {
{
name: "close issue with reason when reason is not supported",
opts: &CloseOptions{
SelectorArg: "13",
IssueNumber: 13,
Reason: "not planned",
Detector: &fd.DisabledDetectorMock{},
},
@ -219,7 +214,7 @@ func TestCloseRun(t *testing.T) {
{
name: "issue already closed",
opts: &CloseOptions{
SelectorArg: "13",
IssueNumber: 13,
},
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
@ -236,7 +231,7 @@ func TestCloseRun(t *testing.T) {
{
name: "issues disabled",
opts: &CloseOptions{
SelectorArg: "13",
IssueNumber: 13,
},
httpStubs: func(reg *httpmock.Registry) {
reg.Register(

29
pkg/cmd/issue/comment/comment.go
View file

@ -3,6 +3,7 @@ package comment
import (
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/cmd/issue/shared"
issueShared "github.com/cli/cli/v2/pkg/cmd/issue/shared"
prShared "github.com/cli/cli/v2/pkg/cmd/pr/shared"
"github.com/cli/cli/v2/pkg/cmdutil"
@ -37,15 +38,41 @@ func NewCmdComment(f *cmdutil.Factory, runF func(*prShared.CommentableOptions) e
Args: cobra.ExactArgs(1),
PreRunE: func(cmd *cobra.Command, args []string) error {
opts.RetrieveCommentable = func() (prShared.Commentable, ghrepo.Interface, error) {
// TODO wm: more testing
issueNumber, parsedBaseRepo, err := shared.ParseIssueFromArg(args[0])
if err != nil {
return nil, nil, err
}
// If the args provided the base repo then use that directly.
var baseRepo ghrepo.Interface
if parsedBaseRepo, present := parsedBaseRepo.Value(); present {
baseRepo = parsedBaseRepo
} else {
// support `-R, --repo` override
baseRepo, err = f.BaseRepo()
if err != nil {
return nil, nil, err
}
}
httpClient, err := f.HttpClient()
if err != nil {
return nil, nil, err
}
fields := []string{"id", "url"}
if opts.EditLast {
fields = append(fields, "comments")
}
return issueShared.IssueFromArgWithFields(httpClient, f.BaseRepo, args[0], fields)
issue, err := issueShared.FindIssueOrPR(httpClient, baseRepo, issueNumber, fields)
if err != nil {
return nil, nil, err
}
return issue, baseRepo, nil
}
return prShared.CommentablePreRun(cmd, opts)
},

29
pkg/cmd/issue/delete/delete.go
View file

@ -21,7 +21,7 @@ type DeleteOptions struct {
BaseRepo func() (ghrepo.Interface, error)
Prompter iprompter
SelectorArg string
IssueNumber int
Confirmed bool
}
@ -42,13 +42,23 @@ func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Co
Short: "Delete issue",
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
// support `-R, --repo` override
opts.BaseRepo = f.BaseRepo
if len(args) > 0 {
opts.SelectorArg = 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 runF != nil {
return runF(opts)
}
@ -71,7 +81,12 @@ func deleteRun(opts *DeleteOptions) error {
return err
}
issue, baseRepo, err := shared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.SelectorArg, []string{"id", "number", "title"})
baseRepo, err := opts.BaseRepo()
if err != nil {
return err
}
issue, err := shared.FindIssueOrPR(httpClient, baseRepo, opts.IssueNumber, []string{"id", "number", "title"})
if err != nil {
return err
}

5
pkg/cmd/issue/delete/delete_test.go
View file

@ -12,6 +12,7 @@ import (
"github.com/cli/cli/v2/internal/gh"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/prompter"
"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"
@ -20,6 +21,10 @@ import (
"github.com/stretchr/testify/assert"
)
func TestNewCmdDelete(t *testing.T) {
argparsetest.TestArgParsing(t, NewCmdDelete)
}
func runCommand(rt http.RoundTripper, pm *prompter.MockPrompter, isTTY bool, cli string) (*test.CmdOut, error) {
ios, _, stdout, stderr := iostreams.Test()
ios.SetStdoutTTY(isTTY)

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

33
pkg/cmd/issue/edit/edit.go
View file

@ -28,7 +28,7 @@ type EditOptions struct {
EditFieldsSurvey func(prShared.EditPrompter, *prShared.Editable, string) error
FetchOptions func(*api.Client, ghrepo.Interface, *prShared.Editable) error
SelectorArgs []string
IssueNumbers []int
Interactive bool
prShared.Editable
@ -69,10 +69,22 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman
`),
Args: cobra.MinimumNArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
// support `-R, --repo` override
opts.BaseRepo = f.BaseRepo
issueNumbers, baseRepo, err := shared.ParseIssuesFromArgs(args)
if err != nil {
return err
}
opts.SelectorArgs = args
// 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.IssueNumbers = issueNumbers
flags := cmd.Flags()
@ -134,7 +146,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman
return cmdutil.FlagErrorf("field to edit flag required when not running interactively")
}
if opts.Interactive && len(opts.SelectorArgs) > 1 {
if opts.Interactive && len(opts.IssueNumbers) > 1 {
return cmdutil.FlagErrorf("multiple issues cannot be edited interactively")
}
@ -167,6 +179,11 @@ func editRun(opts *EditOptions) error {
return err
}
baseRepo, err := opts.BaseRepo()
if err != nil {
return err
}
// Prompt the user which fields they'd like to edit.
editable := opts.Editable
if opts.Interactive {
@ -192,7 +209,7 @@ func editRun(opts *EditOptions) error {
}
// Get all specified issues and make sure they are within the same repo.
issues, repo, err := shared.IssuesFromArgsWithFields(httpClient, opts.BaseRepo, opts.SelectorArgs, lookupFields)
issues, err := shared.FindIssuesOrPRs(httpClient, baseRepo, opts.IssueNumbers, lookupFields)
if err != nil {
return err
}
@ -200,7 +217,7 @@ func editRun(opts *EditOptions) error {
// Fetch editable shared fields once for all issues.
apiClient := api.NewClientFromHTTP(httpClient)
opts.IO.StartProgressIndicatorWithLabel("Fetching repository information")
err = opts.FetchOptions(apiClient, repo, &editable)
err = opts.FetchOptions(apiClient, baseRepo, &editable)
opts.IO.StopProgressIndicator()
if err != nil {
return err
@ -250,7 +267,7 @@ func editRun(opts *EditOptions) error {
go func(issue *api.Issue) {
defer g.Done()
err := prShared.UpdateIssue(httpClient, repo, issue.ID, issue.IsPullRequest(), editable)
err := prShared.UpdateIssue(httpClient, baseRepo, issue.ID, issue.IsPullRequest(), editable)
if err != nil {
failedIssueChan <- fmt.Sprintf("failed to update %s: %s", issue.URL, err)
return

89
pkg/cmd/issue/edit/edit_test.go
View file

@ -26,11 +26,12 @@ func TestNewCmdEdit(t *testing.T) {
require.NoError(t, err)
tests := []struct {
name string
input string
stdin string
output EditOptions
wantsErr bool
name string
input string
stdin string
output EditOptions
expectedBaseRepo ghrepo.Interface
wantsErr bool
}{
{
name: "no argument",
@ -42,7 +43,7 @@ func TestNewCmdEdit(t *testing.T) {
name: "issue number argument",
input: "23",
output: EditOptions{
SelectorArgs: []string{"23"},
IssueNumbers: []int{23},
Interactive: true,
},
wantsErr: false,
@ -51,7 +52,7 @@ func TestNewCmdEdit(t *testing.T) {
name: "title flag",
input: "23 --title test",
output: EditOptions{
SelectorArgs: []string{"23"},
IssueNumbers: []int{23},
Editable: prShared.Editable{
Title: prShared.EditableString{
Value: "test",
@ -65,7 +66,7 @@ func TestNewCmdEdit(t *testing.T) {
name: "body flag",
input: "23 --body test",
output: EditOptions{
SelectorArgs: []string{"23"},
IssueNumbers: []int{23},
Editable: prShared.Editable{
Body: prShared.EditableString{
Value: "test",
@ -80,7 +81,7 @@ func TestNewCmdEdit(t *testing.T) {
input: "23 --body-file -",
stdin: "this is on standard input",
output: EditOptions{
SelectorArgs: []string{"23"},
IssueNumbers: []int{23},
Editable: prShared.Editable{
Body: prShared.EditableString{
Value: "this is on standard input",
@ -94,7 +95,7 @@ func TestNewCmdEdit(t *testing.T) {
name: "body from file",
input: fmt.Sprintf("23 --body-file '%s'", tmpFile),
output: EditOptions{
SelectorArgs: []string{"23"},
IssueNumbers: []int{23},
Editable: prShared.Editable{
Body: prShared.EditableString{
Value: "a body from file",
@ -113,7 +114,7 @@ func TestNewCmdEdit(t *testing.T) {
name: "add-assignee flag",
input: "23 --add-assignee monalisa,hubot",
output: EditOptions{
SelectorArgs: []string{"23"},
IssueNumbers: []int{23},
Editable: prShared.Editable{
Assignees: prShared.EditableSlice{
Add: []string{"monalisa", "hubot"},
@ -127,7 +128,7 @@ func TestNewCmdEdit(t *testing.T) {
name: "remove-assignee flag",
input: "23 --remove-assignee monalisa,hubot",
output: EditOptions{
SelectorArgs: []string{"23"},
IssueNumbers: []int{23},
Editable: prShared.Editable{
Assignees: prShared.EditableSlice{
Remove: []string{"monalisa", "hubot"},
@ -141,7 +142,7 @@ func TestNewCmdEdit(t *testing.T) {
name: "add-label flag",
input: "23 --add-label feature,TODO,bug",
output: EditOptions{
SelectorArgs: []string{"23"},
IssueNumbers: []int{23},
Editable: prShared.Editable{
Labels: prShared.EditableSlice{
Add: []string{"feature", "TODO", "bug"},
@ -155,7 +156,7 @@ func TestNewCmdEdit(t *testing.T) {
name: "remove-label flag",
input: "23 --remove-label feature,TODO,bug",
output: EditOptions{
SelectorArgs: []string{"23"},
IssueNumbers: []int{23},
Editable: prShared.Editable{
Labels: prShared.EditableSlice{
Remove: []string{"feature", "TODO", "bug"},
@ -169,7 +170,7 @@ func TestNewCmdEdit(t *testing.T) {
name: "add-project flag",
input: "23 --add-project Cleanup,Roadmap",
output: EditOptions{
SelectorArgs: []string{"23"},
IssueNumbers: []int{23},
Editable: prShared.Editable{
Projects: prShared.EditableProjects{
EditableSlice: prShared.EditableSlice{
@ -185,7 +186,7 @@ func TestNewCmdEdit(t *testing.T) {
name: "remove-project flag",
input: "23 --remove-project Cleanup,Roadmap",
output: EditOptions{
SelectorArgs: []string{"23"},
IssueNumbers: []int{23},
Editable: prShared.Editable{
Projects: prShared.EditableProjects{
EditableSlice: prShared.EditableSlice{
@ -201,7 +202,7 @@ func TestNewCmdEdit(t *testing.T) {
name: "milestone flag",
input: "23 --milestone GA",
output: EditOptions{
SelectorArgs: []string{"23"},
IssueNumbers: []int{23},
Editable: prShared.Editable{
Milestone: prShared.EditableString{
Value: "GA",
@ -215,7 +216,7 @@ func TestNewCmdEdit(t *testing.T) {
name: "remove-milestone flag",
input: "23 --remove-milestone",
output: EditOptions{
SelectorArgs: []string{"23"},
IssueNumbers: []int{23},
Editable: prShared.Editable{
Milestone: prShared.EditableString{
Value: "",
@ -234,7 +235,7 @@ func TestNewCmdEdit(t *testing.T) {
name: "add label to multiple issues",
input: "23 34 --add-label bug",
output: EditOptions{
SelectorArgs: []string{"23", "34"},
IssueNumbers: []int{23, 34},
Editable: prShared.Editable{
Labels: prShared.EditableSlice{
Add: []string{"bug"},
@ -244,6 +245,31 @@ func TestNewCmdEdit(t *testing.T) {
},
wantsErr: false,
},
{
name: "argument is hash prefixed number",
// Escaping is required here to avoid what I think is shellex treating it as a comment.
input: "\\#23",
output: EditOptions{
IssueNumbers: []int{23},
Interactive: true,
},
wantsErr: false,
},
{
name: "argument is a URL",
input: "https://github.com/cli/cli/issues/23",
output: EditOptions{
IssueNumbers: []int{23},
Interactive: true,
},
expectedBaseRepo: ghrepo.New("cli", "cli"),
wantsErr: false,
},
{
name: "URL arguments parse as different repos",
input: "https://github.com/cli/cli/issues/23 https://github.com/cli/go-gh/issues/23",
wantsErr: true,
},
{
name: "interactive multiple issues",
input: "23 34",
@ -282,14 +308,23 @@ func TestNewCmdEdit(t *testing.T) {
_, err = cmd.ExecuteC()
if tt.wantsErr {
assert.Error(t, err)
require.Error(t, err)
return
}
assert.NoError(t, err)
assert.Equal(t, tt.output.SelectorArgs, gotOpts.SelectorArgs)
require.NoError(t, err)
assert.Equal(t, tt.output.IssueNumbers, gotOpts.IssueNumbers)
assert.Equal(t, tt.output.Interactive, gotOpts.Interactive)
assert.Equal(t, tt.output.Editable, gotOpts.Editable)
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,
)
}
})
}
}
@ -306,7 +341,7 @@ func Test_editRun(t *testing.T) {
{
name: "non-interactive",
input: &EditOptions{
SelectorArgs: []string{"123"},
IssueNumbers: []int{123},
Interactive: false,
Editable: prShared.Editable{
Title: prShared.EditableString{
@ -359,7 +394,7 @@ func Test_editRun(t *testing.T) {
{
name: "non-interactive multiple issues",
input: &EditOptions{
SelectorArgs: []string{"456", "123"},
IssueNumbers: []int{456, 123},
Interactive: false,
Editable: prShared.Editable{
Assignees: prShared.EditableSlice{
@ -409,7 +444,7 @@ func Test_editRun(t *testing.T) {
{
name: "non-interactive multiple issues with fetch failures",
input: &EditOptions{
SelectorArgs: []string{"123", "9999"},
IssueNumbers: []int{123, 9999},
Interactive: false,
Editable: prShared.Editable{
Assignees: prShared.EditableSlice{
@ -454,7 +489,7 @@ func Test_editRun(t *testing.T) {
{
name: "non-interactive multiple issues with update failures",
input: &EditOptions{
SelectorArgs: []string{"123", "456"},
IssueNumbers: []int{123, 456},
Interactive: false,
Editable: prShared.Editable{
Assignees: prShared.EditableSlice{
@ -524,7 +559,7 @@ func Test_editRun(t *testing.T) {
{
name: "interactive",
input: &EditOptions{
SelectorArgs: []string{"123"},
IssueNumbers: []int{123},
Interactive: true,
FieldsToEditSurvey: func(p prShared.EditPrompter, eo *prShared.Editable) error {
eo.Title.Edited = true

47
pkg/cmd/issue/lock/lock.go
View file

@ -19,6 +19,7 @@ import (
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/gh"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/cmd/issue/shared"
issueShared "github.com/cli/cli/v2/pkg/cmd/issue/shared"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/iostreams"
@ -99,20 +100,33 @@ type LockOptions struct {
ParentCmd string
Reason string
SelectorArg string
IssueNumber int
Interactive bool
}
func (opts *LockOptions) setCommonOptions(f *cmdutil.Factory, args []string) {
func (opts *LockOptions) setCommonOptions(f *cmdutil.Factory, args []string) error {
opts.IO = f.IOStreams
opts.HttpClient = f.HttpClient
opts.Config = f.Config
// support `-R, --repo` override
opts.BaseRepo = f.BaseRepo
issueNumber, baseRepo, err := shared.ParseIssueFromArg(args[0])
if err != nil {
return err
}
opts.SelectorArg = args[0]
// 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
return nil
}
func NewCmdLock(f *cmdutil.Factory, parentName string, runF func(string, *LockOptions) error) *cobra.Command {
@ -129,7 +143,9 @@ func NewCmdLock(f *cmdutil.Factory, parentName string, runF func(string, *LockOp
Short: short,
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
opts.setCommonOptions(f, args)
if err := opts.setCommonOptions(f, args); err != nil {
return err
}
reasonProvided := cmd.Flags().Changed("reason")
if reasonProvided {
@ -172,7 +188,9 @@ func NewCmdUnlock(f *cmdutil.Factory, parentName string, runF func(string, *Lock
Short: short,
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
opts.setCommonOptions(f, args)
if err := opts.setCommonOptions(f, args); err != nil {
return err
}
if runF != nil {
return runF(Unlock, opts)
@ -214,13 +232,18 @@ func lockRun(state string, opts *LockOptions) error {
return err
}
issuePr, baseRepo, err := issueShared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.SelectorArg, fields())
parent := alias[opts.ParentCmd]
baseRepo, err := opts.BaseRepo()
if err != nil {
return err
} else if parent.Typename != issuePr.Typename {
}
issuePr, err := issueShared.FindIssueOrPR(httpClient, baseRepo, opts.IssueNumber, fields())
if err != nil {
return err
}
parent := alias[opts.ParentCmd]
if parent.Typename != issuePr.Typename {
currentType := alias[parent.Typename]
correctType := alias[issuePr.Typename]

98
pkg/cmd/issue/lock/lock_test.go
View file

@ -30,7 +30,7 @@ func Test_NewCmdLock(t *testing.T) {
args: "--reason off_topic 451",
want: LockOptions{
Reason: "off_topic",
SelectorArg: "451",
IssueNumber: 451,
},
},
{
@ -41,9 +41,36 @@ func Test_NewCmdLock(t *testing.T) {
name: "no flags",
args: "451",
want: LockOptions{
SelectorArg: "451",
IssueNumber: 451,
},
},
{
name: "issue number argument",
args: "451 --repo owner/repo",
want: LockOptions{
IssueNumber: 451,
},
},
{
name: "argument is hash prefixed number",
// Escaping is required here to avoid what I think is shellex treating it as a comment.
args: "\\#451 --repo owner/repo",
want: LockOptions{
IssueNumber: 451,
},
},
{
name: "argument is a URL",
args: "https://github.com/cli/cli/issues/451",
want: LockOptions{
IssueNumber: 451,
},
},
{
name: "argument cannot be parsed to an issue",
args: "unparseable",
wantErr: "invalid issue format: \"unparseable\"",
},
{
name: "bad reason",
args: "--reason bad 451",
@ -60,7 +87,7 @@ func Test_NewCmdLock(t *testing.T) {
args: "451",
tty: true,
want: LockOptions{
SelectorArg: "451",
IssueNumber: 451,
Interactive: true,
},
},
@ -99,7 +126,7 @@ func Test_NewCmdLock(t *testing.T) {
}
assert.Equal(t, tt.want.Reason, opts.Reason)
assert.Equal(t, tt.want.SelectorArg, opts.SelectorArg)
assert.Equal(t, tt.want.IssueNumber, opts.IssueNumber)
assert.Equal(t, tt.want.Interactive, opts.Interactive)
})
}
@ -121,9 +148,36 @@ func Test_NewCmdUnlock(t *testing.T) {
name: "no flags",
args: "451",
want: LockOptions{
SelectorArg: "451",
IssueNumber: 451,
},
},
{
name: "issue number argument",
args: "451 --repo owner/repo",
want: LockOptions{
IssueNumber: 451,
},
},
{
name: "argument is hash prefixed number",
// Escaping is required here to avoid what I think is shellex treating it as a comment.
args: "\\#451 --repo owner/repo",
want: LockOptions{
IssueNumber: 451,
},
},
{
name: "argument is a URL",
args: "https://github.com/cli/cli/issues/451",
want: LockOptions{
IssueNumber: 451,
},
},
{
name: "argument cannot be parsed to an issue",
args: "unparseable",
wantErr: "invalid issue format: \"unparseable\"",
},
}
for _, tt := range cases {
@ -158,7 +212,7 @@ func Test_NewCmdUnlock(t *testing.T) {
assert.NoError(t, err)
}
assert.Equal(t, tt.want.SelectorArg, opts.SelectorArg)
assert.Equal(t, tt.want.IssueNumber, opts.IssueNumber)
})
}
}
@ -179,7 +233,7 @@ func Test_runLock(t *testing.T) {
name: "lock issue nontty",
state: Lock,
opts: LockOptions{
SelectorArg: "451",
IssueNumber: 451,
ParentCmd: "issue",
},
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
@ -203,7 +257,7 @@ func Test_runLock(t *testing.T) {
tty: true,
opts: LockOptions{
Interactive: true,
SelectorArg: "451",
IssueNumber: 451,
ParentCmd: "issue",
},
state: Lock,
@ -241,7 +295,7 @@ func Test_runLock(t *testing.T) {
tty: true,
state: Lock,
opts: LockOptions{
SelectorArg: "451",
IssueNumber: 451,
ParentCmd: "issue",
Reason: "off_topic",
},
@ -268,7 +322,7 @@ func Test_runLock(t *testing.T) {
tty: true,
state: Unlock,
opts: LockOptions{
SelectorArg: "451",
IssueNumber: 451,
ParentCmd: "issue",
},
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
@ -294,7 +348,7 @@ func Test_runLock(t *testing.T) {
name: "unlock issue nontty",
state: Unlock,
opts: LockOptions{
SelectorArg: "451",
IssueNumber: 451,
ParentCmd: "issue",
},
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
@ -319,7 +373,7 @@ func Test_runLock(t *testing.T) {
name: "lock issue with explicit reason nontty",
state: Lock,
opts: LockOptions{
SelectorArg: "451",
IssueNumber: 451,
ParentCmd: "issue",
Reason: "off_topic",
},
@ -344,7 +398,7 @@ func Test_runLock(t *testing.T) {
name: "relock issue tty",
state: Lock,
opts: LockOptions{
SelectorArg: "451",
IssueNumber: 451,
ParentCmd: "issue",
Reason: "off_topic",
},
@ -388,7 +442,7 @@ func Test_runLock(t *testing.T) {
name: "relock issue nontty",
state: Lock,
opts: LockOptions{
SelectorArg: "451",
IssueNumber: 451,
ParentCmd: "issue",
Reason: "off_topic",
},
@ -409,7 +463,7 @@ func Test_runLock(t *testing.T) {
name: "lock pr nontty",
state: Lock,
opts: LockOptions{
SelectorArg: "451",
IssueNumber: 451,
ParentCmd: "pr",
},
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
@ -433,7 +487,7 @@ func Test_runLock(t *testing.T) {
tty: true,
opts: LockOptions{
Interactive: true,
SelectorArg: "451",
IssueNumber: 451,
ParentCmd: "pr",
},
state: Lock,
@ -469,7 +523,7 @@ func Test_runLock(t *testing.T) {
tty: true,
state: Lock,
opts: LockOptions{
SelectorArg: "451",
IssueNumber: 451,
ParentCmd: "pr",
Reason: "off_topic",
},
@ -495,7 +549,7 @@ func Test_runLock(t *testing.T) {
name: "lock pr with explicit nontty",
state: Lock,
opts: LockOptions{
SelectorArg: "451",
IssueNumber: 451,
ParentCmd: "pr",
Reason: "off_topic",
},
@ -520,7 +574,7 @@ func Test_runLock(t *testing.T) {
name: "unlock pr tty",
state: Unlock,
opts: LockOptions{
SelectorArg: "451",
IssueNumber: 451,
ParentCmd: "pr",
},
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
@ -546,7 +600,7 @@ func Test_runLock(t *testing.T) {
tty: true,
state: Unlock,
opts: LockOptions{
SelectorArg: "451",
IssueNumber: 451,
ParentCmd: "pr",
},
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
@ -572,7 +626,7 @@ func Test_runLock(t *testing.T) {
name: "relock pr tty",
state: Lock,
opts: LockOptions{
SelectorArg: "451",
IssueNumber: 451,
ParentCmd: "pr",
Reason: "off_topic",
},
@ -616,7 +670,7 @@ func Test_runLock(t *testing.T) {
name: "relock pr nontty",
state: Lock,
opts: LockOptions{
SelectorArg: "451",
IssueNumber: 451,
ParentCmd: "pr",
Reason: "off_topic",
},

27
pkg/cmd/issue/pin/pin.go
View file

@ -20,7 +20,7 @@ type PinOptions struct {
Config func() (gh.Config, error)
IO *iostreams.IOStreams
BaseRepo func() (ghrepo.Interface, error)
SelectorArg string
IssueNumber int
}
func NewCmdPin(f *cmdutil.Factory, runF func(*PinOptions) error) *cobra.Command {
@ -51,8 +51,22 @@ func NewCmdPin(f *cmdutil.Factory, runF func(*PinOptions) error) *cobra.Command
`),
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
opts.BaseRepo = f.BaseRepo
opts.SelectorArg = 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 runF != nil {
return runF(opts)
@ -73,7 +87,12 @@ func pinRun(opts *PinOptions) error {
return err
}
issue, baseRepo, err := shared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.SelectorArg, []string{"id", "number", "title", "isPinned"})
baseRepo, err := opts.BaseRepo()
if err != nil {
return err
}
issue, err := shared.FindIssueOrPR(httpClient, baseRepo, opts.IssueNumber, []string{"id", "number", "title", "isPinned"})
if err != nil {
return err
}

69
pkg/cmd/issue/pin/pin_test.go
View file

@ -1,80 +1,21 @@
package pin
import (
"bytes"
"net/http"
"testing"
"github.com/cli/cli/v2/internal/config"
"github.com/cli/cli/v2/internal/gh"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/cmd/issue/argparsetest"
"github.com/cli/cli/v2/pkg/httpmock"
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/google/shlex"
"github.com/stretchr/testify/assert"
)
func TestNewCmdPin(t *testing.T) {
tests := []struct {
name string
input string
output PinOptions
wantErr bool
errMsg string
}{
{
name: "no argument",
input: "",
wantErr: true,
errMsg: "accepts 1 arg(s), received 0",
},
{
name: "issue number",
input: "6",
output: PinOptions{
SelectorArg: "6",
},
},
{
name: "issue url",
input: "https://github.com/cli/cli/6",
output: PinOptions{
SelectorArg: "https://github.com/cli/cli/6",
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ios, _, _, _ := iostreams.Test()
ios.SetStdinTTY(true)
ios.SetStdoutTTY(true)
f := &cmdutil.Factory{
IOStreams: ios,
}
argv, err := shlex.Split(tt.input)
assert.NoError(t, err)
var gotOpts *PinOptions
cmd := NewCmdPin(f, func(opts *PinOptions) error {
gotOpts = opts
return nil
})
cmd.SetArgs(argv)
cmd.SetIn(&bytes.Buffer{})
cmd.SetOut(&bytes.Buffer{})
cmd.SetErr(&bytes.Buffer{})
_, err = cmd.ExecuteC()
if tt.wantErr {
assert.Error(t, err)
assert.Equal(t, tt.errMsg, err.Error())
return
}
assert.NoError(t, err)
assert.Equal(t, tt.output.SelectorArg, gotOpts.SelectorArg)
})
}
// Test shared parsing of issue number / URL.
argparsetest.TestArgParsing(t, NewCmdPin)
}
func TestPinRun(t *testing.T) {
@ -89,7 +30,7 @@ func TestPinRun(t *testing.T) {
{
name: "pin issue",
tty: true,
opts: &PinOptions{SelectorArg: "20"},
opts: &PinOptions{IssueNumber: 20},
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL(`query IssueByNumber\b`),
@ -113,7 +54,7 @@ func TestPinRun(t *testing.T) {
{
name: "issue already pinned",
tty: true,
opts: &PinOptions{SelectorArg: "20"},
opts: &PinOptions{IssueNumber: 20},
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL(`query IssueByNumber\b`),

29
pkg/cmd/issue/reopen/reopen.go
View file

@ -21,7 +21,7 @@ type ReopenOptions struct {
IO *iostreams.IOStreams
BaseRepo func() (ghrepo.Interface, error)
SelectorArg string
IssueNumber int
Comment string
}
@ -37,13 +37,23 @@ func NewCmdReopen(f *cmdutil.Factory, runF func(*ReopenOptions) error) *cobra.Co
Short: "Reopen issue",
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
// support `-R, --repo` override
opts.BaseRepo = f.BaseRepo
if len(args) > 0 {
opts.SelectorArg = 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 runF != nil {
return runF(opts)
}
@ -64,7 +74,12 @@ func reopenRun(opts *ReopenOptions) error {
return err
}
issue, baseRepo, err := shared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.SelectorArg, []string{"id", "number", "title", "state"})
baseRepo, err := opts.BaseRepo()
if err != nil {
return err
}
issue, err := shared.FindIssueOrPR(httpClient, baseRepo, opts.IssueNumber, []string{"id", "number", "title", "state"})
if err != nil {
return err
}

6
pkg/cmd/issue/reopen/reopen_test.go
View file

@ -10,6 +10,7 @@ import (
"github.com/cli/cli/v2/internal/config"
"github.com/cli/cli/v2/internal/gh"
"github.com/cli/cli/v2/internal/ghrepo"
"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"
@ -18,6 +19,11 @@ import (
"github.com/stretchr/testify/assert"
)
func TestNewCmdReopen(t *testing.T) {
// Test shared parsing of issue number / URL.
argparsetest.TestArgParsing(t, NewCmdReopen)
}
func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, error) {
ios, _, stdout, stderr := iostreams.Test()
ios.SetStdoutTTY(isTTY)

175
pkg/cmd/issue/shared/lookup.go
View file

@ -13,69 +13,104 @@ import (
"github.com/cli/cli/v2/api"
fd "github.com/cli/cli/v2/internal/featuredetection"
"github.com/cli/cli/v2/internal/ghrepo"
o "github.com/cli/cli/v2/pkg/option"
"github.com/cli/cli/v2/pkg/set"
"golang.org/x/sync/errgroup"
)
// IssueFromArgWithFields loads an issue or pull request with the specified fields. If some of the fields
// could not be fetched by GraphQL, this returns a non-nil issue and a *PartialLoadError.
func IssueFromArgWithFields(httpClient *http.Client, baseRepoFn func() (ghrepo.Interface, error), arg string, fields []string) (*api.Issue, ghrepo.Interface, error) {
issueNumber, baseRepo, err := IssueNumberAndRepoFromArg(arg)
if err != nil {
return nil, nil, err
}
var issueURLRE = regexp.MustCompile(`^/([^/]+)/([^/]+)/(?:issues|pull)/(\d+)`)
if baseRepo == nil {
var err error
if baseRepo, err = baseRepoFn(); err != nil {
return nil, nil, err
}
}
func ParseIssuesFromArgs(args []string) ([]int, o.Option[ghrepo.Interface], error) {
var repo o.Option[ghrepo.Interface]
issueNumbers := make([]int, len(args))
issue, err := findIssueOrPR(httpClient, baseRepo, issueNumber, fields)
return issue, baseRepo, err
}
// IssuesFromArgsWithFields loads 1 or more issues or pull requests with the specified fields. If some of the fields
// could not be fetched by GraphQL, this returns non-nil issues and a *PartialLoadError.
func IssuesFromArgsWithFields(httpClient *http.Client, baseRepoFn func() (ghrepo.Interface, error), args []string, fields []string) ([]*api.Issue, ghrepo.Interface, error) {
var issuesRepo ghrepo.Interface
issueNumbers := make([]int, 0, len(args))
for _, arg := range args {
issueNumber, baseRepo, err := IssueNumberAndRepoFromArg(arg)
for i, arg := range args {
// For each argument, parse the issue number and an optional repo
issueNumber, issueRepo, err := ParseIssueFromArg(arg)
if err != nil {
return nil, nil, err
return nil, o.None[ghrepo.Interface](), err
}
issueNumbers = append(issueNumbers, issueNumber)
if baseRepo == nil {
var err error
if baseRepo, err = baseRepoFn(); err != nil {
return nil, nil, err
// if this is our first issue repo found, then we need to set it
if repo.IsNone() {
repo = issueRepo
}
// if there is an issue repo returned, then we need to check if it is the same as the previous one
if issueRepo.IsSome() && repo.IsSome() {
// Unwraps are safe because we've checked for presence above
if !ghrepo.IsSame(repo.Unwrap(), issueRepo.Unwrap()) {
return nil, o.None[ghrepo.Interface](), fmt.Errorf(
"multiple issues must be in same repo: found %q, expected %q",
ghrepo.FullName(issueRepo.Unwrap()),
ghrepo.FullName(repo.Unwrap()),
)
}
}
if issuesRepo == nil {
issuesRepo = baseRepo
continue
}
if !ghrepo.IsSame(issuesRepo, baseRepo) {
return nil, nil, fmt.Errorf(
"multiple issues must be in same repo: found %q, expected %q",
ghrepo.FullName(baseRepo),
ghrepo.FullName(issuesRepo),
)
}
// add the issue number to the list
issueNumbers[i] = issueNumber
}
issuesChan := make(chan *api.Issue, len(args))
return issueNumbers, repo, nil
}
func ParseIssueFromArg(arg string) (int, o.Option[ghrepo.Interface], error) {
issueLocator := tryParseIssueFromURL(arg)
if issueLocator, present := issueLocator.Value(); present {
return issueLocator.issueNumber, o.Some(issueLocator.repo), nil
}
issueNumber, err := strconv.Atoi(strings.TrimPrefix(arg, "#"))
if err != nil {
return 0, o.None[ghrepo.Interface](), fmt.Errorf("invalid issue format: %q", arg)
}
return issueNumber, o.None[ghrepo.Interface](), nil
}
type issueLocator struct {
issueNumber int
repo ghrepo.Interface
}
// tryParseIssueFromURL tries to parse an issue number and repo from a URL.
func tryParseIssueFromURL(maybeURL string) o.Option[issueLocator] {
u, err := url.Parse(maybeURL)
if err != nil {
return o.None[issueLocator]()
}
if u.Scheme != "https" && u.Scheme != "http" {
return o.None[issueLocator]()
}
m := issueURLRE.FindStringSubmatch(u.Path)
if m == nil {
return o.None[issueLocator]()
}
repo := ghrepo.NewWithHost(m[1], m[2], u.Hostname())
issueNumber, _ := strconv.Atoi(m[3])
return o.Some(issueLocator{
issueNumber: issueNumber,
repo: repo,
})
}
type PartialLoadError struct {
error
}
// FindIssuesOrPRs loads 1 or more issues or pull requests with the specified fields. If some of the fields
// could not be fetched by GraphQL, this returns non-nil issues and a *PartialLoadError.
func FindIssuesOrPRs(httpClient *http.Client, repo ghrepo.Interface, issueNumbers []int, fields []string) ([]*api.Issue, error) {
issuesChan := make(chan *api.Issue, len(issueNumbers))
g := errgroup.Group{}
for _, num := range issueNumbers {
issueNumber := num
g.Go(func() error {
issue, err := findIssueOrPR(httpClient, issuesRepo, issueNumber, fields)
issue, err := FindIssueOrPR(httpClient, repo, issueNumber, fields)
if err != nil {
return err
}
@ -89,60 +124,18 @@ func IssuesFromArgsWithFields(httpClient *http.Client, baseRepoFn func() (ghrepo
close(issuesChan)
if err != nil {
return nil, nil, err
return nil, err
}
issues := make([]*api.Issue, 0, len(args))
issues := make([]*api.Issue, 0, len(issueNumbers))
for issue := range issuesChan {
issues = append(issues, issue)
}
return issues, issuesRepo, nil
return issues, nil
}
var issueURLRE = regexp.MustCompile(`^/([^/]+)/([^/]+)/(?:issues|pull)/(\d+)`)
func issueMetadataFromURL(s string) (int, ghrepo.Interface) {
u, err := url.Parse(s)
if err != nil {
return 0, nil
}
if u.Scheme != "https" && u.Scheme != "http" {
return 0, nil
}
m := issueURLRE.FindStringSubmatch(u.Path)
if m == nil {
return 0, nil
}
repo := ghrepo.NewWithHost(m[1], m[2], u.Hostname())
issueNumber, _ := strconv.Atoi(m[3])
return issueNumber, repo
}
// Returns the issue number and repo if the issue URL is provided.
// If only the issue number is provided, returns the number and nil repo.
func IssueNumberAndRepoFromArg(arg string) (int, ghrepo.Interface, error) {
issueNumber, baseRepo := issueMetadataFromURL(arg)
if issueNumber == 0 {
var err error
issueNumber, err = strconv.Atoi(strings.TrimPrefix(arg, "#"))
if err != nil {
return 0, nil, fmt.Errorf("invalid issue format: %q", arg)
}
}
return issueNumber, baseRepo, nil
}
type PartialLoadError struct {
error
}
func findIssueOrPR(httpClient *http.Client, repo ghrepo.Interface, number int, fields []string) (*api.Issue, error) {
func FindIssueOrPR(httpClient *http.Client, repo ghrepo.Interface, number int, fields []string) (*api.Issue, error) {
fieldSet := set.NewStringSet()
fieldSet.AddValues(fields)
if fieldSet.Contains("stateReason") {

326
pkg/cmd/issue/shared/lookup_test.go
View file

@ -2,265 +2,94 @@ package shared
import (
"net/http"
"strings"
"testing"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/httpmock"
o "github.com/cli/cli/v2/pkg/option"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestIssueFromArgWithFields(t *testing.T) {
type args struct {
baseRepoFn func() (ghrepo.Interface, error)
selector string
}
func TestParseIssuesFromArgs(t *testing.T) {
tests := []struct {
name string
args args
httpStub func(*httpmock.Registry)
wantIssue int
wantRepo string
wantProjects string
wantErr bool
behavior string
args []string
expectedIssueNumbers []int
expectedRepo o.Option[ghrepo.Interface]
expectedErr bool
}{
{
name: "number argument",
args: args{
selector: "13",
baseRepoFn: func() (ghrepo.Interface, error) {
return ghrepo.FromFullName("OWNER/REPO")
},
},
httpStub: func(r *httpmock.Registry) {
r.Register(
httpmock.GraphQL(`query IssueByNumber\b`),
httpmock.StringResponse(`{"data":{"repository":{
"hasIssuesEnabled": true,
"issue":{"number":13}
}}}`))
},
wantIssue: 13,
wantRepo: "https://github.com/OWNER/REPO",
behavior: "when given issue numbers, returns them with no repo",
args: []string{"1", "2"},
expectedIssueNumbers: []int{1, 2},
expectedRepo: o.None[ghrepo.Interface](),
},
{
name: "number with hash argument",
args: args{
selector: "#13",
baseRepoFn: func() (ghrepo.Interface, error) {
return ghrepo.FromFullName("OWNER/REPO")
},
},
httpStub: func(r *httpmock.Registry) {
r.Register(
httpmock.GraphQL(`query IssueByNumber\b`),
httpmock.StringResponse(`{"data":{"repository":{
"hasIssuesEnabled": true,
"issue":{"number":13}
}}}`))
},
wantIssue: 13,
wantRepo: "https://github.com/OWNER/REPO",
behavior: "when given # prefixed issue numbers, returns them with no repo",
args: []string{"#1", "#2"},
expectedIssueNumbers: []int{1, 2},
expectedRepo: o.None[ghrepo.Interface](),
},
{
name: "URL argument",
args: args{
selector: "https://example.org/OWNER/REPO/issues/13#comment-123",
baseRepoFn: nil,
behavior: "when given URLs, returns them with the repo",
args: []string{
"https://github.com/OWNER/REPO/issues/1",
"https://github.com/OWNER/REPO/issues/2",
},
httpStub: func(r *httpmock.Registry) {
r.Register(
httpmock.GraphQL(`query IssueByNumber\b`),
httpmock.StringResponse(`{"data":{"repository":{
"hasIssuesEnabled": true,
"issue":{"number":13}
}}}`))
},
wantIssue: 13,
wantRepo: "https://example.org/OWNER/REPO",
expectedIssueNumbers: []int{1, 2},
expectedRepo: o.Some(ghrepo.New("OWNER", "REPO")),
},
{
name: "PR URL argument",
args: args{
selector: "https://example.org/OWNER/REPO/pull/13#comment-123",
baseRepoFn: nil,
behavior: "when given URLs in different repos, errors",
args: []string{
"https://github.com/OWNER/REPO/issues/1",
"https://github.com/OWNER/OTHERREPO/issues/2",
},
httpStub: func(r *httpmock.Registry) {
r.Register(
httpmock.GraphQL(`query IssueByNumber\b`),
httpmock.StringResponse(`{"data":{"repository":{
"hasIssuesEnabled": true,
"issue":{"number":13}
}}}`))
},
wantIssue: 13,
wantRepo: "https://example.org/OWNER/REPO",
expectedErr: true,
},
{
name: "project cards permission issue",
args: args{
selector: "https://example.org/OWNER/REPO/issues/13",
baseRepoFn: nil,
},
httpStub: func(r *httpmock.Registry) {
r.Register(
httpmock.GraphQL(`query IssueByNumber\b`),
httpmock.StringResponse(`
{
"data": {
"repository": {
"hasIssuesEnabled": true,
"issue": {
"number": 13,
"projectCards": {
"nodes": [
null,
{
"project": {"name": "myproject"},
"column": {"name": "To Do"}
},
null,
{
"project": {"name": "other project"},
"column": null
}
]
}
}
}
},
"errors": [
{
"type": "FORBIDDEN",
"message": "Resource not accessible by integration",
"path": ["repository", "issue", "projectCards", "nodes", 0]
},
{
"type": "FORBIDDEN",
"message": "Resource not accessible by integration",
"path": ["repository", "issue", "projectCards", "nodes", 2]
}
]
}`))
},
wantErr: true,
wantIssue: 13,
wantProjects: "myproject, other project",
wantRepo: "https://example.org/OWNER/REPO",
behavior: "when given an unparseable argument, errors",
args: []string{"://"},
expectedErr: true,
},
{
name: "projects permission issue",
args: args{
selector: "https://example.org/OWNER/REPO/issues/13",
baseRepoFn: nil,
},
httpStub: func(r *httpmock.Registry) {
r.Register(
httpmock.GraphQL(`query IssueByNumber\b`),
httpmock.StringResponse(`
{
"data": {
"repository": {
"hasIssuesEnabled": true,
"issue": {
"number": 13,
"projectCards": {
"nodes": null,
"totalCount": 0
}
}
}
},
"errors": [
{
"type": "FORBIDDEN",
"message": "Resource not accessible by integration",
"path": ["repository", "issue", "projectCards", "nodes"]
}
]
}`))
},
wantErr: true,
wantIssue: 13,
wantProjects: "",
wantRepo: "https://example.org/OWNER/REPO",
behavior: "when given a URL that isn't an issue or PR url, errors",
args: []string{"https://github.com"},
expectedErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
reg := &httpmock.Registry{}
if tt.httpStub != nil {
tt.httpStub(reg)
}
httpClient := &http.Client{Transport: reg}
issue, repo, err := IssueFromArgWithFields(httpClient, tt.args.baseRepoFn, tt.args.selector, []string{"number"})
if (err != nil) != tt.wantErr {
t.Errorf("IssueFromArgWithFields() error = %v, wantErr %v", err, tt.wantErr)
if issue == nil {
return
}
}
if issue.Number != tt.wantIssue {
t.Errorf("want issue #%d, got #%d", tt.wantIssue, issue.Number)
}
if gotProjects := strings.Join(issue.ProjectCards.ProjectNames(), ", "); gotProjects != tt.wantProjects {
t.Errorf("want projects %q, got %q", tt.wantProjects, gotProjects)
}
repoURL := ghrepo.GenerateRepoURL(repo, "")
if repoURL != tt.wantRepo {
t.Errorf("want repo %s, got %s", tt.wantRepo, repoURL)
for _, tc := range tests {
t.Run(tc.behavior, func(t *testing.T) {
issueNumbers, repo, err := ParseIssuesFromArgs(tc.args)
if tc.expectedErr {
require.Error(t, err)
return
}
require.NoError(t, err)
assert.Equal(t, tc.expectedIssueNumbers, issueNumbers)
assert.Equal(t, tc.expectedRepo, repo)
})
}
}
func TestIssuesFromArgsWithFields(t *testing.T) {
type args struct {
baseRepoFn func() (ghrepo.Interface, error)
selectors []string
}
func TestFindIssuesOrPRs(t *testing.T) {
tests := []struct {
name string
args args
httpStub func(*httpmock.Registry)
wantIssues []int
wantRepo string
wantErr bool
wantErrMsg string
name string
issueNumbers []int
baseRepo ghrepo.Interface
httpStub func(*httpmock.Registry)
wantIssueNumbers []int
wantErr bool
}{
{
name: "multiple repos",
args: args{
selectors: []string{"1", "https://github.com/OWNER/OTHERREPO/issues/2"},
baseRepoFn: func() (ghrepo.Interface, error) {
return ghrepo.New("OWNER", "REPO"), nil
},
},
httpStub: func(r *httpmock.Registry) {
r.Register(
httpmock.GraphQL(`query IssueByNumber\b`),
httpmock.StringResponse(`{"data":{"repository":{
"hasIssuesEnabled": true,
"issue":{"number":1}
}}}`))
r.Register(
httpmock.GraphQL(`query IssueByNumber\b`),
httpmock.StringResponse(`{"data":{"repository":{
"hasIssuesEnabled": true,
"issue":{"number":2}
}}}`))
},
wantErr: true,
wantErrMsg: "multiple issues must be in same repo",
},
{
name: "multiple issues",
args: args{
selectors: []string{"1", "2"},
baseRepoFn: func() (ghrepo.Interface, error) {
return ghrepo.New("OWNER", "REPO"), nil
},
},
name: "multiple issues",
issueNumbers: []int{1, 2},
baseRepo: ghrepo.New("OWNER", "REPO"),
httpStub: func(r *httpmock.Registry) {
r.Register(
httpmock.GraphQL(`query IssueByNumber\b`),
@ -275,43 +104,48 @@ func TestIssuesFromArgsWithFields(t *testing.T) {
"issue":{"number":2}
}}}`))
},
wantIssues: []int{1, 2},
wantRepo: "https://github.com/OWNER/REPO",
wantIssueNumbers: []int{1, 2},
},
{
name: "any find error results in total error",
issueNumbers: []int{1, 2},
baseRepo: ghrepo.New("OWNER", "REPO"),
httpStub: func(r *httpmock.Registry) {
r.Register(
httpmock.GraphQL(`query IssueByNumber\b`),
httpmock.StringResponse(`{"data":{"repository":{
"hasIssuesEnabled": true,
"issue":{"number":1}
}}}`))
r.Register(
httpmock.GraphQL(`query IssueByNumber\b`),
httpmock.StatusStringResponse(500, "internal server error"))
},
wantErr: true,
},
}
for _, tt := range tests {
if !tt.wantErr && len(tt.args.selectors) != len(tt.wantIssues) {
t.Fatal("number of selectors and issues not equal")
}
t.Run(tt.name, func(t *testing.T) {
reg := &httpmock.Registry{}
if tt.httpStub != nil {
tt.httpStub(reg)
}
httpClient := &http.Client{Transport: reg}
issues, repo, err := IssuesFromArgsWithFields(httpClient, tt.args.baseRepoFn, tt.args.selectors, []string{"number"})
issues, err := FindIssuesOrPRs(httpClient, tt.baseRepo, tt.issueNumbers, []string{"number"})
if (err != nil) != tt.wantErr {
t.Errorf("IssuesFromArgsWithFields() error = %v, wantErr %v", err, tt.wantErr)
t.Errorf("FindIssuesOrPRs() error = %v, wantErr %v", err, tt.wantErr)
if issues == nil {
return
}
}
if tt.wantErr {
assert.Error(t, err)
assert.ErrorContains(t, err, tt.wantErrMsg)
require.Error(t, err)
return
}
assert.NoError(t, err)
require.NoError(t, err)
for i := range issues {
assert.Contains(t, tt.wantIssues, issues[i].Number)
}
if repo != nil {
repoURL := ghrepo.GenerateRepoURL(repo, "")
if repoURL != tt.wantRepo {
t.Errorf("want repo %s, got %s", tt.wantRepo, repoURL)
}
} else if tt.wantRepo != "" {
t.Errorf("want repo %sw, got nil", tt.wantRepo)
assert.Contains(t, tt.wantIssueNumbers, issues[i].Number)
}
})
}

28
pkg/cmd/issue/transfer/transfer.go
View file

@ -20,7 +20,7 @@ type TransferOptions struct {
IO *iostreams.IOStreams
BaseRepo func() (ghrepo.Interface, error)
IssueSelector string
IssueNumber int
DestRepoSelector string
}
@ -36,8 +36,23 @@ func NewCmdTransfer(f *cmdutil.Factory, runF func(*TransferOptions) error) *cobr
Short: "Transfer issue to another repository",
Args: cmdutil.ExactArgs(2, "issue and destination repository are required"),
RunE: func(cmd *cobra.Command, args []string) error {
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
opts.DestRepoSelector = args[1]
if runF != nil {
@ -57,7 +72,12 @@ func transferRun(opts *TransferOptions) error {
return err
}
issue, baseRepo, err := shared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.IssueSelector, []string{"id", "number"})
baseRepo, err := opts.BaseRepo()
if err != nil {
return err
}
issue, err := shared.FindIssueOrPR(httpClient, baseRepo, opts.IssueNumber, []string{"id", "number"})
if err != nil {
return err
}

64
pkg/cmd/issue/transfer/transfer_test.go
View file

@ -15,6 +15,7 @@ import (
"github.com/cli/cli/v2/test"
"github.com/google/shlex"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func runCommand(rt http.RoundTripper, cli string) (*test.CmdOut, error) {
@ -57,18 +58,49 @@ func runCommand(rt http.RoundTripper, cli string) (*test.CmdOut, error) {
func TestNewCmdTransfer(t *testing.T) {
tests := []struct {
name string
cli string
wants TransferOptions
wantErr string
name string
cli string
wants TransferOptions
wantBaseRepo ghrepo.Interface
wantErr bool
}{
{
name: "issue name",
cli: "3252 OWNER/REPO",
name: "no argument",
cli: "",
wantErr: true,
},
{
name: "issue number argument",
cli: "--repo cli/repo 23 OWNER/REPO",
wants: TransferOptions{
IssueSelector: "3252",
IssueNumber: 23,
DestRepoSelector: "OWNER/REPO",
},
wantBaseRepo: ghrepo.New("cli", "repo"),
},
{
name: "argument is hash prefixed number",
// Escaping is required here to avoid what I think is shellex treating it as a comment.
cli: "--repo cli/repo \\#23 OWNER/REPO",
wants: TransferOptions{
IssueNumber: 23,
DestRepoSelector: "OWNER/REPO",
},
wantBaseRepo: ghrepo.New("cli", "repo"),
},
{
name: "argument is a URL",
cli: "https://github.com/cli/cli/issues/23 OWNER/REPO",
wants: TransferOptions{
IssueNumber: 23,
DestRepoSelector: "OWNER/REPO",
},
wantBaseRepo: ghrepo.New("cli", "cli"),
},
{
name: "argument cannot be parsed to an issue",
cli: "unparseable OWNER/REPO",
wantErr: true,
},
}
@ -84,15 +116,29 @@ func TestNewCmdTransfer(t *testing.T) {
gotOpts = opts
return nil
})
cmdutil.EnableRepoOverride(cmd, f)
cmd.SetArgs(argv)
cmd.SetIn(&bytes.Buffer{})
cmd.SetOut(&bytes.Buffer{})
cmd.SetErr(&bytes.Buffer{})
_, cErr := cmd.ExecuteC()
assert.NoError(t, cErr)
assert.Equal(t, tt.wants.IssueSelector, gotOpts.IssueSelector)
if tt.wantErr {
require.Error(t, cErr)
return
}
require.NoError(t, cErr)
assert.Equal(t, tt.wants.IssueNumber, gotOpts.IssueNumber)
assert.Equal(t, tt.wants.DestRepoSelector, gotOpts.DestRepoSelector)
actualBaseRepo, err := gotOpts.BaseRepo()
require.NoError(t, err)
assert.True(
t,
ghrepo.IsSame(tt.wantBaseRepo, actualBaseRepo),
"expected base repo %+v, got %+v", tt.wantBaseRepo, actualBaseRepo,
)
})
}
}

36
pkg/cmd/issue/unpin/unpin.go
View file

@ -16,11 +16,12 @@ import (
)
type UnpinOptions struct {
HttpClient func() (*http.Client, error)
Config func() (gh.Config, error)
IO *iostreams.IOStreams
BaseRepo func() (ghrepo.Interface, error)
SelectorArg string
HttpClient func() (*http.Client, error)
Config func() (gh.Config, error)
IO *iostreams.IOStreams
BaseRepo func() (ghrepo.Interface, error)
IssueNumber int
}
func NewCmdUnpin(f *cmdutil.Factory, runF func(*UnpinOptions) error) *cobra.Command {
@ -51,8 +52,22 @@ func NewCmdUnpin(f *cmdutil.Factory, runF func(*UnpinOptions) error) *cobra.Comm
`),
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
opts.BaseRepo = f.BaseRepo
opts.SelectorArg = 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 runF != nil {
return runF(opts)
@ -73,7 +88,12 @@ func unpinRun(opts *UnpinOptions) error {
return err
}
issue, baseRepo, err := shared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.SelectorArg, []string{"id", "number", "title", "isPinned"})
baseRepo, err := opts.BaseRepo()
if err != nil {
return err
}
issue, err := shared.FindIssueOrPR(httpClient, baseRepo, opts.IssueNumber, []string{"id", "number", "title", "isPinned"})
if err != nil {
return err
}

71
pkg/cmd/issue/unpin/unpin_test.go
View file

@ -1,80 +1,21 @@
package unpin
import (
"bytes"
"net/http"
"testing"
"github.com/cli/cli/v2/internal/config"
"github.com/cli/cli/v2/internal/gh"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/cmd/issue/argparsetest"
"github.com/cli/cli/v2/pkg/httpmock"
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/google/shlex"
"github.com/stretchr/testify/assert"
)
func TestNewCmdPin(t *testing.T) {
tests := []struct {
name string
input string
output UnpinOptions
wantErr bool
errMsg string
}{
{
name: "no argument",
input: "",
wantErr: true,
errMsg: "accepts 1 arg(s), received 0",
},
{
name: "issue number",
input: "6",
output: UnpinOptions{
SelectorArg: "6",
},
},
{
name: "issue url",
input: "https://github.com/cli/cli/6",
output: UnpinOptions{
SelectorArg: "https://github.com/cli/cli/6",
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ios, _, _, _ := iostreams.Test()
ios.SetStdinTTY(true)
ios.SetStdoutTTY(true)
f := &cmdutil.Factory{
IOStreams: ios,
}
argv, err := shlex.Split(tt.input)
assert.NoError(t, err)
var gotOpts *UnpinOptions
cmd := NewCmdUnpin(f, func(opts *UnpinOptions) error {
gotOpts = opts
return nil
})
cmd.SetArgs(argv)
cmd.SetIn(&bytes.Buffer{})
cmd.SetOut(&bytes.Buffer{})
cmd.SetErr(&bytes.Buffer{})
_, err = cmd.ExecuteC()
if tt.wantErr {
assert.Error(t, err)
assert.Equal(t, tt.errMsg, err.Error())
return
}
assert.NoError(t, err)
assert.Equal(t, tt.output.SelectorArg, gotOpts.SelectorArg)
})
}
func TestNewCmdUnpin(t *testing.T) {
// Test shared parsing of issue number / URL.
argparsetest.TestArgParsing(t, NewCmdUnpin)
}
func TestUnpinRun(t *testing.T) {
@ -89,7 +30,7 @@ func TestUnpinRun(t *testing.T) {
{
name: "unpin issue",
tty: true,
opts: &UnpinOptions{SelectorArg: "20"},
opts: &UnpinOptions{IssueNumber: 20},
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL(`query IssueByNumber\b`),
@ -113,7 +54,7 @@ func TestUnpinRun(t *testing.T) {
{
name: "issue not pinned",
tty: true,
opts: &UnpinOptions{SelectorArg: "20"},
opts: &UnpinOptions{IssueNumber: 20},
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL(`query IssueByNumber\b`),

59
pkg/cmd/issue/view/view.go
View file

@ -14,6 +14,7 @@ import (
"github.com/cli/cli/v2/internal/browser"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/text"
"github.com/cli/cli/v2/pkg/cmd/issue/shared"
issueShared "github.com/cli/cli/v2/pkg/cmd/issue/shared"
prShared "github.com/cli/cli/v2/pkg/cmd/pr/shared"
"github.com/cli/cli/v2/pkg/cmdutil"
@ -29,7 +30,7 @@ type ViewOptions struct {
BaseRepo func() (ghrepo.Interface, error)
Browser browser.Browser
SelectorArg string
IssueNumber int
WebMode bool
Comments bool
Exporter cmdutil.Exporter
@ -55,13 +56,23 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman
`, "`"),
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
// support `-R, --repo` override
opts.BaseRepo = f.BaseRepo
if len(args) > 0 {
opts.SelectorArg = 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 runF != nil {
return runF(opts)
}
@ -87,6 +98,11 @@ func viewRun(opts *ViewOptions) error {
return err
}
baseRepo, err := opts.BaseRepo()
if err != nil {
return err
}
lookupFields := set.NewStringSet()
if opts.Exporter != nil {
lookupFields.AddValues(opts.Exporter.Fields())
@ -103,7 +119,18 @@ func viewRun(opts *ViewOptions) error {
opts.IO.DetectTerminalTheme()
opts.IO.StartProgressIndicator()
issue, baseRepo, err := findIssue(httpClient, opts.BaseRepo, opts.SelectorArg, lookupFields.ToSlice())
lookupFields.Add("id")
issue, err := issueShared.FindIssueOrPR(httpClient, baseRepo, opts.IssueNumber, lookupFields.ToSlice())
if err != nil {
return err
}
if lookupFields.Contains("comments") {
// FIXME: this re-fetches the comments connection even though the initial set of 100 were
// fetched in the previous request.
err = preloadIssueComments(httpClient, baseRepo, issue)
}
opts.IO.StopProgressIndicator()
if err != nil {
var loadErr *issueShared.PartialLoadError
@ -143,24 +170,6 @@ func viewRun(opts *ViewOptions) error {
return printRawIssuePreview(opts.IO.Out, issue)
}
func findIssue(client *http.Client, baseRepoFn func() (ghrepo.Interface, error), selector string, fields []string) (*api.Issue, ghrepo.Interface, error) {
fieldSet := set.NewStringSet()
fieldSet.AddValues(fields)
fieldSet.Add("id")
issue, repo, err := issueShared.IssueFromArgWithFields(client, baseRepoFn, selector, fieldSet.ToSlice())
if err != nil {
return issue, repo, err
}
if fieldSet.Contains("comments") {
// FIXME: this re-fetches the comments connection even though the initial set of 100 were
// fetched in the previous request.
err = preloadIssueComments(client, repo, issue)
}
return issue, repo, err
}
func printRawIssuePreview(out io.Writer, issue *api.Issue) error {
assignees := issueAssigneeList(*issue)
labels := issueLabelList(issue, nil)

10
pkg/cmd/issue/view/view_test.go
View file

@ -13,6 +13,7 @@ import (
"github.com/cli/cli/v2/internal/gh"
"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"
@ -47,6 +48,11 @@ func TestJSONFields(t *testing.T) {
})
}
func TestNewCmdView(t *testing.T) {
// Test shared parsing of issue number / URL.
argparsetest.TestArgParsing(t, NewCmdView)
}
func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, error) {
ios, _, stdout, stderr := iostreams.Test()
ios.SetStdoutTTY(isTTY)
@ -116,7 +122,7 @@ func TestIssueView_web(t *testing.T) {
return ghrepo.New("OWNER", "REPO"), nil
},
WebMode: true,
SelectorArg: "123",
IssueNumber: 123,
})
if err != nil {
t.Errorf("error running command `issue view`: %v", err)
@ -273,7 +279,7 @@ func TestIssueView_tty_Preview(t *testing.T) {
BaseRepo: func() (ghrepo.Interface, error) {
return ghrepo.New("OWNER", "REPO"), nil
},
SelectorArg: "123",
IssueNumber: 123,
}
err := viewRun(&opts)