X Tutup
Skip to content

Commit d771bef

Browse files
mislavmislav
authored
Merge pull request cli#2858 from dpromanko/dpromanko/remove-set-cmd-prepare
Remove use of run.SetPrepareCmd in tests
2 parents d91b312 + 051fbbc commit d771bef

File tree

9 files changed

+276
-594
lines changed

9 files changed

+276
-594
lines changed

‎internal/run/run.go‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,21 +22,6 @@ var PrepareCmd = func(cmd *exec.Cmd) Runnable {
2222
return &cmdWithStderr{cmd}
2323
}
2424

25-
// Deprecated: use Stub
26-
func SetPrepareCmd(fn func(*exec.Cmd) Runnable) func() {
27-
origPrepare := PrepareCmd
28-
PrepareCmd = func(cmd *exec.Cmd) Runnable {
29-
// normalize git executable name for consistency in tests
30-
if baseName := filepath.Base(cmd.Args[0]); baseName == "git" || baseName == "git.exe" {
31-
cmd.Args[0] = "git"
32-
}
33-
return fn(cmd)
34-
}
35-
return func() {
36-
PrepareCmd = origPrepare
37-
}
38-
}
39-
4025
// cmdWithStderr augments exec.Cmd by adding stderr to the error message
4126
type cmdWithStderr struct {
4227
*exec.Cmd

‎internal/run/stub.go‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package run
33
import (
44
"fmt"
55
"os/exec"
6+
"path/filepath"
67
"regexp"
78
"strings"
89
)
@@ -12,9 +13,11 @@ type T interface {
1213
Errorf(string, ...interface{})
1314
}
1415

16+
// Stub installs a catch-all for all external commands invoked from gh. It returns a restore func that, when
17+
// invoked from tests, fails the current test if some stubs that were registered were never matched.
1518
func Stub() (*CommandStubber, func(T)) {
1619
cs := &CommandStubber{}
17-
teardown := SetPrepareCmd(func(cmd *exec.Cmd) Runnable {
20+
teardown := setPrepareCmd(func(cmd *exec.Cmd) Runnable {
1821
s := cs.find(cmd.Args)
1922
if s == nil {
2023
panic(fmt.Sprintf("no exec stub for `%s`", strings.Join(cmd.Args, " ")))
@@ -43,13 +46,33 @@ func Stub() (*CommandStubber, func(T)) {
4346
}
4447
}
4548

49+
func setPrepareCmd(fn func(*exec.Cmd) Runnable) func() {
50+
origPrepare := PrepareCmd
51+
PrepareCmd = func(cmd *exec.Cmd) Runnable {
52+
// normalize git executable name for consistency in tests
53+
if baseName := filepath.Base(cmd.Args[0]); baseName == "git" || baseName == "git.exe" {
54+
cmd.Args[0] = "git"
55+
}
56+
return fn(cmd)
57+
}
58+
return func() {
59+
PrepareCmd = origPrepare
60+
}
61+
}
62+
63+
// CommandStubber stubs out invocations to external commands.
4664
type CommandStubber struct {
4765
stubs []*commandStub
4866
}
4967

50-
func (cs *CommandStubber) Register(p string, exitStatus int, output string, callbacks ...CommandCallback) {
68+
// Register a stub for an external command. Pattern is a regular expression, output is the standard output
69+
// from a command. Pass callbacks to inspect raw arguments that the command was invoked with.
70+
func (cs *CommandStubber) Register(pattern string, exitStatus int, output string, callbacks ...CommandCallback) {
71+
if len(pattern) < 1 {
72+
panic("cannot use empty regexp pattern")
73+
}
5174
cs.stubs = append(cs.stubs, &commandStub{
52-
pattern: regexp.MustCompile(p),
75+
pattern: regexp.MustCompile(pattern),
5376
exitStatus: exitStatus,
5477
stdout: output,
5578
callbacks: callbacks,
@@ -76,13 +99,15 @@ type commandStub struct {
7699
callbacks []CommandCallback
77100
}
78101

102+
// Run satisfies Runnable
79103
func (s *commandStub) Run() error {
80104
if s.exitStatus != 0 {
81105
return fmt.Errorf("%s exited with status %d", s.pattern, s.exitStatus)
82106
}
83107
return nil
84108
}
85109

110+
// Output satisfies Runnable
86111
func (s *commandStub) Output() ([]byte, error) {
87112
if s.exitStatus != 0 {
88113
return []byte(nil), fmt.Errorf("%s exited with status %d", s.pattern, s.exitStatus)

‎pkg/cmd/issue/create/create_test.go‎

Lines changed: 37 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"io/ioutil"
88
"net/http"
99
"os"
10-
"os/exec"
1110
"strings"
1211
"testing"
1312

@@ -281,13 +280,14 @@ func TestIssueCreate_continueInBrowser(t *testing.T) {
281280
},
282281
})
283282

284-
var seenCmd *exec.Cmd
285-
//nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub
286-
restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
287-
seenCmd = cmd
288-
return &test.OutputStub{}
283+
cs, cmdTeardown := run.Stub()
284+
defer cmdTeardown(t)
285+
286+
cs.Register(`git rev-parse --show-toplevel`, 0, "")
287+
cs.Register(`https://github\.com`, 0, "", func(args []string) {
288+
url := strings.ReplaceAll(args[len(args)-1], "^", "")
289+
assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?body=body&title=hello", url)
289290
})
290-
defer restoreCmd()
291291

292292
output, err := runCommand(http, true, `-b body`)
293293
if err != nil {
@@ -296,17 +296,11 @@ func TestIssueCreate_continueInBrowser(t *testing.T) {
296296

297297
assert.Equal(t, "", output.String())
298298
assert.Equal(t, heredoc.Doc(`
299-
299+
300300
Creating issue in OWNER/REPO
301-
301+
302302
Opening github.com/OWNER/REPO/issues/new in your browser.
303303
`), output.Stderr())
304-
305-
if seenCmd == nil {
306-
t.Fatal("expected a command to run")
307-
}
308-
url := strings.ReplaceAll(seenCmd.Args[len(seenCmd.Args)-1], "^", "")
309-
assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?body=body&title=hello", url)
310304
}
311305

312306
func TestIssueCreate_metadata(t *testing.T) {
@@ -419,24 +413,20 @@ func TestIssueCreate_web(t *testing.T) {
419413
`),
420414
)
421415

422-
var seenCmd *exec.Cmd
423-
//nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub
424-
restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
425-
seenCmd = cmd
426-
return &test.OutputStub{}
416+
cs, cmdTeardown := run.Stub()
417+
defer cmdTeardown(t)
418+
419+
cs.Register(`git rev-parse --show-toplevel`, 0, "")
420+
cs.Register(`https://github\.com`, 0, "", func(args []string) {
421+
url := strings.ReplaceAll(args[len(args)-1], "^", "")
422+
assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?assignees=MonaLisa", url)
427423
})
428-
defer restoreCmd()
429424

430425
output, err := runCommand(http, true, `--web -a @me`)
431426
if err != nil {
432427
t.Errorf("error running command `issue create`: %v", err)
433428
}
434429

435-
if seenCmd == nil {
436-
t.Fatal("expected a command to run")
437-
}
438-
url := seenCmd.Args[len(seenCmd.Args)-1]
439-
assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?assignees=MonaLisa", url)
440430
assert.Equal(t, "", output.String())
441431
assert.Equal(t, "Opening github.com/OWNER/REPO/issues/new in your browser.\n", output.Stderr())
442432
}
@@ -445,24 +435,20 @@ func TestIssueCreate_webTitleBody(t *testing.T) {
445435
http := &httpmock.Registry{}
446436
defer http.Verify(t)
447437

448-
var seenCmd *exec.Cmd
449-
//nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub
450-
restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
451-
seenCmd = cmd
452-
return &test.OutputStub{}
438+
cs, cmdTeardown := run.Stub()
439+
defer cmdTeardown(t)
440+
441+
cs.Register(`git rev-parse --show-toplevel`, 0, "")
442+
cs.Register(`https://github\.com`, 0, "", func(args []string) {
443+
url := strings.ReplaceAll(args[len(args)-1], "^", "")
444+
assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?body=mybody&title=mytitle", url)
453445
})
454-
defer restoreCmd()
455446

456447
output, err := runCommand(http, true, `-w -t mytitle -b mybody`)
457448
if err != nil {
458449
t.Errorf("error running command `issue create`: %v", err)
459450
}
460451

461-
if seenCmd == nil {
462-
t.Fatal("expected a command to run")
463-
}
464-
url := strings.ReplaceAll(seenCmd.Args[len(seenCmd.Args)-1], "^", "")
465-
assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?body=mybody&title=mytitle", url)
466452
assert.Equal(t, "", output.String())
467453
assert.Equal(t, "Opening github.com/OWNER/REPO/issues/new in your browser.\n", output.Stderr())
468454
}
@@ -480,24 +466,20 @@ func TestIssueCreate_webTitleBodyAtMeAssignee(t *testing.T) {
480466
`),
481467
)
482468

483-
var seenCmd *exec.Cmd
484-
//nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub
485-
restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
486-
seenCmd = cmd
487-
return &test.OutputStub{}
469+
cs, cmdTeardown := run.Stub()
470+
defer cmdTeardown(t)
471+
472+
cs.Register(`git rev-parse --show-toplevel`, 0, "")
473+
cs.Register(`https://github\.com`, 0, "", func(args []string) {
474+
url := strings.ReplaceAll(args[len(args)-1], "^", "")
475+
assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?assignees=MonaLisa&body=mybody&title=mytitle", url)
488476
})
489-
defer restoreCmd()
490477

491478
output, err := runCommand(http, true, `-w -t mytitle -b mybody -a @me`)
492479
if err != nil {
493480
t.Errorf("error running command `issue create`: %v", err)
494481
}
495482

496-
if seenCmd == nil {
497-
t.Fatal("expected a command to run")
498-
}
499-
url := strings.ReplaceAll(seenCmd.Args[len(seenCmd.Args)-1], "^", "")
500-
assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?assignees=MonaLisa&body=mybody&title=mytitle", url)
501483
assert.Equal(t, "", output.String())
502484
assert.Equal(t, "Opening github.com/OWNER/REPO/issues/new in your browser.\n", output.Stderr())
503485
}
@@ -580,24 +562,20 @@ func TestIssueCreate_webProject(t *testing.T) {
580562
} } } }
581563
`))
582564

583-
var seenCmd *exec.Cmd
584-
//nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub
585-
restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
586-
seenCmd = cmd
587-
return &test.OutputStub{}
565+
cs, cmdTeardown := run.Stub()
566+
defer cmdTeardown(t)
567+
568+
cs.Register(`git rev-parse --show-toplevel`, 0, "")
569+
cs.Register(`https://github\.com`, 0, "", func(args []string) {
570+
url := strings.ReplaceAll(args[len(args)-1], "^", "")
571+
assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?projects=OWNER%2FREPO%2F1&title=Title", url)
588572
})
589-
defer restoreCmd()
590573

591574
output, err := runCommand(http, true, `-w -t Title -p Cleanup`)
592575
if err != nil {
593576
t.Errorf("error running command `issue create`: %v", err)
594577
}
595578

596-
if seenCmd == nil {
597-
t.Fatal("expected a command to run")
598-
}
599-
url := strings.ReplaceAll(seenCmd.Args[len(seenCmd.Args)-1], "^", "")
600-
assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?projects=OWNER%2FREPO%2F1&title=Title", url)
601579
assert.Equal(t, "", output.String())
602580
assert.Equal(t, "Opening github.com/OWNER/REPO/issues/new in your browser.\n", output.Stderr())
603581
}

‎pkg/cmd/issue/list/list_test.go‎

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ import (
55
"encoding/json"
66
"io/ioutil"
77
"net/http"
8-
"os/exec"
98
"regexp"
9+
"strings"
1010
"testing"
1111

1212
"github.com/MakeNowJust/heredoc"
@@ -235,29 +235,21 @@ func TestIssueList_web(t *testing.T) {
235235
http := &httpmock.Registry{}
236236
defer http.Verify(t)
237237

238-
var seenCmd *exec.Cmd
239-
//nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub
240-
restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
241-
seenCmd = cmd
242-
return &test.OutputStub{}
238+
cs, cmdTeardown := run.Stub()
239+
defer cmdTeardown(t)
240+
241+
cs.Register(`https://github\.com`, 0, "", func(args []string) {
242+
url := strings.ReplaceAll(args[len(args)-1], "^", "")
243+
assert.Equal(t, "https://github.com/OWNER/REPO/issues?q=is%3Aissue+assignee%3Apeter+label%3Abug+label%3Adocs+author%3Ajohn+mentions%3Afrank+milestone%3Av1.1", url)
243244
})
244-
defer restoreCmd()
245245

246246
output, err := runCommand(http, true, "--web -a peter -A john -l bug -l docs -L 10 -s all --mention frank --milestone v1.1")
247247
if err != nil {
248248
t.Errorf("error running command `issue list` with `--web` flag: %v", err)
249249
}
250250

251-
expectedURL := "https://github.com/OWNER/REPO/issues?q=is%3Aissue+assignee%3Apeter+label%3Abug+label%3Adocs+author%3Ajohn+mentions%3Afrank+milestone%3Av1.1"
252-
253251
assert.Equal(t, "", output.String())
254252
assert.Equal(t, "Opening github.com/OWNER/REPO/issues in your browser.\n", output.Stderr())
255-
256-
if seenCmd == nil {
257-
t.Fatal("expected a command to run")
258-
}
259-
url := seenCmd.Args[len(seenCmd.Args)-1]
260-
assert.Equal(t, expectedURL, url)
261253
}
262254

263255
func TestIssueList_milestoneNotFound(t *testing.T) {

0 commit comments

Comments
 (0)
X Tutup