From 2621bccc44d1b5dec9d71a2cd16ebcefd88c124c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 3 Dec 2019 17:24:50 +0100 Subject: [PATCH] Use RunCommand in tests to ensure flags reset between runs --- command/completion_test.go | 21 ++----------- command/issue.go | 17 +++++----- command/issue_test.go | 17 ++++------ command/pr.go | 2 +- command/pr_checkout_test.go | 28 ++++++++--------- command/pr_create_test.go | 20 ++++-------- command/pr_test.go | 62 ++++++++++++++++++++----------------- go.mod | 1 + go.sum | 2 ++ test/helpers.go | 39 ----------------------- 10 files changed, 76 insertions(+), 133 deletions(-) diff --git a/command/completion_test.go b/command/completion_test.go index 8cea652b1..5ea8e8ed8 100644 --- a/command/completion_test.go +++ b/command/completion_test.go @@ -1,49 +1,34 @@ package command import ( - "bytes" "strings" "testing" ) func TestCompletion_bash(t *testing.T) { - out := bytes.Buffer{} - completionCmd.SetOut(&out) - - RootCmd.SetArgs([]string{"completion"}) - _, err := RootCmd.ExecuteC() + outStr, err := RunCommand(completionCmd, `completion`) if err != nil { t.Fatal(err) } - outStr := out.String() if !strings.Contains(outStr, "complete -o default -F __start_gh gh") { t.Errorf("problem in bash completion:\n%s", outStr) } } func TestCompletion_zsh(t *testing.T) { - out := bytes.Buffer{} - completionCmd.SetOut(&out) - - RootCmd.SetArgs([]string{"completion", "-s", "zsh"}) - _, err := RootCmd.ExecuteC() + outStr, err := RunCommand(completionCmd, `completion -s zsh`) if err != nil { t.Fatal(err) } - outStr := out.String() if !strings.Contains(outStr, "#compdef _gh gh") { t.Errorf("problem in zsh completion:\n%s", outStr) } } func TestCompletion_unsupported(t *testing.T) { - out := bytes.Buffer{} - completionCmd.SetOut(&out) - - RootCmd.SetArgs([]string{"completion", "-s", "fish"}) - _, err := RootCmd.ExecuteC() + _, err := RunCommand(completionCmd, `completion -s fish`) if err == nil || err.Error() != "unsupported shell type: fish" { t.Fatal(err) } diff --git a/command/issue.go b/command/issue.go index 80b0a980d..d8811f5c8 100644 --- a/command/issue.go +++ b/command/issue.go @@ -15,25 +15,21 @@ import ( func init() { RootCmd.AddCommand(issueCmd) - issueCmd.AddCommand(issueCreateCmd) issueCmd.AddCommand(issueStatusCmd) issueCmd.AddCommand(issueViewCmd) + + issueCmd.AddCommand(issueCreateCmd) issueCreateCmd.Flags().StringP("title", "t", "", "Supply a title. Will prompt for one otherwise.") issueCreateCmd.Flags().StringP("body", "b", "", "Supply a body. Will prompt for one otherwise.") issueCreateCmd.Flags().BoolP("web", "w", false, "Open the web browser to create an issue") - issueListCmd := &cobra.Command{ - Use: "list", - Short: "List and filter issues in this repository", - RunE: issueList, - } + issueCmd.AddCommand(issueListCmd) issueListCmd.Flags().StringP("assignee", "a", "", "Filter by assignee") issueListCmd.Flags().StringSliceP("label", "l", nil, "Filter by label") issueListCmd.Flags().StringP("state", "s", "", "Filter by state (open|closed|all)") issueListCmd.Flags().IntP("limit", "L", 30, "Maximum number of issues to fetch") - issueCmd.AddCommand((issueListCmd)) } var issueCmd = &cobra.Command{ @@ -46,6 +42,11 @@ var issueCreateCmd = &cobra.Command{ Short: "Create a new issue", RunE: issueCreate, } +var issueListCmd = &cobra.Command{ + Use: "list", + Short: "List and filter issues in this repository", + RunE: issueList, +} var issueStatusCmd = &cobra.Command{ Use: "status", Short: "Show status of relevant issues", @@ -191,7 +192,7 @@ func issueView(cmd *cobra.Command, args []string) error { return fmt.Errorf("invalid issue number: '%s'", args[0]) } - fmt.Printf("Opening %s in your browser.\n", openURL) + cmd.Printf("Opening %s in your browser.\n", openURL) return utils.OpenInBrowser(openURL) } diff --git a/command/issue_test.go b/command/issue_test.go index 37db69b10..aaf5e2517 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -9,7 +9,6 @@ import ( "regexp" "testing" - "github.com/github/gh-cli/test" "github.com/github/gh-cli/utils" ) @@ -21,7 +20,7 @@ func TestIssueStatus(t *testing.T) { defer jsonFile.Close() http.StubResponse(200, jsonFile) - output, err := test.RunCommand(RootCmd, "issue status") + output, err := RunCommand(issueStatusCmd, "issue status") if err != nil { t.Errorf("error running command `issue status`: %v", err) } @@ -49,7 +48,7 @@ func TestIssueList(t *testing.T) { defer jsonFile.Close() http.StubResponse(200, jsonFile) - output, err := test.RunCommand(RootCmd, "issue list") + output, err := RunCommand(issueListCmd, "issue list") if err != nil { t.Errorf("error running command `issue list`: %v", err) } @@ -73,7 +72,7 @@ func TestIssueList_withFlags(t *testing.T) { http.StubResponse(200, bytes.NewBufferString(`{"data": {}}`)) // Since we are testing that the flags are passed, we don't care about the response - _, err := test.RunCommand(RootCmd, "issue list -a probablyCher -l web,bug -s open") + _, err := RunCommand(issueListCmd, "issue list -a probablyCher -l web,bug -s open") if err != nil { t.Errorf("error running command `issue list`: %v", err) } @@ -108,7 +107,7 @@ func TestIssueView(t *testing.T) { }) defer restoreCmd() - output, err := test.RunCommand(RootCmd, "issue view 8") + output, err := RunCommand(issueViewCmd, "issue view 8") if err != nil { t.Errorf("error running command `issue view`: %v", err) } @@ -141,11 +140,7 @@ func TestIssueCreate(t *testing.T) { } } } } `)) - out := bytes.Buffer{} - issueCreateCmd.SetOut(&out) - - RootCmd.SetArgs([]string{"issue", "create", "-t", "hello", "-b", "cash rules everything around me"}) - _, err := RootCmd.ExecuteC() + output, err := RunCommand(issueCreateCmd, `issue create -t hello -b "cash rules everything around me"`) if err != nil { t.Errorf("error running command `issue create`: %v", err) } @@ -166,5 +161,5 @@ func TestIssueCreate(t *testing.T) { eq(t, reqBody.Variables.Input.Title, "hello") eq(t, reqBody.Variables.Input.Body, "cash rules everything around me") - eq(t, out.String(), "https://github.com/OWNER/REPO/issues/12\n") + eq(t, output, "https://github.com/OWNER/REPO/issues/12\n") } diff --git a/command/pr.go b/command/pr.go index d4b923a91..78c576ffe 100644 --- a/command/pr.go +++ b/command/pr.go @@ -249,7 +249,7 @@ func prView(cmd *cobra.Command, args []string) error { } } - fmt.Printf("Opening %s in your browser.\n", openURL) + cmd.Printf("Opening %s in your browser.\n", openURL) return utils.OpenInBrowser(openURL) } diff --git a/command/pr_checkout_test.go b/command/pr_checkout_test.go index 930d2b4d8..f4dd95dcc 100644 --- a/command/pr_checkout_test.go +++ b/command/pr_checkout_test.go @@ -50,9 +50,9 @@ func TestPRCheckout_sameRepo(t *testing.T) { }) defer restoreCmd() - RootCmd.SetArgs([]string{"pr", "checkout", "123"}) - _, err := prCheckoutCmd.ExecuteC() + output, err := RunCommand(prCheckoutCmd, `pr checkout 123`) eq(t, err, nil) + eq(t, output, "") eq(t, len(ranCommands), 4) eq(t, strings.Join(ranCommands[0], " "), "git fetch origin +refs/heads/feature:refs/remotes/origin/feature") @@ -101,9 +101,9 @@ func TestPRCheckout_existingBranch(t *testing.T) { }) defer restoreCmd() - RootCmd.SetArgs([]string{"pr", "checkout", "123"}) - _, err := prCheckoutCmd.ExecuteC() + output, err := RunCommand(prCheckoutCmd, `pr checkout 123`) eq(t, err, nil) + eq(t, output, "") eq(t, len(ranCommands), 3) eq(t, strings.Join(ranCommands[0], " "), "git fetch origin +refs/heads/feature:refs/remotes/origin/feature") @@ -152,9 +152,9 @@ func TestPRCheckout_differentRepo_remoteExists(t *testing.T) { }) defer restoreCmd() - RootCmd.SetArgs([]string{"pr", "checkout", "123"}) - _, err := prCheckoutCmd.ExecuteC() + output, err := RunCommand(prCheckoutCmd, `pr checkout 123`) eq(t, err, nil) + eq(t, output, "") eq(t, len(ranCommands), 4) eq(t, strings.Join(ranCommands[0], " "), "git fetch robot-fork +refs/heads/feature:refs/remotes/robot-fork/feature") @@ -203,9 +203,9 @@ func TestPRCheckout_differentRepo(t *testing.T) { }) defer restoreCmd() - RootCmd.SetArgs([]string{"pr", "checkout", "123"}) - _, err := prCheckoutCmd.ExecuteC() + output, err := RunCommand(prCheckoutCmd, `pr checkout 123`) eq(t, err, nil) + eq(t, output, "") eq(t, len(ranCommands), 4) eq(t, strings.Join(ranCommands[0], " "), "git fetch origin refs/pull/123/head:feature") @@ -254,9 +254,9 @@ func TestPRCheckout_differentRepo_existingBranch(t *testing.T) { }) defer restoreCmd() - RootCmd.SetArgs([]string{"pr", "checkout", "123"}) - _, err := prCheckoutCmd.ExecuteC() + output, err := RunCommand(prCheckoutCmd, `pr checkout 123`) eq(t, err, nil) + eq(t, output, "") eq(t, len(ranCommands), 2) eq(t, strings.Join(ranCommands[0], " "), "git fetch origin refs/pull/123/head:feature") @@ -303,9 +303,9 @@ func TestPRCheckout_differentRepo_currentBranch(t *testing.T) { }) defer restoreCmd() - RootCmd.SetArgs([]string{"pr", "checkout", "123"}) - _, err := prCheckoutCmd.ExecuteC() + output, err := RunCommand(prCheckoutCmd, `pr checkout 123`) eq(t, err, nil) + eq(t, output, "") eq(t, len(ranCommands), 2) eq(t, strings.Join(ranCommands[0], " "), "git fetch origin refs/pull/123/head") @@ -352,9 +352,9 @@ func TestPRCheckout_maintainerCanModify(t *testing.T) { }) defer restoreCmd() - RootCmd.SetArgs([]string{"pr", "checkout", "123"}) - _, err := prCheckoutCmd.ExecuteC() + output, err := RunCommand(prCheckoutCmd, `pr checkout 123`) eq(t, err, nil) + eq(t, output, "") eq(t, len(ranCommands), 4) eq(t, strings.Join(ranCommands[0], " "), "git fetch origin refs/pull/123/head:feature") diff --git a/command/pr_create_test.go b/command/pr_create_test.go index 191043be3..af609f5a4 100644 --- a/command/pr_create_test.go +++ b/command/pr_create_test.go @@ -65,11 +65,7 @@ func TestPRCreate(t *testing.T) { git.GitCommand = origGitCommand }() - out := bytes.Buffer{} - prCreateCmd.SetOut(&out) - - RootCmd.SetArgs([]string{"pr", "create", "-t", "mytitle", "-b", "mybody"}) - _, err := prCreateCmd.ExecuteC() + output, err := RunCommand(prCreateCmd, `pr create -t "my title" -b "my body"`) eq(t, err, nil) bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body) @@ -87,12 +83,12 @@ func TestPRCreate(t *testing.T) { json.Unmarshal(bodyBytes, &reqBody) eq(t, reqBody.Variables.Input.RepositoryID, "REPOID") - eq(t, reqBody.Variables.Input.Title, "mytitle") - eq(t, reqBody.Variables.Input.Body, "mybody") + eq(t, reqBody.Variables.Input.Title, "my title") + eq(t, reqBody.Variables.Input.Body, "my body") eq(t, reqBody.Variables.Input.BaseRefName, "master") eq(t, reqBody.Variables.Input.HeadRefName, "feature") - eq(t, out.String(), "https://github.com/OWNER/REPO/pull/12\n") + eq(t, output, "https://github.com/OWNER/REPO/pull/12\n") } func TestPRCreate_ReportsUncommittedChanges(t *testing.T) { @@ -123,14 +119,10 @@ func TestPRCreate_ReportsUncommittedChanges(t *testing.T) { git.GitCommand = origGitCommand }() - out := bytes.Buffer{} - prCreateCmd.SetOut(&out) - - RootCmd.SetArgs([]string{"pr", "create", "-t", "mytitle", "-b", "mybody"}) - _, err := prCreateCmd.ExecuteC() + output, err := RunCommand(prCreateCmd, `pr create -t "my title" -b "my body"`) eq(t, err, nil) - eq(t, out.String(), `Warning: 1 uncommitted change + eq(t, output, `Warning: 1 uncommitted change https://github.com/OWNER/REPO/pull/12 `) } diff --git a/command/pr_test.go b/command/pr_test.go index da796d947..c62fd30ce 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -11,8 +11,9 @@ import ( "strings" "testing" - "github.com/github/gh-cli/test" "github.com/github/gh-cli/utils" + "github.com/google/shlex" + "github.com/spf13/cobra" "github.com/spf13/pflag" ) @@ -23,6 +24,28 @@ func eq(t *testing.T, got interface{}, expected interface{}) { } } +func RunCommand(cmd *cobra.Command, args string) (string, error) { + rootCmd := cmd.Root() + argv, err := shlex.Split(args) + if err != nil { + return "", err + } + rootCmd.SetArgs(argv) + + outBuf := bytes.Buffer{} + cmd.SetOut(&outBuf) + + // Reset flag slice values so they don't leak between tests + cmd.Flags().Visit(func(f *pflag.Flag) { + if v, ok := f.Value.(pflag.SliceValue); ok { + v.Replace([]string{}) + } + }) + + _, err = rootCmd.ExecuteC() + return outBuf.String(), err +} + func TestPRStatus(t *testing.T) { initBlankContext("OWNER/REPO", "blueberries") http := initFakeHTTP() @@ -31,7 +54,7 @@ func TestPRStatus(t *testing.T) { defer jsonFile.Close() http.StubResponse(200, jsonFile) - output, err := test.RunCommand(RootCmd, "pr status") + output, err := RunCommand(prStatusCmd, "pr status") if err != nil { t.Errorf("error running command `pr status`: %v", err) } @@ -58,16 +81,12 @@ func TestPRList(t *testing.T) { defer jsonFile.Close() http.StubResponse(200, jsonFile) - out := bytes.Buffer{} - prListCmd.SetOut(&out) - - RootCmd.SetArgs([]string{"pr", "list"}) - _, err := RootCmd.ExecuteC() + output, err := RunCommand(prListCmd, "pr list") if err != nil { t.Fatal(err) } - eq(t, out.String(), `32 New feature feature + eq(t, output, `32 New feature feature 29 Fixed bad bug hubot:bug-fix 28 Improve documentation docs `) @@ -80,10 +99,7 @@ func TestPRList_filtering(t *testing.T) { respBody := bytes.NewBufferString(`{ "data": {} }`) http.StubResponse(200, respBody) - prListCmd.SetOut(ioutil.Discard) - - RootCmd.SetArgs([]string{"pr", "list", "-s", "all", "-l", "one", "-l", "two"}) - _, err := RootCmd.ExecuteC() + _, err := RunCommand(prListCmd, `pr list -s all -l one,two -l three`) if err != nil { t.Fatal(err) } @@ -98,7 +114,7 @@ func TestPRList_filtering(t *testing.T) { json.Unmarshal(bodyBytes, &reqBody) eq(t, reqBody.Variables.State, []string{"OPEN", "CLOSED", "MERGED"}) - eq(t, reqBody.Variables.Labels, []string{"one", "two"}) + eq(t, reqBody.Variables.Labels, []string{"one", "two", "three"}) } func TestPRList_filteringAssignee(t *testing.T) { @@ -108,17 +124,7 @@ func TestPRList_filteringAssignee(t *testing.T) { respBody := bytes.NewBufferString(`{ "data": {} }`) http.StubResponse(200, respBody) - prListCmd.SetOut(ioutil.Discard) - // Reset flag slice values so they don't leak between tests - // TODO: generalize this hack - prListCmd.Flags().Visit(func(f *pflag.Flag) { - if v, ok := f.Value.(pflag.SliceValue); ok { - v.Replace([]string{}) - } - }) - - RootCmd.SetArgs([]string{"pr", "list", "-s", "merged", "-l", "one two", "-a", "hubot", "-B", "develop"}) - _, err := RootCmd.ExecuteC() + _, err := RunCommand(prListCmd, `pr list -s merged -l "needs tests" -a hubot -B develop`) if err != nil { t.Fatal(err) } @@ -131,7 +137,7 @@ func TestPRList_filteringAssignee(t *testing.T) { }{} json.Unmarshal(bodyBytes, &reqBody) - eq(t, reqBody.Variables.Q, `repo:OWNER/REPO assignee:hubot is:pr sort:created-desc is:merged label:"one two" base:"develop"`) + eq(t, reqBody.Variables.Q, `repo:OWNER/REPO assignee:hubot is:pr sort:created-desc is:merged label:"needs tests" base:"develop"`) } func TestPRView_currentBranch(t *testing.T) { @@ -154,7 +160,7 @@ func TestPRView_currentBranch(t *testing.T) { }) defer restoreCmd() - output, err := test.RunCommand(RootCmd, "pr view") + output, err := RunCommand(prViewCmd, "pr view") if err != nil { t.Errorf("error running command `pr view`: %v", err) } @@ -192,7 +198,7 @@ func TestPRView_noResultsForBranch(t *testing.T) { }) defer restoreCmd() - _, err := test.RunCommand(RootCmd, "pr view") + _, err := RunCommand(prViewCmd, "pr view") if err == nil || err.Error() != `no open pull requests found for branch "blueberries"` { t.Errorf("error running command `pr view`: %v", err) } @@ -219,7 +225,7 @@ func TestPRView_numberArg(t *testing.T) { }) defer restoreCmd() - output, err := test.RunCommand(RootCmd, "pr view 23") + output, err := RunCommand(prViewCmd, "pr view 23") if err != nil { t.Errorf("error running command `pr view`: %v", err) } diff --git a/go.mod b/go.mod index c61e8489e..18562337e 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.13 require ( github.com/AlecAivazis/survey/v2 v2.0.4 + github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 github.com/mattn/go-colorable v0.1.2 github.com/mattn/go-isatty v0.0.9 diff --git a/go.sum b/go.sum index e428fb9a6..b31b3328c 100644 --- a/go.sum +++ b/go.sum @@ -13,6 +13,8 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo= +github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4= +github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ= github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ= github.com/hinshun/vt10x v0.0.0-20180616224451-1954e6464174 h1:WlZsjVhE8Af9IcZDGgJGQpNflI3+MJSBhsgT5PCtzBQ= github.com/hinshun/vt10x v0.0.0-20180616224451-1954e6464174/go.mod h1:DqJ97dSdRW1W22yXSB90986pcOyQ7r45iio1KN2ez1A= diff --git a/test/helpers.go b/test/helpers.go index dca930be5..f21bb57e0 100644 --- a/test/helpers.go +++ b/test/helpers.go @@ -6,9 +6,6 @@ import ( "os" "os/exec" "path/filepath" - "strings" - - "github.com/spf13/cobra" ) func GetTestHelperProcessArgs() []string { @@ -96,39 +93,3 @@ func UseTempGitRepo() *TempGitRepo { return &TempGitRepo{Remote: remotePath, TearDown: tearDown} } - -func RunCommand(root *cobra.Command, s string) (string, error) { - var err error - output := captureOutput(func() { - root.SetArgs(strings.Split(s, " ")) - _, err = root.ExecuteC() - }) - - if err != nil { - return "", err - } - return output, nil -} - -func captureOutput(f func()) string { - originalStdout := os.Stdout - defer func() { - os.Stdout = originalStdout - }() - - r, w, err := os.Pipe() - if err != nil { - panic("failed to pipe stdout") - } - os.Stdout = w - - f() - - w.Close() - out, err := ioutil.ReadAll(r) - if err != nil { - panic("failed to read captured input from stdout") - } - - return string(out) -}