Assert stdout separarely from stderr in command tests

This stubs stderr separately from stdout in command tests (before those
streams were combined) and improves test assertions around output.

Additionally, no longer use the `cmd.Print*()` family of Cobra functions
because their name sounds like the text will go to stdout, but they
write to stderr instead. Use the more explicit `cmd.ErrOrStderr()` as
output destination instead.
This commit is contained in:
Mislav Marohnić 2019-12-16 15:46:42 +01:00
parent e93ab66107
commit 48aeff1ca7
9 changed files with 67 additions and 58 deletions

View file

@ -6,24 +6,24 @@ import (
)
func TestCompletion_bash(t *testing.T) {
outStr, err := RunCommand(completionCmd, `completion`)
output, err := RunCommand(completionCmd, `completion`)
if err != nil {
t.Fatal(err)
}
if !strings.Contains(outStr, "complete -o default -F __start_gh gh") {
t.Errorf("problem in bash completion:\n%s", outStr)
if !strings.Contains(output.String(), "complete -o default -F __start_gh gh") {
t.Errorf("problem in bash completion:\n%s", output)
}
}
func TestCompletion_zsh(t *testing.T) {
outStr, err := RunCommand(completionCmd, `completion -s zsh`)
output, err := RunCommand(completionCmd, `completion -s zsh`)
if err != nil {
t.Fatal(err)
}
if !strings.Contains(outStr, "#compdef _gh gh") {
t.Errorf("problem in zsh completion:\n%s", outStr)
if !strings.Contains(output.String(), "#compdef _gh gh") {
t.Errorf("problem in zsh completion:\n%s", output)
}
}

View file

@ -214,7 +214,7 @@ func issueView(cmd *cobra.Command, args []string) error {
}
openURL := issue.URL
cmd.Printf("Opening %s in your browser.\n", openURL)
fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", openURL)
return utils.OpenInBrowser(openURL)
}

View file

@ -33,7 +33,7 @@ func TestIssueStatus(t *testing.T) {
}
for _, r := range expectedIssues {
if !r.MatchString(output) {
if !r.MatchString(output.String()) {
t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output)
return
}
@ -60,7 +60,7 @@ func TestIssueList(t *testing.T) {
}
for _, r := range expectedIssues {
if !r.MatchString(output) {
if !r.MatchString(output.String()) {
t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output)
return
}
@ -72,11 +72,14 @@ 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 := RunCommand(issueListCmd, "issue list -a probablyCher -l web,bug -s open")
output, err := RunCommand(issueListCmd, "issue list -a probablyCher -l web,bug -s open")
if err != nil {
t.Errorf("error running command `issue list`: %v", err)
}
eq(t, output.String(), "")
eq(t, output.Stderr(), "No issues match your search\n")
bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body)
reqBody := struct {
Variables struct {
@ -115,9 +118,8 @@ func TestIssueView(t *testing.T) {
t.Errorf("error running command `issue view`: %v", err)
}
if output == "" {
t.Errorf("command output expected got an empty string")
}
eq(t, output.String(), "")
eq(t, output.Stderr(), "Opening https://github.com/OWNER/REPO/issues/123 in your browser.\n")
if seenCmd == nil {
t.Fatal("expected a command to run")
@ -176,9 +178,7 @@ func TestIssueView_urlArg(t *testing.T) {
t.Errorf("error running command `issue view`: %v", err)
}
if output == "" {
t.Errorf("command output expected got an empty string")
}
eq(t, output.String(), "")
if seenCmd == nil {
t.Fatal("expected a command to run")
@ -223,5 +223,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, output, "https://github.com/OWNER/REPO/issues/12\n")
eq(t, output.String(), "https://github.com/OWNER/REPO/issues/12\n")
}

View file

@ -280,7 +280,7 @@ func prView(cmd *cobra.Command, args []string) error {
}
}
cmd.Printf("Opening %s in your browser.\n", openURL)
fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", openURL)
return utils.OpenInBrowser(openURL)
}

View file

@ -53,7 +53,7 @@ func TestPRCheckout_sameRepo(t *testing.T) {
output, err := RunCommand(prCheckoutCmd, `pr checkout 123`)
eq(t, err, nil)
eq(t, output, "")
eq(t, output.String(), "")
eq(t, len(ranCommands), 4)
eq(t, strings.Join(ranCommands[0], " "), "git fetch origin +refs/heads/feature:refs/remotes/origin/feature")
@ -105,7 +105,7 @@ func TestPRCheckout_urlArg(t *testing.T) {
output, err := RunCommand(prCheckoutCmd, `pr checkout https://github.com/OWNER/REPO/pull/123/files`)
eq(t, err, nil)
eq(t, output, "")
eq(t, output.String(), "")
eq(t, len(ranCommands), 4)
eq(t, strings.Join(ranCommands[1], " "), "git checkout -b feature --no-track origin/feature")
@ -154,7 +154,7 @@ func TestPRCheckout_branchArg(t *testing.T) {
output, err := RunCommand(prCheckoutCmd, `pr checkout hubot:feature`)
eq(t, err, nil)
eq(t, output, "")
eq(t, output.String(), "")
eq(t, len(ranCommands), 5)
eq(t, strings.Join(ranCommands[1], " "), "git fetch origin refs/pull/123/head:feature")
@ -203,7 +203,7 @@ func TestPRCheckout_existingBranch(t *testing.T) {
output, err := RunCommand(prCheckoutCmd, `pr checkout 123`)
eq(t, err, nil)
eq(t, output, "")
eq(t, output.String(), "")
eq(t, len(ranCommands), 3)
eq(t, strings.Join(ranCommands[0], " "), "git fetch origin +refs/heads/feature:refs/remotes/origin/feature")
@ -255,7 +255,7 @@ func TestPRCheckout_differentRepo_remoteExists(t *testing.T) {
output, err := RunCommand(prCheckoutCmd, `pr checkout 123`)
eq(t, err, nil)
eq(t, output, "")
eq(t, output.String(), "")
eq(t, len(ranCommands), 4)
eq(t, strings.Join(ranCommands[0], " "), "git fetch robot-fork +refs/heads/feature:refs/remotes/robot-fork/feature")
@ -307,7 +307,7 @@ func TestPRCheckout_differentRepo(t *testing.T) {
output, err := RunCommand(prCheckoutCmd, `pr checkout 123`)
eq(t, err, nil)
eq(t, output, "")
eq(t, output.String(), "")
eq(t, len(ranCommands), 4)
eq(t, strings.Join(ranCommands[0], " "), "git fetch origin refs/pull/123/head:feature")
@ -359,7 +359,7 @@ func TestPRCheckout_differentRepo_existingBranch(t *testing.T) {
output, err := RunCommand(prCheckoutCmd, `pr checkout 123`)
eq(t, err, nil)
eq(t, output, "")
eq(t, output.String(), "")
eq(t, len(ranCommands), 2)
eq(t, strings.Join(ranCommands[0], " "), "git fetch origin refs/pull/123/head:feature")
@ -409,7 +409,7 @@ func TestPRCheckout_differentRepo_currentBranch(t *testing.T) {
output, err := RunCommand(prCheckoutCmd, `pr checkout 123`)
eq(t, err, nil)
eq(t, output, "")
eq(t, output.String(), "")
eq(t, len(ranCommands), 2)
eq(t, strings.Join(ranCommands[0], " "), "git fetch origin refs/pull/123/head")
@ -459,7 +459,7 @@ func TestPRCheckout_maintainerCanModify(t *testing.T) {
output, err := RunCommand(prCheckoutCmd, `pr checkout 123`)
eq(t, err, nil)
eq(t, output, "")
eq(t, output.String(), "")
eq(t, len(ranCommands), 4)
eq(t, strings.Join(ranCommands[0], " "), "git fetch origin refs/pull/123/head:feature")

View file

@ -27,7 +27,7 @@ func prCreate(cmd *cobra.Command, _ []string) error {
noun = noun + "s"
}
cmd.Printf("Warning: %d uncommitted %s\n", ucc, noun)
fmt.Fprintf(cmd.ErrOrStderr(), "Warning: %d uncommitted %s\n", ucc, noun)
}
repo, err := ctx.BaseRepo()
@ -55,7 +55,7 @@ func prCreate(cmd *cobra.Command, _ []string) error {
}
if isWeb {
openURL := fmt.Sprintf(`https://github.com/%s/%s/pull/%s`, repo.RepoOwner(), repo.RepoName(), head)
cmd.Printf("Opening %s in your browser.\n", openURL)
fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", openURL)
return utils.OpenInBrowser(openURL)
}

View file

@ -91,7 +91,7 @@ func TestPRCreate(t *testing.T) {
eq(t, reqBody.Variables.Input.BaseRefName, "master")
eq(t, reqBody.Variables.Input.HeadRefName, "feature")
eq(t, output, "https://github.com/OWNER/REPO/pull/12\n")
eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n")
}
func TestPRCreate_web(t *testing.T) {
@ -115,9 +115,8 @@ func TestPRCreate_web(t *testing.T) {
output, err := RunCommand(prCreateCmd, `pr create --web`)
eq(t, err, nil)
if output == "" {
t.Fatal("expected output")
}
eq(t, output.String(), "")
eq(t, output.Stderr(), "Opening https://github.com/OWNER/REPO/pull/feature in your browser.\n")
eq(t, len(ranCommands), 3)
eq(t, strings.Join(ranCommands[1], " "), "git push --set-upstream origin HEAD:feature")
@ -155,7 +154,6 @@ func TestPRCreate_ReportsUncommittedChanges(t *testing.T) {
output, err := RunCommand(prCreateCmd, `pr create -t "my title" -b "my body"`)
eq(t, err, nil)
eq(t, output, `Warning: 1 uncommitted change
https://github.com/OWNER/REPO/pull/12
`)
eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n")
eq(t, output.Stderr(), "Warning: 1 uncommitted change\n")
}

View file

@ -24,16 +24,30 @@ func eq(t *testing.T, got interface{}, expected interface{}) {
}
}
func RunCommand(cmd *cobra.Command, args string) (string, error) {
type cmdOut struct {
outBuf, errBuf *bytes.Buffer
}
func (c cmdOut) String() string {
return c.outBuf.String()
}
func (c cmdOut) Stderr() string {
return c.errBuf.String()
}
func RunCommand(cmd *cobra.Command, args string) (*cmdOut, error) {
rootCmd := cmd.Root()
argv, err := shlex.Split(args)
if err != nil {
return "", err
return nil, err
}
rootCmd.SetArgs(argv)
outBuf := bytes.Buffer{}
cmd.SetOut(&outBuf)
errBuf := bytes.Buffer{}
cmd.SetErr(&errBuf)
// Reset flag values so they don't leak between tests
cmd.Flags().VisitAll(func(f *pflag.Flag) {
@ -48,7 +62,10 @@ func RunCommand(cmd *cobra.Command, args string) (string, error) {
})
_, err = rootCmd.ExecuteC()
return outBuf.String(), err
cmd.SetOut(nil)
cmd.SetErr(nil)
return &cmdOut{&outBuf, &errBuf}, err
}
func TestPRStatus(t *testing.T) {
@ -72,7 +89,7 @@ func TestPRStatus(t *testing.T) {
}
for _, r := range expectedPrs {
if !r.MatchString(output) {
if !r.MatchString(output.String()) {
t.Errorf("output did not match regexp /%s/", r)
}
}
@ -91,7 +108,7 @@ func TestPRList(t *testing.T) {
t.Fatal(err)
}
eq(t, output, `32 New feature feature
eq(t, output.String(), `32 New feature feature
29 Fixed bad bug hubot:bug-fix
28 Improve documentation docs
`)
@ -104,11 +121,14 @@ func TestPRList_filtering(t *testing.T) {
respBody := bytes.NewBufferString(`{ "data": {} }`)
http.StubResponse(200, respBody)
_, err := RunCommand(prListCmd, `pr list -s all -l one,two -l three`)
output, err := RunCommand(prListCmd, `pr list -s all -l one,two -l three`)
if err != nil {
t.Fatal(err)
}
eq(t, output.String(), "")
eq(t, output.Stderr(), "No pull requests match your search\n")
bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body)
reqBody := struct {
Variables struct {
@ -183,9 +203,8 @@ func TestPRView_currentBranch(t *testing.T) {
t.Errorf("error running command `pr view`: %v", err)
}
if output == "" {
t.Errorf("command output expected got an empty string")
}
eq(t, output.String(), "")
eq(t, output.Stderr(), "Opening https://github.com/OWNER/REPO/pull/10 in your browser.\n")
if seenCmd == nil {
t.Fatal("expected a command to run")
@ -248,9 +267,7 @@ func TestPRView_numberArg(t *testing.T) {
t.Errorf("error running command `pr view`: %v", err)
}
if output == "" {
t.Errorf("command output expected got an empty string")
}
eq(t, output.String(), "")
if seenCmd == nil {
t.Fatal("expected a command to run")
@ -281,9 +298,7 @@ func TestPRView_urlArg(t *testing.T) {
t.Errorf("error running command `pr view`: %v", err)
}
if output == "" {
t.Errorf("command output expected got an empty string")
}
eq(t, output.String(), "")
if seenCmd == nil {
t.Fatal("expected a command to run")
@ -316,9 +331,7 @@ func TestPRView_branchArg(t *testing.T) {
t.Errorf("error running command `pr view`: %v", err)
}
if output == "" {
t.Errorf("command output expected got an empty string")
}
eq(t, output.String(), "")
if seenCmd == nil {
t.Fatal("expected a command to run")
@ -352,9 +365,7 @@ func TestPRView_branchWithOwnerArg(t *testing.T) {
t.Errorf("error running command `pr view`: %v", err)
}
if output == "" {
t.Errorf("command output expected got an empty string")
}
eq(t, output.String(), "")
if seenCmd == nil {
t.Fatal("expected a command to run")

View file

@ -92,7 +92,7 @@ func titleBodySurvey(cmd *cobra.Command, providedTitle string, providedBody stri
case _unconfirmed:
continue
case _cancel:
cmd.Println("Discarding.")
fmt.Fprintln(cmd.ErrOrStderr(), "Discarding.")
return nil, nil
default:
panic("reached unreachable case")