From 877cbbb5dc22d9b54f34eb91ae0d7c240785da18 Mon Sep 17 00:00:00 2001 From: Devon Romanko <28825608+dpromanko@users.noreply.github.com> Date: Tue, 26 Jan 2021 07:59:03 -0500 Subject: [PATCH 01/11] use Stub instead of SetPrepareCmd in 'issue list' tests --- pkg/cmd/issue/list/list_test.go | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/pkg/cmd/issue/list/list_test.go b/pkg/cmd/issue/list/list_test.go index 003cb4407..b40701023 100644 --- a/pkg/cmd/issue/list/list_test.go +++ b/pkg/cmd/issue/list/list_test.go @@ -5,8 +5,8 @@ import ( "encoding/json" "io/ioutil" "net/http" - "os/exec" "regexp" + "strings" "testing" "github.com/MakeNowJust/heredoc" @@ -235,29 +235,21 @@ func TestIssueList_web(t *testing.T) { http := &httpmock.Registry{} defer http.Verify(t) - var seenCmd *exec.Cmd - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`https://github\.com`, 0, "", func(args []string) { + url := strings.ReplaceAll(args[len(args)-1], "^", "") + 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) }) - defer restoreCmd() output, err := runCommand(http, true, "--web -a peter -A john -l bug -l docs -L 10 -s all --mention frank --milestone v1.1") if err != nil { t.Errorf("error running command `issue list` with `--web` flag: %v", err) } - expectedURL := "https://github.com/OWNER/REPO/issues?q=is%3Aissue+assignee%3Apeter+label%3Abug+label%3Adocs+author%3Ajohn+mentions%3Afrank+milestone%3Av1.1" - assert.Equal(t, "", output.String()) assert.Equal(t, "Opening github.com/OWNER/REPO/issues in your browser.\n", output.Stderr()) - - if seenCmd == nil { - t.Fatal("expected a command to run") - } - url := seenCmd.Args[len(seenCmd.Args)-1] - assert.Equal(t, expectedURL, url) } func TestIssueList_milestoneNotFound(t *testing.T) { From 2eed1593ce28a1bd098f81d3486230ee0b8697cd Mon Sep 17 00:00:00 2001 From: Devon Romanko <28825608+dpromanko@users.noreply.github.com> Date: Tue, 26 Jan 2021 08:02:32 -0500 Subject: [PATCH 02/11] use Stub instead of SetPrepareCmd in 'pr list' tests --- pkg/cmd/pr/list/list_test.go | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/pkg/cmd/pr/list/list_test.go b/pkg/cmd/pr/list/list_test.go index b1e2bdf58..b8c5ecdb7 100644 --- a/pkg/cmd/pr/list/list_test.go +++ b/pkg/cmd/pr/list/list_test.go @@ -4,7 +4,6 @@ import ( "bytes" "io/ioutil" "net/http" - "os/exec" "strings" "testing" @@ -72,7 +71,7 @@ func TestPRList(t *testing.T) { assert.Equal(t, heredoc.Doc(` Showing 3 of 3 open pull requests in OWNER/REPO - + #32 New feature feature #29 Fixed bad bug hubot:bug-fix #28 Improve documentation docs @@ -199,27 +198,19 @@ func TestPRList_web(t *testing.T) { http := initFakeHTTP() defer http.Verify(t) - var seenCmd *exec.Cmd - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`https://github\.com`, 0, "", func(args []string) { + url := strings.ReplaceAll(args[len(args)-1], "^", "") + assert.Equal(t, "https://github.com/OWNER/REPO/pulls?q=is%3Apr+is%3Amerged+assignee%3Apeter+label%3Abug+label%3Adocs+base%3Atrunk", url) }) - defer restoreCmd() output, err := runCommand(http, true, "--web -a peter -l bug -l docs -L 10 -s merged -B trunk") if err != nil { t.Errorf("error running command `pr list` with `--web` flag: %v", err) } - expectedURL := "https://github.com/OWNER/REPO/pulls?q=is%3Apr+is%3Amerged+assignee%3Apeter+label%3Abug+label%3Adocs+base%3Atrunk" - assert.Equal(t, "", output.String()) assert.Equal(t, "Opening github.com/OWNER/REPO/pulls in your browser.\n", output.Stderr()) - - if seenCmd == nil { - t.Fatal("expected a command to run") - } - url := seenCmd.Args[len(seenCmd.Args)-1] - assert.Equal(t, url, expectedURL) } From 3068d80a279f8b2f986bd62c1be86e7918a8f88b Mon Sep 17 00:00:00 2001 From: Devon Romanko <28825608+dpromanko@users.noreply.github.com> Date: Tue, 26 Jan 2021 08:08:56 -0500 Subject: [PATCH 03/11] use Stub instead of SetPrepareCmd in 'issue view' tests --- pkg/cmd/issue/view/view_test.go | 68 +++++++++------------------------ 1 file changed, 19 insertions(+), 49 deletions(-) diff --git a/pkg/cmd/issue/view/view_test.go b/pkg/cmd/issue/view/view_test.go index 1cfc0db88..6189c93ff 100644 --- a/pkg/cmd/issue/view/view_test.go +++ b/pkg/cmd/issue/view/view_test.go @@ -5,7 +5,7 @@ import ( "fmt" "io/ioutil" "net/http" - "os/exec" + "strings" "testing" "time" @@ -72,13 +72,13 @@ func TestIssueView_web(t *testing.T) { `), ) - var seenCmd *exec.Cmd - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`https://github\.com`, 0, "", func(args []string) { + url := strings.ReplaceAll(args[len(args)-1], "^", "") + assert.Equal(t, "https://github.com/OWNER/REPO/issues/123", url) }) - defer restoreCmd() output, err := runCommand(http, true, "-w 123") if err != nil { @@ -87,12 +87,6 @@ func TestIssueView_web(t *testing.T) { assert.Equal(t, "", output.String()) assert.Equal(t, "Opening github.com/OWNER/REPO/issues/123 in your browser.\n", output.Stderr()) - - if seenCmd == nil { - t.Fatal("expected a command to run") - } - url := seenCmd.Args[len(seenCmd.Args)-1] - assert.Equal(t, "https://github.com/OWNER/REPO/issues/123", url) } func TestIssueView_web_numberArgWithHash(t *testing.T) { @@ -109,13 +103,13 @@ func TestIssueView_web_numberArgWithHash(t *testing.T) { `), ) - var seenCmd *exec.Cmd - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`https://github\.com`, 0, "", func(args []string) { + url := strings.ReplaceAll(args[len(args)-1], "^", "") + assert.Equal(t, "https://github.com/OWNER/REPO/issues/123", url) }) - defer restoreCmd() output, err := runCommand(http, true, "-w \"#123\"") if err != nil { @@ -124,12 +118,6 @@ func TestIssueView_web_numberArgWithHash(t *testing.T) { assert.Equal(t, "", output.String()) assert.Equal(t, "Opening github.com/OWNER/REPO/issues/123 in your browser.\n", output.Stderr()) - - if seenCmd == nil { - t.Fatal("expected a command to run") - } - url := seenCmd.Args[len(seenCmd.Args)-1] - assert.Equal(t, "https://github.com/OWNER/REPO/issues/123", url) } func TestIssueView_nontty_Preview(t *testing.T) { @@ -301,22 +289,10 @@ func TestIssueView_web_notFound(t *testing.T) { `), ) - var seenCmd *exec.Cmd - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} - }) - defer restoreCmd() - _, err := runCommand(http, true, "-w 9999") if err == nil || err.Error() != "GraphQL error: Could not resolve to an Issue with the number of 9999." { t.Errorf("error running command `issue view`: %v", err) } - - if seenCmd != nil { - t.Fatal("did not expect any command to run") - } } func TestIssueView_disabledIssues(t *testing.T) { @@ -353,13 +329,13 @@ func TestIssueView_web_urlArg(t *testing.T) { `), ) - var seenCmd *exec.Cmd - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`https://github\.com`, 0, "", func(args []string) { + url := strings.ReplaceAll(args[len(args)-1], "^", "") + assert.Equal(t, "https://github.com/OWNER/REPO/issues/123", url) }) - defer restoreCmd() output, err := runCommand(http, true, "-w https://github.com/OWNER/REPO/issues/123") if err != nil { @@ -367,12 +343,6 @@ func TestIssueView_web_urlArg(t *testing.T) { } assert.Equal(t, "", output.String()) - - if seenCmd == nil { - t.Fatal("expected a command to run") - } - url := seenCmd.Args[len(seenCmd.Args)-1] - assert.Equal(t, "https://github.com/OWNER/REPO/issues/123", url) } func TestIssueView_tty_Comments(t *testing.T) { From 9dcd3fbacf10ce218486844e8d5025aef8d10924 Mon Sep 17 00:00:00 2001 From: Devon Romanko <28825608+dpromanko@users.noreply.github.com> Date: Tue, 26 Jan 2021 08:24:22 -0500 Subject: [PATCH 04/11] use Stub instead of SetPrepareCmd in 'issue create' tests --- pkg/cmd/issue/create/create_test.go | 96 +++++++++++------------------ 1 file changed, 37 insertions(+), 59 deletions(-) diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go index e0bd6cbb5..3bbec9d05 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -7,7 +7,6 @@ import ( "io/ioutil" "net/http" "os" - "os/exec" "strings" "testing" @@ -281,13 +280,14 @@ func TestIssueCreate_continueInBrowser(t *testing.T) { }, }) - var seenCmd *exec.Cmd - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git rev-parse --show-toplevel`, 0, "") + cs.Register(`https://github\.com`, 0, "", func(args []string) { + url := strings.ReplaceAll(args[len(args)-1], "^", "") + assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?body=body&title=hello", url) }) - defer restoreCmd() output, err := runCommand(http, true, `-b body`) if err != nil { @@ -296,17 +296,11 @@ func TestIssueCreate_continueInBrowser(t *testing.T) { assert.Equal(t, "", output.String()) assert.Equal(t, heredoc.Doc(` - + Creating issue in OWNER/REPO - + Opening github.com/OWNER/REPO/issues/new in your browser. `), output.Stderr()) - - if seenCmd == nil { - t.Fatal("expected a command to run") - } - url := strings.ReplaceAll(seenCmd.Args[len(seenCmd.Args)-1], "^", "") - assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?body=body&title=hello", url) } func TestIssueCreate_metadata(t *testing.T) { @@ -419,24 +413,20 @@ func TestIssueCreate_web(t *testing.T) { `), ) - var seenCmd *exec.Cmd - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git rev-parse --show-toplevel`, 0, "") + cs.Register(`https://github\.com`, 0, "", func(args []string) { + url := strings.ReplaceAll(args[len(args)-1], "^", "") + assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?assignees=MonaLisa", url) }) - defer restoreCmd() output, err := runCommand(http, true, `--web -a @me`) if err != nil { t.Errorf("error running command `issue create`: %v", err) } - if seenCmd == nil { - t.Fatal("expected a command to run") - } - url := seenCmd.Args[len(seenCmd.Args)-1] - assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?assignees=MonaLisa", url) assert.Equal(t, "", output.String()) assert.Equal(t, "Opening github.com/OWNER/REPO/issues/new in your browser.\n", output.Stderr()) } @@ -445,24 +435,20 @@ func TestIssueCreate_webTitleBody(t *testing.T) { http := &httpmock.Registry{} defer http.Verify(t) - var seenCmd *exec.Cmd - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git rev-parse --show-toplevel`, 0, "") + cs.Register(`https://github\.com`, 0, "", func(args []string) { + url := strings.ReplaceAll(args[len(args)-1], "^", "") + assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?body=mybody&title=mytitle", url) }) - defer restoreCmd() output, err := runCommand(http, true, `-w -t mytitle -b mybody`) if err != nil { t.Errorf("error running command `issue create`: %v", err) } - if seenCmd == nil { - t.Fatal("expected a command to run") - } - url := strings.ReplaceAll(seenCmd.Args[len(seenCmd.Args)-1], "^", "") - assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?body=mybody&title=mytitle", url) assert.Equal(t, "", output.String()) assert.Equal(t, "Opening github.com/OWNER/REPO/issues/new in your browser.\n", output.Stderr()) } @@ -480,24 +466,20 @@ func TestIssueCreate_webTitleBodyAtMeAssignee(t *testing.T) { `), ) - var seenCmd *exec.Cmd - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git rev-parse --show-toplevel`, 0, "") + cs.Register(`https://github\.com`, 0, "", func(args []string) { + url := strings.ReplaceAll(args[len(args)-1], "^", "") + assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?assignees=MonaLisa&body=mybody&title=mytitle", url) }) - defer restoreCmd() output, err := runCommand(http, true, `-w -t mytitle -b mybody -a @me`) if err != nil { t.Errorf("error running command `issue create`: %v", err) } - if seenCmd == nil { - t.Fatal("expected a command to run") - } - url := strings.ReplaceAll(seenCmd.Args[len(seenCmd.Args)-1], "^", "") - assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?assignees=MonaLisa&body=mybody&title=mytitle", url) assert.Equal(t, "", output.String()) assert.Equal(t, "Opening github.com/OWNER/REPO/issues/new in your browser.\n", output.Stderr()) } @@ -580,24 +562,20 @@ func TestIssueCreate_webProject(t *testing.T) { } } } } `)) - var seenCmd *exec.Cmd - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git rev-parse --show-toplevel`, 0, "") + cs.Register(`https://github\.com`, 0, "", func(args []string) { + url := strings.ReplaceAll(args[len(args)-1], "^", "") + assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?projects=OWNER%2FREPO%2F1&title=Title", url) }) - defer restoreCmd() output, err := runCommand(http, true, `-w -t Title -p Cleanup`) if err != nil { t.Errorf("error running command `issue create`: %v", err) } - if seenCmd == nil { - t.Fatal("expected a command to run") - } - url := strings.ReplaceAll(seenCmd.Args[len(seenCmd.Args)-1], "^", "") - assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?projects=OWNER%2FREPO%2F1&title=Title", url) assert.Equal(t, "", output.String()) assert.Equal(t, "Opening github.com/OWNER/REPO/issues/new in your browser.\n", output.Stderr()) } From 45bc1d787c09319287106d55c1c565137c6a8244 Mon Sep 17 00:00:00 2001 From: Devon Romanko <28825608+dpromanko@users.noreply.github.com> Date: Wed, 27 Jan 2021 07:23:49 -0500 Subject: [PATCH 05/11] use Stub instead of SetPrepareCmd in 'pr view' tests --- pkg/cmd/pr/view/view_test.go | 134 ++++++++++------------------------- 1 file changed, 37 insertions(+), 97 deletions(-) diff --git a/pkg/cmd/pr/view/view_test.go b/pkg/cmd/pr/view/view_test.go index c71618cd1..525ba9de2 100644 --- a/pkg/cmd/pr/view/view_test.go +++ b/pkg/cmd/pr/view/view_test.go @@ -5,7 +5,6 @@ import ( "fmt" "io/ioutil" "net/http" - "os/exec" "strings" "testing" @@ -564,18 +563,14 @@ func TestPRView_web_currentBranch(t *testing.T) { defer http.Verify(t) http.Register(httpmock.GraphQL(`query PullRequestForBranch\b`), httpmock.FileResponse("./fixtures/prView.json")) - var seenCmd *exec.Cmd - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - switch strings.Join(cmd.Args, " ") { - case `git config --get-regexp ^branch\.blueberries\.(remote|merge)$`: - return &test.OutputStub{} - default: - seenCmd = cmd - return &test.OutputStub{} - } + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(``, 0, "") + cs.Register(`https://github\.com`, 0, "", func(args []string) { + url := strings.ReplaceAll(args[len(args)-1], "^", "") + assert.Equal(t, "https://github.com/OWNER/REPO/pull/10", url) }) - defer restoreCmd() output, err := runCommand(http, "blueberries", true, "-w") if err != nil { @@ -584,14 +579,6 @@ func TestPRView_web_currentBranch(t *testing.T) { assert.Equal(t, "", output.String()) assert.Equal(t, "Opening github.com/OWNER/REPO/pull/10 in your browser.\n", output.Stderr()) - - if seenCmd == nil { - t.Fatal("expected a command to run") - } - url := seenCmd.Args[len(seenCmd.Args)-1] - if url != "https://github.com/OWNER/REPO/pull/10" { - t.Errorf("got: %q", url) - } } func TestPRView_web_noResultsForBranch(t *testing.T) { @@ -599,27 +586,10 @@ func TestPRView_web_noResultsForBranch(t *testing.T) { defer http.Verify(t) http.Register(httpmock.GraphQL(`query PullRequestForBranch\b`), httpmock.FileResponse("./fixtures/prView_NoActiveBranch.json")) - var seenCmd *exec.Cmd - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - switch strings.Join(cmd.Args, " ") { - case `git config --get-regexp ^branch\.blueberries\.(remote|merge)$`: - return &test.OutputStub{} - default: - seenCmd = cmd - return &test.OutputStub{} - } - }) - defer restoreCmd() - _, err := runCommand(http, "blueberries", true, "-w") if err == nil || err.Error() != `no pull requests found for branch "blueberries"` { t.Errorf("error running command `pr view`: %v", err) } - - if seenCmd != nil { - t.Fatalf("unexpected command: %v", seenCmd.Args) - } } func TestPRView_web_numberArg(t *testing.T) { @@ -634,13 +604,13 @@ func TestPRView_web_numberArg(t *testing.T) { } } } }`), ) - var seenCmd *exec.Cmd - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`https://github\.com`, 0, "", func(args []string) { + url := strings.ReplaceAll(args[len(args)-1], "^", "") + assert.Equal(t, "https://github.com/OWNER/REPO/pull/23", url) }) - defer restoreCmd() output, err := runCommand(http, "master", true, "-w 23") if err != nil { @@ -648,12 +618,6 @@ func TestPRView_web_numberArg(t *testing.T) { } assert.Equal(t, "", output.String()) - - if seenCmd == nil { - t.Fatal("expected a command to run") - } - url := seenCmd.Args[len(seenCmd.Args)-1] - assert.Equal(t, "https://github.com/OWNER/REPO/pull/23", url) } func TestPRView_web_numberArgWithHash(t *testing.T) { @@ -668,13 +632,13 @@ func TestPRView_web_numberArgWithHash(t *testing.T) { } } } }`), ) - var seenCmd *exec.Cmd - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`https://github\.com`, 0, "", func(args []string) { + url := strings.ReplaceAll(args[len(args)-1], "^", "") + assert.Equal(t, "https://github.com/OWNER/REPO/pull/23", url) }) - defer restoreCmd() output, err := runCommand(http, "master", true, `-w "#23"`) if err != nil { @@ -682,12 +646,6 @@ func TestPRView_web_numberArgWithHash(t *testing.T) { } assert.Equal(t, "", output.String()) - - if seenCmd == nil { - t.Fatal("expected a command to run") - } - url := seenCmd.Args[len(seenCmd.Args)-1] - assert.Equal(t, "https://github.com/OWNER/REPO/pull/23", url) } func TestPRView_web_urlArg(t *testing.T) { @@ -702,13 +660,13 @@ func TestPRView_web_urlArg(t *testing.T) { } } } }`), ) - var seenCmd *exec.Cmd - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`https://github\.com`, 0, "", func(args []string) { + url := strings.ReplaceAll(args[len(args)-1], "^", "") + assert.Equal(t, "https://github.com/OWNER/REPO/pull/23", url) }) - defer restoreCmd() output, err := runCommand(http, "master", true, "-w https://github.com/OWNER/REPO/pull/23/files") if err != nil { @@ -716,12 +674,6 @@ func TestPRView_web_urlArg(t *testing.T) { } assert.Equal(t, "", output.String()) - - if seenCmd == nil { - t.Fatal("expected a command to run") - } - url := seenCmd.Args[len(seenCmd.Args)-1] - assert.Equal(t, "https://github.com/OWNER/REPO/pull/23", url) } func TestPRView_web_branchArg(t *testing.T) { @@ -738,13 +690,13 @@ func TestPRView_web_branchArg(t *testing.T) { ] } } } }`), ) - var seenCmd *exec.Cmd - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`https://github\.com`, 0, "", func(args []string) { + url := strings.ReplaceAll(args[len(args)-1], "^", "") + assert.Equal(t, "https://github.com/OWNER/REPO/pull/23", url) }) - defer restoreCmd() output, err := runCommand(http, "master", true, "-w blueberries") if err != nil { @@ -752,12 +704,6 @@ func TestPRView_web_branchArg(t *testing.T) { } assert.Equal(t, "", output.String()) - - if seenCmd == nil { - t.Fatal("expected a command to run") - } - url := seenCmd.Args[len(seenCmd.Args)-1] - assert.Equal(t, "https://github.com/OWNER/REPO/pull/23", url) } func TestPRView_web_branchWithOwnerArg(t *testing.T) { @@ -775,13 +721,13 @@ func TestPRView_web_branchWithOwnerArg(t *testing.T) { ] } } } }`), ) - var seenCmd *exec.Cmd - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`https://github\.com`, 0, "", func(args []string) { + url := strings.ReplaceAll(args[len(args)-1], "^", "") + assert.Equal(t, "https://github.com/hubot/REPO/pull/23", url) }) - defer restoreCmd() output, err := runCommand(http, "master", true, "-w hubot:blueberries") if err != nil { @@ -789,12 +735,6 @@ func TestPRView_web_branchWithOwnerArg(t *testing.T) { } assert.Equal(t, "", output.String()) - - if seenCmd == nil { - t.Fatal("expected a command to run") - } - url := seenCmd.Args[len(seenCmd.Args)-1] - assert.Equal(t, "https://github.com/hubot/REPO/pull/23", url) } func TestPRView_tty_Comments(t *testing.T) { From a04e0ece717fead00d5b7a8e84539c5dde88230d Mon Sep 17 00:00:00 2001 From: Devon Romanko <28825608+dpromanko@users.noreply.github.com> Date: Wed, 27 Jan 2021 07:53:21 -0500 Subject: [PATCH 06/11] use Stub instead of SetPrepareCmd in 'pr checkout' tests --- pkg/cmd/pr/checkout/checkout_test.go | 345 ++++++++------------------- 1 file changed, 105 insertions(+), 240 deletions(-) diff --git a/pkg/cmd/pr/checkout/checkout_test.go b/pkg/cmd/pr/checkout/checkout_test.go index bd0ba36ce..a92a64ccd 100644 --- a/pkg/cmd/pr/checkout/checkout_test.go +++ b/pkg/cmd/pr/checkout/checkout_test.go @@ -6,8 +6,6 @@ import ( "errors" "io/ioutil" "net/http" - "os/exec" - "strings" "testing" "github.com/cli/cli/api" @@ -108,18 +106,13 @@ func TestPRCheckout_sameRepo(t *testing.T) { } } } } `)) - ranCommands := [][]string{} - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - switch strings.Join(cmd.Args, " ") { - case "git show-ref --verify -- refs/heads/feature": - return &errorStub{"exit status: 1"} - default: - ranCommands = append(ranCommands, cmd.Args) - return &test.OutputStub{} - } - }) - defer restoreCmd() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "") + cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") + cs.Register(`git checkout feature`, 0, "") + cs.Register(`git merge --ff-only refs/remotes/origin/feature`, 0, "") output, err := runCommand(http, nil, "master", `123`) if !assert.NoError(t, err) { @@ -127,13 +120,6 @@ func TestPRCheckout_sameRepo(t *testing.T) { } assert.Equal(t, "", output.String()) - if !assert.Equal(t, 4, len(ranCommands)) { - return - } - assert.Equal(t, "git fetch origin +refs/heads/feature:refs/remotes/origin/feature", strings.Join(ranCommands[0], " ")) - assert.Equal(t, "git checkout -b feature --no-track origin/feature", strings.Join(ranCommands[1], " ")) - assert.Equal(t, "git config branch.feature.remote origin", strings.Join(ranCommands[2], " ")) - assert.Equal(t, "git config branch.feature.merge refs/heads/feature", strings.Join(ranCommands[3], " ")) } func TestPRCheckout_urlArg(t *testing.T) { @@ -154,25 +140,17 @@ func TestPRCheckout_urlArg(t *testing.T) { } } } } `)) - ranCommands := [][]string{} - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - switch strings.Join(cmd.Args, " ") { - case "git show-ref --verify -- refs/heads/feature": - return &errorStub{"exit status: 1"} - default: - ranCommands = append(ranCommands, cmd.Args) - return &test.OutputStub{} - } - }) - defer restoreCmd() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "") + cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") + cs.Register(`git checkout feature`, 0, "") + cs.Register(`git merge --ff-only refs/remotes/origin/feature`, 0, "") output, err := runCommand(http, nil, "master", `https://github.com/OWNER/REPO/pull/123/files`) assert.NoError(t, err) assert.Equal(t, "", output.String()) - - assert.Equal(t, 4, len(ranCommands)) - assert.Equal(t, "git checkout -b feature --no-track origin/feature", strings.Join(ranCommands[1], " ")) } func TestPRCheckout_urlArg_differentBase(t *testing.T) { @@ -194,18 +172,14 @@ func TestPRCheckout_urlArg_differentBase(t *testing.T) { `)) http.StubRepoInfoResponse("OWNER", "REPO", "master") - ranCommands := [][]string{} - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - switch strings.Join(cmd.Args, " ") { - case "git show-ref --verify -- refs/heads/feature": - return &errorStub{"exit status: 1"} - default: - ranCommands = append(ranCommands, cmd.Args) - return &test.OutputStub{} - } - }) - defer restoreCmd() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git fetch https://github\.com/OTHER/POE\.git refs/pull/123/head:feature`, 0, "") + cs.Register(`git config branch\.feature\.merge`, 0, "") + cs.Register(`git checkout feature`, 0, "") + cs.Register(`git config branch\.feature\.remote https://github\.com/OTHER/POE\.git`, 0, "") + cs.Register(`git config branch\.feature\.merge refs/pull/123/head`, 0, "") output, err := runCommand(http, nil, "master", `https://github.com/OTHER/POE/pull/123/files`) assert.NoError(t, err) @@ -222,10 +196,6 @@ func TestPRCheckout_urlArg_differentBase(t *testing.T) { assert.Equal(t, "OTHER", reqBody.Variables.Owner) assert.Equal(t, "POE", reqBody.Variables.Repo) - - assert.Equal(t, 5, len(ranCommands)) - assert.Equal(t, "git fetch https://github.com/OTHER/POE.git refs/pull/123/head:feature", strings.Join(ranCommands[1], " ")) - assert.Equal(t, "git config branch.feature.remote https://github.com/OTHER/POE.git", strings.Join(ranCommands[3], " ")) } func TestPRCheckout_branchArg(t *testing.T) { @@ -247,25 +217,18 @@ func TestPRCheckout_branchArg(t *testing.T) { ] } } } } `)) - ranCommands := [][]string{} - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - switch strings.Join(cmd.Args, " ") { - case "git show-ref --verify -- refs/heads/feature": - return &errorStub{"exit status: 1"} - default: - ranCommands = append(ranCommands, cmd.Args) - return &test.OutputStub{} - } - }) - defer restoreCmd() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "") + cs.Register(`git config branch\.feature\.merge`, 0, "") + cs.Register(`git checkout feature`, 0, "") + cs.Register(`git config branch\.feature\.remote origin`, 0, "") + cs.Register(`git config branch\.feature\.merge refs/pull/123/head`, 0, "") output, err := runCommand(http, nil, "master", `hubot:feature`) assert.NoError(t, err) assert.Equal(t, "", output.String()) - - assert.Equal(t, 5, len(ranCommands)) - assert.Equal(t, "git fetch origin refs/pull/123/head:feature", strings.Join(ranCommands[1], " ")) } func TestPRCheckout_existingBranch(t *testing.T) { @@ -287,27 +250,17 @@ func TestPRCheckout_existingBranch(t *testing.T) { } } } } `)) - ranCommands := [][]string{} - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - switch strings.Join(cmd.Args, " ") { - case "git show-ref --verify -- refs/heads/feature": - return &test.OutputStub{} - default: - ranCommands = append(ranCommands, cmd.Args) - return &test.OutputStub{} - } - }) - defer restoreCmd() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "") + cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") + cs.Register(`git checkout feature`, 0, "") + cs.Register(`git merge --ff-only refs/remotes/origin/feature`, 0, "") output, err := runCommand(http, nil, "master", `123`) assert.NoError(t, err) assert.Equal(t, "", output.String()) - - assert.Equal(t, 3, len(ranCommands)) - assert.Equal(t, "git fetch origin +refs/heads/feature:refs/remotes/origin/feature", strings.Join(ranCommands[0], " ")) - assert.Equal(t, "git checkout feature", strings.Join(ranCommands[1], " ")) - assert.Equal(t, "git merge --ff-only refs/remotes/origin/feature", strings.Join(ranCommands[2], " ")) } func TestPRCheckout_differentRepo_remoteExists(t *testing.T) { @@ -340,28 +293,17 @@ func TestPRCheckout_differentRepo_remoteExists(t *testing.T) { } } } } `)) - ranCommands := [][]string{} - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - switch strings.Join(cmd.Args, " ") { - case "git show-ref --verify -- refs/heads/feature": - return &errorStub{"exit status: 1"} - default: - ranCommands = append(ranCommands, cmd.Args) - return &test.OutputStub{} - } - }) - defer restoreCmd() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git fetch robot-fork \+refs/heads/feature:refs/remotes/robot-fork/feature`, 0, "") + cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") + cs.Register(`git checkout feature`, 0, "") + cs.Register(`git merge --ff-only refs/remotes/robot-fork/feature`, 0, "") output, err := runCommand(http, remotes, "master", `123`) assert.NoError(t, err) assert.Equal(t, "", output.String()) - - assert.Equal(t, 4, len(ranCommands)) - assert.Equal(t, "git fetch robot-fork +refs/heads/feature:refs/remotes/robot-fork/feature", strings.Join(ranCommands[0], " ")) - assert.Equal(t, "git checkout -b feature --no-track robot-fork/feature", strings.Join(ranCommands[1], " ")) - assert.Equal(t, "git config branch.feature.remote robot-fork", strings.Join(ranCommands[2], " ")) - assert.Equal(t, "git config branch.feature.merge refs/heads/feature", strings.Join(ranCommands[3], " ")) } func TestPRCheckout_differentRepo(t *testing.T) { @@ -383,28 +325,18 @@ func TestPRCheckout_differentRepo(t *testing.T) { } } } } `)) - ranCommands := [][]string{} - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - switch strings.Join(cmd.Args, " ") { - case "git config branch.feature.merge": - return &errorStub{"exit status 1"} - default: - ranCommands = append(ranCommands, cmd.Args) - return &test.OutputStub{} - } - }) - defer restoreCmd() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "") + cs.Register(`git config branch\.feature\.merge`, 0, "") + cs.Register(`git checkout feature`, 0, "") + cs.Register(`git config branch\.feature\.remote origin`, 0, "") + cs.Register(`git config branch\.feature\.merge refs/pull/123/head`, 0, "") output, err := runCommand(http, nil, "master", `123`) assert.NoError(t, err) assert.Equal(t, "", output.String()) - - assert.Equal(t, 4, len(ranCommands)) - assert.Equal(t, "git fetch origin refs/pull/123/head:feature", strings.Join(ranCommands[0], " ")) - assert.Equal(t, "git checkout feature", strings.Join(ranCommands[1], " ")) - assert.Equal(t, "git config branch.feature.remote origin", strings.Join(ranCommands[2], " ")) - assert.Equal(t, "git config branch.feature.merge refs/pull/123/head", strings.Join(ranCommands[3], " ")) } func TestPRCheckout_differentRepo_existingBranch(t *testing.T) { @@ -426,26 +358,18 @@ func TestPRCheckout_differentRepo_existingBranch(t *testing.T) { } } } } `)) - ranCommands := [][]string{} - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - switch strings.Join(cmd.Args, " ") { - case "git config branch.feature.merge": - return &test.OutputStub{Out: []byte("refs/heads/feature\n")} - default: - ranCommands = append(ranCommands, cmd.Args) - return &test.OutputStub{} - } - }) - defer restoreCmd() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "") + cs.Register(`git config branch\.feature\.merge`, 0, "") + cs.Register(`git checkout feature`, 0, "") + cs.Register(`git config branch\.feature\.remote origin`, 0, "") + cs.Register(`git config branch\.feature\.merge refs/pull/123/head`, 0, "") output, err := runCommand(http, nil, "master", `123`) assert.NoError(t, err) assert.Equal(t, "", output.String()) - - assert.Equal(t, 2, len(ranCommands)) - assert.Equal(t, "git fetch origin refs/pull/123/head:feature", strings.Join(ranCommands[0], " ")) - assert.Equal(t, "git checkout feature", strings.Join(ranCommands[1], " ")) } func TestPRCheckout_detachedHead(t *testing.T) { @@ -467,26 +391,18 @@ func TestPRCheckout_detachedHead(t *testing.T) { } } } } `)) - ranCommands := [][]string{} - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - switch strings.Join(cmd.Args, " ") { - case "git config branch.feature.merge": - return &test.OutputStub{Out: []byte("refs/heads/feature\n")} - default: - ranCommands = append(ranCommands, cmd.Args) - return &test.OutputStub{} - } - }) - defer restoreCmd() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "") + cs.Register(`git config branch\.feature\.merge`, 0, "") + cs.Register(`git checkout feature`, 0, "") + cs.Register(`git config branch\.feature\.remote https://github\.com/hubot/REPO\.git`, 0, "") + cs.Register(`git config branch\.feature\.merge refs/heads/feature`, 0, "") output, err := runCommand(http, nil, "", `123`) assert.NoError(t, err) assert.Equal(t, "", output.String()) - - assert.Equal(t, 2, len(ranCommands)) - assert.Equal(t, "git fetch origin refs/pull/123/head:feature", strings.Join(ranCommands[0], " ")) - assert.Equal(t, "git checkout feature", strings.Join(ranCommands[1], " ")) } func TestPRCheckout_differentRepo_currentBranch(t *testing.T) { @@ -508,26 +424,18 @@ func TestPRCheckout_differentRepo_currentBranch(t *testing.T) { } } } } `)) - ranCommands := [][]string{} - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - switch strings.Join(cmd.Args, " ") { - case "git config branch.feature.merge": - return &test.OutputStub{Out: []byte("refs/heads/feature\n")} - default: - ranCommands = append(ranCommands, cmd.Args) - return &test.OutputStub{} - } - }) - defer restoreCmd() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git fetch origin refs/pull/123/head`, 0, "") + cs.Register(`git config branch\.feature\.merge`, 0, "") + cs.Register(`git merge --ff-only FETCH_HEAD`, 0, "") + cs.Register(`git config branch\.feature\.remote origin`, 0, "") + cs.Register(`git config branch\.feature\.merge refs/pull/123/head`, 0, "") output, err := runCommand(http, nil, "feature", `123`) assert.NoError(t, err) assert.Equal(t, "", output.String()) - - assert.Equal(t, 2, len(ranCommands)) - assert.Equal(t, "git fetch origin refs/pull/123/head", strings.Join(ranCommands[0], " ")) - assert.Equal(t, "git merge --ff-only FETCH_HEAD", strings.Join(ranCommands[1], " ")) } func TestPRCheckout_differentRepo_invalidBranchName(t *testing.T) { @@ -549,13 +457,6 @@ func TestPRCheckout_differentRepo_invalidBranchName(t *testing.T) { } } } } `)) - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - t.Errorf("unexpected external invocation: %v", cmd.Args) - return &test.OutputStub{} - }) - defer restoreCmd() - output, err := runCommand(http, nil, "master", `123`) assert.EqualError(t, err, `invalid branch name: "-foo"`) assert.Equal(t, "", output.Stderr()) @@ -580,28 +481,18 @@ func TestPRCheckout_maintainerCanModify(t *testing.T) { } } } } `)) - ranCommands := [][]string{} - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - switch strings.Join(cmd.Args, " ") { - case "git config branch.feature.merge": - return &errorStub{"exit status 1"} - default: - ranCommands = append(ranCommands, cmd.Args) - return &test.OutputStub{} - } - }) - defer restoreCmd() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "") + cs.Register(`git config branch\.feature\.merge`, 0, "") + cs.Register(`git checkout feature`, 0, "") + cs.Register(`git config branch\.feature\.remote https://github\.com/hubot/REPO\.git`, 0, "") + cs.Register(`git config branch\.feature\.merge refs/heads/feature`, 0, "") output, err := runCommand(http, nil, "master", `123`) assert.NoError(t, err) assert.Equal(t, "", output.String()) - - assert.Equal(t, 4, len(ranCommands)) - assert.Equal(t, "git fetch origin refs/pull/123/head:feature", strings.Join(ranCommands[0], " ")) - assert.Equal(t, "git checkout feature", strings.Join(ranCommands[1], " ")) - assert.Equal(t, "git config branch.feature.remote https://github.com/hubot/REPO.git", strings.Join(ranCommands[2], " ")) - assert.Equal(t, "git config branch.feature.merge refs/heads/feature", strings.Join(ranCommands[3], " ")) } func TestPRCheckout_recurseSubmodules(t *testing.T) { @@ -622,29 +513,19 @@ func TestPRCheckout_recurseSubmodules(t *testing.T) { } } } } `)) - ranCommands := [][]string{} - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - switch strings.Join(cmd.Args, " ") { - case "git show-ref --verify -- refs/heads/feature": - return &test.OutputStub{} - default: - ranCommands = append(ranCommands, cmd.Args) - return &test.OutputStub{} - } - }) - defer restoreCmd() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "") + cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") + cs.Register(`git checkout feature`, 0, "") + cs.Register(`git merge --ff-only refs/remotes/origin/feature`, 0, "") + cs.Register(`git submodule sync --recursive`, 0, "") + cs.Register(`git submodule update --init --recursive`, 0, "") output, err := runCommand(http, nil, "master", `123 --recurse-submodules`) assert.NoError(t, err) assert.Equal(t, "", output.String()) - - assert.Equal(t, 5, len(ranCommands)) - assert.Equal(t, "git fetch origin +refs/heads/feature:refs/remotes/origin/feature", strings.Join(ranCommands[0], " ")) - assert.Equal(t, "git checkout feature", strings.Join(ranCommands[1], " ")) - assert.Equal(t, "git merge --ff-only refs/remotes/origin/feature", strings.Join(ranCommands[2], " ")) - assert.Equal(t, "git submodule sync --recursive", strings.Join(ranCommands[3], " ")) - assert.Equal(t, "git submodule update --init --recursive", strings.Join(ranCommands[4], " ")) } func TestPRCheckout_force(t *testing.T) { @@ -665,28 +546,18 @@ func TestPRCheckout_force(t *testing.T) { } } } } `)) - ranCommands := [][]string{} - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - switch strings.Join(cmd.Args, " ") { - case "git show-ref --verify -- refs/heads/feature": - return &test.OutputStub{} - default: - ranCommands = append(ranCommands, cmd.Args) - return &test.OutputStub{} - } - }) - defer restoreCmd() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "") + cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") + cs.Register(`git checkout feature`, 0, "") + cs.Register(`git reset --hard refs/remotes/origin/feature`, 0, "") output, err := runCommand(http, nil, "master", `123 --force`) assert.NoError(t, err) assert.Equal(t, "", output.String()) - - assert.Equal(t, len(ranCommands), 3) - assert.Equal(t, "git fetch origin +refs/heads/feature:refs/remotes/origin/feature", strings.Join(ranCommands[0], " ")) - assert.Equal(t, "git checkout feature", strings.Join(ranCommands[1], " ")) - assert.Equal(t, "git reset --hard refs/remotes/origin/feature", strings.Join(ranCommands[2], " ")) } func TestPRCheckout_detach(t *testing.T) { @@ -708,19 +579,13 @@ func TestPRCheckout_detach(t *testing.T) { } } } } `)) - ranCommands := [][]string{} - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - ranCommands = append(ranCommands, cmd.Args) - return &test.OutputStub{} - }) - defer restoreCmd() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git checkout --detach FETCH_HEAD`, 0, "") + cs.Register(`git fetch origin refs/pull/123/head`, 0, "") output, err := runCommand(http, nil, "", `123 --detach`) assert.Nil(t, err) assert.Equal(t, "", output.String()) - - assert.Equal(t, 2, len(ranCommands)) - assert.Equal(t, "git fetch origin refs/pull/123/head", strings.Join(ranCommands[0], " ")) - assert.Equal(t, "git checkout --detach FETCH_HEAD", strings.Join(ranCommands[1], " ")) } From 696cbfc8d1443f8263e415184fb533cd993335dd Mon Sep 17 00:00:00 2001 From: Devon Romanko <28825608+dpromanko@users.noreply.github.com> Date: Wed, 27 Jan 2021 07:59:32 -0500 Subject: [PATCH 07/11] use Stub instead of SetPrepareCmd in 'repo create' tests --- pkg/cmd/repo/create/create_test.go | 120 ++++++++--------------------- 1 file changed, 30 insertions(+), 90 deletions(-) diff --git a/pkg/cmd/repo/create/create_test.go b/pkg/cmd/repo/create/create_test.go index 70d6cc763..81df0ebd6 100644 --- a/pkg/cmd/repo/create/create_test.go +++ b/pkg/cmd/repo/create/create_test.go @@ -3,11 +3,8 @@ package create import ( "bytes" "encoding/json" - "errors" "io/ioutil" "net/http" - "os/exec" - "strings" "testing" "github.com/cli/cli/internal/config" @@ -84,13 +81,11 @@ func TestRepoCreate(t *testing.T) { httpClient := &http.Client{Transport: reg} - var seenCmd *exec.Cmd - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} - }) - defer restoreCmd() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git remote add -f origin https://github\.com/OWNER/REPO\.git`, 0, "") + cs.Register(`git rev-parse --show-toplevel`, 0, "") as, surveyTearDown := prompt.InitAskStubber() defer surveyTearDown() @@ -116,11 +111,6 @@ func TestRepoCreate(t *testing.T) { assert.Equal(t, "", output.String()) assert.Equal(t, "✓ Created repository OWNER/REPO on GitHub\n✓ Added remote https://github.com/OWNER/REPO.git\n", output.Stderr()) - if seenCmd == nil { - t.Fatal("expected a command to run") - } - assert.Equal(t, "git remote add -f origin https://github.com/OWNER/REPO.git", strings.Join(seenCmd.Args, " ")) - var reqBody struct { Query string Variables struct { @@ -163,25 +153,11 @@ func TestRepoCreate_outsideGitWorkDir(t *testing.T) { httpClient := &http.Client{Transport: reg} - var seenCmds []*exec.Cmd - cmdOutputs := []test.OutputStub{ - { - Error: errors.New("Not a git repository"), - }, - {}, - {}, - } - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - if len(cmdOutputs) == 0 { - t.Fatal("Too many calls to git command") - } - out := cmdOutputs[0] - cmdOutputs = cmdOutputs[1:] - seenCmds = append(seenCmds, cmd) - return &out - }) - defer restoreCmd() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git remote add -f origin https://github\.com/OWNER/REPO\.git`, 0, "") + cs.Register(`git rev-parse --show-toplevel`, 0, "") output, err := runCommand(httpClient, "REPO --private --confirm", false) if err != nil { @@ -191,14 +167,6 @@ func TestRepoCreate_outsideGitWorkDir(t *testing.T) { assert.Equal(t, "https://github.com/OWNER/REPO\n", output.String()) assert.Equal(t, "", output.Stderr()) - if len(seenCmds) != 3 { - t.Fatal("expected three commands to run") - } - - assert.Equal(t, "git rev-parse --show-toplevel", strings.Join(seenCmds[0].Args, " ")) - assert.Equal(t, "git init REPO", strings.Join(seenCmds[1].Args, " ")) - assert.Equal(t, "git -C REPO remote add origin https://github.com/OWNER/REPO.git", strings.Join(seenCmds[2].Args, " ")) - var reqBody struct { Query string Variables struct { @@ -245,13 +213,11 @@ func TestRepoCreate_org(t *testing.T) { } } }`)) httpClient := &http.Client{Transport: reg} - var seenCmd *exec.Cmd - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} - }) - defer restoreCmd() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git remote add -f origin https://github\.com/ORG/REPO\.git`, 0, "") + cs.Register(`git rev-parse --show-toplevel`, 0, "") as, surveyTearDown := prompt.InitAskStubber() defer surveyTearDown() @@ -277,11 +243,6 @@ func TestRepoCreate_org(t *testing.T) { assert.Equal(t, "", output.String()) assert.Equal(t, "✓ Created repository ORG/REPO on GitHub\n✓ Added remote https://github.com/ORG/REPO.git\n", output.Stderr()) - if seenCmd == nil { - t.Fatal("expected a command to run") - } - assert.Equal(t, "git remote add -f origin https://github.com/ORG/REPO.git", strings.Join(seenCmd.Args, " ")) - var reqBody struct { Query string Variables struct { @@ -328,13 +289,11 @@ func TestRepoCreate_orgWithTeam(t *testing.T) { } } }`)) httpClient := &http.Client{Transport: reg} - var seenCmd *exec.Cmd - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} - }) - defer restoreCmd() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git remote add -f origin https://github\.com/ORG/REPO\.git`, 0, "") + cs.Register(`git rev-parse --show-toplevel`, 0, "") as, surveyTearDown := prompt.InitAskStubber() defer surveyTearDown() @@ -360,11 +319,6 @@ func TestRepoCreate_orgWithTeam(t *testing.T) { assert.Equal(t, "", output.String()) assert.Equal(t, "✓ Created repository ORG/REPO on GitHub\n✓ Added remote https://github.com/ORG/REPO.git\n", output.Stderr()) - if seenCmd == nil { - t.Fatal("expected a command to run") - } - assert.Equal(t, "git remote add -f origin https://github.com/ORG/REPO.git", strings.Join(seenCmd.Args, " ")) - var reqBody struct { Query string Variables struct { @@ -412,13 +366,11 @@ func TestRepoCreate_template(t *testing.T) { httpClient := &http.Client{Transport: reg} - var seenCmd *exec.Cmd - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} - }) - defer restoreCmd() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git remote add -f origin https://github\.com/OWNER/REPO\.git`, 0, "") + cs.Register(`git rev-parse --show-toplevel`, 0, "") as, surveyTearDown := prompt.InitAskStubber() defer surveyTearDown() @@ -444,11 +396,6 @@ func TestRepoCreate_template(t *testing.T) { assert.Equal(t, "", output.String()) assert.Equal(t, "✓ Created repository OWNER/REPO on GitHub\n✓ Added remote https://github.com/OWNER/REPO.git\n", output.Stderr()) - if seenCmd == nil { - t.Fatal("expected a command to run") - } - assert.Equal(t, "git remote add -f origin https://github.com/OWNER/REPO.git", strings.Join(seenCmd.Args, " ")) - var reqBody struct { Query string Variables struct { @@ -493,13 +440,11 @@ func TestRepoCreate_withoutNameArg(t *testing.T) { } } }`)) httpClient := &http.Client{Transport: reg} - var seenCmd *exec.Cmd - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} - }) - defer restoreCmd() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git remote add -f origin https://github\.com/OWNER/REPO\.git`, 0, "") + cs.Register(`git rev-parse --show-toplevel`, 0, "") as, surveyTearDown := prompt.InitAskStubber() defer surveyTearDown() @@ -533,11 +478,6 @@ func TestRepoCreate_withoutNameArg(t *testing.T) { assert.Equal(t, "", output.String()) assert.Equal(t, "✓ Created repository OWNER/REPO on GitHub\n✓ Added remote https://github.com/OWNER/REPO.git\n", output.Stderr()) - if seenCmd == nil { - t.Fatal("expected a command to run") - } - assert.Equal(t, "git remote add -f origin https://github.com/OWNER/REPO.git", strings.Join(seenCmd.Args, " ")) - var reqBody struct { Query string Variables struct { From cb897fd7e2b953bba02ddaff0d5a241f0050fc49 Mon Sep 17 00:00:00 2001 From: Devon Romanko <28825608+dpromanko@users.noreply.github.com> Date: Wed, 27 Jan 2021 08:04:57 -0500 Subject: [PATCH 08/11] remove unused errorStub from 'pr checkout' test --- pkg/cmd/pr/checkout/checkout_test.go | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/pkg/cmd/pr/checkout/checkout_test.go b/pkg/cmd/pr/checkout/checkout_test.go index a92a64ccd..ed1a4722e 100644 --- a/pkg/cmd/pr/checkout/checkout_test.go +++ b/pkg/cmd/pr/checkout/checkout_test.go @@ -3,7 +3,6 @@ package checkout import ( "bytes" "encoding/json" - "errors" "io/ioutil" "net/http" "testing" @@ -22,18 +21,6 @@ import ( "github.com/stretchr/testify/assert" ) -type errorStub struct { - message string -} - -func (s errorStub) Output() ([]byte, error) { - return nil, errors.New(s.message) -} - -func (s errorStub) Run() error { - return errors.New(s.message) -} - func runCommand(rt http.RoundTripper, remotes context.Remotes, branch string, cli string) (*test.CmdOut, error) { io, _, stdout, stderr := iostreams.Test() From 2964895a7746f9bb36e37e72b3f9c9869ae0a3d8 Mon Sep 17 00:00:00 2001 From: Devon Romanko <28825608+dpromanko@users.noreply.github.com> Date: Wed, 27 Jan 2021 17:55:28 -0500 Subject: [PATCH 09/11] fix test behavior changes from migration to run.Stub --- pkg/cmd/issue/view/view_test.go | 3 +++ pkg/cmd/pr/checkout/checkout_test.go | 40 ++++++++++++++-------------- pkg/cmd/pr/view/view_test.go | 7 ++++- pkg/cmd/repo/create/create_test.go | 5 ++-- 4 files changed, 32 insertions(+), 23 deletions(-) diff --git a/pkg/cmd/issue/view/view_test.go b/pkg/cmd/issue/view/view_test.go index 6189c93ff..3fba1eea8 100644 --- a/pkg/cmd/issue/view/view_test.go +++ b/pkg/cmd/issue/view/view_test.go @@ -289,6 +289,9 @@ func TestIssueView_web_notFound(t *testing.T) { `), ) + _, cmdTeardown := run.Stub() + defer cmdTeardown(t) + _, err := runCommand(http, true, "-w 9999") if err == nil || err.Error() != "GraphQL error: Could not resolve to an Issue with the number of 9999." { t.Errorf("error running command `issue view`: %v", err) diff --git a/pkg/cmd/pr/checkout/checkout_test.go b/pkg/cmd/pr/checkout/checkout_test.go index ed1a4722e..b9e7308de 100644 --- a/pkg/cmd/pr/checkout/checkout_test.go +++ b/pkg/cmd/pr/checkout/checkout_test.go @@ -97,9 +97,10 @@ func TestPRCheckout_sameRepo(t *testing.T) { defer cmdTeardown(t) cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "") - cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") - cs.Register(`git checkout feature`, 0, "") - cs.Register(`git merge --ff-only refs/remotes/origin/feature`, 0, "") + cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "") + cs.Register(`git checkout -b feature --no-track origin/feature`, 0, "") + cs.Register(`git config branch\.feature\.remote origin`, 0, "") + cs.Register(`git config branch\.feature\.merge refs/heads/feature`, 0, "") output, err := runCommand(http, nil, "master", `123`) if !assert.NoError(t, err) { @@ -131,9 +132,10 @@ func TestPRCheckout_urlArg(t *testing.T) { defer cmdTeardown(t) cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "") - cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") - cs.Register(`git checkout feature`, 0, "") - cs.Register(`git merge --ff-only refs/remotes/origin/feature`, 0, "") + cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "") + cs.Register(`git checkout -b feature --no-track origin/feature`, 0, "") + cs.Register(`git config branch\.feature\.remote origin`, 0, "") + cs.Register(`git config branch\.feature\.merge refs/heads/feature`, 0, "") output, err := runCommand(http, nil, "master", `https://github.com/OWNER/REPO/pull/123/files`) assert.NoError(t, err) @@ -284,9 +286,10 @@ func TestPRCheckout_differentRepo_remoteExists(t *testing.T) { defer cmdTeardown(t) cs.Register(`git fetch robot-fork \+refs/heads/feature:refs/remotes/robot-fork/feature`, 0, "") - cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") - cs.Register(`git checkout feature`, 0, "") - cs.Register(`git merge --ff-only refs/remotes/robot-fork/feature`, 0, "") + cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "") + cs.Register(`git checkout -b feature --no-track robot-fork/feature`, 0, "") + cs.Register(`git config branch\.feature\.remote robot-fork`, 0, "") + cs.Register(`git config branch\.feature\.merge refs/heads/feature`, 0, "") output, err := runCommand(http, remotes, "master", `123`) assert.NoError(t, err) @@ -316,7 +319,7 @@ func TestPRCheckout_differentRepo(t *testing.T) { defer cmdTeardown(t) cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "") - cs.Register(`git config branch\.feature\.merge`, 0, "") + cs.Register(`git config branch\.feature\.merge`, 1, "") cs.Register(`git checkout feature`, 0, "") cs.Register(`git config branch\.feature\.remote origin`, 0, "") cs.Register(`git config branch\.feature\.merge refs/pull/123/head`, 0, "") @@ -349,10 +352,8 @@ func TestPRCheckout_differentRepo_existingBranch(t *testing.T) { defer cmdTeardown(t) cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "") - cs.Register(`git config branch\.feature\.merge`, 0, "") + cs.Register(`git config branch\.feature\.merge`, 0, "refs/heads/feature\n") cs.Register(`git checkout feature`, 0, "") - cs.Register(`git config branch\.feature\.remote origin`, 0, "") - cs.Register(`git config branch\.feature\.merge refs/pull/123/head`, 0, "") output, err := runCommand(http, nil, "master", `123`) assert.NoError(t, err) @@ -382,10 +383,8 @@ func TestPRCheckout_detachedHead(t *testing.T) { defer cmdTeardown(t) cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "") - cs.Register(`git config branch\.feature\.merge`, 0, "") + cs.Register(`git config branch\.feature\.merge`, 0, "refs/heads/feature\n") cs.Register(`git checkout feature`, 0, "") - cs.Register(`git config branch\.feature\.remote https://github\.com/hubot/REPO\.git`, 0, "") - cs.Register(`git config branch\.feature\.merge refs/heads/feature`, 0, "") output, err := runCommand(http, nil, "", `123`) assert.NoError(t, err) @@ -415,10 +414,8 @@ func TestPRCheckout_differentRepo_currentBranch(t *testing.T) { defer cmdTeardown(t) cs.Register(`git fetch origin refs/pull/123/head`, 0, "") - cs.Register(`git config branch\.feature\.merge`, 0, "") + cs.Register(`git config branch\.feature\.merge`, 0, "refs/heads/feature\n") cs.Register(`git merge --ff-only FETCH_HEAD`, 0, "") - cs.Register(`git config branch\.feature\.remote origin`, 0, "") - cs.Register(`git config branch\.feature\.merge refs/pull/123/head`, 0, "") output, err := runCommand(http, nil, "feature", `123`) assert.NoError(t, err) @@ -444,6 +441,9 @@ func TestPRCheckout_differentRepo_invalidBranchName(t *testing.T) { } } } } `)) + _, cmdTeardown := run.Stub() + defer cmdTeardown(t) + output, err := runCommand(http, nil, "master", `123`) assert.EqualError(t, err, `invalid branch name: "-foo"`) assert.Equal(t, "", output.Stderr()) @@ -472,7 +472,7 @@ func TestPRCheckout_maintainerCanModify(t *testing.T) { defer cmdTeardown(t) cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "") - cs.Register(`git config branch\.feature\.merge`, 0, "") + cs.Register(`git config branch\.feature\.merge`, 1, "") cs.Register(`git checkout feature`, 0, "") cs.Register(`git config branch\.feature\.remote https://github\.com/hubot/REPO\.git`, 0, "") cs.Register(`git config branch\.feature\.merge refs/heads/feature`, 0, "") diff --git a/pkg/cmd/pr/view/view_test.go b/pkg/cmd/pr/view/view_test.go index 525ba9de2..879dcd8f9 100644 --- a/pkg/cmd/pr/view/view_test.go +++ b/pkg/cmd/pr/view/view_test.go @@ -566,7 +566,7 @@ func TestPRView_web_currentBranch(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register(``, 0, "") + cs.Register(`git config --get-regexp.+branch\\\.blueberries\\\.`, 0, "") cs.Register(`https://github\.com`, 0, "", func(args []string) { url := strings.ReplaceAll(args[len(args)-1], "^", "") assert.Equal(t, "https://github.com/OWNER/REPO/pull/10", url) @@ -586,6 +586,11 @@ func TestPRView_web_noResultsForBranch(t *testing.T) { defer http.Verify(t) http.Register(httpmock.GraphQL(`query PullRequestForBranch\b`), httpmock.FileResponse("./fixtures/prView_NoActiveBranch.json")) + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git config --get-regexp.+branch\\\.blueberries\\\.`, 0, "") + _, err := runCommand(http, "blueberries", true, "-w") if err == nil || err.Error() != `no pull requests found for branch "blueberries"` { t.Errorf("error running command `pr view`: %v", err) diff --git a/pkg/cmd/repo/create/create_test.go b/pkg/cmd/repo/create/create_test.go index 81df0ebd6..9914807ea 100644 --- a/pkg/cmd/repo/create/create_test.go +++ b/pkg/cmd/repo/create/create_test.go @@ -156,8 +156,9 @@ func TestRepoCreate_outsideGitWorkDir(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register(`git remote add -f origin https://github\.com/OWNER/REPO\.git`, 0, "") - cs.Register(`git rev-parse --show-toplevel`, 0, "") + cs.Register(`git rev-parse --show-toplevel`, 1, "") + cs.Register(`git init REPO`, 0, "") + cs.Register(`git -C REPO remote add origin https://github\.com/OWNER/REPO\.git`, 0, "") output, err := runCommand(httpClient, "REPO --private --confirm", false) if err != nil { From 88c27934a1724ff5eda0c4c42b2b8fde9bf2e2e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 28 Jan 2021 21:58:45 +0100 Subject: [PATCH 10/11] Update some stubs to be closer to how git behaves --- pkg/cmd/pr/checkout/checkout_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/pr/checkout/checkout_test.go b/pkg/cmd/pr/checkout/checkout_test.go index b9e7308de..ed1775f44 100644 --- a/pkg/cmd/pr/checkout/checkout_test.go +++ b/pkg/cmd/pr/checkout/checkout_test.go @@ -165,7 +165,7 @@ func TestPRCheckout_urlArg_differentBase(t *testing.T) { defer cmdTeardown(t) cs.Register(`git fetch https://github\.com/OTHER/POE\.git refs/pull/123/head:feature`, 0, "") - cs.Register(`git config branch\.feature\.merge`, 0, "") + cs.Register(`git config branch\.feature\.merge`, 1, "") cs.Register(`git checkout feature`, 0, "") cs.Register(`git config branch\.feature\.remote https://github\.com/OTHER/POE\.git`, 0, "") cs.Register(`git config branch\.feature\.merge refs/pull/123/head`, 0, "") @@ -210,7 +210,7 @@ func TestPRCheckout_branchArg(t *testing.T) { defer cmdTeardown(t) cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "") - cs.Register(`git config branch\.feature\.merge`, 0, "") + cs.Register(`git config branch\.feature\.merge`, 1, "") cs.Register(`git checkout feature`, 0, "") cs.Register(`git config branch\.feature\.remote origin`, 0, "") cs.Register(`git config branch\.feature\.merge refs/pull/123/head`, 0, "") From d86cfe4627742123e9a55101a693b3591f8573f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 28 Jan 2021 21:59:30 +0100 Subject: [PATCH 11/11] Unpublish SetPrepareCmd --- internal/run/run.go | 15 --------------- internal/run/stub.go | 31 ++++++++++++++++++++++++++++--- 2 files changed, 28 insertions(+), 18 deletions(-) diff --git a/internal/run/run.go b/internal/run/run.go index 34fad8500..58fb189e3 100644 --- a/internal/run/run.go +++ b/internal/run/run.go @@ -22,21 +22,6 @@ var PrepareCmd = func(cmd *exec.Cmd) Runnable { return &cmdWithStderr{cmd} } -// Deprecated: use Stub -func SetPrepareCmd(fn func(*exec.Cmd) Runnable) func() { - origPrepare := PrepareCmd - PrepareCmd = func(cmd *exec.Cmd) Runnable { - // normalize git executable name for consistency in tests - if baseName := filepath.Base(cmd.Args[0]); baseName == "git" || baseName == "git.exe" { - cmd.Args[0] = "git" - } - return fn(cmd) - } - return func() { - PrepareCmd = origPrepare - } -} - // cmdWithStderr augments exec.Cmd by adding stderr to the error message type cmdWithStderr struct { *exec.Cmd diff --git a/internal/run/stub.go b/internal/run/stub.go index f11834c19..bcb359cee 100644 --- a/internal/run/stub.go +++ b/internal/run/stub.go @@ -3,6 +3,7 @@ package run import ( "fmt" "os/exec" + "path/filepath" "regexp" "strings" ) @@ -12,9 +13,11 @@ type T interface { Errorf(string, ...interface{}) } +// Stub installs a catch-all for all external commands invoked from gh. It returns a restore func that, when +// invoked from tests, fails the current test if some stubs that were registered were never matched. func Stub() (*CommandStubber, func(T)) { cs := &CommandStubber{} - teardown := SetPrepareCmd(func(cmd *exec.Cmd) Runnable { + teardown := setPrepareCmd(func(cmd *exec.Cmd) Runnable { s := cs.find(cmd.Args) if s == nil { panic(fmt.Sprintf("no exec stub for `%s`", strings.Join(cmd.Args, " "))) @@ -43,13 +46,33 @@ func Stub() (*CommandStubber, func(T)) { } } +func setPrepareCmd(fn func(*exec.Cmd) Runnable) func() { + origPrepare := PrepareCmd + PrepareCmd = func(cmd *exec.Cmd) Runnable { + // normalize git executable name for consistency in tests + if baseName := filepath.Base(cmd.Args[0]); baseName == "git" || baseName == "git.exe" { + cmd.Args[0] = "git" + } + return fn(cmd) + } + return func() { + PrepareCmd = origPrepare + } +} + +// CommandStubber stubs out invocations to external commands. type CommandStubber struct { stubs []*commandStub } -func (cs *CommandStubber) Register(p string, exitStatus int, output string, callbacks ...CommandCallback) { +// Register a stub for an external command. Pattern is a regular expression, output is the standard output +// from a command. Pass callbacks to inspect raw arguments that the command was invoked with. +func (cs *CommandStubber) Register(pattern string, exitStatus int, output string, callbacks ...CommandCallback) { + if len(pattern) < 1 { + panic("cannot use empty regexp pattern") + } cs.stubs = append(cs.stubs, &commandStub{ - pattern: regexp.MustCompile(p), + pattern: regexp.MustCompile(pattern), exitStatus: exitStatus, stdout: output, callbacks: callbacks, @@ -76,6 +99,7 @@ type commandStub struct { callbacks []CommandCallback } +// Run satisfies Runnable func (s *commandStub) Run() error { if s.exitStatus != 0 { return fmt.Errorf("%s exited with status %d", s.pattern, s.exitStatus) @@ -83,6 +107,7 @@ func (s *commandStub) Run() error { return nil } +// Output satisfies Runnable func (s *commandStub) Output() ([]byte, error) { if s.exitStatus != 0 { return []byte(nil), fmt.Errorf("%s exited with status %d", s.pattern, s.exitStatus)