diff --git a/pkg/cmd/pr/diff/diff.go b/pkg/cmd/pr/diff/diff.go index 108041008..c6902e5b5 100644 --- a/pkg/cmd/pr/diff/diff.go +++ b/pkg/cmd/pr/diff/diff.go @@ -2,6 +2,7 @@ package diff import ( "bufio" + "errors" "fmt" "io" "net/http" @@ -46,6 +47,10 @@ func NewCmdDiff(f *cmdutil.Factory, runF func(*DiffOptions) error) *cobra.Comman // support `-R, --repo` override opts.BaseRepo = f.BaseRepo + if repoOverride, _ := cmd.Flags().GetString("repo"); repoOverride != "" && len(args) == 0 { + return &cmdutil.FlagError{Err: errors.New("argument required when using the --repo flag")} + } + if len(args) > 0 { opts.SelectorArg = args[0] } diff --git a/pkg/cmd/pr/diff/diff_test.go b/pkg/cmd/pr/diff/diff_test.go index 3f52435a9..dc93970b6 100644 --- a/pkg/cmd/pr/diff/diff_test.go +++ b/pkg/cmd/pr/diff/diff_test.go @@ -19,8 +19,97 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/shlex" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +func Test_NewCmdDiff(t *testing.T) { + tests := []struct { + name string + args string + isTTY bool + want DiffOptions + wantErr string + }{ + { + name: "number argument", + args: "123", + isTTY: true, + want: DiffOptions{ + SelectorArg: "123", + UseColor: "auto", + }, + }, + { + name: "no argument", + args: "", + isTTY: true, + want: DiffOptions{ + SelectorArg: "", + UseColor: "auto", + }, + }, + { + name: "no color when redirected", + args: "", + isTTY: false, + want: DiffOptions{ + SelectorArg: "", + UseColor: "never", + }, + }, + { + name: "no argument with --repo override", + args: "-R owner/repo", + isTTY: true, + wantErr: "argument required when using the --repo flag", + }, + { + name: "invalid --color argument", + args: "--color doublerainbow", + isTTY: true, + wantErr: `did not understand color: "doublerainbow". Expected one of always, never, or auto`, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + io, _, _, _ := iostreams.Test() + io.SetStdoutTTY(tt.isTTY) + io.SetStdinTTY(tt.isTTY) + io.SetStderrTTY(tt.isTTY) + + f := &cmdutil.Factory{ + IOStreams: io, + } + + var opts *DiffOptions + cmd := NewCmdDiff(f, func(o *DiffOptions) error { + opts = o + return nil + }) + cmd.PersistentFlags().StringP("repo", "R", "", "") + + argv, err := shlex.Split(tt.args) + require.NoError(t, err) + cmd.SetArgs(argv) + + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(ioutil.Discard) + cmd.SetErr(ioutil.Discard) + + _, err = cmd.ExecuteC() + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + return + } else { + require.NoError(t, err) + } + + assert.Equal(t, tt.want.SelectorArg, opts.SelectorArg) + assert.Equal(t, tt.want.UseColor, opts.UseColor) + }) + } +} + func runCommand(rt http.RoundTripper, remotes context.Remotes, isTTY bool, cli string) (*test.CmdOut, error) { io, _, stdout, stderr := iostreams.Test() io.SetStdoutTTY(isTTY) @@ -74,14 +163,6 @@ func runCommand(rt http.RoundTripper, remotes context.Remotes, isTTY bool, cli s }, err } -func TestPRDiff_validation(t *testing.T) { - _, err := runCommand(nil, nil, false, "--color=doublerainbow") - if err == nil { - t.Fatal("expected error") - } - assert.Equal(t, `did not understand color: "doublerainbow". Expected one of always, never, or auto`, err.Error()) -} - func TestPRDiff_no_current_pr(t *testing.T) { http := &httpmock.Registry{} defer http.Verify(t) diff --git a/pkg/cmd/pr/merge/merge.go b/pkg/cmd/pr/merge/merge.go index b3c20b208..94e0c959c 100644 --- a/pkg/cmd/pr/merge/merge.go +++ b/pkg/cmd/pr/merge/merge.go @@ -64,6 +64,10 @@ func NewCmdMerge(f *cmdutil.Factory, runF func(*MergeOptions) error) *cobra.Comm // support `-R, --repo` override opts.BaseRepo = f.BaseRepo + if repoOverride, _ := cmd.Flags().GetString("repo"); repoOverride != "" && len(args) == 0 { + return &cmdutil.FlagError{Err: errors.New("argument required when using the --repo flag")} + } + if len(args) > 0 { opts.SelectorArg = args[0] } diff --git a/pkg/cmd/pr/merge/merge_test.go b/pkg/cmd/pr/merge/merge_test.go index b479e36a9..2b5f89e7b 100644 --- a/pkg/cmd/pr/merge/merge_test.go +++ b/pkg/cmd/pr/merge/merge_test.go @@ -20,8 +20,97 @@ import ( "github.com/cli/cli/test" "github.com/google/shlex" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +func Test_NewCmdMerge(t *testing.T) { + tests := []struct { + name string + args string + isTTY bool + want MergeOptions + wantErr string + }{ + { + name: "number argument", + args: "123", + isTTY: true, + want: MergeOptions{ + SelectorArg: "123", + DeleteBranch: true, + DeleteLocalBranch: true, + MergeMethod: api.PullRequestMergeMethodMerge, + InteractiveMode: true, + }, + }, + { + name: "no argument with --repo override", + args: "-R owner/repo", + isTTY: true, + wantErr: "argument required when using the --repo flag", + }, + { + name: "insufficient flags in non-interactive mode", + args: "123", + isTTY: false, + wantErr: "--merge, --rebase, or --squash required when not attached to a terminal", + }, + { + name: "multiple merge methods", + args: "123 --merge --rebase", + isTTY: true, + wantErr: "only one of --merge, --rebase, or --squash can be enabled", + }, + { + name: "multiple merge methods, non-tty", + args: "123 --merge --rebase", + isTTY: false, + wantErr: "only one of --merge, --rebase, or --squash can be enabled", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + io, _, _, _ := iostreams.Test() + io.SetStdoutTTY(tt.isTTY) + io.SetStdinTTY(tt.isTTY) + io.SetStderrTTY(tt.isTTY) + + f := &cmdutil.Factory{ + IOStreams: io, + } + + var opts *MergeOptions + cmd := NewCmdMerge(f, func(o *MergeOptions) error { + opts = o + return nil + }) + cmd.PersistentFlags().StringP("repo", "R", "", "") + + argv, err := shlex.Split(tt.args) + require.NoError(t, err) + cmd.SetArgs(argv) + + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(ioutil.Discard) + cmd.SetErr(ioutil.Discard) + + _, err = cmd.ExecuteC() + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + return + } else { + require.NoError(t, err) + } + + assert.Equal(t, tt.want.SelectorArg, opts.SelectorArg) + assert.Equal(t, tt.want.DeleteBranch, opts.DeleteBranch) + assert.Equal(t, tt.want.DeleteLocalBranch, opts.DeleteLocalBranch) + assert.Equal(t, tt.want.MergeMethod, opts.MergeMethod) + assert.Equal(t, tt.want.InteractiveMode, opts.InteractiveMode) + }) + } +} + func runCommand(rt http.RoundTripper, branch string, isTTY bool, cli string) (*test.CmdOut, error) { io, _, stdout, stderr := iostreams.Test() io.SetStdoutTTY(isTTY) @@ -168,16 +257,6 @@ func TestPrMerge_nontty(t *testing.T) { assert.Equal(t, "", output.Stderr()) } -func TestPrMerge_nontty_insufficient_flags(t *testing.T) { - output, err := runCommand(nil, "master", false, "pr merge 1") - if err == nil { - t.Fatal("expected error") - } - - assert.Equal(t, "--merge, --rebase, or --squash required when not attached to a terminal", err.Error()) - assert.Equal(t, "", output.String()) -} - func TestPrMerge_withRepoFlag(t *testing.T) { http := initFakeHTTP() defer http.Verify(t) @@ -489,17 +568,3 @@ func TestPRMerge_interactive(t *testing.T) { test.ExpectLines(t, output.Stderr(), "Merged pull request #3", `Deleted branch.*blueberries`) } - -func TestPrMerge_multipleMergeMethods(t *testing.T) { - _, err := runCommand(nil, "master", true, "1 --merge --squash") - if err == nil { - t.Fatal("expected error running `pr merge` with multiple merge methods") - } -} - -func TestPrMerge_multipleMergeMethods_nontty(t *testing.T) { - _, err := runCommand(nil, "master", false, "1 --merge --squash") - if err == nil { - t.Fatal("expected error running `pr merge` with multiple merge methods") - } -} diff --git a/pkg/cmd/pr/ready/ready.go b/pkg/cmd/pr/ready/ready.go index c5a248f19..933dec082 100644 --- a/pkg/cmd/pr/ready/ready.go +++ b/pkg/cmd/pr/ready/ready.go @@ -1,6 +1,7 @@ package ready import ( + "errors" "fmt" "net/http" @@ -43,6 +44,10 @@ func NewCmdReady(f *cmdutil.Factory, runF func(*ReadyOptions) error) *cobra.Comm // support `-R, --repo` override opts.BaseRepo = f.BaseRepo + if repoOverride, _ := cmd.Flags().GetString("repo"); repoOverride != "" && len(args) == 0 { + return &cmdutil.FlagError{Err: errors.New("argument required when using the --repo flag")} + } + if len(args) > 0 { opts.SelectorArg = args[0] } diff --git a/pkg/cmd/pr/ready/ready_test.go b/pkg/cmd/pr/ready/ready_test.go index 8f5df5832..4dc5d9eee 100644 --- a/pkg/cmd/pr/ready/ready_test.go +++ b/pkg/cmd/pr/ready/ready_test.go @@ -16,8 +16,80 @@ import ( "github.com/cli/cli/pkg/iostreams" "github.com/cli/cli/test" "github.com/google/shlex" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +func Test_NewCmdReady(t *testing.T) { + tests := []struct { + name string + args string + isTTY bool + want ReadyOptions + wantErr string + }{ + { + name: "number argument", + args: "123", + isTTY: true, + want: ReadyOptions{ + SelectorArg: "123", + }, + }, + { + name: "no argument", + args: "", + isTTY: true, + want: ReadyOptions{ + SelectorArg: "", + }, + }, + { + name: "no argument with --repo override", + args: "-R owner/repo", + isTTY: true, + wantErr: "argument required when using the --repo flag", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + io, _, _, _ := iostreams.Test() + io.SetStdoutTTY(tt.isTTY) + io.SetStdinTTY(tt.isTTY) + io.SetStderrTTY(tt.isTTY) + + f := &cmdutil.Factory{ + IOStreams: io, + } + + var opts *ReadyOptions + cmd := NewCmdReady(f, func(o *ReadyOptions) error { + opts = o + return nil + }) + cmd.PersistentFlags().StringP("repo", "R", "", "") + + argv, err := shlex.Split(tt.args) + require.NoError(t, err) + cmd.SetArgs(argv) + + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(ioutil.Discard) + cmd.SetErr(ioutil.Discard) + + _, err = cmd.ExecuteC() + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + return + } else { + require.NoError(t, err) + } + + assert.Equal(t, tt.want.SelectorArg, opts.SelectorArg) + }) + } +} + func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, error) { io, _, stdout, stderr := iostreams.Test() io.SetStdoutTTY(isTTY) diff --git a/pkg/cmd/pr/review/review.go b/pkg/cmd/pr/review/review.go index dba5dff10..19f33ca84 100644 --- a/pkg/cmd/pr/review/review.go +++ b/pkg/cmd/pr/review/review.go @@ -28,11 +28,10 @@ type ReviewOptions struct { Remotes func() (context.Remotes, error) Branch func() (string, error) - SelectorArg string - Approve bool - RequestChanges bool - Comment bool - Body string + SelectorArg string + InteractiveMode bool + ReviewType api.PullRequestReviewState + Body string } func NewCmdReview(f *cmdutil.Factory, runF func(*ReviewOptions) error) *cobra.Command { @@ -44,6 +43,12 @@ func NewCmdReview(f *cmdutil.Factory, runF func(*ReviewOptions) error) *cobra.Co Branch: f.Branch, } + var ( + flagApprove bool + flagRequestChanges bool + flagComment bool + ) + cmd := &cobra.Command{ Use: "review [ | | ]", Short: "Add a review to a pull request", @@ -70,10 +75,45 @@ func NewCmdReview(f *cmdutil.Factory, runF func(*ReviewOptions) error) *cobra.Co // support `-R, --repo` override opts.BaseRepo = f.BaseRepo + if repoOverride, _ := cmd.Flags().GetString("repo"); repoOverride != "" && len(args) == 0 { + return &cmdutil.FlagError{Err: errors.New("argument required when using the --repo flag")} + } + if len(args) > 0 { opts.SelectorArg = args[0] } + found := 0 + if flagApprove { + found++ + opts.ReviewType = api.ReviewApprove + } + if flagRequestChanges { + found++ + opts.ReviewType = api.ReviewRequestChanges + if opts.Body == "" { + return &cmdutil.FlagError{Err: errors.New("body cannot be blank for request-changes review")} + } + } + if flagComment { + found++ + opts.ReviewType = api.ReviewComment + if opts.Body == "" { + return &cmdutil.FlagError{Err: errors.New("body cannot be blank for comment review")} + } + } + + if found == 0 && opts.Body == "" { + if !opts.IO.IsStdoutTTY() || !opts.IO.IsStdinTTY() { + return &cmdutil.FlagError{Err: errors.New("--approve, --request-changes, or --comment required when not attached to a tty")} + } + opts.InteractiveMode = true + } else if found == 0 && opts.Body != "" { + return &cmdutil.FlagError{Err: errors.New("--body unsupported without --approve, --request-changes, or --comment")} + } else if found > 1 { + return &cmdutil.FlagError{Err: errors.New("need exactly one of --approve, --request-changes, or --comment")} + } + if runF != nil { return runF(opts) } @@ -81,9 +121,9 @@ func NewCmdReview(f *cmdutil.Factory, runF func(*ReviewOptions) error) *cobra.Co }, } - cmd.Flags().BoolVarP(&opts.Approve, "approve", "a", false, "Approve pull request") - cmd.Flags().BoolVarP(&opts.RequestChanges, "request-changes", "r", false, "Request changes on a pull request") - cmd.Flags().BoolVarP(&opts.Comment, "comment", "c", false, "Comment on a pull request") + cmd.Flags().BoolVarP(&flagApprove, "approve", "a", false, "Approve pull request") + cmd.Flags().BoolVarP(&flagRequestChanges, "request-changes", "r", false, "Request changes on a pull request") + cmd.Flags().BoolVarP(&flagComment, "comment", "c", false, "Comment on a pull request") cmd.Flags().StringVarP(&opts.Body, "body", "b", "", "Specify the body of a review") return cmd @@ -96,17 +136,13 @@ func reviewRun(opts *ReviewOptions) error { } apiClient := api.NewClientFromHTTP(httpClient) - reviewData, err := processReviewOpt(opts) - if err != nil { - return fmt.Errorf("did not understand desired review action: %w", err) - } - pr, baseRepo, err := shared.PRFromArgs(apiClient, opts.BaseRepo, opts.Branch, opts.Remotes, opts.SelectorArg) if err != nil { return err } - if reviewData == nil { + var reviewData *api.PullRequestReviewInput + if opts.InteractiveMode { editorCommand, err := cmdutil.DetermineEditor(opts.Config) if err != nil { return err @@ -119,6 +155,11 @@ func reviewRun(opts *ReviewOptions) error { fmt.Fprint(opts.IO.ErrOut, "Discarding.\n") return nil } + } else { + reviewData = &api.PullRequestReviewInput{ + State: opts.ReviewType, + Body: opts.Body, + } } err = api.AddReview(apiClient, baseRepo, pr, reviewData) @@ -142,51 +183,6 @@ func reviewRun(opts *ReviewOptions) error { return nil } -// TODO: move to Command.Args, raise FlagError -func processReviewOpt(opts *ReviewOptions) (*api.PullRequestReviewInput, error) { - found := 0 - flag := "" - var state api.PullRequestReviewState - - if opts.Approve { - found++ - flag = "approve" - state = api.ReviewApprove - } - if opts.RequestChanges { - found++ - flag = "request-changes" - state = api.ReviewRequestChanges - } - if opts.Comment { - found++ - flag = "comment" - state = api.ReviewComment - } - - body := opts.Body - - if found == 0 && body == "" { - if opts.IO.IsStdoutTTY() && opts.IO.IsStderrTTY() { - return nil, nil // signal interactive mode - } - return nil, errors.New("--approve, --request-changes, or --comment required when not attached to a tty") - } else if found == 0 && body != "" { - return nil, errors.New("--body unsupported without --approve, --request-changes, or --comment") - } else if found > 1 { - return nil, errors.New("need exactly one of --approve, --request-changes, or --comment") - } - - if (flag == "request-changes" || flag == "comment") && body == "" { - return nil, fmt.Errorf("body cannot be blank for %s review", flag) - } - - return &api.PullRequestReviewInput{ - Body: body, - State: state, - }, nil -} - func reviewSurvey(io *iostreams.IOStreams, editorCommand string) (*api.PullRequestReviewInput, error) { typeAnswers := struct { ReviewType string diff --git a/pkg/cmd/pr/review/review_test.go b/pkg/cmd/pr/review/review_test.go index fa033d0a5..1dfe4a53d 100644 --- a/pkg/cmd/pr/review/review_test.go +++ b/pkg/cmd/pr/review/review_test.go @@ -19,8 +19,114 @@ import ( "github.com/cli/cli/test" "github.com/google/shlex" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +func Test_NewCmdReview(t *testing.T) { + tests := []struct { + name string + args string + isTTY bool + want ReviewOptions + wantErr string + }{ + { + name: "number argument", + args: "123", + isTTY: true, + want: ReviewOptions{ + SelectorArg: "123", + ReviewType: 0, + Body: "", + }, + }, + { + name: "no argument", + args: "", + isTTY: true, + want: ReviewOptions{ + SelectorArg: "", + ReviewType: 0, + Body: "", + }, + }, + { + name: "no argument with --repo override", + args: "-R owner/repo", + isTTY: true, + wantErr: "argument required when using the --repo flag", + }, + { + name: "no arguments in non-interactive mode", + args: "", + isTTY: false, + wantErr: "--approve, --request-changes, or --comment required when not attached to a tty", + }, + { + name: "mutually exclusive review types", + args: `--approve --comment -b hello`, + isTTY: true, + wantErr: "need exactly one of --approve, --request-changes, or --comment", + }, + { + name: "comment without body", + args: `--comment`, + isTTY: true, + wantErr: "body cannot be blank for comment review", + }, + { + name: "request changes without body", + args: `--request-changes`, + isTTY: true, + wantErr: "body cannot be blank for request-changes review", + }, + { + name: "only body argument", + args: `-b hello`, + isTTY: true, + wantErr: "--body unsupported without --approve, --request-changes, or --comment", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + io, _, _, _ := iostreams.Test() + io.SetStdoutTTY(tt.isTTY) + io.SetStdinTTY(tt.isTTY) + io.SetStderrTTY(tt.isTTY) + + f := &cmdutil.Factory{ + IOStreams: io, + } + + var opts *ReviewOptions + cmd := NewCmdReview(f, func(o *ReviewOptions) error { + opts = o + return nil + }) + cmd.PersistentFlags().StringP("repo", "R", "", "") + + argv, err := shlex.Split(tt.args) + require.NoError(t, err) + cmd.SetArgs(argv) + + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(ioutil.Discard) + cmd.SetErr(ioutil.Discard) + + _, err = cmd.ExecuteC() + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + return + } else { + require.NoError(t, err) + } + + assert.Equal(t, tt.want.SelectorArg, opts.SelectorArg) + assert.Equal(t, tt.want.Body, opts.Body) + }) + } +} + func runCommand(rt http.RoundTripper, remotes context.Remotes, isTTY bool, cli string) (*test.CmdOut, error) { io, _, stdout, stderr := iostreams.Test() io.SetStdoutTTY(isTTY) @@ -74,36 +180,6 @@ func runCommand(rt http.RoundTripper, remotes context.Remotes, isTTY bool, cli s }, err } -func TestPRReview_validation(t *testing.T) { - for _, cmd := range []string{ - `--approve --comment 123`, - `--approve --comment -b"hey" 123`, - } { - t.Run(cmd, func(t *testing.T) { - http := &httpmock.Registry{} - defer http.Verify(t) - - _, err := runCommand(http, nil, false, cmd) - if err == nil { - t.Fatal("expected error") - } - assert.Equal(t, "did not understand desired review action: need exactly one of --approve, --request-changes, or --comment", err.Error()) - }) - - } -} - -func TestPRReview_bad_body(t *testing.T) { - http := &httpmock.Registry{} - defer http.Verify(t) - - _, err := runCommand(http, nil, false, `123 -b "radical"`) - if err == nil { - t.Fatal("expected error") - } - assert.Equal(t, "did not understand desired review action: --body unsupported without --approve, --request-changes, or --comment", err.Error()) -} - func TestPRReview_url_arg(t *testing.T) { http := &httpmock.Registry{} defer http.Verify(t) @@ -233,22 +309,6 @@ func TestPRReview_no_arg(t *testing.T) { assert.Equal(t, "cool story", reqBody.Variables.Input.Body) } -func TestPRReview_blank_comment(t *testing.T) { - http := &httpmock.Registry{} - defer http.Verify(t) - - _, err := runCommand(http, nil, false, `--comment 123`) - assert.Equal(t, "did not understand desired review action: body cannot be blank for comment review", err.Error()) -} - -func TestPRReview_blank_request_changes(t *testing.T) { - http := &httpmock.Registry{} - defer http.Verify(t) - - _, err := runCommand(http, nil, false, `-r 123`) - assert.Equal(t, "did not understand desired review action: body cannot be blank for request-changes review", err.Error()) -} - func TestPRReview(t *testing.T) { type c struct { Cmd string @@ -298,20 +358,6 @@ func TestPRReview(t *testing.T) { } } -func TestPRReview_nontty_insufficient_flags(t *testing.T) { - http := &httpmock.Registry{} - defer http.Verify(t) - - output, err := runCommand(http, nil, false, "") - if err == nil { - t.Fatal("expected error") - } - - assert.Equal(t, "", output.String()) - - assert.Equal(t, "did not understand desired review action: --approve, --request-changes, or --comment required when not attached to a tty", err.Error()) -} - func TestPRReview_nontty(t *testing.T) { http := &httpmock.Registry{} defer http.Verify(t) diff --git a/pkg/cmd/pr/view/view.go b/pkg/cmd/pr/view/view.go index e473bd212..b653e5ffe 100644 --- a/pkg/cmd/pr/view/view.go +++ b/pkg/cmd/pr/view/view.go @@ -1,6 +1,7 @@ package view import ( + "errors" "fmt" "io" "net/http" @@ -56,6 +57,10 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman // support `-R, --repo` override opts.BaseRepo = f.BaseRepo + if repoOverride, _ := cmd.Flags().GetString("repo"); repoOverride != "" && len(args) == 0 { + return &cmdutil.FlagError{Err: errors.New("argument required when using the --repo flag")} + } + if len(args) > 0 { opts.SelectorArg = args[0] } diff --git a/pkg/cmd/pr/view/view_test.go b/pkg/cmd/pr/view/view_test.go index 827c7f1c0..fa9e3941a 100644 --- a/pkg/cmd/pr/view/view_test.go +++ b/pkg/cmd/pr/view/view_test.go @@ -19,8 +19,91 @@ import ( "github.com/cli/cli/pkg/iostreams" "github.com/cli/cli/test" "github.com/google/shlex" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +func Test_NewCmdView(t *testing.T) { + tests := []struct { + name string + args string + isTTY bool + want ViewOptions + wantErr string + }{ + { + name: "number argument", + args: "123", + isTTY: true, + want: ViewOptions{ + SelectorArg: "123", + BrowserMode: false, + }, + }, + { + name: "no argument", + args: "", + isTTY: true, + want: ViewOptions{ + SelectorArg: "", + BrowserMode: false, + }, + }, + { + name: "web mode", + args: "123 -w", + isTTY: true, + want: ViewOptions{ + SelectorArg: "123", + BrowserMode: true, + }, + }, + { + name: "no argument with --repo override", + args: "-R owner/repo", + isTTY: true, + wantErr: "argument required when using the --repo flag", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + io, _, _, _ := iostreams.Test() + io.SetStdoutTTY(tt.isTTY) + io.SetStdinTTY(tt.isTTY) + io.SetStderrTTY(tt.isTTY) + + f := &cmdutil.Factory{ + IOStreams: io, + } + + var opts *ViewOptions + cmd := NewCmdView(f, func(o *ViewOptions) error { + opts = o + return nil + }) + cmd.PersistentFlags().StringP("repo", "R", "", "") + + argv, err := shlex.Split(tt.args) + require.NoError(t, err) + cmd.SetArgs(argv) + + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(ioutil.Discard) + cmd.SetErr(ioutil.Discard) + + _, err = cmd.ExecuteC() + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + return + } else { + require.NoError(t, err) + } + + assert.Equal(t, tt.want.SelectorArg, opts.SelectorArg) + }) + } +} + func eq(t *testing.T, got interface{}, expected interface{}) { t.Helper() if !reflect.DeepEqual(got, expected) {