fully restore fork remote renaming behavior (#2982)

* fully restore fork remote renaming behavior

* catch blank remote name and error + arg tests

* hard wrap fork usage

* do not rename if remote-name supplied

* tweak error text
This commit is contained in:
Nate Smith 2021-02-17 12:33:22 -06:00 committed by GitHub
parent 4a897f70c3
commit e91b97b4c5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 180 additions and 22 deletions

View file

@ -1,6 +1,7 @@
package fork
import (
"errors"
"fmt"
"net/http"
"net/url"
@ -35,6 +36,7 @@ type ForkOptions struct {
PromptClone bool
PromptRemote bool
RemoteName string
Rename bool
}
var Since = func(t time.Time) time.Duration {
@ -61,7 +63,12 @@ func NewCmdFork(f *cmdutil.Factory, runF func(*ForkOptions) error) *cobra.Comman
Short: "Create a fork of a repository",
Long: `Create a fork of a repository.
With no argument, creates a fork of the current repository. Otherwise, forks the specified repository.
With no argument, creates a fork of the current repository. Otherwise, forks
the specified repository.
By default, the new fork is set to be your 'origin' remote and any existing
origin remote is renamed to 'upstream'. To alter this behavior, you can set
a name for the new fork's remote with --remote-name.
Additional 'git clone' flags can be passed in by listing them after '--'.`,
RunE: func(cmd *cobra.Command, args []string) error {
@ -71,6 +78,10 @@ Additional 'git clone' flags can be passed in by listing them after '--'.`,
opts.GitArgs = args[1:]
}
if opts.RemoteName == "" {
return &cmdutil.FlagError{Err: errors.New("--remote-name cannot be blank")}
}
if promptOk && !cmd.Flags().Changed("clone") {
opts.PromptClone = true
}
@ -79,6 +90,10 @@ Additional 'git clone' flags can be passed in by listing them after '--'.`,
opts.PromptRemote = true
}
if !cmd.Flags().Changed("remote-name") {
opts.Rename = true
}
if runF != nil {
return runF(opts)
}
@ -237,11 +252,7 @@ func forkRun(opts *ForkOptions) error {
}
if _, err := remotes.FindByName(remoteName); err == nil {
if connectedToTerminal {
return fmt.Errorf("a remote called '%s' already exists. You can rerun this command with --remote-name to specify a different remote name.", remoteName)
} else {
// TODO next major version we should break this behavior and force users to opt into
// remote renaming in a scripting context via --remote-name
if opts.Rename {
renameTarget := "upstream"
renameCmd, err := git.GitCommand("remote", "rename", remoteName, renameTarget)
if err != nil {
@ -251,6 +262,8 @@ func forkRun(opts *ForkOptions) error {
if err != nil {
return err
}
} else {
return fmt.Errorf("a git remote named '%s' already exists", remoteName)
}
}

View file

@ -1,6 +1,7 @@
package fork
import (
"bytes"
"net/http"
"net/url"
"regexp"
@ -21,6 +22,129 @@ import (
"github.com/stretchr/testify/assert"
)
func TestNewCmdFork(t *testing.T) {
tests := []struct {
name string
cli string
tty bool
wants ForkOptions
wantErr bool
}{
{
name: "repo with git args",
cli: "foo/bar -- --foo=bar",
wants: ForkOptions{
Repository: "foo/bar",
GitArgs: []string{"TODO"},
RemoteName: "origin",
Rename: true,
},
},
{
name: "git args without repo",
cli: "-- --foo bar",
wantErr: true,
},
{
name: "repo",
cli: "foo/bar",
wants: ForkOptions{
Repository: "foo/bar",
RemoteName: "origin",
Rename: true,
},
},
{
name: "blank remote name",
cli: "--remote --remote-name=''",
wantErr: true,
},
{
name: "remote name",
cli: "--remote --remote-name=foo",
wants: ForkOptions{
RemoteName: "foo",
Rename: false,
Remote: true,
},
},
{
name: "blank nontty",
cli: "",
wants: ForkOptions{
RemoteName: "origin",
Rename: true,
},
},
{
name: "blank tty",
cli: "",
tty: true,
wants: ForkOptions{
RemoteName: "origin",
PromptClone: true,
PromptRemote: true,
Rename: true,
},
},
{
name: "clone",
cli: "--clone",
wants: ForkOptions{
RemoteName: "origin",
Rename: true,
},
},
{
name: "remote",
cli: "--remote",
wants: ForkOptions{
RemoteName: "origin",
Remote: true,
Rename: true,
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
io, _, _, _ := iostreams.Test()
f := &cmdutil.Factory{
IOStreams: io,
}
io.SetStdoutTTY(tt.tty)
io.SetStdinTTY(tt.tty)
argv, err := shlex.Split(tt.cli)
assert.NoError(t, err)
var gotOpts *ForkOptions
cmd := NewCmdFork(f, func(opts *ForkOptions) error {
gotOpts = opts
return nil
})
cmd.SetArgs(argv)
cmd.SetIn(&bytes.Buffer{})
cmd.SetOut(&bytes.Buffer{})
cmd.SetErr(&bytes.Buffer{})
_, err = cmd.ExecuteC()
if tt.wantErr {
assert.Error(t, err)
return
}
assert.NoError(t, err)
assert.Equal(t, tt.wants.RemoteName, gotOpts.RemoteName)
assert.Equal(t, tt.wants.Remote, gotOpts.Remote)
assert.Equal(t, tt.wants.PromptRemote, gotOpts.PromptRemote)
assert.Equal(t, tt.wants.PromptClone, gotOpts.PromptClone)
})
}
}
func runCommand(httpClient *http.Client, remotes []*context.Remote, isTTY bool, cli string) (*test.CmdOut, error) {
io, stdin, stdout, stderr := iostreams.Test()
io.SetStdoutTTY(isTTY)
@ -98,22 +222,6 @@ func TestRepoFork_nontty(t *testing.T) {
}
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")
if err == nil {
t.Fatal("expected error running command `repo fork`")
}
assert.Equal(t, "a remote called 'origin' already exists. You can rerun this command with --remote-name to specify a different remote name.", err.Error())
reg.Verify(t)
}
func TestRepoFork_no_conflicting_remote(t *testing.T) {
remotes := []*context.Remote{
{
@ -144,6 +252,43 @@ func TestRepoFork_no_conflicting_remote(t *testing.T) {
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{}