From bf4bc1511f593d7ed44be7cfa066b058fe043e66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 18 Jan 2021 20:15:40 +0100 Subject: [PATCH 1/8] Migrate to new cmd stubber in `merge` tests --- pkg/cmd/pr/merge/merge_test.go | 113 +++++++++++---------------------- 1 file changed, 38 insertions(+), 75 deletions(-) diff --git a/pkg/cmd/pr/merge/merge_test.go b/pkg/cmd/pr/merge/merge_test.go index 963fe13fe..3f2c55204 100644 --- a/pkg/cmd/pr/merge/merge_test.go +++ b/pkg/cmd/pr/merge/merge_test.go @@ -208,14 +208,8 @@ func TestPrMerge(t *testing.T) { assert.NotContains(t, input, "commitHeadline") })) - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("branch.blueberries.remote origin\nbranch.blueberries.merge refs/heads/blueberries") // git config --get-regexp ^branch\.master\.(remote|merge) - cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ - cs.Stub("") // git symbolic-ref --quiet --short HEAD - cs.Stub("") // git checkout master - cs.Stub("") + _, cmdTeardown := run.Stub() + defer cmdTeardown(t) output, err := runCommand(http, "master", true, "pr merge 1 --merge") if err != nil { @@ -251,14 +245,8 @@ func TestPrMerge_nontty(t *testing.T) { assert.NotContains(t, input, "commitHeadline") })) - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("branch.blueberries.remote origin\nbranch.blueberries.merge refs/heads/blueberries") // git config --get-regexp ^branch\.master\.(remote|merge) - cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ - cs.Stub("") // git symbolic-ref --quiet --short HEAD - cs.Stub("") // git checkout master - cs.Stub("") + _, cmdTeardown := run.Stub() + defer cmdTeardown(t) output, err := runCommand(http, "master", false, "pr merge 1 --merge") if err != nil { @@ -291,16 +279,14 @@ func TestPrMerge_withRepoFlag(t *testing.T) { assert.NotContains(t, input, "commitHeadline") })) - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() + _, cmdTeardown := run.Stub() + defer cmdTeardown(t) output, err := runCommand(http, "master", true, "pr merge 1 --merge -R OWNER/REPO") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) } - assert.Equal(t, 0, len(cs.Calls)) - r := regexp.MustCompile(`Merged pull request #1 \(The title of the PR\)`) if !r.MatchString(output.Stderr()) { @@ -326,14 +312,13 @@ func TestPrMerge_deleteBranch(t *testing.T) { httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), httpmock.StringResponse(`{}`)) - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) - cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ - cs.Stub("") // git checkout master - cs.Stub("") // git rev-parse --verify blueberries` - cs.Stub("") // git branch -d - cs.Stub("") // git push origin --delete blueberries + cs.Register(`git config --get-regexp.+branch\\\.blueberries\\\.`, 0, "") + cs.Register(`git checkout master`, 0, "") + cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "") + cs.Register(`git branch -D blueberries`, 0, "") output, err := runCommand(http, "blueberries", true, `pr merge --merge --delete-branch`) if err != nil { @@ -361,13 +346,11 @@ func TestPrMerge_deleteNonCurrentBranch(t *testing.T) { httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), httpmock.StringResponse(`{}`)) - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) - // We don't expect the default branch to be checked out, just that blueberries is deleted - cs.Stub("") // git rev-parse --verify blueberries - cs.Stub("") // git branch -d blueberries - cs.Stub("") // git push origin --delete blueberries + cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "") + cs.Register(`git branch -D blueberries`, 0, "") output, err := runCommand(http, "master", true, `pr merge --merge --delete-branch blueberries`) if err != nil { @@ -392,14 +375,10 @@ func TestPrMerge_noPrNumberGiven(t *testing.T) { assert.NotContains(t, input, "commitHeadline") })) - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) - cs.Stub("branch.blueberries.remote origin\nbranch.blueberries.merge refs/heads/blueberries") // git config --get-regexp ^branch\.master\.(remote|merge) - cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ - cs.Stub("") // git symbolic-ref --quiet --short HEAD - cs.Stub("") // git checkout master - cs.Stub("") // git branch -d + cs.Register(`git config --get-regexp.+branch\\\.blueberries\\\.`, 0, "") output, err := runCommand(http, "blueberries", true, "pr merge --merge") if err != nil { @@ -435,13 +414,8 @@ func TestPrMerge_rebase(t *testing.T) { assert.NotContains(t, input, "commitHeadline") })) - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ - cs.Stub("") // git symbolic-ref --quiet --short HEAD - cs.Stub("") // git checkout master - cs.Stub("") // git branch -d + _, cmdTeardown := run.Stub() + defer cmdTeardown(t) output, err := runCommand(http, "master", true, "pr merge 2 --rebase") if err != nil { @@ -477,13 +451,8 @@ func TestPrMerge_squash(t *testing.T) { assert.Equal(t, "The title of the PR (#3)", input["commitHeadline"].(string)) })) - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ - cs.Stub("") // git symbolic-ref --quiet --short HEAD - cs.Stub("") // git checkout master - cs.Stub("") // git branch -d + _, cmdTeardown := run.Stub() + defer cmdTeardown(t) output, err := runCommand(http, "master", true, "pr merge 3 --squash") if err != nil { @@ -577,14 +546,13 @@ func TestPRMerge_interactive(t *testing.T) { httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), httpmock.StringResponse(`{}`)) - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) - cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ - cs.Stub("") // git symbolic-ref --quiet --short HEAD - cs.Stub("") // git checkout master - cs.Stub("") // git push origin --delete blueberries - cs.Stub("") // git branch -d + cs.Register(`git config --get-regexp.+branch\\\.blueberries\\\.`, 0, "") + cs.Register(`git checkout master`, 0, "") + cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "") + cs.Register(`git branch -D blueberries`, 0, "") as, surveyTeardown := prompt.InitAskStubber() defer surveyTeardown() @@ -635,14 +603,13 @@ func TestPRMerge_interactiveWithDeleteBranch(t *testing.T) { httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), httpmock.StringResponse(`{}`)) - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) - cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ - cs.Stub("") // git symbolic-ref --quiet --short HEAD - cs.Stub("") // git checkout master - cs.Stub("") // git push origin --delete blueberries - cs.Stub("") // git branch -d + cs.Register(`git config --get-regexp.+branch\\\.blueberries\\\.`, 0, "") + cs.Register(`git checkout master`, 0, "") + cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "") + cs.Register(`git branch -D blueberries`, 0, "") as, surveyTeardown := prompt.InitAskStubber() defer surveyTeardown() @@ -679,14 +646,10 @@ func TestPRMerge_interactiveCancelled(t *testing.T) { "number": 3 }] } } } }`)) - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) - cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ - cs.Stub("") // git symbolic-ref --quiet --short HEAD - cs.Stub("") // git checkout master - cs.Stub("") // git push origin --delete blueberries - cs.Stub("") // git branch -d + cs.Register(`git config --get-regexp.+branch\\\.blueberries\\\.`, 0, "") as, surveyTeardown := prompt.InitAskStubber() defer surveyTeardown() From 1717c8d0838ae3a46be34eebc39e305375349f0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 18 Jan 2021 22:39:59 +0100 Subject: [PATCH 2/8] Migrate to new cmd stubber in git tests --- git/git.go | 15 ++++++------ git/git_test.go | 64 +++++++++++++------------------------------------ 2 files changed, 23 insertions(+), 56 deletions(-) diff --git a/git/git.go b/git/git.go index 3cda262f0..56224abbc 100644 --- a/git/git.go +++ b/git/git.go @@ -67,22 +67,21 @@ func CurrentBranch() (string, error) { return "", err } + stderr := bytes.Buffer{} + refCmd.Stderr = &stderr + output, err := run.PrepareCmd(refCmd).Output() if err == nil { // Found the branch name return getBranchShortName(output), nil } - var cmdErr *run.CmdError - if errors.As(err, &cmdErr) { - if cmdErr.Stderr.Len() == 0 { - // Detached head - return "", ErrNotOnAnyBranch - } + if stderr.Len() == 0 { + // Detached head + return "", ErrNotOnAnyBranch } - // Unknown error - return "", err + return "", fmt.Errorf("%sgit: %s", stderr.String(), err) } func listRemotes() ([]string, error) { diff --git a/git/git_test.go b/git/git_test.go index 34cc9c7cb..9afc97655 100644 --- a/git/git_test.go +++ b/git/git_test.go @@ -1,12 +1,10 @@ package git import ( - "os/exec" "reflect" "testing" "github.com/cli/cli/internal/run" - "github.com/cli/cli/test" ) func Test_UncommittedChangeCount(t *testing.T) { @@ -21,20 +19,17 @@ func Test_UncommittedChangeCount(t *testing.T) { {Label: "untracked file", Expected: 2, Output: " M poem.txt\n?? new.txt"}, } - teardown := run.SetPrepareCmd(func(*exec.Cmd) run.Runnable { - return &test.OutputStub{} - }) - defer teardown() - for _, v := range cases { - _ = run.SetPrepareCmd(func(*exec.Cmd) run.Runnable { - return &test.OutputStub{Out: []byte(v.Output)} - }) - ucc, _ := UncommittedChangeCount() + t.Run(v.Label, func(t *testing.T) { + cs, restore := run.Stub() + defer restore(t) + cs.Register(`git status --porcelain`, 0, v.Output) - if ucc != v.Expected { - t.Errorf("got unexpected ucc value: %d for case %s", ucc, v.Label) - } + ucc, _ := UncommittedChangeCount() + if ucc != v.Expected { + t.Errorf("UncommittedChangeCount() = %d, expected %d", ucc, v.Expected) + } + }) } } @@ -59,59 +54,32 @@ func Test_CurrentBranch(t *testing.T) { } for _, v := range cases { - cs, teardown := test.InitCmdStubber() - cs.Stub(v.Stub) + cs, teardown := run.Stub() + cs.Register(`git symbolic-ref --quiet HEAD`, 0, v.Stub) result, err := CurrentBranch() if err != nil { t.Errorf("got unexpected error: %w", err) } - if len(cs.Calls) != 1 { - t.Errorf("expected 1 git call, saw %d", len(cs.Calls)) - } if result != v.Expected { t.Errorf("unexpected branch name: %s instead of %s", result, v.Expected) } - teardown() + teardown(t) } } func Test_CurrentBranch_detached_head(t *testing.T) { - cs, teardown := test.InitCmdStubber() - defer teardown() - - cs.StubError("") + cs, teardown := run.Stub() + defer teardown(t) + cs.Register(`git symbolic-ref --quiet HEAD`, 1, "") _, err := CurrentBranch() if err == nil { - t.Errorf("expected an error") + t.Fatal("expected an error, got nil") } if err != ErrNotOnAnyBranch { t.Errorf("got unexpected error: %s instead of %s", err, ErrNotOnAnyBranch) } - if len(cs.Calls) != 1 { - t.Errorf("expected 1 git call, saw %d", len(cs.Calls)) - } -} - -func Test_CurrentBranch_unexpected_error(t *testing.T) { - cs, teardown := test.InitCmdStubber() - defer teardown() - - cs.StubError("lol") - - expectedError := "lol\nstub: lol" - - _, err := CurrentBranch() - if err == nil { - t.Errorf("expected an error") - } - if err.Error() != expectedError { - t.Errorf("got unexpected error: %s instead of %s", err.Error(), expectedError) - } - if len(cs.Calls) != 1 { - t.Errorf("expected 1 git call, saw %d", len(cs.Calls)) - } } func TestParseExtraCloneArgs(t *testing.T) { From c63acf672821f6a362dcf76960253afbb3ffae20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 18 Jan 2021 22:42:01 +0100 Subject: [PATCH 3/8] Migrate to new cmd stubber in misc. tests --- pkg/cmd/gist/clone/clone_test.go | 14 +++++++------- pkg/cmd/gist/create/create_test.go | 15 ++++----------- pkg/cmd/pr/checks/checks_test.go | 9 ++++----- pkg/cmd/pr/close/close_test.go | 11 +++++------ pkg/cmd/repo/view/view_test.go | 12 ++++-------- 5 files changed, 24 insertions(+), 37 deletions(-) diff --git a/pkg/cmd/gist/clone/clone_test.go b/pkg/cmd/gist/clone/clone_test.go index 8faf3d40c..f1e5b6d4d 100644 --- a/pkg/cmd/gist/clone/clone_test.go +++ b/pkg/cmd/gist/clone/clone_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/run" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/httpmock" "github.com/cli/cli/pkg/iostreams" @@ -88,13 +89,15 @@ func Test_GistClone(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { reg := &httpmock.Registry{} + defer reg.Verify(t) httpClient := &http.Client{Transport: reg} - cs, restore := test.InitCmdStubber() - defer restore() - - cs.Stub("") // git clone + cs, restore := run.Stub() + defer restore(t) + cs.Register(`git clone`, 0, "", func(s []string) { + assert.Equal(t, tt.want, strings.Join(s, " ")) + }) output, err := runCloneCommand(httpClient, tt.args) if err != nil { @@ -103,9 +106,6 @@ func Test_GistClone(t *testing.T) { assert.Equal(t, "", output.String()) assert.Equal(t, "", output.Stderr()) - assert.Equal(t, 1, cs.Count) - assert.Equal(t, tt.want, strings.Join(cs.Calls[0].Args, " ")) - reg.Verify(t) }) } } diff --git a/pkg/cmd/gist/create/create_test.go b/pkg/cmd/gist/create/create_test.go index 548abd743..70011d6e8 100644 --- a/pkg/cmd/gist/create/create_test.go +++ b/pkg/cmd/gist/create/create_test.go @@ -3,12 +3,12 @@ package create import ( "bytes" "encoding/json" - "github.com/cli/cli/test" "io/ioutil" "net/http" "strings" "testing" + "github.com/cli/cli/internal/run" "github.com/cli/cli/pkg/cmd/gist/shared" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/httpmock" @@ -291,11 +291,10 @@ func Test_createRun(t *testing.T) { io, stdin, stdout, stderr := iostreams.Test() tt.opts.IO = io - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - + cs, teardown := run.Stub() + defer teardown(t) if tt.opts.WebMode { - cs.Stub("") + cs.Register(`https://gist\.github\.com/aa5a315d61ae9438b18d$`, 0, "") } t.Run(tt.name, func(t *testing.T) { @@ -313,12 +312,6 @@ func Test_createRun(t *testing.T) { assert.Equal(t, tt.wantOut, stdout.String()) assert.Equal(t, tt.wantStderr, stderr.String()) assert.Equal(t, tt.wantParams, reqBody) - - if tt.opts.WebMode { - browserCall := cs.Calls[0].Args - assert.Equal(t, browserCall[len(browserCall)-1], "https://gist.github.com/aa5a315d61ae9438b18d") - } - reg.Verify(t) }) } diff --git a/pkg/cmd/pr/checks/checks_test.go b/pkg/cmd/pr/checks/checks_test.go index 64565a963..af4eb11e8 100644 --- a/pkg/cmd/pr/checks/checks_test.go +++ b/pkg/cmd/pr/checks/checks_test.go @@ -6,10 +6,10 @@ import ( "testing" "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/internal/run" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/httpmock" "github.com/cli/cli/pkg/iostreams" - "github.com/cli/cli/test" "github.com/google/shlex" "github.com/stretchr/testify/assert" ) @@ -252,10 +252,9 @@ func TestChecksRun_web(t *testing.T) { opts.IO = io - cs, teardown := test.InitCmdStubber() - defer teardown() - - cs.Stub("") // browser open + cs, teardown := run.Stub() + defer teardown(t) + cs.Register(`https://github\.com/OWNER/REPO/pull/123/checks$`, 0, "") err := checksRun(opts) assert.NoError(t, err) diff --git a/pkg/cmd/pr/close/close_test.go b/pkg/cmd/pr/close/close_test.go index 3154b4e4b..0e5a1d942 100644 --- a/pkg/cmd/pr/close/close_test.go +++ b/pkg/cmd/pr/close/close_test.go @@ -9,6 +9,7 @@ import ( "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/internal/run" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/httpmock" "github.com/cli/cli/pkg/iostreams" @@ -135,13 +136,11 @@ func TestPrClose_deleteBranch(t *testing.T) { httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), httpmock.StringResponse(`{}`)) - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) - cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ - cs.Stub("") // git rev-parse --verify blueberries` - cs.Stub("") // git branch -d - cs.Stub("") // git push origin --delete blueberries + cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "") + cs.Register(`git branch -D blueberries`, 0, "") output, err := runCommand(http, true, `96 --delete-branch`) if err != nil { diff --git a/pkg/cmd/repo/view/view_test.go b/pkg/cmd/repo/view/view_test.go index 945fab965..005ec8515 100644 --- a/pkg/cmd/repo/view/view_test.go +++ b/pkg/cmd/repo/view/view_test.go @@ -8,10 +8,10 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/internal/run" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/httpmock" "github.com/cli/cli/pkg/iostreams" - "github.com/cli/cli/test" "github.com/google/shlex" "github.com/stretchr/testify/assert" ) @@ -132,18 +132,14 @@ func Test_RepoView_Web(t *testing.T) { t.Run(tt.name, func(t *testing.T) { io.SetStdoutTTY(tt.stdoutTTY) - cs, teardown := test.InitCmdStubber() - defer teardown() - - cs.Stub("") // browser open + cs, teardown := run.Stub() + defer teardown(t) + cs.Register(`https://github\.com/OWNER/REPO$`, 0, "") if err := viewRun(opts); err != nil { t.Errorf("viewRun() error = %v", err) } assert.Equal(t, "", stdout.String()) - assert.Equal(t, 1, len(cs.Calls)) - call := cs.Calls[0] - assert.Equal(t, "https://github.com/OWNER/REPO", call.Args[len(call.Args)-1]) assert.Equal(t, tt.wantStderr, stderr.String()) reg.Verify(t) }) From 584b33e79ce85535abf4d6a54d2d2c7a6759bbaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 18 Jan 2021 22:42:13 +0100 Subject: [PATCH 4/8] Migrate to new cmd stubber in `repo clone` tests --- pkg/cmd/repo/clone/clone_test.go | 42 ++++++++++++-------------------- 1 file changed, 15 insertions(+), 27 deletions(-) diff --git a/pkg/cmd/repo/clone/clone_test.go b/pkg/cmd/repo/clone/clone_test.go index f6b612202..b34ab168c 100644 --- a/pkg/cmd/repo/clone/clone_test.go +++ b/pkg/cmd/repo/clone/clone_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/run" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/httpmock" "github.com/cli/cli/pkg/iostreams" @@ -182,6 +183,7 @@ func Test_RepoClone(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { reg := &httpmock.Registry{} + defer reg.Verify(t) reg.Register( httpmock.GraphQL(`query RepositoryInfo\b`), httpmock.StringResponse(` @@ -196,10 +198,11 @@ func Test_RepoClone(t *testing.T) { httpClient := &http.Client{Transport: reg} - cs, restore := test.InitCmdStubber() - defer restore() - - cs.Stub("") // git clone + cs, restore := run.Stub() + defer restore(t) + cs.Register(`git clone`, 0, "", func(s []string) { + assert.Equal(t, tt.want, strings.Join(s, " ")) + }) output, err := runCloneCommand(httpClient, tt.args) if err != nil { @@ -208,9 +211,6 @@ func Test_RepoClone(t *testing.T) { assert.Equal(t, "", output.String()) assert.Equal(t, "", output.Stderr()) - assert.Equal(t, 1, cs.Count) - assert.Equal(t, tt.want, strings.Join(cs.Calls[0].Args, " ")) - reg.Verify(t) }) } } @@ -236,23 +236,20 @@ func Test_RepoClone_hasParent(t *testing.T) { httpClient := &http.Client{Transport: reg} - cs, restore := test.InitCmdStubber() - defer restore() - - cs.Stub("") // git clone - cs.Stub("") // git remote add + cs, restore := run.Stub() + defer restore(t) + cs.Register(`git clone`, 0, "") + cs.Register(`git -C REPO remote add -f upstream https://github\.com/hubot/ORIG\.git`, 0, "") _, err := runCloneCommand(httpClient, "OWNER/REPO") if err != nil { t.Fatalf("error running command `repo clone`: %v", err) } - - assert.Equal(t, 2, cs.Count) - assert.Equal(t, "git -C REPO remote add -f upstream https://github.com/hubot/ORIG.git", strings.Join(cs.Calls[1].Args, " ")) } func Test_RepoClone_withoutUsername(t *testing.T) { reg := &httpmock.Registry{} + defer reg.Verify(t) reg.Register( httpmock.GraphQL(`query UserCurrent\b`), httpmock.StringResponse(` @@ -269,19 +266,12 @@ func Test_RepoClone_withoutUsername(t *testing.T) { } } } } `)) - reg.Register( - httpmock.GraphQL(`query RepositoryFindParent\b`), - httpmock.StringResponse(` - { "data": { "repository": { - "parent": null - } } }`)) httpClient := &http.Client{Transport: reg} - cs, restore := test.InitCmdStubber() - defer restore() - - cs.Stub("") // git clone + cs, restore := run.Stub() + defer restore(t) + cs.Register(`git clone https://github\.com/OWNER/REPO\.git`, 0, "") output, err := runCloneCommand(httpClient, "REPO") if err != nil { @@ -290,6 +280,4 @@ func Test_RepoClone_withoutUsername(t *testing.T) { assert.Equal(t, "", output.String()) assert.Equal(t, "", output.Stderr()) - assert.Equal(t, 1, cs.Count) - assert.Equal(t, "git clone https://github.com/OWNER/REPO.git", strings.Join(cs.Calls[0].Args, " ")) } From 5531498f27a158edc4e58c95221ea4ece0aea04f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 18 Jan 2021 22:42:27 +0100 Subject: [PATCH 5/8] Migrate to new cmd stubber in `repo fork` tests --- pkg/cmd/repo/fork/fork_test.go | 152 ++++++++++----------------------- 1 file changed, 47 insertions(+), 105 deletions(-) diff --git a/pkg/cmd/repo/fork/fork_test.go b/pkg/cmd/repo/fork/fork_test.go index 20f396394..2558f2c2c 100644 --- a/pkg/cmd/repo/fork/fork_test.go +++ b/pkg/cmd/repo/fork/fork_test.go @@ -3,9 +3,7 @@ package fork import ( "net/http" "net/url" - "os/exec" "regexp" - "strings" "testing" "time" @@ -85,21 +83,21 @@ func runCommand(httpClient *http.Client, remotes []*context.Remote, isTTY bool, func TestRepoFork_nontty(t *testing.T) { defer stubSince(2 * time.Second)() reg := &httpmock.Registry{} + defer reg.Verify(t) defer reg.StubWithFixturePath(200, "./forkResult.json")() httpClient := &http.Client{Transport: reg} - cs, restore := test.InitCmdStubber() - defer restore() + _, restore := run.Stub() + defer restore(t) output, err := runCommand(httpClient, nil, false, "") if err != nil { t.Fatalf("error running command `repo fork`: %v", err) } - assert.Equal(t, 0, len(cs.Calls)) assert.Equal(t, "", output.String()) assert.Equal(t, "", output.Stderr()) - reg.Verify(t) + } func TestRepoFork_in_parent_nontty(t *testing.T) { @@ -108,21 +106,17 @@ func TestRepoFork_in_parent_nontty(t *testing.T) { defer reg.StubWithFixturePath(200, "./forkResult.json")() httpClient := &http.Client{Transport: reg} - cs, restore := test.InitCmdStubber() - defer restore() + cs, restore := run.Stub() + defer restore(t) - cs.Stub("") // git remote rename - cs.Stub("") // git remote add + cs.Register(`git remote rename origin upstream`, 0, "") + cs.Register(`git remote add -f origin https://github\.com/someone/REPO\.git`, 0, "") output, err := runCommand(httpClient, nil, false, "--remote") if err != nil { t.Fatalf("error running command `repo fork`: %v", err) } - assert.Equal(t, 2, len(cs.Calls)) - assert.Equal(t, "git remote rename origin upstream", strings.Join(cs.Calls[0].Args, " ")) - assert.Equal(t, "git remote add -f origin https://github.com/someone/REPO.git", strings.Join(cs.Calls[1].Args, " ")) - assert.Equal(t, "", output.String()) assert.Equal(t, "", output.Stderr()) reg.Verify(t) @@ -131,14 +125,15 @@ func TestRepoFork_in_parent_nontty(t *testing.T) { func TestRepoFork_outside_parent_nontty(t *testing.T) { defer stubSince(2 * time.Second)() reg := &httpmock.Registry{} + reg.Verify(t) defer reg.StubWithFixturePath(200, "./forkResult.json")() httpClient := &http.Client{Transport: reg} - cs, restore := test.InitCmdStubber() - defer restore() + cs, restore := run.Stub() + defer restore(t) - cs.Stub("") // git clone - cs.Stub("") // git remote add + cs.Register(`git clone https://github.com/someone/REPO\.git`, 0, "") + cs.Register(`git -C REPO remote add -f upstream https://github\.com/OWNER/REPO\.git`, 0, "") output, err := runCommand(httpClient, nil, false, "--clone OWNER/REPO") if err != nil { @@ -146,12 +141,8 @@ func TestRepoFork_outside_parent_nontty(t *testing.T) { } assert.Equal(t, "", output.String()) - - assert.Equal(t, "git clone https://github.com/someone/REPO.git", strings.Join(cs.Calls[0].Args, " ")) - assert.Equal(t, "git -C REPO remote add -f upstream https://github.com/OWNER/REPO.git", strings.Join(cs.Calls[1].Args, " ")) - assert.Equal(t, output.Stderr(), "") - reg.Verify(t) + } func TestRepoFork_already_forked(t *testing.T) { @@ -160,16 +151,14 @@ func TestRepoFork_already_forked(t *testing.T) { defer reg.StubWithFixturePath(200, "./forkResult.json")() httpClient := &http.Client{Transport: reg} - cs, restore := test.InitCmdStubber() - defer restore() + _, restore := run.Stub() + defer restore(t) output, err := runCommand(httpClient, nil, true, "--remote=false") if err != nil { t.Errorf("got unexpected error: %v", err) } - assert.Equal(t, 0, len(cs.Calls)) - r := regexp.MustCompile(`someone/REPO.*already exists`) if !r.MatchString(output.Stderr()) { t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output.Stderr()) @@ -213,8 +202,8 @@ func TestRepoFork_in_parent(t *testing.T) { defer reg.StubWithFixturePath(200, "./forkResult.json")() httpClient := &http.Client{Transport: reg} - cs, restore := test.InitCmdStubber() - defer restore() + _, restore := run.Stub() + defer restore(t) defer stubSince(2 * time.Second)() output, err := runCommand(httpClient, nil, true, "--remote=false") @@ -222,7 +211,6 @@ func TestRepoFork_in_parent(t *testing.T) { t.Errorf("error running command `repo fork`: %v", err) } - assert.Equal(t, 0, len(cs.Calls)) assert.Equal(t, "", output.String()) r := regexp.MustCompile(`Created fork.*someone/REPO`) @@ -279,28 +267,18 @@ func TestRepoFork_in_parent_yes(t *testing.T) { defer reg.StubWithFixturePath(200, "./forkResult.json")() httpClient := &http.Client{Transport: reg} - var seenCmds []*exec.Cmd - defer run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmds = append(seenCmds, cmd) - return &test.OutputStub{} - })() + cs, restore := run.Stub() + defer restore(t) + + cs.Register(`git remote rename origin upstream`, 0, "") + cs.Register(`git remote add -f origin https://github\.com/someone/REPO\.git`, 0, "") output, err := runCommand(httpClient, nil, true, "--remote") if err != nil { t.Errorf("error running command `repo fork`: %v", err) } - expectedCmds := []string{ - "git remote rename origin upstream", - "git remote add -f origin https://github.com/someone/REPO.git", - } - - for x, cmd := range seenCmds { - assert.Equal(t, expectedCmds[x], strings.Join(cmd.Args, " ")) - } - assert.Equal(t, "", output.String()) - test.ExpectLines(t, output.Stderr(), "Created fork.*someone/REPO", "Added remote.*origin") @@ -314,11 +292,11 @@ func TestRepoFork_outside_yes(t *testing.T) { defer reg.StubWithFixturePath(200, "./forkResult.json")() httpClient := &http.Client{Transport: reg} - cs, restore := test.InitCmdStubber() - defer restore() + cs, restore := run.Stub() + defer restore(t) - cs.Stub("") // git clone - cs.Stub("") // git remote add + cs.Register(`git clone https://github\.com/someone/REPO\.git`, 0, "") + cs.Register(`git -C REPO remote add -f upstream https://github\.com/OWNER/REPO\.git`, 0, "") output, err := runCommand(httpClient, nil, true, "--clone OWNER/REPO") if err != nil { @@ -326,10 +304,6 @@ func TestRepoFork_outside_yes(t *testing.T) { } assert.Equal(t, "", output.String()) - - assert.Equal(t, "git clone https://github.com/someone/REPO.git", strings.Join(cs.Calls[0].Args, " ")) - assert.Equal(t, "git -C REPO remote add -f upstream https://github.com/OWNER/REPO.git", strings.Join(cs.Calls[1].Args, " ")) - test.ExpectLines(t, output.Stderr(), "Created fork.*someone/REPO", "Cloned fork") @@ -343,11 +317,11 @@ func TestRepoFork_outside_survey_yes(t *testing.T) { defer reg.StubWithFixturePath(200, "./forkResult.json")() httpClient := &http.Client{Transport: reg} - cs, restore := test.InitCmdStubber() - defer restore() + cs, restore := run.Stub() + defer restore(t) - cs.Stub("") // git clone - cs.Stub("") // git remote add + cs.Register(`git clone https://github\.com/someone/REPO\.git`, 0, "") + cs.Register(`git -C REPO remote add -f upstream https://github\.com/OWNER/REPO\.git`, 0, "") defer prompt.StubConfirm(true)() @@ -357,10 +331,6 @@ func TestRepoFork_outside_survey_yes(t *testing.T) { } assert.Equal(t, "", output.String()) - - assert.Equal(t, "git clone https://github.com/someone/REPO.git", strings.Join(cs.Calls[0].Args, " ")) - assert.Equal(t, "git -C REPO remote add -f upstream https://github.com/OWNER/REPO.git", strings.Join(cs.Calls[1].Args, " ")) - test.ExpectLines(t, output.Stderr(), "Created fork.*someone/REPO", "Cloned fork") @@ -374,11 +344,8 @@ func TestRepoFork_outside_survey_no(t *testing.T) { defer reg.StubWithFixturePath(200, "./forkResult.json")() httpClient := &http.Client{Transport: reg} - cmdRun := false - defer run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - cmdRun = true - return &test.OutputStub{} - })() + _, restore := run.Stub() + defer restore(t) defer prompt.StubConfirm(false)() @@ -389,8 +356,6 @@ func TestRepoFork_outside_survey_no(t *testing.T) { assert.Equal(t, "", output.String()) - assert.Equal(t, false, cmdRun) - r := regexp.MustCompile(`Created fork.*someone/REPO`) if !r.MatchString(output.Stderr()) { t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) @@ -406,11 +371,11 @@ func TestRepoFork_in_parent_survey_yes(t *testing.T) { httpClient := &http.Client{Transport: reg} defer stubSince(2 * time.Second)() - var seenCmds []*exec.Cmd - defer run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmds = append(seenCmds, cmd) - return &test.OutputStub{} - })() + cs, restore := run.Stub() + defer restore(t) + + cs.Register(`git remote rename origin upstream`, 0, "") + cs.Register(`git remote add -f origin https://github\.com/someone/REPO\.git`, 0, "") defer prompt.StubConfirm(true)() @@ -419,15 +384,6 @@ func TestRepoFork_in_parent_survey_yes(t *testing.T) { t.Errorf("error running command `repo fork`: %v", err) } - expectedCmds := []string{ - "git remote rename origin upstream", - "git remote add -f origin https://github.com/someone/REPO.git", - } - - for x, cmd := range seenCmds { - assert.Equal(t, expectedCmds[x], strings.Join(cmd.Args, " ")) - } - assert.Equal(t, "", output.String()) test.ExpectLines(t, output.Stderr(), @@ -440,15 +396,13 @@ func TestRepoFork_in_parent_survey_yes(t *testing.T) { func TestRepoFork_in_parent_survey_no(t *testing.T) { stubSpinner() reg := &httpmock.Registry{} + defer reg.Verify(t) defer reg.StubWithFixturePath(200, "./forkResult.json")() httpClient := &http.Client{Transport: reg} defer stubSince(2 * time.Second)() - cmdRun := false - defer run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - cmdRun = true - return &test.OutputStub{} - })() + _, restore := run.Stub() + defer restore(t) defer prompt.StubConfirm(false)() @@ -459,28 +413,26 @@ func TestRepoFork_in_parent_survey_no(t *testing.T) { assert.Equal(t, "", output.String()) - assert.Equal(t, false, cmdRun) - r := regexp.MustCompile(`Created fork.*someone/REPO`) if !r.MatchString(output.Stderr()) { t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) return } - reg.Verify(t) } func TestRepoFork_in_parent_match_protocol(t *testing.T) { stubSpinner() defer stubSince(2 * time.Second)() reg := &httpmock.Registry{} + defer reg.Verify(t) defer reg.StubWithFixturePath(200, "./forkResult.json")() httpClient := &http.Client{Transport: reg} - var seenCmds []*exec.Cmd - defer run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmds = append(seenCmds, cmd) - return &test.OutputStub{} - })() + cs, restore := run.Stub() + defer restore(t) + + cs.Register(`git remote rename origin upstream`, 0, "") + cs.Register(`git remote add -f origin git@github\.com:someone/REPO\.git`, 0, "") remotes := []*context.Remote{ { @@ -496,21 +448,11 @@ func TestRepoFork_in_parent_match_protocol(t *testing.T) { t.Errorf("error running command `repo fork`: %v", err) } - expectedCmds := []string{ - "git remote rename origin upstream", - "git remote add -f origin git@github.com:someone/REPO.git", - } - - for x, cmd := range seenCmds { - assert.Equal(t, expectedCmds[x], strings.Join(cmd.Args, " ")) - } - assert.Equal(t, "", output.String()) test.ExpectLines(t, output.Stderr(), "Created fork.*someone/REPO", "Added remote.*origin") - reg.Verify(t) } func stubSpinner() { From c308f1cd910d52acf73bbb6f6eb6bfe469916490 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 18 Jan 2021 22:44:53 +0100 Subject: [PATCH 6/8] Prevent further use of SetPrepareCmd and InitCmdStubber --- .golangci.yml | 7 ++++++- internal/run/run.go | 2 +- pkg/cmd/issue/create/create_test.go | 3 +++ pkg/cmd/issue/list/list_test.go | 1 + pkg/cmd/issue/view/view_test.go | 4 ++++ pkg/cmd/pr/checkout/checkout_test.go | 13 +++++++++++++ pkg/cmd/pr/create/create_test.go | 13 +++++++++++++ pkg/cmd/pr/list/list_test.go | 1 + pkg/cmd/pr/view/view_test.go | 7 +++++++ pkg/cmd/repo/create/create_test.go | 6 ++++++ test/helpers.go | 1 + 11 files changed, 56 insertions(+), 2 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 57e53d6fb..ff7f37014 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,3 +1,8 @@ linters: enable: - gofmt + - gofmt + - nolintlint + +issues: + max-issues-per-linter: 0 + max-same-issues: 0 diff --git a/internal/run/run.go b/internal/run/run.go index bc2c92191..34fad8500 100644 --- a/internal/run/run.go +++ b/internal/run/run.go @@ -22,7 +22,7 @@ var PrepareCmd = func(cmd *exec.Cmd) Runnable { return &cmdWithStderr{cmd} } -// SetPrepareCmd overrides PrepareCmd and returns a func to revert it back +// Deprecated: use Stub func SetPrepareCmd(fn func(*exec.Cmd) Runnable) func() { origPrepare := PrepareCmd PrepareCmd = func(cmd *exec.Cmd) Runnable { diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go index 21177164a..63eb552c9 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -294,6 +294,7 @@ 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{} @@ -422,6 +423,7 @@ func TestIssueCreate_web(t *testing.T) { 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{} @@ -447,6 +449,7 @@ func TestIssueCreate_webTitleBody(t *testing.T) { 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{} diff --git a/pkg/cmd/issue/list/list_test.go b/pkg/cmd/issue/list/list_test.go index 456741a91..3e46a0c0b 100644 --- a/pkg/cmd/issue/list/list_test.go +++ b/pkg/cmd/issue/list/list_test.go @@ -218,6 +218,7 @@ func TestIssueList_web(t *testing.T) { 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{} diff --git a/pkg/cmd/issue/view/view_test.go b/pkg/cmd/issue/view/view_test.go index 82890eac9..448e5aedd 100644 --- a/pkg/cmd/issue/view/view_test.go +++ b/pkg/cmd/issue/view/view_test.go @@ -74,6 +74,7 @@ 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{} @@ -110,6 +111,7 @@ 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{} @@ -281,6 +283,7 @@ 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{} @@ -332,6 +335,7 @@ 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{} diff --git a/pkg/cmd/pr/checkout/checkout_test.go b/pkg/cmd/pr/checkout/checkout_test.go index 2ca060587..b8d3c89b1 100644 --- a/pkg/cmd/pr/checkout/checkout_test.go +++ b/pkg/cmd/pr/checkout/checkout_test.go @@ -117,6 +117,7 @@ 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": @@ -162,6 +163,7 @@ 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": @@ -201,6 +203,7 @@ 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": @@ -253,6 +256,7 @@ 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": @@ -292,6 +296,7 @@ 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": @@ -344,6 +349,7 @@ 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": @@ -386,6 +392,7 @@ 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": @@ -428,6 +435,7 @@ 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": @@ -468,6 +476,7 @@ 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": @@ -508,6 +517,7 @@ 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": @@ -547,6 +557,7 @@ 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{} @@ -580,6 +591,7 @@ 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": @@ -621,6 +633,7 @@ 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": diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index 63963aa77..5ed0028d3 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -105,6 +105,7 @@ 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() @@ -178,6 +179,7 @@ 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() @@ -254,6 +256,7 @@ func TestPRCreate_nontty(t *testing.T) { }), ) + //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() @@ -296,6 +299,7 @@ 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() @@ -351,6 +355,7 @@ 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() @@ -502,6 +507,7 @@ 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() @@ -635,6 +641,7 @@ func TestPRCreate_metadata(t *testing.T) { eq(t, inputs["union"], true) })) + //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() @@ -662,6 +669,7 @@ func TestPRCreate_alreadyExists(t *testing.T) { ] } } } }`), ) + //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() @@ -688,6 +696,7 @@ 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() @@ -715,6 +724,7 @@ func TestPRCreate_web(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() @@ -730,6 +740,7 @@ 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() @@ -755,6 +766,7 @@ 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() @@ -786,6 +798,7 @@ deadbeef refs/remotes/upstream/feature`) // git show-ref --verify (ShowRefs) } func Test_determineTrackingBranch_respectTrackingConfig(t *testing.T) { + //nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() diff --git a/pkg/cmd/pr/list/list_test.go b/pkg/cmd/pr/list/list_test.go index ab99e4aa9..2e05eb6bf 100644 --- a/pkg/cmd/pr/list/list_test.go +++ b/pkg/cmd/pr/list/list_test.go @@ -207,6 +207,7 @@ func TestPRList_web(t *testing.T) { 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{} diff --git a/pkg/cmd/pr/view/view_test.go b/pkg/cmd/pr/view/view_test.go index e61ccd614..aeaf5ee01 100644 --- a/pkg/cmd/pr/view/view_test.go +++ b/pkg/cmd/pr/view/view_test.go @@ -494,6 +494,7 @@ func TestPRView_web_currentBranch(t *testing.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)$`: @@ -528,6 +529,7 @@ func TestPRView_web_noResultsForBranch(t *testing.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)$`: @@ -562,6 +564,7 @@ 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{} @@ -595,6 +598,7 @@ 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{} @@ -628,6 +632,7 @@ 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{} @@ -663,6 +668,7 @@ 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{} @@ -699,6 +705,7 @@ 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{} diff --git a/pkg/cmd/repo/create/create_test.go b/pkg/cmd/repo/create/create_test.go index f9f3ea12d..70d6cc763 100644 --- a/pkg/cmd/repo/create/create_test.go +++ b/pkg/cmd/repo/create/create_test.go @@ -85,6 +85,7 @@ 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{} @@ -170,6 +171,7 @@ func TestRepoCreate_outsideGitWorkDir(t *testing.T) { {}, {}, } + //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") @@ -244,6 +246,7 @@ 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{} @@ -326,6 +329,7 @@ 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{} @@ -409,6 +413,7 @@ 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{} @@ -489,6 +494,7 @@ 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{} diff --git a/test/helpers.go b/test/helpers.go index edfa25bda..5a493dcae 100644 --- a/test/helpers.go +++ b/test/helpers.go @@ -49,6 +49,7 @@ type CmdStubber struct { Calls []*exec.Cmd } +// Deprecated: use run.Stub func InitCmdStubber() (*CmdStubber, func()) { cs := CmdStubber{} teardown := run.SetPrepareCmd(createStubbedPrepareCmd(&cs)) From 411bd4a70e1a73b9f29e45c171ae2415e8416c2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 18 Jan 2021 22:53:03 +0100 Subject: [PATCH 7/8] :fire: unused `test/fixtures/` --- test/fixtures/test.git/HEAD | 1 - test/fixtures/test.git/config | 6 ------ test/fixtures/test.git/info/exclude | 6 ------ .../08/f4b7b6513dffc6245857e497cfd6101dc47818 | 3 --- .../8a/1cdac440b4a3c44b988e300758a903a9866905 | Bin 54 -> 0 bytes .../9b/5a719a3d76ac9dc2fa635d9b1f34fd73994c06 | 3 --- .../9d/aeafb9864cf43055ae93beb0afd6c7d144bfa4 | Bin 20 -> 0 bytes .../ca/93b49848670d03b3968c8a481eca55f5fb2150 | Bin 56 -> 0 bytes .../e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391 | Bin 15 -> 0 bytes test/fixtures/test.git/refs/heads/master | 1 - 10 files changed, 20 deletions(-) delete mode 100644 test/fixtures/test.git/HEAD delete mode 100644 test/fixtures/test.git/config delete mode 100644 test/fixtures/test.git/info/exclude delete mode 100644 test/fixtures/test.git/objects/08/f4b7b6513dffc6245857e497cfd6101dc47818 delete mode 100644 test/fixtures/test.git/objects/8a/1cdac440b4a3c44b988e300758a903a9866905 delete mode 100644 test/fixtures/test.git/objects/9b/5a719a3d76ac9dc2fa635d9b1f34fd73994c06 delete mode 100644 test/fixtures/test.git/objects/9d/aeafb9864cf43055ae93beb0afd6c7d144bfa4 delete mode 100644 test/fixtures/test.git/objects/ca/93b49848670d03b3968c8a481eca55f5fb2150 delete mode 100644 test/fixtures/test.git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391 delete mode 100644 test/fixtures/test.git/refs/heads/master diff --git a/test/fixtures/test.git/HEAD b/test/fixtures/test.git/HEAD deleted file mode 100644 index cb089cd89..000000000 --- a/test/fixtures/test.git/HEAD +++ /dev/null @@ -1 +0,0 @@ -ref: refs/heads/master diff --git a/test/fixtures/test.git/config b/test/fixtures/test.git/config deleted file mode 100644 index 4f7d452b3..000000000 --- a/test/fixtures/test.git/config +++ /dev/null @@ -1,6 +0,0 @@ -[core] - repositoryformatversion = 0 - filemode = true - bare = true - ignorecase = true - precomposeunicode = false diff --git a/test/fixtures/test.git/info/exclude b/test/fixtures/test.git/info/exclude deleted file mode 100644 index a5196d1be..000000000 --- a/test/fixtures/test.git/info/exclude +++ /dev/null @@ -1,6 +0,0 @@ -# git ls-files --others --exclude-from=.git/info/exclude -# Lines that start with '#' are comments. -# For a project mostly in C, the following would be a good set of -# exclude patterns (uncomment them if you want to use them): -# *.[oa] -# *~ diff --git a/test/fixtures/test.git/objects/08/f4b7b6513dffc6245857e497cfd6101dc47818 b/test/fixtures/test.git/objects/08/f4b7b6513dffc6245857e497cfd6101dc47818 deleted file mode 100644 index 37162913a..000000000 --- a/test/fixtures/test.git/objects/08/f4b7b6513dffc6245857e497cfd6101dc47818 +++ /dev/null @@ -1,3 +0,0 @@ -xM -0F]seb~nxcMi)^ߠ7p>xb9U7ub{bgٴă#h8>4o -'yȜ],ڄFk-jUW*zI); \ No newline at end of file diff --git a/test/fixtures/test.git/objects/8a/1cdac440b4a3c44b988e300758a903a9866905 b/test/fixtures/test.git/objects/8a/1cdac440b4a3c44b988e300758a903a9866905 deleted file mode 100644 index 27427c2680fa0fe5d2277e77e623ccb000ece3a9..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 54 zcmb Date: Mon, 18 Jan 2021 23:25:45 +0100 Subject: [PATCH 8/8] Deprecate `test.ExpectLines` For asserting command output, exact string matches are preferred in most cases. In cases when a pattern match is needed, the test can use regexp ad hoc. --- pkg/cmd/alias/set/set_test.go | 11 +++++++++++ pkg/cmd/issue/list/list_test.go | 1 + pkg/cmd/issue/view/view_test.go | 4 ++++ pkg/cmd/pr/close/close_test.go | 1 + pkg/cmd/pr/merge/merge_test.go | 6 ++++++ pkg/cmd/pr/review/review_test.go | 6 ++++++ pkg/cmd/pr/shared/preserve_test.go | 1 + pkg/cmd/pr/view/view_test.go | 4 ++++ pkg/cmd/repo/fork/fork_test.go | 5 +++++ pkg/cmd/secret/list/list_test.go | 1 + test/helpers.go | 1 + 11 files changed, 41 insertions(+) diff --git a/pkg/cmd/alias/set/set_test.go b/pkg/cmd/alias/set/set_test.go index 95f3a5031..bb7e26325 100644 --- a/pkg/cmd/alias/set/set_test.go +++ b/pkg/cmd/alias/set/set_test.go @@ -86,7 +86,9 @@ func TestAliasSet_empty_aliases(t *testing.T) { t.Fatalf("unexpected error: %s", err) } + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Added alias") + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.String(), "") expected := `aliases: @@ -108,6 +110,7 @@ func TestAliasSet_existing_alias(t *testing.T) { output, err := runCommand(cfg, true, "co 'pr checkout -Rcool/repo'") require.NoError(t, err) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Changed alias.*co.*from.*pr checkout.*to.*pr checkout -Rcool/repo") } @@ -120,8 +123,10 @@ func TestAliasSet_space_args(t *testing.T) { output, err := runCommand(cfg, true, `il 'issue list -l "cool story"'`) require.NoError(t, err) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), `Adding alias for.*il.*issue list -l "cool story"`) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, mainBuf.String(), `il: issue list -l "cool story"`) } @@ -156,7 +161,9 @@ func TestAliasSet_arg_processing(t *testing.T) { t.Fatalf("got unexpected error running %s: %s", c.Cmd, err) } + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), c.ExpectedOutputLine) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, mainBuf.String(), c.ExpectedConfigLine) }) } @@ -178,6 +185,7 @@ aliases: diff: pr diff ` + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Adding alias for.*diff.*pr diff", "Added alias.") assert.Equal(t, expected, mainBuf.String()) } @@ -199,6 +207,7 @@ func TestAliasSet_existing_aliases(t *testing.T) { view: pr view ` + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Adding alias for.*view.*pr view", "Added alias.") assert.Equal(t, expected, mainBuf.String()) @@ -226,6 +235,7 @@ func TestShellAlias_flag(t *testing.T) { t.Fatalf("unexpected error: %s", err) } + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Adding alias for.*igrep") expected := `aliases: @@ -243,6 +253,7 @@ func TestShellAlias_bang(t *testing.T) { output, err := runCommand(cfg, true, "igrep '!gh issue list | grep'") require.NoError(t, err) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Adding alias for.*igrep") expected := `aliases: diff --git a/pkg/cmd/issue/list/list_test.go b/pkg/cmd/issue/list/list_test.go index 3e46a0c0b..98b954ee3 100644 --- a/pkg/cmd/issue/list/list_test.go +++ b/pkg/cmd/issue/list/list_test.go @@ -80,6 +80,7 @@ func TestIssueList_nontty(t *testing.T) { } eq(t, output.Stderr(), "") + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.String(), `1[\t]+number won[\t]+label[\t]+\d+`, `2[\t]+number too[\t]+label[\t]+\d+`, diff --git a/pkg/cmd/issue/view/view_test.go b/pkg/cmd/issue/view/view_test.go index 448e5aedd..03548c554 100644 --- a/pkg/cmd/issue/view/view_test.go +++ b/pkg/cmd/issue/view/view_test.go @@ -198,6 +198,7 @@ func TestIssueView_nontty_Preview(t *testing.T) { assert.Equal(t, "", output.Stderr()) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.String(), tc.expectedOutputs...) }) } @@ -264,6 +265,7 @@ func TestIssueView_tty_Preview(t *testing.T) { assert.Equal(t, "", output.Stderr()) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.String(), tc.expectedOutputs...) }) } @@ -422,6 +424,7 @@ func TestIssueView_tty_Comments(t *testing.T) { } assert.NoError(t, err) assert.Equal(t, "", output.Stderr()) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.String(), tc.expectedOutputs...) }) } @@ -496,6 +499,7 @@ func TestIssueView_nontty_Comments(t *testing.T) { } assert.NoError(t, err) assert.Equal(t, "", output.Stderr()) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.String(), tc.expectedOutputs...) }) } diff --git a/pkg/cmd/pr/close/close_test.go b/pkg/cmd/pr/close/close_test.go index 0e5a1d942..09d8e521b 100644 --- a/pkg/cmd/pr/close/close_test.go +++ b/pkg/cmd/pr/close/close_test.go @@ -147,5 +147,6 @@ func TestPrClose_deleteBranch(t *testing.T) { t.Fatalf("Got unexpected error running `pr close` %s", err) } + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), `Closed pull request #96 \(The title of the PR\)`, `Deleted branch blueberries`) } diff --git a/pkg/cmd/pr/merge/merge_test.go b/pkg/cmd/pr/merge/merge_test.go index 3f2c55204..37049d0ef 100644 --- a/pkg/cmd/pr/merge/merge_test.go +++ b/pkg/cmd/pr/merge/merge_test.go @@ -325,6 +325,7 @@ func TestPrMerge_deleteBranch(t *testing.T) { t.Fatalf("Got unexpected error running `pr merge` %s", err) } + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), `Merged pull request #10 \(Blueberries are a good fruit\)`, `Deleted branch.*blueberries`) } @@ -357,6 +358,7 @@ func TestPrMerge_deleteNonCurrentBranch(t *testing.T) { t.Fatalf("Got unexpected error running `pr merge` %s", err) } + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), `Merged pull request #10 \(Blueberries are a good fruit\)`, `Deleted branch.*blueberries`) } @@ -459,6 +461,7 @@ func TestPrMerge_squash(t *testing.T) { t.Fatalf("error running command `pr merge`: %v", err) } + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Squashed and merged pull request #3") } @@ -498,6 +501,7 @@ func TestPrMerge_alreadyMerged(t *testing.T) { t.Fatalf("Got unexpected error running `pr merge` %s", err) } + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "✔ Deleted branch blueberries and switched to branch master") } @@ -577,6 +581,7 @@ func TestPRMerge_interactive(t *testing.T) { t.Fatalf("Got unexpected error running `pr merge` %s", err) } + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Merged pull request #3") } @@ -630,6 +635,7 @@ func TestPRMerge_interactiveWithDeleteBranch(t *testing.T) { t.Fatalf("Got unexpected error running `pr merge` %s", err) } + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Merged pull request #3", "Deleted branch blueberries and switched to branch master") } diff --git a/pkg/cmd/pr/review/review_test.go b/pkg/cmd/pr/review/review_test.go index 749495c7a..2919526d3 100644 --- a/pkg/cmd/pr/review/review_test.go +++ b/pkg/cmd/pr/review/review_test.go @@ -218,6 +218,7 @@ func TestPRReview_url_arg(t *testing.T) { t.Fatalf("error running pr review: %s", err) } + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Approved pull request #123") } @@ -260,6 +261,7 @@ func TestPRReview_number_arg(t *testing.T) { t.Fatalf("error running pr review: %s", err) } + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Approved pull request #123") } @@ -293,6 +295,7 @@ func TestPRReview_no_arg(t *testing.T) { t.Fatalf("error running pr review: %s", err) } + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Reviewed pull request #123") } @@ -425,8 +428,10 @@ func TestPRReview_interactive(t *testing.T) { t.Fatalf("got unexpected error running pr review: %s", err) } + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Approved pull request #123") + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.String(), "Got:", "cool.*story") @@ -532,5 +537,6 @@ func TestPRReview_interactive_blank_approve(t *testing.T) { t.Errorf("did not expect to see body printed in %s", output.String()) } + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Approved pull request #123") } diff --git a/pkg/cmd/pr/shared/preserve_test.go b/pkg/cmd/pr/shared/preserve_test.go index 28949d957..892973f26 100644 --- a/pkg/cmd/pr/shared/preserve_test.go +++ b/pkg/cmd/pr/shared/preserve_test.go @@ -98,6 +98,7 @@ func Test_PreserveInput(t *testing.T) { assert.NoError(t, err) if tt.wantPreservation { + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, errOut.String(), tt.wantErrLine) preserved := &IssueMetadataState{} assert.NoError(t, json.Unmarshal(data, preserved)) diff --git a/pkg/cmd/pr/view/view_test.go b/pkg/cmd/pr/view/view_test.go index aeaf5ee01..3ceac3526 100644 --- a/pkg/cmd/pr/view/view_test.go +++ b/pkg/cmd/pr/view/view_test.go @@ -338,6 +338,7 @@ func TestPRView_Preview_nontty(t *testing.T) { assert.Equal(t, "", output.Stderr()) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.String(), tc.expectedOutputs...) }) } @@ -483,6 +484,7 @@ func TestPRView_Preview(t *testing.T) { assert.Equal(t, "", output.Stderr()) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.String(), tc.expectedOutputs...) }) } @@ -798,6 +800,7 @@ func TestPRView_tty_Comments(t *testing.T) { } assert.NoError(t, err) assert.Equal(t, "", output.Stderr()) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.String(), tt.expectedOutputs...) }) } @@ -876,6 +879,7 @@ func TestPRView_nontty_Comments(t *testing.T) { } assert.NoError(t, err) assert.Equal(t, "", output.Stderr()) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.String(), tt.expectedOutputs...) }) } diff --git a/pkg/cmd/repo/fork/fork_test.go b/pkg/cmd/repo/fork/fork_test.go index 2558f2c2c..ade9d4d7d 100644 --- a/pkg/cmd/repo/fork/fork_test.go +++ b/pkg/cmd/repo/fork/fork_test.go @@ -279,6 +279,7 @@ func TestRepoFork_in_parent_yes(t *testing.T) { } assert.Equal(t, "", output.String()) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Created fork.*someone/REPO", "Added remote.*origin") @@ -304,6 +305,7 @@ func TestRepoFork_outside_yes(t *testing.T) { } assert.Equal(t, "", output.String()) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Created fork.*someone/REPO", "Cloned fork") @@ -331,6 +333,7 @@ func TestRepoFork_outside_survey_yes(t *testing.T) { } assert.Equal(t, "", output.String()) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Created fork.*someone/REPO", "Cloned fork") @@ -386,6 +389,7 @@ func TestRepoFork_in_parent_survey_yes(t *testing.T) { assert.Equal(t, "", output.String()) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Created fork.*someone/REPO", "Renamed.*origin.*remote to.*upstream", @@ -450,6 +454,7 @@ func TestRepoFork_in_parent_match_protocol(t *testing.T) { assert.Equal(t, "", output.String()) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Created fork.*someone/REPO", "Added remote.*origin") diff --git a/pkg/cmd/secret/list/list_test.go b/pkg/cmd/secret/list/list_test.go index 601c13c27..a791bb13c 100644 --- a/pkg/cmd/secret/list/list_test.go +++ b/pkg/cmd/secret/list/list_test.go @@ -191,6 +191,7 @@ func Test_listRun(t *testing.T) { reg.Verify(t) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, stdout.String(), tt.wantOut...) }) } diff --git a/test/helpers.go b/test/helpers.go index 5a493dcae..f1dc5875f 100644 --- a/test/helpers.go +++ b/test/helpers.go @@ -91,6 +91,7 @@ type T interface { Errorf(string, ...interface{}) } +// Deprecated: prefer exact matches for command output func ExpectLines(t T, output string, lines ...string) { t.Helper() var r *regexp.Regexp