From 395355d075b9c632fa6b2e7e0ea04cb2075cad99 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 8 Jun 2021 15:49:06 -0500 Subject: [PATCH 1/5] make prompt.Confirm stubbable --- pkg/prompt/prompt.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/prompt/prompt.go b/pkg/prompt/prompt.go index 169b0f42c..1c6fc7a61 100644 --- a/pkg/prompt/prompt.go +++ b/pkg/prompt/prompt.go @@ -18,7 +18,7 @@ var Confirm = func(prompt string, result *bool) error { Message: prompt, Default: true, } - return survey.AskOne(p, result) + return SurveyAskOne(p, result) } var SurveyAskOne = func(p survey.Prompt, response interface{}, opts ...survey.AskOpt) error { From 14de70a0112ada64cbd90467ead60ea0bf6b609f Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 9 Jun 2021 14:10:19 -0500 Subject: [PATCH 2/5] add defaultRemoteName --- pkg/cmd/repo/fork/fork.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/repo/fork/fork.go b/pkg/cmd/repo/fork/fork.go index 1bebcd7df..ea0a16e89 100644 --- a/pkg/cmd/repo/fork/fork.go +++ b/pkg/cmd/repo/fork/fork.go @@ -22,6 +22,8 @@ import ( "github.com/spf13/pflag" ) +const defaultRemoteName = "origin" + type ForkOptions struct { HttpClient func() (*http.Client, error) Config func() (config.Config, error) @@ -110,7 +112,7 @@ Additional 'git clone' flags can be passed in by listing them after '--'.`, cmd.Flags().BoolVar(&opts.Clone, "clone", false, "Clone the fork {true|false}") cmd.Flags().BoolVar(&opts.Remote, "remote", false, "Add remote for fork {true|false}") - cmd.Flags().StringVar(&opts.RemoteName, "remote-name", "origin", "Specify a name for a fork's new remote.") + cmd.Flags().StringVar(&opts.RemoteName, "remote-name", defaultRemoteName, "Specify a name for a fork's new remote.") cmd.Flags().StringVar(&opts.Organization, "org", "", "Create the fork in an organization") return cmd From 4a7ec7f4f6f79a457375f8d55556da50a4743a52 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Fri, 4 Jun 2021 12:47:58 -0500 Subject: [PATCH 3/5] cleaning up fork tests --- pkg/cmd/repo/fork/fork.go | 5 + pkg/cmd/repo/fork/fork_test.go | 1079 +++++++++++++++----------------- 2 files changed, 521 insertions(+), 563 deletions(-) diff --git a/pkg/cmd/repo/fork/fork.go b/pkg/cmd/repo/fork/fork.go index ea0a16e89..b1ca35193 100644 --- a/pkg/cmd/repo/fork/fork.go +++ b/pkg/cmd/repo/fork/fork.go @@ -42,10 +42,15 @@ type ForkOptions struct { Rename bool } +// TODO get this into the Options var Since = func(t time.Time) time.Duration { return time.Since(t) } +// TODO warn about useless flags (--remote, --remote-name) when running from outside a repository +// TODO output over STDOUT not STDERR +// TODO remote-name has no effect on its own; error that or change behavior + func NewCmdFork(f *cmdutil.Factory, runF func(*ForkOptions) error) *cobra.Command { opts := &ForkOptions{ IO: f.IOStreams, diff --git a/pkg/cmd/repo/fork/fork_test.go b/pkg/cmd/repo/fork/fork_test.go index 26f69aa97..7515c1167 100644 --- a/pkg/cmd/repo/fork/fork_test.go +++ b/pkg/cmd/repo/fork/fork_test.go @@ -5,11 +5,10 @@ import ( "io/ioutil" "net/http" "net/url" - "regexp" + "strings" "testing" "time" - "github.com/MakeNowJust/heredoc" "github.com/cli/cli/context" "github.com/cli/cli/git" "github.com/cli/cli/internal/config" @@ -19,7 +18,6 @@ import ( "github.com/cli/cli/pkg/httpmock" "github.com/cli/cli/pkg/iostreams" "github.com/cli/cli/pkg/prompt" - "github.com/cli/cli/test" "github.com/google/shlex" "github.com/stretchr/testify/assert" ) @@ -31,13 +29,14 @@ func TestNewCmdFork(t *testing.T) { tty bool wants ForkOptions wantErr bool + errMsg string }{ { name: "repo with git args", cli: "foo/bar -- --foo=bar", wants: ForkOptions{ Repository: "foo/bar", - GitArgs: []string{"TODO"}, + GitArgs: []string{"--foo=bar"}, RemoteName: "origin", Rename: true, }, @@ -46,6 +45,7 @@ func TestNewCmdFork(t *testing.T) { name: "git args without repo", cli: "-- --foo bar", wantErr: true, + errMsg: "repository argument required when passing 'git clone' flags", }, { name: "repo", @@ -54,12 +54,14 @@ func TestNewCmdFork(t *testing.T) { Repository: "foo/bar", RemoteName: "origin", Rename: true, + GitArgs: []string{}, }, }, { name: "blank remote name", cli: "--remote --remote-name=''", wantErr: true, + errMsg: "--remote-name cannot be blank", }, { name: "remote name", @@ -118,6 +120,12 @@ func TestNewCmdFork(t *testing.T) { Organization: "batmanshome", }, }, + { + name: "git flags in wrong place", + cli: "--depth 1 OWNER/REPO", + wantErr: true, + errMsg: "unknown flag: --depth\nSeparate git clone flags with '--'.", + }, } for _, tt := range tests { @@ -147,6 +155,7 @@ func TestNewCmdFork(t *testing.T) { _, err = cmd.ExecuteC() if tt.wantErr { assert.Error(t, err) + assert.Equal(t, tt.errMsg, err.Error()) return } assert.NoError(t, err) @@ -156,28 +165,491 @@ func TestNewCmdFork(t *testing.T) { assert.Equal(t, tt.wants.PromptRemote, gotOpts.PromptRemote) assert.Equal(t, tt.wants.PromptClone, gotOpts.PromptClone) assert.Equal(t, tt.wants.Organization, gotOpts.Organization) + assert.Equal(t, tt.wants.GitArgs, gotOpts.GitArgs) }) } } -func runCommand(httpClient *http.Client, remotes []*context.Remote, isTTY bool, cli string) (*test.CmdOut, error) { - io, stdin, stdout, stderr := iostreams.Test() - io.SetStdoutTTY(isTTY) - io.SetStdinTTY(isTTY) - io.SetStderrTTY(isTTY) - fac := &cmdutil.Factory{ - IOStreams: io, - HttpClient: func() (*http.Client, error) { - return httpClient, nil +// TODO try to replace with a Now in the opts +func stubSince(d time.Duration) func() { + originalSince := Since + Since = func(t time.Time) time.Duration { + return d + } + return func() { + Since = originalSince + } +} + +func TestRepoFork(t *testing.T) { + forkResult := `{ + "node_id": "123", + "name": "REPO", + "clone_url": "https://github.com/someone/repo.git", + "created_at": "2011-01-26T19:01:12Z", + "owner": { + "login": "someone" + } + }` + tests := []struct { + name string + opts *ForkOptions + cfg func(config.Config) + tty bool + httpStubs func(*httpmock.Registry) + execStubs func(*run.CommandStubber) + askStubs func(*prompt.AskStubber) + remotes []*context.Remote + since time.Duration + wantOut string + wantErrOut string + wantErr bool + errMsg string + }{ + + // TODO i don't like passing since every time, clean that up + + // TODO implicit, override existing remote's protocol with configured protocol + { + name: "implicit match existing remote's protocol", + tty: true, + opts: &ForkOptions{ + Remote: true, + RemoteName: "fork", + }, + remotes: []*context.Remote{ + { + Remote: &git.Remote{Name: "origin", PushURL: &url.URL{ + Scheme: "ssh", + }}, + Repo: ghrepo.New("OWNER", "REPO"), + }, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("POST", "repos/OWNER/REPO/forks"), + httpmock.StringResponse(forkResult)) + }, + execStubs: func(cs *run.CommandStubber) { + cs.Register(`git remote add -f fork git@github\.com:someone/REPO\.git`, 0, "") + }, + since: 2 * time.Second, + wantErrOut: "✓ Created fork someone/REPO\n✓ Added remote fork\n", }, - Config: func() (config.Config, error) { - return config.NewBlankConfig(), nil + { + name: "implicit with negative interactive choices", + tty: true, + opts: &ForkOptions{ + PromptRemote: true, + Rename: true, + RemoteName: defaultRemoteName, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("POST", "repos/OWNER/REPO/forks"), + httpmock.StringResponse(forkResult)) + }, + askStubs: func(as *prompt.AskStubber) { + as.StubOne(false) + }, + since: 2 * time.Second, + wantErrOut: "✓ Created fork someone/REPO\n", }, - BaseRepo: func() (ghrepo.Interface, error) { + { + name: "implicit with interactive choices", + tty: true, + opts: &ForkOptions{ + PromptRemote: true, + Rename: true, + RemoteName: defaultRemoteName, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("POST", "repos/OWNER/REPO/forks"), + httpmock.StringResponse(forkResult)) + }, + execStubs: func(cs *run.CommandStubber) { + cs.Register("git remote rename origin upstream", 0, "") + cs.Register(`git remote add -f origin https://github.com/someone/REPO.git`, 0, "") + }, + askStubs: func(as *prompt.AskStubber) { + as.StubOne(true) + }, + since: 2 * time.Second, + wantErrOut: "✓ Created fork someone/REPO\n✓ Added remote origin\n", + }, + { + name: "implicit tty reuse existing remote", + tty: true, + opts: &ForkOptions{ + Remote: true, + RemoteName: defaultRemoteName, + }, + remotes: []*context.Remote{ + { + Remote: &git.Remote{Name: "origin", FetchURL: &url.URL{}}, + Repo: ghrepo.New("someone", "REPO"), + }, + { + Remote: &git.Remote{Name: "upstream", FetchURL: &url.URL{}}, + Repo: ghrepo.New("OWNER", "REPO"), + }, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("POST", "repos/OWNER/REPO/forks"), + httpmock.StringResponse(forkResult)) + }, + since: 2 * time.Second, + wantErrOut: "✓ Created fork someone/REPO\n✓ Using existing remote origin\n", + }, + { + name: "implicit tty remote exists", + // gh repo fork --remote --remote-name origin | cat + tty: true, + opts: &ForkOptions{ + Remote: true, + RemoteName: defaultRemoteName, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("POST", "repos/OWNER/REPO/forks"), + httpmock.StringResponse(forkResult)) + }, + wantErr: true, + errMsg: "a git remote named 'origin' already exists", + since: 2 * time.Second, + }, + { + name: "implicit tty already forked", + tty: true, + opts: &ForkOptions{}, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("POST", "repos/OWNER/REPO/forks"), + httpmock.StringResponse(forkResult)) + }, + wantErrOut: "! someone/REPO already exists\n", + }, + { + name: "implicit tty --remote", + tty: true, + opts: &ForkOptions{ + Remote: true, + RemoteName: defaultRemoteName, + Rename: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("POST", "repos/OWNER/REPO/forks"), + httpmock.StringResponse(forkResult)) + }, + execStubs: func(cs *run.CommandStubber) { + cs.Register("git remote rename origin upstream", 0, "") + cs.Register(`git remote add -f origin https://github.com/someone/REPO.git`, 0, "") + }, + since: 2 * time.Second, + wantErrOut: "✓ Created fork someone/REPO\n✓ Added remote origin\n", + }, + { + name: "implicit nontty reuse existing remote", + opts: &ForkOptions{ + Remote: true, + RemoteName: defaultRemoteName, + Rename: true, + }, + remotes: []*context.Remote{ + { + Remote: &git.Remote{Name: "origin", FetchURL: &url.URL{}}, + Repo: ghrepo.New("someone", "REPO"), + }, + { + Remote: &git.Remote{Name: "upstream", FetchURL: &url.URL{}}, + Repo: ghrepo.New("OWNER", "REPO"), + }, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("POST", "repos/OWNER/REPO/forks"), + httpmock.StringResponse(forkResult)) + }, + since: 2 * time.Second, + }, + { + name: "implicit nontty remote exists", + // gh repo fork --remote --remote-name origin | cat + opts: &ForkOptions{ + Remote: true, + RemoteName: defaultRemoteName, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("POST", "repos/OWNER/REPO/forks"), + httpmock.StringResponse(forkResult)) + }, + wantErr: true, + errMsg: "a git remote named 'origin' already exists", + since: 2 * time.Second, + }, + { + name: "implicit nontty already forked", + opts: &ForkOptions{}, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("POST", "repos/OWNER/REPO/forks"), + httpmock.StringResponse(forkResult)) + }, + wantErrOut: "someone/REPO already exists", + }, + { + name: "implicit nontty --remote", + opts: &ForkOptions{ + Remote: true, + RemoteName: defaultRemoteName, + Rename: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("POST", "repos/OWNER/REPO/forks"), + httpmock.StringResponse(forkResult)) + }, + execStubs: func(cs *run.CommandStubber) { + cs.Register("git remote rename origin upstream", 0, "") + cs.Register(`git remote add -f origin https://github.com/someone/REPO.git`, 0, "") + }, + since: 2 * time.Second, + }, + { + name: "implicit nontty no args", + opts: &ForkOptions{}, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("POST", "repos/OWNER/REPO/forks"), + httpmock.StringResponse(forkResult)) + }, + since: 2 * time.Second, + }, + { + name: "passes git flags", + tty: true, + opts: &ForkOptions{ + Repository: "OWNER/REPO", + GitArgs: []string{"--depth", "1"}, + Clone: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("POST", "repos/OWNER/REPO/forks"), + httpmock.StringResponse(forkResult)) + }, + execStubs: func(cs *run.CommandStubber) { + cs.Register(`git clone --depth 1 https://github.com/someone/REPO\.git`, 0, "") + cs.Register(`git -C REPO remote add -f upstream https://github\.com/OWNER/REPO\.git`, 0, "") + }, + since: 2 * time.Second, + wantErrOut: "✓ Created fork someone/REPO\n✓ Cloned fork\n", + }, + { + name: "repo arg fork to org", + tty: true, + opts: &ForkOptions{ + Repository: "OWNER/REPO", + Organization: "gamehendge", + Clone: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("POST", "repos/OWNER/REPO/forks"), + func(req *http.Request) (*http.Response, error) { + bb, err := ioutil.ReadAll(req.Body) + if err != nil { + return nil, err + } + assert.Equal(t, `{"organization":"gamehendge"}`, strings.TrimSpace(string(bb))) + return &http.Response{ + Request: req, + StatusCode: 200, + Body: ioutil.NopCloser(bytes.NewBufferString(`{"name":"REPO", "owner":{"login":"gamehendge"}}`)), + }, nil + }) + }, + execStubs: func(cs *run.CommandStubber) { + cs.Register(`git clone https://github.com/gamehendge/REPO\.git`, 0, "") + cs.Register(`git -C REPO remote add -f upstream https://github\.com/OWNER/REPO\.git`, 0, "") + }, + since: 2 * time.Second, + wantErrOut: "✓ Created fork gamehendge/REPO\n✓ Cloned fork\n", + }, + { + name: "repo arg url arg", + tty: true, + opts: &ForkOptions{ + Repository: "https://github.com/OWNER/REPO.git", + Clone: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("POST", "repos/OWNER/REPO/forks"), + httpmock.StringResponse(forkResult)) + }, + execStubs: func(cs *run.CommandStubber) { + 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, "") + }, + since: 2 * time.Second, + wantErrOut: "✓ Created fork someone/REPO\n✓ Cloned fork\n", + }, + { + name: "repo arg interactive no clone", + tty: true, + opts: &ForkOptions{ + Repository: "OWNER/REPO", + PromptClone: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("POST", "repos/OWNER/REPO/forks"), + httpmock.StringResponse(forkResult)) + }, + askStubs: func(as *prompt.AskStubber) { + as.StubOne(false) + }, + since: 2 * time.Second, + wantErrOut: "✓ Created fork someone/REPO\n", + }, + { + name: "repo arg interactive", + tty: true, + opts: &ForkOptions{ + Repository: "OWNER/REPO", + PromptClone: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("POST", "repos/OWNER/REPO/forks"), + httpmock.StringResponse(forkResult)) + }, + askStubs: func(as *prompt.AskStubber) { + as.StubOne(true) + }, + execStubs: func(cs *run.CommandStubber) { + 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, "") + }, + since: 2 * time.Second, + wantErrOut: "✓ Created fork someone/REPO\n✓ Cloned fork\n", + }, + { + name: "repo arg interactive already forked", + tty: true, + opts: &ForkOptions{ + Repository: "OWNER/REPO", + PromptClone: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("POST", "repos/OWNER/REPO/forks"), + httpmock.StringResponse(forkResult)) + }, + askStubs: func(as *prompt.AskStubber) { + as.StubOne(true) + }, + execStubs: func(cs *run.CommandStubber) { + 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, "") + }, + wantErrOut: "! someone/REPO already exists\n✓ Cloned fork\n", + }, + { + name: "repo arg nontty no flags", + opts: &ForkOptions{ + Repository: "OWNER/REPO", + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("POST", "repos/OWNER/REPO/forks"), + httpmock.StringResponse(forkResult)) + }, + since: 2 * time.Second, + }, + { + name: "repo arg nontty repo already exists", + opts: &ForkOptions{ + Repository: "OWNER/REPO", + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("POST", "repos/OWNER/REPO/forks"), + httpmock.StringResponse(forkResult)) + }, + wantErrOut: "someone/REPO already exists", + }, + { + name: "repo arg nontty clone arg already exists", + opts: &ForkOptions{ + Repository: "OWNER/REPO", + Clone: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("POST", "repos/OWNER/REPO/forks"), + httpmock.StringResponse(forkResult)) + }, + execStubs: func(cs *run.CommandStubber) { + 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, "") + }, + wantErrOut: "someone/REPO already exists", + }, + { + name: "repo arg nontty clone arg", + opts: &ForkOptions{ + Repository: "OWNER/REPO", + Clone: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("POST", "repos/OWNER/REPO/forks"), + httpmock.StringResponse(forkResult)) + }, + execStubs: func(cs *run.CommandStubber) { + 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, "") + }, + since: 2 * time.Second, + }, + } + + for _, tt := range tests { + io, _, stdout, stderr := iostreams.Test() + io.SetStdinTTY(tt.tty) + io.SetStdoutTTY(tt.tty) + io.SetStderrTTY(tt.tty) + tt.opts.IO = io + + tt.opts.BaseRepo = func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil - }, - Remotes: func() (context.Remotes, error) { - if remotes == nil { + } + + reg := &httpmock.Registry{} + if tt.httpStubs != nil { + tt.httpStubs(reg) + } + tt.opts.HttpClient = func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + } + + cfg := config.NewBlankConfig() + if tt.cfg != nil { + tt.cfg(cfg) + } + + tt.opts.Config = func() (config.Config, error) { + return cfg, nil + } + + tt.opts.Remotes = func() (context.Remotes, error) { + if tt.remotes == nil { return []*context.Remote{ { Remote: &git.Remote{ @@ -188,555 +660,36 @@ func runCommand(httpClient *http.Client, remotes []*context.Remote, isTTY bool, }, }, nil } + return tt.remotes, nil + } - return remotes, nil - }, - } + as, teardown := prompt.InitAskStubber() + defer teardown() + if tt.askStubs != nil { + tt.askStubs(as) + } + cs, restoreRun := run.Stub() + defer restoreRun(t) + if tt.execStubs != nil { + tt.execStubs(cs) + } - cmd := NewCmdFork(fac, nil) - - argv, err := shlex.Split(cli) - cmd.SetArgs(argv) - - cmd.SetIn(stdin) - cmd.SetOut(stdout) - cmd.SetErr(stderr) - - if err != nil { - panic(err) - } - - _, err = cmd.ExecuteC() - - if err != nil { - return nil, err - } - - return &test.CmdOut{ - OutBuf: stdout, - ErrBuf: stderr}, nil -} - -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} - - _, 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, "", output.String()) - assert.Equal(t, "", output.Stderr()) - -} - -func TestRepoFork_no_conflicting_remote(t *testing.T) { - remotes := []*context.Remote{ - { - Remote: &git.Remote{ - Name: "upstream", - FetchURL: &url.URL{}, - }, - Repo: ghrepo.New("OWNER", "REPO"), - }, - } - 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 := run.Stub() - defer restore(t) - - cs.Register(`git remote add -f origin https://github\.com/someone/REPO\.git`, 0, "") - - output, err := runCommand(httpClient, remotes, false, "--remote") - if err != nil { - t.Fatalf("error running command `repo fork`: %v", err) - } - - assert.Equal(t, "", output.String()) - assert.Equal(t, "", output.Stderr()) -} - -func TestRepoFork_existing_remote_error(t *testing.T) { - defer stubSince(2 * time.Second)() - reg := &httpmock.Registry{} - defer reg.StubWithFixturePath(200, "./forkResult.json")() - httpClient := &http.Client{Transport: reg} - - _, err := runCommand(httpClient, nil, true, "--remote --remote-name='origin'") - if err == nil { - t.Fatal("expected error running command `repo fork`") - } - - assert.Equal(t, "a git remote named 'origin' already exists", err.Error()) - - reg.Verify(t) -} - -func TestRepoFork_in_parent_tty(t *testing.T) { - defer stubSince(2 * time.Second)() - reg := &httpmock.Registry{} - defer reg.StubWithFixturePath(200, "./forkResult.json")() - httpClient := &http.Client{Transport: reg} - - 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.Fatalf("error running command `repo fork`: %v", err) - } - - assert.Equal(t, "", output.String()) - assert.Equal(t, "✓ Created fork someone/REPO\n✓ Added remote origin\n", output.Stderr()) - reg.Verify(t) -} - -func TestRepoFork_in_parent_nontty(t *testing.T) { - defer stubSince(2 * time.Second)() - reg := &httpmock.Registry{} - defer reg.StubWithFixturePath(200, "./forkResult.json")() - httpClient := &http.Client{Transport: reg} - - 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, false, "--remote") - if err != nil { - t.Fatalf("error running command `repo fork`: %v", err) - } - - assert.Equal(t, "", output.String()) - assert.Equal(t, "", output.Stderr()) - reg.Verify(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 := run.Stub() - defer restore(t) - - 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 { - t.Errorf("error running command `repo fork`: %v", err) - } - - assert.Equal(t, "", output.String()) - assert.Equal(t, output.Stderr(), "") - -} - -func TestRepoFork_already_forked(t *testing.T) { - reg := &httpmock.Registry{} - defer reg.StubWithFixturePath(200, "./forkResult.json")() - httpClient := &http.Client{Transport: reg} - - _, restore := run.Stub() - defer restore(t) - - output, err := runCommand(httpClient, nil, true, "--remote=false") - if err != nil { - t.Errorf("got unexpected error: %v", err) - } - - 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()) - return - } - - reg.Verify(t) -} - -func TestRepoFork_reuseRemote(t *testing.T) { - remotes := []*context.Remote{ - { - Remote: &git.Remote{Name: "origin", FetchURL: &url.URL{}}, - Repo: ghrepo.New("someone", "REPO"), - }, - { - Remote: &git.Remote{Name: "upstream", FetchURL: &url.URL{}}, - Repo: ghrepo.New("OWNER", "REPO"), - }, - } - reg := &httpmock.Registry{} - defer reg.StubWithFixturePath(200, "./forkResult.json")() - httpClient := &http.Client{Transport: reg} - - output, err := runCommand(httpClient, remotes, true, "--remote") - if err != nil { - t.Errorf("got unexpected error: %v", err) - } - r := regexp.MustCompile(`Using existing remote.*origin`) - if !r.MatchString(output.Stderr()) { - t.Errorf("output did not match: %q", output.Stderr()) - return - } - reg.Verify(t) -} - -func TestRepoFork_in_parent(t *testing.T) { - reg := &httpmock.Registry{} - defer reg.StubWithFixturePath(200, "./forkResult.json")() - httpClient := &http.Client{Transport: reg} - - _, restore := run.Stub() - defer restore(t) - defer stubSince(2 * time.Second)() - - output, err := runCommand(httpClient, nil, true, "--remote=false") - if err != nil { - t.Errorf("error running command `repo fork`: %v", err) - } - - assert.Equal(t, "", output.String()) - - 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_outside(t *testing.T) { - tests := []struct { - name string - args string - postBody string - responseBody string - wantStderr string - }{ - { - name: "url arg", - args: "--clone=false http://github.com/OWNER/REPO.git", - postBody: "{}\n", - responseBody: `{"name":"REPO", "owner":{"login":"monalisa"}}`, - wantStderr: heredoc.Doc(` - ✓ Created fork monalisa/REPO - `), - }, - { - name: "full name arg", - args: "--clone=false OWNER/REPO", - postBody: "{}\n", - responseBody: `{"name":"REPO", "owner":{"login":"monalisa"}}`, - wantStderr: heredoc.Doc(` - ✓ Created fork monalisa/REPO - `), - }, - { - name: "fork to org without clone", - args: "--clone=false OWNER/REPO --org batmanshome", - postBody: "{\"organization\":\"batmanshome\"}\n", - responseBody: `{"name":"REPO", "owner":{"login":"BatmansHome"}}`, - wantStderr: heredoc.Doc(` - ✓ Created fork BatmansHome/REPO - `), - }, - } - for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - defer stubSince(2 * time.Second)() + if tt.since > 0 { + // TODO hate this + defer stubSince(tt.since)() + } + defer reg.Verify(t) + err := forkRun(tt.opts) + if tt.wantErr { + assert.Error(t, err) + assert.Equal(t, tt.errMsg, err.Error()) + return + } - reg := &httpmock.Registry{} - reg.Register( - httpmock.REST("POST", "repos/OWNER/REPO/forks"), - func(req *http.Request) (*http.Response, error) { - bb, err := ioutil.ReadAll(req.Body) - if err != nil { - return nil, err - } - assert.Equal(t, tt.postBody, string(bb)) - return &http.Response{ - Request: req, - StatusCode: 200, - Body: ioutil.NopCloser(bytes.NewBufferString(tt.responseBody)), - }, nil - }) - - httpClient := &http.Client{Transport: reg} - output, err := runCommand(httpClient, nil, true, tt.args) assert.NoError(t, err) - assert.Equal(t, "", output.String()) - assert.Equal(t, tt.wantStderr, output.Stderr()) - reg.Verify(t) + assert.Equal(t, tt.wantOut, stdout.String()) + assert.Equal(t, tt.wantErrOut, stderr.String()) }) } } - -func TestRepoFork_in_parent_yes(t *testing.T) { - defer stubSince(2 * time.Second)() - reg := &httpmock.Registry{} - defer reg.StubWithFixturePath(200, "./forkResult.json")() - httpClient := &http.Client{Transport: reg} - - cs, restore := run.Stub() - defer restore(t) - - cs.Register(`git remote add -f fork https://github\.com/someone/REPO\.git`, 0, "") - - output, err := runCommand(httpClient, nil, true, "--remote --remote-name=fork") - if err != nil { - t.Errorf("error running command `repo fork`: %v", err) - } - - assert.Equal(t, "", output.String()) - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.Stderr(), - "Created fork.*someone/REPO", - "Added remote.*fork") - reg.Verify(t) -} - -func TestRepoFork_outside_yes(t *testing.T) { - defer stubSince(2 * time.Second)() - reg := &httpmock.Registry{} - defer reg.StubWithFixturePath(200, "./forkResult.json")() - httpClient := &http.Client{Transport: reg} - - cs, restore := run.Stub() - defer restore(t) - - 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 { - t.Errorf("error running command `repo fork`: %v", err) - } - - assert.Equal(t, "", output.String()) - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.Stderr(), - "Created fork.*someone/REPO", - "Cloned fork") - reg.Verify(t) -} - -func TestRepoFork_ForkAlreadyExistsAndCloneNonTty(t *testing.T) { - defer stubSince(2 * time.Minute)() - reg := &httpmock.Registry{} - defer reg.StubWithFixturePath(200, "./forkResult.json")() - httpClient := &http.Client{Transport: reg} - - cs, restore := run.Stub() - defer restore(t) - - 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 { - t.Errorf("error running command `repo fork`: %v", err) - } - - assert.Equal(t, "", output.String()) - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.Stderr(), - "someone/REPO.*already exists") - reg.Verify(t) -} - -func TestRepoFork_outside_survey_yes(t *testing.T) { - defer stubSince(2 * time.Second)() - reg := &httpmock.Registry{} - defer reg.StubWithFixturePath(200, "./forkResult.json")() - httpClient := &http.Client{Transport: reg} - - cs, restore := run.Stub() - defer restore(t) - - 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)() - - output, err := runCommand(httpClient, nil, true, "OWNER/REPO") - if err != nil { - t.Errorf("error running command `repo fork`: %v", err) - } - - assert.Equal(t, "", output.String()) - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.Stderr(), - "Created fork.*someone/REPO", - "Cloned fork") - reg.Verify(t) -} - -func TestRepoFork_outside_survey_no(t *testing.T) { - defer stubSince(2 * time.Second)() - reg := &httpmock.Registry{} - defer reg.StubWithFixturePath(200, "./forkResult.json")() - httpClient := &http.Client{Transport: reg} - - _, restore := run.Stub() - defer restore(t) - - defer prompt.StubConfirm(false)() - - output, err := runCommand(httpClient, nil, true, "OWNER/REPO") - if err != nil { - t.Errorf("error running command `repo fork`: %v", err) - } - - assert.Equal(t, "", output.String()) - - 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_survey_yes(t *testing.T) { - reg := &httpmock.Registry{} - defer reg.StubWithFixturePath(200, "./forkResult.json")() - httpClient := &http.Client{Transport: reg} - defer stubSince(2 * time.Second)() - - cs, restore := run.Stub() - defer restore(t) - - cs.Register(`git remote add -f fork https://github\.com/someone/REPO\.git`, 0, "") - - defer prompt.StubConfirm(true)() - - output, err := runCommand(httpClient, nil, true, "--remote-name=fork") - if err != nil { - t.Errorf("error running command `repo fork`: %v", err) - } - - assert.Equal(t, "", output.String()) - - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.Stderr(), - "Created fork.*someone/REPO", - "Added remote.*fork") - reg.Verify(t) -} - -func TestRepoFork_in_parent_survey_no(t *testing.T) { - reg := &httpmock.Registry{} - defer reg.Verify(t) - defer reg.StubWithFixturePath(200, "./forkResult.json")() - httpClient := &http.Client{Transport: reg} - defer stubSince(2 * time.Second)() - - _, restore := run.Stub() - defer restore(t) - - defer prompt.StubConfirm(false)() - - output, err := runCommand(httpClient, nil, true, "") - if err != nil { - t.Errorf("error running command `repo fork`: %v", err) - } - - assert.Equal(t, "", output.String()) - - 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 - } -} - -func Test_RepoFork_gitFlags(t *testing.T) { - defer stubSince(2 * time.Second)() - reg := &httpmock.Registry{} - defer reg.StubWithFixturePath(200, "./forkResult.json")() - httpClient := &http.Client{Transport: reg} - - cs, cmdTeardown := run.Stub() - defer cmdTeardown(t) - - cs.Register(`git clone --depth 1 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 -- --depth 1") - if err != nil { - t.Errorf("error running command `repo fork`: %v", err) - } - - assert.Equal(t, "", output.String()) - assert.Equal(t, output.Stderr(), "") - reg.Verify(t) -} - -func Test_RepoFork_flagError(t *testing.T) { - _, err := runCommand(nil, nil, true, "--depth 1 OWNER/REPO") - if err == nil || err.Error() != "unknown flag: --depth\nSeparate git clone flags with '--'." { - t.Errorf("unexpected error %v", err) - } -} - -func TestRepoFork_in_parent_match_protocol(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 := run.Stub() - defer restore(t) - - cs.Register(`git remote add -f fork git@github\.com:someone/REPO\.git`, 0, "") - - remotes := []*context.Remote{ - { - Remote: &git.Remote{Name: "origin", PushURL: &url.URL{ - Scheme: "ssh", - }}, - Repo: ghrepo.New("OWNER", "REPO"), - }, - } - - output, err := runCommand(httpClient, remotes, true, "--remote --remote-name=fork") - if err != nil { - t.Errorf("error running command `repo fork`: %v", err) - } - - assert.Equal(t, "", output.String()) - - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.Stderr(), - "Created fork.*someone/REPO", - "Added remote.*fork") -} - -func stubSince(d time.Duration) func() { - originalSince := Since - Since = func(t time.Time) time.Duration { - return d - } - return func() { - Since = originalSince - } -} From f31a31e2edc0b30880dfddddb7bbf7c83e30c89a Mon Sep 17 00:00:00 2001 From: Nate Smith Date: Thu, 10 Jun 2021 21:30:04 +0000 Subject: [PATCH 4/5] stop stubbing out a Since function --- pkg/cmd/repo/fork/fork.go | 9 ++---- pkg/cmd/repo/fork/fork_test.go | 59 +++++++++++++--------------------- 2 files changed, 26 insertions(+), 42 deletions(-) diff --git a/pkg/cmd/repo/fork/fork.go b/pkg/cmd/repo/fork/fork.go index b1ca35193..a441b1af5 100644 --- a/pkg/cmd/repo/fork/fork.go +++ b/pkg/cmd/repo/fork/fork.go @@ -30,6 +30,7 @@ type ForkOptions struct { IO *iostreams.IOStreams BaseRepo func() (ghrepo.Interface, error) Remotes func() (context.Remotes, error) + Since func(time.Time) time.Duration GitArgs []string Repository string @@ -42,11 +43,6 @@ type ForkOptions struct { Rename bool } -// TODO get this into the Options -var Since = func(t time.Time) time.Duration { - return time.Since(t) -} - // TODO warn about useless flags (--remote, --remote-name) when running from outside a repository // TODO output over STDOUT not STDERR // TODO remote-name has no effect on its own; error that or change behavior @@ -58,6 +54,7 @@ func NewCmdFork(f *cmdutil.Factory, runF func(*ForkOptions) error) *cobra.Comman Config: f.Config, BaseRepo: f.BaseRepo, Remotes: f.Remotes, + Since: time.Since, } cmd := &cobra.Command{ @@ -189,7 +186,7 @@ func forkRun(opts *ForkOptions) error { // returns the fork repo data even if it already exists -- with no change in status code or // anything. We thus check the created time to see if the repo is brand new or not; if it's not, // we assume the fork already existed and report an error. - createdAgo := Since(forkedRepo.CreatedAt) + createdAgo := opts.Since(forkedRepo.CreatedAt) if createdAgo > time.Minute { if connectedToTerminal { fmt.Fprintf(stderr, "%s %s %s\n", diff --git a/pkg/cmd/repo/fork/fork_test.go b/pkg/cmd/repo/fork/fork_test.go index 7515c1167..5a39972f8 100644 --- a/pkg/cmd/repo/fork/fork_test.go +++ b/pkg/cmd/repo/fork/fork_test.go @@ -170,17 +170,6 @@ func TestNewCmdFork(t *testing.T) { } } -// TODO try to replace with a Now in the opts -func stubSince(d time.Duration) func() { - originalSince := Since - Since = func(t time.Time) time.Duration { - return d - } - return func() { - Since = originalSince - } -} - func TestRepoFork(t *testing.T) { forkResult := `{ "node_id": "123", @@ -206,9 +195,6 @@ func TestRepoFork(t *testing.T) { wantErr bool errMsg string }{ - - // TODO i don't like passing since every time, clean that up - // TODO implicit, override existing remote's protocol with configured protocol { name: "implicit match existing remote's protocol", @@ -233,7 +219,6 @@ func TestRepoFork(t *testing.T) { execStubs: func(cs *run.CommandStubber) { cs.Register(`git remote add -f fork git@github\.com:someone/REPO\.git`, 0, "") }, - since: 2 * time.Second, wantErrOut: "✓ Created fork someone/REPO\n✓ Added remote fork\n", }, { @@ -252,7 +237,6 @@ func TestRepoFork(t *testing.T) { askStubs: func(as *prompt.AskStubber) { as.StubOne(false) }, - since: 2 * time.Second, wantErrOut: "✓ Created fork someone/REPO\n", }, { @@ -275,7 +259,6 @@ func TestRepoFork(t *testing.T) { askStubs: func(as *prompt.AskStubber) { as.StubOne(true) }, - since: 2 * time.Second, wantErrOut: "✓ Created fork someone/REPO\n✓ Added remote origin\n", }, { @@ -300,7 +283,6 @@ func TestRepoFork(t *testing.T) { httpmock.REST("POST", "repos/OWNER/REPO/forks"), httpmock.StringResponse(forkResult)) }, - since: 2 * time.Second, wantErrOut: "✓ Created fork someone/REPO\n✓ Using existing remote origin\n", }, { @@ -318,12 +300,15 @@ func TestRepoFork(t *testing.T) { }, wantErr: true, errMsg: "a git remote named 'origin' already exists", - since: 2 * time.Second, }, { name: "implicit tty already forked", tty: true, - opts: &ForkOptions{}, + opts: &ForkOptions{ + Since: func(t time.Time) time.Duration { + return 120 * time.Second + }, + }, httpStubs: func(reg *httpmock.Registry) { reg.Register( httpmock.REST("POST", "repos/OWNER/REPO/forks"), @@ -348,7 +333,6 @@ func TestRepoFork(t *testing.T) { cs.Register("git remote rename origin upstream", 0, "") cs.Register(`git remote add -f origin https://github.com/someone/REPO.git`, 0, "") }, - since: 2 * time.Second, wantErrOut: "✓ Created fork someone/REPO\n✓ Added remote origin\n", }, { @@ -373,7 +357,6 @@ func TestRepoFork(t *testing.T) { httpmock.REST("POST", "repos/OWNER/REPO/forks"), httpmock.StringResponse(forkResult)) }, - since: 2 * time.Second, }, { name: "implicit nontty remote exists", @@ -389,11 +372,14 @@ func TestRepoFork(t *testing.T) { }, wantErr: true, errMsg: "a git remote named 'origin' already exists", - since: 2 * time.Second, }, { name: "implicit nontty already forked", - opts: &ForkOptions{}, + opts: &ForkOptions{ + Since: func(t time.Time) time.Duration { + return 120 * time.Second + }, + }, httpStubs: func(reg *httpmock.Registry) { reg.Register( httpmock.REST("POST", "repos/OWNER/REPO/forks"), @@ -417,7 +403,6 @@ func TestRepoFork(t *testing.T) { cs.Register("git remote rename origin upstream", 0, "") cs.Register(`git remote add -f origin https://github.com/someone/REPO.git`, 0, "") }, - since: 2 * time.Second, }, { name: "implicit nontty no args", @@ -427,7 +412,6 @@ func TestRepoFork(t *testing.T) { httpmock.REST("POST", "repos/OWNER/REPO/forks"), httpmock.StringResponse(forkResult)) }, - since: 2 * time.Second, }, { name: "passes git flags", @@ -446,7 +430,6 @@ func TestRepoFork(t *testing.T) { cs.Register(`git clone --depth 1 https://github.com/someone/REPO\.git`, 0, "") cs.Register(`git -C REPO remote add -f upstream https://github\.com/OWNER/REPO\.git`, 0, "") }, - since: 2 * time.Second, wantErrOut: "✓ Created fork someone/REPO\n✓ Cloned fork\n", }, { @@ -477,7 +460,6 @@ func TestRepoFork(t *testing.T) { cs.Register(`git clone https://github.com/gamehendge/REPO\.git`, 0, "") cs.Register(`git -C REPO remote add -f upstream https://github\.com/OWNER/REPO\.git`, 0, "") }, - since: 2 * time.Second, wantErrOut: "✓ Created fork gamehendge/REPO\n✓ Cloned fork\n", }, { @@ -496,7 +478,6 @@ func TestRepoFork(t *testing.T) { 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, "") }, - since: 2 * time.Second, wantErrOut: "✓ Created fork someone/REPO\n✓ Cloned fork\n", }, { @@ -514,7 +495,6 @@ func TestRepoFork(t *testing.T) { askStubs: func(as *prompt.AskStubber) { as.StubOne(false) }, - since: 2 * time.Second, wantErrOut: "✓ Created fork someone/REPO\n", }, { @@ -536,7 +516,6 @@ func TestRepoFork(t *testing.T) { 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, "") }, - since: 2 * time.Second, wantErrOut: "✓ Created fork someone/REPO\n✓ Cloned fork\n", }, { @@ -545,6 +524,9 @@ func TestRepoFork(t *testing.T) { opts: &ForkOptions{ Repository: "OWNER/REPO", PromptClone: true, + Since: func(t time.Time) time.Duration { + return 120 * time.Second + }, }, httpStubs: func(reg *httpmock.Registry) { reg.Register( @@ -570,12 +552,14 @@ func TestRepoFork(t *testing.T) { httpmock.REST("POST", "repos/OWNER/REPO/forks"), httpmock.StringResponse(forkResult)) }, - since: 2 * time.Second, }, { name: "repo arg nontty repo already exists", opts: &ForkOptions{ Repository: "OWNER/REPO", + Since: func(t time.Time) time.Duration { + return 120 * time.Second + }, }, httpStubs: func(reg *httpmock.Registry) { reg.Register( @@ -589,6 +573,9 @@ func TestRepoFork(t *testing.T) { opts: &ForkOptions{ Repository: "OWNER/REPO", Clone: true, + Since: func(t time.Time) time.Duration { + return 120 * time.Second + }, }, httpStubs: func(reg *httpmock.Registry) { reg.Register( @@ -616,7 +603,6 @@ func TestRepoFork(t *testing.T) { 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, "") }, - since: 2 * time.Second, }, } @@ -675,9 +661,10 @@ func TestRepoFork(t *testing.T) { } t.Run(tt.name, func(t *testing.T) { - if tt.since > 0 { - // TODO hate this - defer stubSince(tt.since)() + if tt.opts.Since == nil { + tt.opts.Since = func(t time.Time) time.Duration { + return 2 * time.Second + } } defer reg.Verify(t) err := forkRun(tt.opts) From b0998772ae6843659609e1410dae0a7b9ebd91b5 Mon Sep 17 00:00:00 2001 From: Nate Smith Date: Thu, 10 Jun 2021 21:40:56 +0000 Subject: [PATCH 5/5] more cleanup --- pkg/cmd/repo/fork/fork_test.go | 167 ++++++++------------------------- 1 file changed, 41 insertions(+), 126 deletions(-) diff --git a/pkg/cmd/repo/fork/fork_test.go b/pkg/cmd/repo/fork/fork_test.go index 5a39972f8..914f60af8 100644 --- a/pkg/cmd/repo/fork/fork_test.go +++ b/pkg/cmd/repo/fork/fork_test.go @@ -171,25 +171,28 @@ func TestNewCmdFork(t *testing.T) { } func TestRepoFork(t *testing.T) { - forkResult := `{ - "node_id": "123", - "name": "REPO", - "clone_url": "https://github.com/someone/repo.git", - "created_at": "2011-01-26T19:01:12Z", - "owner": { - "login": "someone" - } - }` + forkPost := func(reg *httpmock.Registry) { + forkResult := `{ + "node_id": "123", + "name": "REPO", + "clone_url": "https://github.com/someone/repo.git", + "created_at": "2011-01-26T19:01:12Z", + "owner": { + "login": "someone" + } + }` + reg.Register( + httpmock.REST("POST", "repos/OWNER/REPO/forks"), + httpmock.StringResponse(forkResult)) + } tests := []struct { name string opts *ForkOptions - cfg func(config.Config) tty bool httpStubs func(*httpmock.Registry) execStubs func(*run.CommandStubber) askStubs func(*prompt.AskStubber) remotes []*context.Remote - since time.Duration wantOut string wantErrOut string wantErr bool @@ -211,11 +214,7 @@ func TestRepoFork(t *testing.T) { Repo: ghrepo.New("OWNER", "REPO"), }, }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.REST("POST", "repos/OWNER/REPO/forks"), - httpmock.StringResponse(forkResult)) - }, + httpStubs: forkPost, execStubs: func(cs *run.CommandStubber) { cs.Register(`git remote add -f fork git@github\.com:someone/REPO\.git`, 0, "") }, @@ -229,11 +228,7 @@ func TestRepoFork(t *testing.T) { Rename: true, RemoteName: defaultRemoteName, }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.REST("POST", "repos/OWNER/REPO/forks"), - httpmock.StringResponse(forkResult)) - }, + httpStubs: forkPost, askStubs: func(as *prompt.AskStubber) { as.StubOne(false) }, @@ -247,11 +242,7 @@ func TestRepoFork(t *testing.T) { Rename: true, RemoteName: defaultRemoteName, }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.REST("POST", "repos/OWNER/REPO/forks"), - httpmock.StringResponse(forkResult)) - }, + httpStubs: forkPost, execStubs: func(cs *run.CommandStubber) { cs.Register("git remote rename origin upstream", 0, "") cs.Register(`git remote add -f origin https://github.com/someone/REPO.git`, 0, "") @@ -278,11 +269,7 @@ func TestRepoFork(t *testing.T) { Repo: ghrepo.New("OWNER", "REPO"), }, }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.REST("POST", "repos/OWNER/REPO/forks"), - httpmock.StringResponse(forkResult)) - }, + httpStubs: forkPost, wantErrOut: "✓ Created fork someone/REPO\n✓ Using existing remote origin\n", }, { @@ -293,13 +280,9 @@ func TestRepoFork(t *testing.T) { Remote: true, RemoteName: defaultRemoteName, }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.REST("POST", "repos/OWNER/REPO/forks"), - httpmock.StringResponse(forkResult)) - }, - wantErr: true, - errMsg: "a git remote named 'origin' already exists", + httpStubs: forkPost, + wantErr: true, + errMsg: "a git remote named 'origin' already exists", }, { name: "implicit tty already forked", @@ -309,11 +292,7 @@ func TestRepoFork(t *testing.T) { return 120 * time.Second }, }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.REST("POST", "repos/OWNER/REPO/forks"), - httpmock.StringResponse(forkResult)) - }, + httpStubs: forkPost, wantErrOut: "! someone/REPO already exists\n", }, { @@ -324,11 +303,7 @@ func TestRepoFork(t *testing.T) { RemoteName: defaultRemoteName, Rename: true, }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.REST("POST", "repos/OWNER/REPO/forks"), - httpmock.StringResponse(forkResult)) - }, + httpStubs: forkPost, execStubs: func(cs *run.CommandStubber) { cs.Register("git remote rename origin upstream", 0, "") cs.Register(`git remote add -f origin https://github.com/someone/REPO.git`, 0, "") @@ -352,11 +327,7 @@ func TestRepoFork(t *testing.T) { Repo: ghrepo.New("OWNER", "REPO"), }, }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.REST("POST", "repos/OWNER/REPO/forks"), - httpmock.StringResponse(forkResult)) - }, + httpStubs: forkPost, }, { name: "implicit nontty remote exists", @@ -365,13 +336,9 @@ func TestRepoFork(t *testing.T) { Remote: true, RemoteName: defaultRemoteName, }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.REST("POST", "repos/OWNER/REPO/forks"), - httpmock.StringResponse(forkResult)) - }, - wantErr: true, - errMsg: "a git remote named 'origin' already exists", + httpStubs: forkPost, + wantErr: true, + errMsg: "a git remote named 'origin' already exists", }, { name: "implicit nontty already forked", @@ -380,11 +347,7 @@ func TestRepoFork(t *testing.T) { return 120 * time.Second }, }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.REST("POST", "repos/OWNER/REPO/forks"), - httpmock.StringResponse(forkResult)) - }, + httpStubs: forkPost, wantErrOut: "someone/REPO already exists", }, { @@ -394,24 +357,16 @@ func TestRepoFork(t *testing.T) { RemoteName: defaultRemoteName, Rename: true, }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.REST("POST", "repos/OWNER/REPO/forks"), - httpmock.StringResponse(forkResult)) - }, + httpStubs: forkPost, execStubs: func(cs *run.CommandStubber) { cs.Register("git remote rename origin upstream", 0, "") cs.Register(`git remote add -f origin https://github.com/someone/REPO.git`, 0, "") }, }, { - name: "implicit nontty no args", - opts: &ForkOptions{}, - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.REST("POST", "repos/OWNER/REPO/forks"), - httpmock.StringResponse(forkResult)) - }, + name: "implicit nontty no args", + opts: &ForkOptions{}, + httpStubs: forkPost, }, { name: "passes git flags", @@ -421,11 +376,7 @@ func TestRepoFork(t *testing.T) { GitArgs: []string{"--depth", "1"}, Clone: true, }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.REST("POST", "repos/OWNER/REPO/forks"), - httpmock.StringResponse(forkResult)) - }, + httpStubs: forkPost, execStubs: func(cs *run.CommandStubber) { cs.Register(`git clone --depth 1 https://github.com/someone/REPO\.git`, 0, "") cs.Register(`git -C REPO remote add -f upstream https://github\.com/OWNER/REPO\.git`, 0, "") @@ -469,11 +420,7 @@ func TestRepoFork(t *testing.T) { Repository: "https://github.com/OWNER/REPO.git", Clone: true, }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.REST("POST", "repos/OWNER/REPO/forks"), - httpmock.StringResponse(forkResult)) - }, + httpStubs: forkPost, execStubs: func(cs *run.CommandStubber) { 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, "") @@ -487,11 +434,7 @@ func TestRepoFork(t *testing.T) { Repository: "OWNER/REPO", PromptClone: true, }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.REST("POST", "repos/OWNER/REPO/forks"), - httpmock.StringResponse(forkResult)) - }, + httpStubs: forkPost, askStubs: func(as *prompt.AskStubber) { as.StubOne(false) }, @@ -504,11 +447,7 @@ func TestRepoFork(t *testing.T) { Repository: "OWNER/REPO", PromptClone: true, }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.REST("POST", "repos/OWNER/REPO/forks"), - httpmock.StringResponse(forkResult)) - }, + httpStubs: forkPost, askStubs: func(as *prompt.AskStubber) { as.StubOne(true) }, @@ -528,11 +467,7 @@ func TestRepoFork(t *testing.T) { return 120 * time.Second }, }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.REST("POST", "repos/OWNER/REPO/forks"), - httpmock.StringResponse(forkResult)) - }, + httpStubs: forkPost, askStubs: func(as *prompt.AskStubber) { as.StubOne(true) }, @@ -547,11 +482,7 @@ func TestRepoFork(t *testing.T) { opts: &ForkOptions{ Repository: "OWNER/REPO", }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.REST("POST", "repos/OWNER/REPO/forks"), - httpmock.StringResponse(forkResult)) - }, + httpStubs: forkPost, }, { name: "repo arg nontty repo already exists", @@ -561,11 +492,7 @@ func TestRepoFork(t *testing.T) { return 120 * time.Second }, }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.REST("POST", "repos/OWNER/REPO/forks"), - httpmock.StringResponse(forkResult)) - }, + httpStubs: forkPost, wantErrOut: "someone/REPO already exists", }, { @@ -577,11 +504,7 @@ func TestRepoFork(t *testing.T) { return 120 * time.Second }, }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.REST("POST", "repos/OWNER/REPO/forks"), - httpmock.StringResponse(forkResult)) - }, + httpStubs: forkPost, execStubs: func(cs *run.CommandStubber) { 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, "") @@ -594,11 +517,7 @@ func TestRepoFork(t *testing.T) { Repository: "OWNER/REPO", Clone: true, }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.REST("POST", "repos/OWNER/REPO/forks"), - httpmock.StringResponse(forkResult)) - }, + httpStubs: forkPost, execStubs: func(cs *run.CommandStubber) { 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, "") @@ -626,10 +545,6 @@ func TestRepoFork(t *testing.T) { } cfg := config.NewBlankConfig() - if tt.cfg != nil { - tt.cfg(cfg) - } - tt.opts.Config = func() (config.Config, error) { return cfg, nil }