X Tutup
Skip to content

Commit 48aeff1

Browse files
committed
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.
1 parent e93ab66 commit 48aeff1

File tree

9 files changed

+67
-58
lines changed

9 files changed

+67
-58
lines changed

command/completion_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,24 +6,24 @@ import (
66
)
77

88
func TestCompletion_bash(t *testing.T) {
9-
outStr, err := RunCommand(completionCmd, `completion`)
9+
output, err := RunCommand(completionCmd, `completion`)
1010
if err != nil {
1111
t.Fatal(err)
1212
}
1313

14-
if !strings.Contains(outStr, "complete -o default -F __start_gh gh") {
15-
t.Errorf("problem in bash completion:\n%s", outStr)
14+
if !strings.Contains(output.String(), "complete -o default -F __start_gh gh") {
15+
t.Errorf("problem in bash completion:\n%s", output)
1616
}
1717
}
1818

1919
func TestCompletion_zsh(t *testing.T) {
20-
outStr, err := RunCommand(completionCmd, `completion -s zsh`)
20+
output, err := RunCommand(completionCmd, `completion -s zsh`)
2121
if err != nil {
2222
t.Fatal(err)
2323
}
2424

25-
if !strings.Contains(outStr, "#compdef _gh gh") {
26-
t.Errorf("problem in zsh completion:\n%s", outStr)
25+
if !strings.Contains(output.String(), "#compdef _gh gh") {
26+
t.Errorf("problem in zsh completion:\n%s", output)
2727
}
2828
}
2929

command/issue.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ func issueView(cmd *cobra.Command, args []string) error {
214214
}
215215
openURL := issue.URL
216216

217-
cmd.Printf("Opening %s in your browser.\n", openURL)
217+
fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", openURL)
218218
return utils.OpenInBrowser(openURL)
219219
}
220220

command/issue_test.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ func TestIssueStatus(t *testing.T) {
3333
}
3434

3535
for _, r := range expectedIssues {
36-
if !r.MatchString(output) {
36+
if !r.MatchString(output.String()) {
3737
t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output)
3838
return
3939
}
@@ -60,7 +60,7 @@ func TestIssueList(t *testing.T) {
6060
}
6161

6262
for _, r := range expectedIssues {
63-
if !r.MatchString(output) {
63+
if !r.MatchString(output.String()) {
6464
t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output)
6565
return
6666
}
@@ -72,11 +72,14 @@ func TestIssueList_withFlags(t *testing.T) {
7272

7373
http.StubResponse(200, bytes.NewBufferString(`{"data": {}}`)) // Since we are testing that the flags are passed, we don't care about the response
7474

75-
_, err := RunCommand(issueListCmd, "issue list -a probablyCher -l web,bug -s open")
75+
output, err := RunCommand(issueListCmd, "issue list -a probablyCher -l web,bug -s open")
7676
if err != nil {
7777
t.Errorf("error running command `issue list`: %v", err)
7878
}
7979

80+
eq(t, output.String(), "")
81+
eq(t, output.Stderr(), "No issues match your search\n")
82+
8083
bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body)
8184
reqBody := struct {
8285
Variables struct {
@@ -115,9 +118,8 @@ func TestIssueView(t *testing.T) {
115118
t.Errorf("error running command `issue view`: %v", err)
116119
}
117120

118-
if output == "" {
119-
t.Errorf("command output expected got an empty string")
120-
}
121+
eq(t, output.String(), "")
122+
eq(t, output.Stderr(), "Opening https://github.com/OWNER/REPO/issues/123 in your browser.\n")
121123

122124
if seenCmd == nil {
123125
t.Fatal("expected a command to run")
@@ -176,9 +178,7 @@ func TestIssueView_urlArg(t *testing.T) {
176178
t.Errorf("error running command `issue view`: %v", err)
177179
}
178180

179-
if output == "" {
180-
t.Errorf("command output expected got an empty string")
181-
}
181+
eq(t, output.String(), "")
182182

183183
if seenCmd == nil {
184184
t.Fatal("expected a command to run")
@@ -223,5 +223,5 @@ func TestIssueCreate(t *testing.T) {
223223
eq(t, reqBody.Variables.Input.Title, "hello")
224224
eq(t, reqBody.Variables.Input.Body, "cash rules everything around me")
225225

226-
eq(t, output, "https://github.com/OWNER/REPO/issues/12\n")
226+
eq(t, output.String(), "https://github.com/OWNER/REPO/issues/12\n")
227227
}

command/pr.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ func prView(cmd *cobra.Command, args []string) error {
280280
}
281281
}
282282

283-
cmd.Printf("Opening %s in your browser.\n", openURL)
283+
fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", openURL)
284284
return utils.OpenInBrowser(openURL)
285285
}
286286

command/pr_checkout_test.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ func TestPRCheckout_sameRepo(t *testing.T) {
5353

5454
output, err := RunCommand(prCheckoutCmd, `pr checkout 123`)
5555
eq(t, err, nil)
56-
eq(t, output, "")
56+
eq(t, output.String(), "")
5757

5858
eq(t, len(ranCommands), 4)
5959
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) {
105105

106106
output, err := RunCommand(prCheckoutCmd, `pr checkout https://github.com/OWNER/REPO/pull/123/files`)
107107
eq(t, err, nil)
108-
eq(t, output, "")
108+
eq(t, output.String(), "")
109109

110110
eq(t, len(ranCommands), 4)
111111
eq(t, strings.Join(ranCommands[1], " "), "git checkout -b feature --no-track origin/feature")
@@ -154,7 +154,7 @@ func TestPRCheckout_branchArg(t *testing.T) {
154154

155155
output, err := RunCommand(prCheckoutCmd, `pr checkout hubot:feature`)
156156
eq(t, err, nil)
157-
eq(t, output, "")
157+
eq(t, output.String(), "")
158158

159159
eq(t, len(ranCommands), 5)
160160
eq(t, strings.Join(ranCommands[1], " "), "git fetch origin refs/pull/123/head:feature")
@@ -203,7 +203,7 @@ func TestPRCheckout_existingBranch(t *testing.T) {
203203

204204
output, err := RunCommand(prCheckoutCmd, `pr checkout 123`)
205205
eq(t, err, nil)
206-
eq(t, output, "")
206+
eq(t, output.String(), "")
207207

208208
eq(t, len(ranCommands), 3)
209209
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) {
255255

256256
output, err := RunCommand(prCheckoutCmd, `pr checkout 123`)
257257
eq(t, err, nil)
258-
eq(t, output, "")
258+
eq(t, output.String(), "")
259259

260260
eq(t, len(ranCommands), 4)
261261
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) {
307307

308308
output, err := RunCommand(prCheckoutCmd, `pr checkout 123`)
309309
eq(t, err, nil)
310-
eq(t, output, "")
310+
eq(t, output.String(), "")
311311

312312
eq(t, len(ranCommands), 4)
313313
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) {
359359

360360
output, err := RunCommand(prCheckoutCmd, `pr checkout 123`)
361361
eq(t, err, nil)
362-
eq(t, output, "")
362+
eq(t, output.String(), "")
363363

364364
eq(t, len(ranCommands), 2)
365365
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) {
409409

410410
output, err := RunCommand(prCheckoutCmd, `pr checkout 123`)
411411
eq(t, err, nil)
412-
eq(t, output, "")
412+
eq(t, output.String(), "")
413413

414414
eq(t, len(ranCommands), 2)
415415
eq(t, strings.Join(ranCommands[0], " "), "git fetch origin refs/pull/123/head")
@@ -459,7 +459,7 @@ func TestPRCheckout_maintainerCanModify(t *testing.T) {
459459

460460
output, err := RunCommand(prCheckoutCmd, `pr checkout 123`)
461461
eq(t, err, nil)
462-
eq(t, output, "")
462+
eq(t, output.String(), "")
463463

464464
eq(t, len(ranCommands), 4)
465465
eq(t, strings.Join(ranCommands[0], " "), "git fetch origin refs/pull/123/head:feature")

command/pr_create.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ func prCreate(cmd *cobra.Command, _ []string) error {
2727
noun = noun + "s"
2828
}
2929

30-
cmd.Printf("Warning: %d uncommitted %s\n", ucc, noun)
30+
fmt.Fprintf(cmd.ErrOrStderr(), "Warning: %d uncommitted %s\n", ucc, noun)
3131
}
3232

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

command/pr_create_test.go

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ func TestPRCreate(t *testing.T) {
9191
eq(t, reqBody.Variables.Input.BaseRefName, "master")
9292
eq(t, reqBody.Variables.Input.HeadRefName, "feature")
9393

94-
eq(t, output, "https://github.com/OWNER/REPO/pull/12\n")
94+
eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n")
9595
}
9696

9797
func TestPRCreate_web(t *testing.T) {
@@ -115,9 +115,8 @@ func TestPRCreate_web(t *testing.T) {
115115
output, err := RunCommand(prCreateCmd, `pr create --web`)
116116
eq(t, err, nil)
117117

118-
if output == "" {
119-
t.Fatal("expected output")
120-
}
118+
eq(t, output.String(), "")
119+
eq(t, output.Stderr(), "Opening https://github.com/OWNER/REPO/pull/feature in your browser.\n")
121120

122121
eq(t, len(ranCommands), 3)
123122
eq(t, strings.Join(ranCommands[1], " "), "git push --set-upstream origin HEAD:feature")
@@ -155,7 +154,6 @@ func TestPRCreate_ReportsUncommittedChanges(t *testing.T) {
155154
output, err := RunCommand(prCreateCmd, `pr create -t "my title" -b "my body"`)
156155
eq(t, err, nil)
157156

158-
eq(t, output, `Warning: 1 uncommitted change
159-
https://github.com/OWNER/REPO/pull/12
160-
`)
157+
eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n")
158+
eq(t, output.Stderr(), "Warning: 1 uncommitted change\n")
161159
}

command/pr_test.go

Lines changed: 32 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -24,16 +24,30 @@ func eq(t *testing.T, got interface{}, expected interface{}) {
2424
}
2525
}
2626

27-
func RunCommand(cmd *cobra.Command, args string) (string, error) {
27+
type cmdOut struct {
28+
outBuf, errBuf *bytes.Buffer
29+
}
30+
31+
func (c cmdOut) String() string {
32+
return c.outBuf.String()
33+
}
34+
35+
func (c cmdOut) Stderr() string {
36+
return c.errBuf.String()
37+
}
38+
39+
func RunCommand(cmd *cobra.Command, args string) (*cmdOut, error) {
2840
rootCmd := cmd.Root()
2941
argv, err := shlex.Split(args)
3042
if err != nil {
31-
return "", err
43+
return nil, err
3244
}
3345
rootCmd.SetArgs(argv)
3446

3547
outBuf := bytes.Buffer{}
3648
cmd.SetOut(&outBuf)
49+
errBuf := bytes.Buffer{}
50+
cmd.SetErr(&errBuf)
3751

3852
// Reset flag values so they don't leak between tests
3953
cmd.Flags().VisitAll(func(f *pflag.Flag) {
@@ -48,7 +62,10 @@ func RunCommand(cmd *cobra.Command, args string) (string, error) {
4862
})
4963

5064
_, err = rootCmd.ExecuteC()
51-
return outBuf.String(), err
65+
cmd.SetOut(nil)
66+
cmd.SetErr(nil)
67+
68+
return &cmdOut{&outBuf, &errBuf}, err
5269
}
5370

5471
func TestPRStatus(t *testing.T) {
@@ -72,7 +89,7 @@ func TestPRStatus(t *testing.T) {
7289
}
7390

7491
for _, r := range expectedPrs {
75-
if !r.MatchString(output) {
92+
if !r.MatchString(output.String()) {
7693
t.Errorf("output did not match regexp /%s/", r)
7794
}
7895
}
@@ -91,7 +108,7 @@ func TestPRList(t *testing.T) {
91108
t.Fatal(err)
92109
}
93110

94-
eq(t, output, `32 New feature feature
111+
eq(t, output.String(), `32 New feature feature
95112
29 Fixed bad bug hubot:bug-fix
96113
28 Improve documentation docs
97114
`)
@@ -104,11 +121,14 @@ func TestPRList_filtering(t *testing.T) {
104121
respBody := bytes.NewBufferString(`{ "data": {} }`)
105122
http.StubResponse(200, respBody)
106123

107-
_, err := RunCommand(prListCmd, `pr list -s all -l one,two -l three`)
124+
output, err := RunCommand(prListCmd, `pr list -s all -l one,two -l three`)
108125
if err != nil {
109126
t.Fatal(err)
110127
}
111128

129+
eq(t, output.String(), "")
130+
eq(t, output.Stderr(), "No pull requests match your search\n")
131+
112132
bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body)
113133
reqBody := struct {
114134
Variables struct {
@@ -183,9 +203,8 @@ func TestPRView_currentBranch(t *testing.T) {
183203
t.Errorf("error running command `pr view`: %v", err)
184204
}
185205

186-
if output == "" {
187-
t.Errorf("command output expected got an empty string")
188-
}
206+
eq(t, output.String(), "")
207+
eq(t, output.Stderr(), "Opening https://github.com/OWNER/REPO/pull/10 in your browser.\n")
189208

190209
if seenCmd == nil {
191210
t.Fatal("expected a command to run")
@@ -248,9 +267,7 @@ func TestPRView_numberArg(t *testing.T) {
248267
t.Errorf("error running command `pr view`: %v", err)
249268
}
250269

251-
if output == "" {
252-
t.Errorf("command output expected got an empty string")
253-
}
270+
eq(t, output.String(), "")
254271

255272
if seenCmd == nil {
256273
t.Fatal("expected a command to run")
@@ -281,9 +298,7 @@ func TestPRView_urlArg(t *testing.T) {
281298
t.Errorf("error running command `pr view`: %v", err)
282299
}
283300

284-
if output == "" {
285-
t.Errorf("command output expected got an empty string")
286-
}
301+
eq(t, output.String(), "")
287302

288303
if seenCmd == nil {
289304
t.Fatal("expected a command to run")
@@ -316,9 +331,7 @@ func TestPRView_branchArg(t *testing.T) {
316331
t.Errorf("error running command `pr view`: %v", err)
317332
}
318333

319-
if output == "" {
320-
t.Errorf("command output expected got an empty string")
321-
}
334+
eq(t, output.String(), "")
322335

323336
if seenCmd == nil {
324337
t.Fatal("expected a command to run")
@@ -352,9 +365,7 @@ func TestPRView_branchWithOwnerArg(t *testing.T) {
352365
t.Errorf("error running command `pr view`: %v", err)
353366
}
354367

355-
if output == "" {
356-
t.Errorf("command output expected got an empty string")
357-
}
368+
eq(t, output.String(), "")
358369

359370
if seenCmd == nil {
360371
t.Fatal("expected a command to run")

command/title_body_survey.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ func titleBodySurvey(cmd *cobra.Command, providedTitle string, providedBody stri
9292
case _unconfirmed:
9393
continue
9494
case _cancel:
95-
cmd.Println("Discarding.")
95+
fmt.Fprintln(cmd.ErrOrStderr(), "Discarding.")
9696
return nil, nil
9797
default:
9898
panic("reached unreachable case")

0 commit comments

Comments
 (0)
X Tutup