From ac5bfc09b8b32bed4caa5c78e1378c5d33c978c1 Mon Sep 17 00:00:00 2001 From: Devon Romanko <28825608+dpromanko@users.noreply.github.com> Date: Wed, 20 Jan 2021 07:19:53 -0500 Subject: [PATCH 1/5] remove use of deprecated InitCmdStubber --- pkg/cmd/pr/create/create_test.go | 166 +++++++++++-------------------- 1 file changed, 58 insertions(+), 108 deletions(-) diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index 986c29507..adff4c855 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -98,24 +98,18 @@ func TestPRCreate_nontty_web(t *testing.T) { http.StubRepoInfoResponse("OWNER", "REPO", "master") - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) - cs.Stub("") // git status - cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log - cs.Stub("") // browser + cs.Register(`git status --porcelain`, 0, "") + cs.Register(`git -c log.ShowSignature=false log --pretty=format:%H,%s --cherry origin/master...feature`, 0, "") + cs.Register(``, 0, "") // browser output, err := runCommand(http, nil, "feature", false, `--web --head=feature`) require.NoError(t, err) assert.Equal(t, "", output.String()) assert.Equal(t, "", output.Stderr()) - - assert.Equal(t, 3, len(cs.Calls)) - browserCall := cs.Calls[2].Args - assert.Equal(t, "https://github.com/OWNER/REPO/compare/master...feature?expand=1", browserCall[len(browserCall)-1]) - } func TestPRCreate_nontty_insufficient_flags(t *testing.T) { @@ -168,12 +162,11 @@ func TestPRCreate_recover(t *testing.T) { assert.Equal(t, "recovered body", input["body"].(string)) })) - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) - cs.Stub("") // git status - cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log + cs.Register(`git status --porcelain`, 0, "") + cs.Register(`git -c log.ShowSignature=false log --pretty=format:\%H,\%s --cherry origin/master...feature`, 0, "") as, teardown := prompt.InitAskStubber() defer teardown() @@ -245,12 +238,10 @@ func TestPRCreate_nontty(t *testing.T) { }), ) - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) - cs.Stub("") // git status - cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log + cs.Register(`git status --porcelain`, 0, "") output, err := runCommand(http, nil, "feature", false, `-t "my title" -b "my body" -H feature`) require.NoError(t, err) @@ -288,15 +279,13 @@ func TestPRCreate(t *testing.T) { assert.Equal(t, "feature", input["headRefName"].(string)) })) - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) - cs.Stub("") // git status - cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log - cs.Stub("") // git push + cs.Register(`git status --porcelain`, 0, "") + cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "") + cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/feature`, 0, "") + cs.Register(`git push --set-upstream origin HEAD:feature`, 0, "") ask, cleanupAsk := prompt.InitAskStubber() defer cleanupAsk() @@ -396,15 +385,14 @@ func TestPRCreate_createFork(t *testing.T) { assert.Equal(t, "monalisa:feature", input["headRefName"].(string)) })) - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) - cs.Stub("") // git status - cs.Stub("") // git remote add - cs.Stub("") // git push + cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "") + cs.Register(`git status --porcelain`, 0, "") + cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/feature`, 0, "") + cs.Register(`git remote add -f fork https://github.com/monalisa/REPO.git`, 0, "") + cs.Register(`git push --set-upstream fork HEAD:feature`, 0, "") ask, cleanupAsk := prompt.InitAskStubber() defer cleanupAsk() @@ -413,9 +401,6 @@ func TestPRCreate_createFork(t *testing.T) { output, err := runCommand(http, nil, "feature", true, `-t title -b body`) require.NoError(t, err) - assert.Equal(t, []string{"git", "remote", "add", "-f", "fork", "https://github.com/monalisa/REPO.git"}, cs.Calls[3].Args) - assert.Equal(t, []string{"git", "push", "--set-upstream", "fork", "HEAD:feature"}, cs.Calls[4].Args) - assert.Equal(t, "https://github.com/OWNER/REPO/pull/12\n", output.String()) } @@ -548,12 +533,11 @@ func TestPRCreate_nonLegacyTemplate(t *testing.T) { assert.Equal(t, "- commit 1\n- commit 0\n\nFixes a bug and Closes an issue", input["body"].(string)) })) - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) - cs.Stub("") // git status - cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log + cs.Register(`git -c log.ShowSignature=false log --pretty=format:%H,%s --cherry origin/master...feature`, 0, "1234567890,commit 0\n2345678901,commit 1") + cs.Register(`git status --porcelain`, 0, "") as, teardown := prompt.InitAskStubber() defer teardown() @@ -682,13 +666,6 @@ func TestPRCreate_metadata(t *testing.T) { assert.Equal(t, true, inputs["union"]) })) - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git status - cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log - output, err := runCommand(http, nil, "feature", true, `-t TITLE -b BODY -H feature -a monalisa -l bug -l todo -p roadmap -m 'big one.oh' -r hubot -r monalisa -r /core -r /robots`) assert.NoError(t, err) @@ -710,15 +687,7 @@ func TestPRCreate_alreadyExists(t *testing.T) { ] } } } }`), ) - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) - cs.Stub("") // git status - - _, err := runCommand(http, nil, "feature", true, `-ttitle -bbody -H feature`) + _, err := runCommand(http, nil, "feature", true, `-title -body -H feature`) if err == nil { t.Fatal("error expected, got nil") } @@ -737,16 +706,15 @@ func TestPRCreate_web(t *testing.T) { httpmock.GraphQL(`query UserCurrent\b`), httpmock.StringResponse(`{"data": {"viewer": {"login": "OWNER"} } }`)) - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) - cs.Stub("") // git status - cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log - cs.Stub("") // git push - cs.Stub("") // browser + cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "") + cs.Register(`git status --porcelain`, 0, "") + cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/feature`, 0, "") + cs.Register(`git -c log.ShowSignature=false log --pretty=format:%H,%s --cherry origin/master...feature`, 0, "") + cs.Register(`git push --set-upstream origin HEAD:feature`, 0, "") + cs.Register(``, 0, "") // browser ask, cleanupAsk := prompt.InitAskStubber() defer cleanupAsk() @@ -757,11 +725,6 @@ func TestPRCreate_web(t *testing.T) { assert.Equal(t, "", output.String()) assert.Equal(t, "Opening github.com/OWNER/REPO/compare/master...feature in your browser.\n", output.Stderr()) - - assert.Equal(t, 6, len(cs.Calls)) - assert.Equal(t, "git push --set-upstream origin HEAD:feature", strings.Join(cs.Calls[4].Args, " ")) - browserCall := cs.Calls[5].Args - assert.Equal(t, "https://github.com/OWNER/REPO/compare/master...feature?expand=1", browserCall[len(browserCall)-1]) } func TestPRCreate_webProject(t *testing.T) { @@ -823,15 +786,14 @@ func TestPRCreate_webProject(t *testing.T) { } func Test_determineTrackingBranch_empty(t *testing.T) { - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "") + cs.Register(`git show-ref --verify -- HEAD`, 0, "abc HEAD") remotes := context.Remotes{} - cs.Stub("") // git config --get-regexp (ReadBranchConfig) - cs.Stub("deadbeef HEAD") // git show-ref --verify (ShowRefs) - ref := determineTrackingBranch(remotes, "feature") if ref != nil { t.Errorf("expected nil result, got %v", ref) @@ -839,9 +801,11 @@ func Test_determineTrackingBranch_empty(t *testing.T) { } func Test_determineTrackingBranch_noMatch(t *testing.T) { - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "") + cs.Register("git show-ref --verify -- HEAD refs/remotes/origin/feature refs/remotes/upstream/feature", 0, "abc HEAD\nbca refs/remotes/origin/feature") remotes := context.Remotes{ &context.Remote{ @@ -854,10 +818,6 @@ func Test_determineTrackingBranch_noMatch(t *testing.T) { }, } - cs.Stub("") // git config --get-regexp (ReadBranchConfig) - cs.Stub(`deadbeef HEAD -deadb00f refs/remotes/origin/feature`) // git show-ref --verify (ShowRefs) - ref := determineTrackingBranch(remotes, "feature") if ref != nil { t.Errorf("expected nil result, got %v", ref) @@ -865,9 +825,11 @@ deadb00f refs/remotes/origin/feature`) // git show-ref --verify (ShowRefs) } func Test_determineTrackingBranch_hasMatch(t *testing.T) { - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "") + cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/feature refs/remotes/upstream/feature`, 0, "abc HEAD\nbca refs/remotes/origin/feature\nabc refs/remotes/upstream/feature") remotes := context.Remotes{ &context.Remote{ @@ -880,26 +842,21 @@ func Test_determineTrackingBranch_hasMatch(t *testing.T) { }, } - cs.Stub("") // git config --get-regexp (ReadBranchConfig) - cs.Stub(`deadbeef HEAD -deadb00f refs/remotes/origin/feature -deadbeef refs/remotes/upstream/feature`) // git show-ref --verify (ShowRefs) - ref := determineTrackingBranch(remotes, "feature") if ref == nil { t.Fatal("expected result, got nil") } - assert.Equal(t, []string{"git", "show-ref", "--verify", "--", "HEAD", "refs/remotes/origin/feature", "refs/remotes/upstream/feature"}, cs.Calls[1].Args) - assert.Equal(t, "upstream", ref.RemoteName) assert.Equal(t, "feature", ref.BranchName) } func Test_determineTrackingBranch_respectTrackingConfig(t *testing.T) { - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "") + cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/feature`, 0, "abc HEAD\nbca refs/remotes/origin/feature") remotes := context.Remotes{ &context.Remote{ @@ -908,17 +865,10 @@ func Test_determineTrackingBranch_respectTrackingConfig(t *testing.T) { }, } - cs.Stub(`branch.feature.remote origin -branch.feature.merge refs/heads/great-feat`) // git config --get-regexp (ReadBranchConfig) - cs.Stub(`deadbeef HEAD -deadb00f refs/remotes/origin/feature`) // git show-ref --verify (ShowRefs) - ref := determineTrackingBranch(remotes, "feature") if ref != nil { t.Errorf("expected nil result, got %v", ref) } - - assert.Equal(t, []string{"git", "show-ref", "--verify", "--", "HEAD", "refs/remotes/origin/great-feat", "refs/remotes/origin/feature"}, cs.Calls[1].Args) } func Test_generateCompareURL(t *testing.T) { From 5d23f116c993b8cc51766026be8a3f587b5696b5 Mon Sep 17 00:00:00 2001 From: Devon Romanko <28825608+dpromanko@users.noreply.github.com> Date: Wed, 20 Jan 2021 07:57:48 -0500 Subject: [PATCH 2/5] removed no longer used CmdStubber --- test/helpers.go | 48 ------------------------------------------------ 1 file changed, 48 deletions(-) diff --git a/test/helpers.go b/test/helpers.go index f1dc5875f..0cef94802 100644 --- a/test/helpers.go +++ b/test/helpers.go @@ -2,12 +2,7 @@ package test import ( "bytes" - "errors" - "fmt" - "os/exec" "regexp" - - "github.com/cli/cli/internal/run" ) // TODO copypasta from command package @@ -43,49 +38,6 @@ func (s OutputStub) Run() error { return nil } -type CmdStubber struct { - Stubs []*OutputStub - Count int - Calls []*exec.Cmd -} - -// Deprecated: use run.Stub -func InitCmdStubber() (*CmdStubber, func()) { - cs := CmdStubber{} - teardown := run.SetPrepareCmd(createStubbedPrepareCmd(&cs)) - return &cs, teardown -} - -func (cs *CmdStubber) Stub(desiredOutput string) { - // TODO maybe have some kind of command mapping but going simple for now - cs.Stubs = append(cs.Stubs, &OutputStub{[]byte(desiredOutput), nil}) -} - -func (cs *CmdStubber) StubError(errText string) { - // TODO support error types beyond CmdError - stderrBuff := bytes.NewBufferString(errText) - args := []string{"stub"} // TODO make more real? - err := errors.New(errText) - cs.Stubs = append(cs.Stubs, &OutputStub{Error: &run.CmdError{ - Stderr: stderrBuff, - Args: args, - Err: err, - }}) -} - -func createStubbedPrepareCmd(cs *CmdStubber) func(*exec.Cmd) run.Runnable { - return func(cmd *exec.Cmd) run.Runnable { - cs.Calls = append(cs.Calls, cmd) - call := cs.Count - cs.Count += 1 - if call >= len(cs.Stubs) { - panic(fmt.Sprintf("more execs than stubs. most recent call: %v", cmd)) - } - // fmt.Printf("Called stub for `%v`\n", cmd) // Helpful for debugging - return cs.Stubs[call] - } -} - type T interface { Helper() Errorf(string, ...interface{}) From e39c9d8f9fd8799f3333cb2928cb1c7182f7eebc Mon Sep 17 00:00:00 2001 From: Devon Romanko <28825608+dpromanko@users.noreply.github.com> Date: Sat, 23 Jan 2021 08:31:49 -0500 Subject: [PATCH 3/5] remove new uses of InitCmdStubber after rebase --- pkg/cmd/pr/create/create_test.go | 40 ++++++++++++-------------------- 1 file changed, 15 insertions(+), 25 deletions(-) diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index adff4c855..65028b557 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -7,7 +7,6 @@ import ( "io/ioutil" "net/http" "os" - "strings" "testing" "github.com/MakeNowJust/heredoc" @@ -166,7 +165,7 @@ func TestPRCreate_recover(t *testing.T) { defer cmdTeardown(t) cs.Register(`git status --porcelain`, 0, "") - cs.Register(`git -c log.ShowSignature=false log --pretty=format:\%H,\%s --cherry origin/master...feature`, 0, "") + cs.Register(`git -c log.ShowSignature=false log --pretty=format:%H,%s --cherry origin/master...feature`, 0, "") as, teardown := prompt.InitAskStubber() defer teardown() @@ -329,15 +328,13 @@ func TestPRCreate_NoMaintainerModify(t *testing.T) { assert.Equal(t, "feature", input["headRefName"].(string)) })) - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) - cs.Stub("") // git status - cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log - cs.Stub("") // git push + cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "") + cs.Register(`git status --porcelain`, 0, "") + cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/feature`, 0, "") + cs.Register(`git push --set-upstream origin HEAD:feature`, 0, "") ask, cleanupAsk := prompt.InitAskStubber() defer cleanupAsk() @@ -757,16 +754,15 @@ func TestPRCreate_webProject(t *testing.T) { } } } } `)) - //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) - cs.Stub("") // git status - cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log - cs.Stub("") // git push - cs.Stub("") // browser + cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "") + cs.Register(`git status --porcelain`, 0, "") + cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/feature`, 0, "") + cs.Register(`git -c log.ShowSignature=false log --pretty=format:%H,%s --cherry origin/master...feature`, 0, "") + cs.Register(`git push --set-upstream origin HEAD:feature`, 0, "") + cs.Register(``, 0, "") // browser ask, cleanupAsk := prompt.InitAskStubber() defer cleanupAsk() @@ -777,12 +773,6 @@ func TestPRCreate_webProject(t *testing.T) { assert.Equal(t, "", output.String()) assert.Equal(t, "Opening github.com/OWNER/REPO/compare/master...feature in your browser.\n", output.Stderr()) - - assert.Equal(t, 6, len(cs.Calls)) - assert.Equal(t, "git push --set-upstream origin HEAD:feature", strings.Join(cs.Calls[4].Args, " ")) - browserCall := cs.Calls[5].Args - url := strings.ReplaceAll(browserCall[len(browserCall)-1], "^", "") - assert.Equal(t, "https://github.com/OWNER/REPO/compare/master...feature?expand=1&projects=ORG%2F1", url) } func Test_determineTrackingBranch_empty(t *testing.T) { From 8cba14b564454952c4b3d0c49fc310c2c514764d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 25 Jan 2021 13:13:36 +0100 Subject: [PATCH 4/5] :nail_care: cleanup command stub assertions --- pkg/cmd/pr/create/create_test.go | 51 +++++++++++++++++++++----------- 1 file changed, 33 insertions(+), 18 deletions(-) diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index 65028b557..e1a5183f2 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -7,6 +7,7 @@ import ( "io/ioutil" "net/http" "os" + "strings" "testing" "github.com/MakeNowJust/heredoc" @@ -101,8 +102,11 @@ func TestPRCreate_nontty_web(t *testing.T) { defer cmdTeardown(t) cs.Register(`git status --porcelain`, 0, "") - cs.Register(`git -c log.ShowSignature=false log --pretty=format:%H,%s --cherry origin/master...feature`, 0, "") - cs.Register(``, 0, "") // browser + cs.Register(`git( .+)? log( .+)? origin/master\.\.\.feature`, 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/compare/master...feature?expand=1", url) + }) output, err := runCommand(http, nil, "feature", false, `--web --head=feature`) require.NoError(t, err) @@ -165,7 +169,7 @@ func TestPRCreate_recover(t *testing.T) { defer cmdTeardown(t) cs.Register(`git status --porcelain`, 0, "") - cs.Register(`git -c log.ShowSignature=false log --pretty=format:%H,%s --cherry origin/master...feature`, 0, "") + cs.Register(`git( .+)? log( .+)? origin/master\.\.\.feature`, 0, "") as, teardown := prompt.InitAskStubber() defer teardown() @@ -533,7 +537,7 @@ func TestPRCreate_nonLegacyTemplate(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register(`git -c log.ShowSignature=false log --pretty=format:%H,%s --cherry origin/master...feature`, 0, "1234567890,commit 0\n2345678901,commit 1") + cs.Register(`git( .+)? log( .+)? origin/master\.\.\.feature`, 0, "1234567890,commit 0\n2345678901,commit 1") cs.Register(`git status --porcelain`, 0, "") as, teardown := prompt.InitAskStubber() @@ -684,13 +688,8 @@ func TestPRCreate_alreadyExists(t *testing.T) { ] } } } }`), ) - _, err := runCommand(http, nil, "feature", true, `-title -body -H feature`) - if err == nil { - t.Fatal("error expected, got nil") - } - if err.Error() != "a pull request for branch \"feature\" into branch \"master\" already exists:\nhttps://github.com/OWNER/REPO/pull/123" { - t.Errorf("got error %q", err) - } + _, err := runCommand(http, nil, "feature", true, `-t title -b body -H feature`) + assert.EqualError(t, err, "a pull request for branch \"feature\" into branch \"master\" already exists:\nhttps://github.com/OWNER/REPO/pull/123") } func TestPRCreate_web(t *testing.T) { @@ -709,9 +708,12 @@ func TestPRCreate_web(t *testing.T) { cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "") cs.Register(`git status --porcelain`, 0, "") cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/feature`, 0, "") - cs.Register(`git -c log.ShowSignature=false log --pretty=format:%H,%s --cherry origin/master...feature`, 0, "") + cs.Register(`git( .+)? log( .+)? origin/master\.\.\.feature`, 0, "") cs.Register(`git push --set-upstream origin HEAD:feature`, 0, "") - cs.Register(``, 0, "") // browser + cs.Register(`https://github\.com`, 0, "", func(args []string) { + url := strings.ReplaceAll(args[len(args)-1], "^", "") + assert.Equal(t, "https://github.com/OWNER/REPO/compare/master...feature?expand=1", url) + }) ask, cleanupAsk := prompt.InitAskStubber() defer cleanupAsk() @@ -760,9 +762,12 @@ func TestPRCreate_webProject(t *testing.T) { cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "") cs.Register(`git status --porcelain`, 0, "") cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/feature`, 0, "") - cs.Register(`git -c log.ShowSignature=false log --pretty=format:%H,%s --cherry origin/master...feature`, 0, "") + cs.Register(`git( .+)? log( .+)? origin/master\.\.\.feature`, 0, "") cs.Register(`git push --set-upstream origin HEAD:feature`, 0, "") - cs.Register(``, 0, "") // browser + cs.Register(`https://github\.com`, 0, "", func(args []string) { + url := strings.ReplaceAll(args[len(args)-1], "^", "") + assert.Equal(t, "https://github.com/OWNER/REPO/compare/master...feature?expand=1&projects=ORG%2F1", url) + }) ask, cleanupAsk := prompt.InitAskStubber() defer cleanupAsk() @@ -819,7 +824,11 @@ func Test_determineTrackingBranch_hasMatch(t *testing.T) { defer cmdTeardown(t) cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "") - cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/feature refs/remotes/upstream/feature`, 0, "abc HEAD\nbca refs/remotes/origin/feature\nabc refs/remotes/upstream/feature") + cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/feature refs/remotes/upstream/feature$`, 0, heredoc.Doc(` + deadbeef HEAD + deadb00f refs/remotes/origin/feature + deadbeef refs/remotes/upstream/feature + `)) remotes := context.Remotes{ &context.Remote{ @@ -845,8 +854,14 @@ func Test_determineTrackingBranch_respectTrackingConfig(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "") - cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/feature`, 0, "abc HEAD\nbca refs/remotes/origin/feature") + cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, heredoc.Doc(` + branch.feature.remote origin + branch.feature.merge refs/heads/great-feat + `)) + cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/great-feat refs/remotes/origin/feature$`, 0, heredoc.Doc(` + deadbeef HEAD + deadb00f refs/remotes/origin/feature + `)) remotes := context.Remotes{ &context.Remote{ From d465b7f5d5852f35f9539a7c6d6451e9b899f4ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 25 Jan 2021 13:46:30 +0100 Subject: [PATCH 5/5] Freeze time in `issue view` test --- pkg/cmd/issue/view/view.go | 8 ++++---- pkg/cmd/issue/view/view_test.go | 35 +++++++++++++++++++++++++-------- 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/pkg/cmd/issue/view/view.go b/pkg/cmd/issue/view/view.go index 24867b32b..917bdd0a2 100644 --- a/pkg/cmd/issue/view/view.go +++ b/pkg/cmd/issue/view/view.go @@ -9,7 +9,6 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/api" - "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/pkg/cmd/issue/shared" issueShared "github.com/cli/cli/pkg/cmd/issue/shared" @@ -23,20 +22,21 @@ import ( type ViewOptions struct { HttpClient func() (*http.Client, error) - Config func() (config.Config, error) IO *iostreams.IOStreams BaseRepo func() (ghrepo.Interface, error) SelectorArg string WebMode bool Comments bool + + Now func() time.Time } func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Command { opts := &ViewOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, - Config: f.Config, + Now: time.Now, } cmd := &cobra.Command{ @@ -141,7 +141,7 @@ func printRawIssuePreview(out io.Writer, issue *api.Issue) error { func printHumanIssuePreview(opts *ViewOptions, issue *api.Issue) error { out := opts.IO.Out - now := time.Now() + now := opts.Now() ago := now.Sub(issue.CreatedAt) cs := opts.IO.ColorScheme() diff --git a/pkg/cmd/issue/view/view_test.go b/pkg/cmd/issue/view/view_test.go index c908aa6c5..1cfc0db88 100644 --- a/pkg/cmd/issue/view/view_test.go +++ b/pkg/cmd/issue/view/view_test.go @@ -7,6 +7,7 @@ import ( "net/http" "os/exec" "testing" + "time" "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/ghrepo" @@ -251,20 +252,38 @@ func TestIssueView_tty_Preview(t *testing.T) { } for name, tc := range tests { t.Run(name, func(t *testing.T) { - http := &httpmock.Registry{} - defer http.Verify(t) + io, _, stdout, stderr := iostreams.Test() + io.SetStdoutTTY(true) + io.SetStdinTTY(true) + io.SetStderrTTY(true) - http.Register(httpmock.GraphQL(`query IssueByNumber\b`), httpmock.FileResponse(tc.fixture)) + httpReg := &httpmock.Registry{} + defer httpReg.Verify(t) - output, err := runCommand(http, true, "123") - if err != nil { - t.Errorf("error running `issue view`: %v", err) + httpReg.Register(httpmock.GraphQL(`query IssueByNumber\b`), httpmock.FileResponse(tc.fixture)) + + opts := ViewOptions{ + IO: io, + Now: func() time.Time { + t, _ := time.Parse(time.RFC822, "03 Nov 20 15:04 UTC") + return t + }, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: httpReg}, nil + }, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + SelectorArg: "123", } - assert.Equal(t, "", output.Stderr()) + err := viewRun(&opts) + assert.NoError(t, err) + + assert.Equal(t, "", stderr.String()) //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.String(), tc.expectedOutputs...) + test.ExpectLines(t, stdout.String(), tc.expectedOutputs...) }) } }