From b4d2bce6fcf15704fc0cdc8faba17f758a6e3899 Mon Sep 17 00:00:00 2001 From: Parth Patel Date: Mon, 25 Oct 2021 17:07:48 -0400 Subject: [PATCH] res comments --- pkg/cmd/repo/rename/http.go | 54 ++++++++++ pkg/cmd/repo/rename/http_test.go | 168 +++++++++++++++++++++++++++++ pkg/cmd/repo/rename/rename.go | 67 +++--------- pkg/cmd/repo/rename/rename_test.go | 144 ------------------------- 4 files changed, 238 insertions(+), 195 deletions(-) create mode 100644 pkg/cmd/repo/rename/http.go create mode 100644 pkg/cmd/repo/rename/http_test.go diff --git a/pkg/cmd/repo/rename/http.go b/pkg/cmd/repo/rename/http.go new file mode 100644 index 000000000..bc0bfd0ab --- /dev/null +++ b/pkg/cmd/repo/rename/http.go @@ -0,0 +1,54 @@ +package rename + +import ( + "bytes" + "encoding/json" + "fmt" + "net/http" + + "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/internal/ghinstance" + "github.com/cli/cli/v2/internal/ghrepo" +) + +type renameRepo struct { + RepoHost string + RepoOwner string + RepoName string + Name string `json:"name,omitempty"` +} + +func runRename(client *http.Client, repo ghrepo.Interface, newRepoName string) error { + + input := renameRepo{ + RepoHost: repo.RepoHost(), + RepoOwner: repo.RepoOwner(), + RepoName: repo.RepoName(), + Name: newRepoName, + } + + body, err := json.Marshal(input) + if err != nil { + return err + } + + path := fmt.Sprintf("%srepos/%s", + ghinstance.RESTPrefix(repo.RepoHost()), + ghrepo.FullName(repo)) + + request, err := http.NewRequest("PATCH", path, bytes.NewBuffer(body)) + if err != nil { + return err + } + + response, err := client.Do(request) + if err != nil { + return err + } + defer response.Body.Close() + + if response.StatusCode > 299 { + return api.HandleHTTPError(response) + } + return nil +} diff --git a/pkg/cmd/repo/rename/http_test.go b/pkg/cmd/repo/rename/http_test.go new file mode 100644 index 000000000..56e8dc365 --- /dev/null +++ b/pkg/cmd/repo/rename/http_test.go @@ -0,0 +1,168 @@ +package rename + +import ( + "net/http" + "testing" + + "github.com/cli/cli/v2/context" + "github.com/cli/cli/v2/git" + "github.com/cli/cli/v2/internal/config" + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/run" + "github.com/cli/cli/v2/pkg/httpmock" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/cli/cli/v2/pkg/prompt" + "github.com/stretchr/testify/assert" +) + +func TestRenameRun(t *testing.T) { + testCases := []struct { + name string + opts RenameOptions + httpStubs func(*httpmock.Registry) + execStubs func(*run.CommandStubber) + askStubs func(*prompt.AskStubber) + wantOut string + tty bool + prompt bool + }{ + { + name: "none argument", + wantOut: "✓ Renamed repository OWNER/NEW_REPO\n✓ Updated the \"origin\" remote \n", + askStubs: func(q *prompt.AskStubber) { + q.StubOne("NEW_REPO") + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("PATCH", "repos/OWNER/REPO"), + httpmock.StatusStringResponse(204, "{}")) + }, + execStubs: func(cs *run.CommandStubber) { + cs.Register(`git remote set-url origin https://github.com/OWNER/REPO.git`, 0, "") + }, + tty: true, + }, + { + name: "owner repo change name prompt", + opts: RenameOptions{ + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + }, + wantOut: "✓ Renamed repository OWNER/NEW_REPO\n✓ Updated the \"origin\" remote \n", + askStubs: func(q *prompt.AskStubber) { + q.StubOne("NEW_REPO") + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("PATCH", "repos/OWNER/REPO"), + httpmock.StatusStringResponse(204, "{}")) + }, + execStubs: func(cs *run.CommandStubber) { + cs.Register(`git remote set-url origin https://github.com/OWNER/REPO.git`, 0, "") + }, + tty: true, + }, + { + name: "owner repo change name prompt no tty", + opts: RenameOptions{ + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + }, + askStubs: func(q *prompt.AskStubber) { + q.StubOne("NEW_REPO") + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("PATCH", "repos/OWNER/REPO"), + httpmock.StatusStringResponse(204, "{}")) + }, + execStubs: func(cs *run.CommandStubber) { + cs.Register(`git remote set-url origin https://github.com/OWNER/REPO.git`, 0, "") + }, + }, + { + name: "owner repo change name argument tty", + opts: RenameOptions{ + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + newRepoSelector: "NEW_REPO", + }, + wantOut: "✓ Renamed repository OWNER/NEW_REPO\n✓ Updated the \"origin\" remote \n", + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("PATCH", "repos/OWNER/REPO"), + httpmock.StatusStringResponse(204, "{}")) + }, + execStubs: func(cs *run.CommandStubber) { + cs.Register(`git remote set-url origin https://github.com/OWNER/REPO.git`, 0, "") + }, + tty: true, + }, + { + name: "owner repo change name argument no tty", + opts: RenameOptions{ + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + newRepoSelector: "REPO", + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("PATCH", "repos/OWNER/REPO"), + httpmock.StatusStringResponse(204, "{}")) + }, + execStubs: func(cs *run.CommandStubber) { + cs.Register(`git remote set-url origin https://github.com/OWNER/REPO.git`, 0, "") + }, + }, + } + + for _, tt := range testCases { + q, teardown := prompt.InitAskStubber() + defer teardown() + if tt.askStubs != nil { + tt.askStubs(q) + } + + repo, _ := ghrepo.FromFullName("OWNER/REPO") + tt.opts.BaseRepo = func() (ghrepo.Interface, error) { + return repo, nil + } + + tt.opts.Config = func() (config.Config, error) { + return config.NewBlankConfig(), nil + } + + tt.opts.Remotes = func() (context.Remotes, error) { + return []*context.Remote{ + { + Remote: &git.Remote{Name: "origin"}, + Repo: repo, + }, + }, nil + } + + reg := &httpmock.Registry{} + if tt.httpStubs != nil { + tt.httpStubs(reg) + } + tt.opts.HttpClient = func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + } + + io, _, stdout, _ := iostreams.Test() + io.SetStdinTTY(tt.tty) + io.SetStdoutTTY(tt.tty) + tt.opts.IO = io + + t.Run(tt.name, func(t *testing.T) { + defer reg.Verify(t) + err := renameRun(&tt.opts) + assert.NoError(t, err) + assert.Equal(t, tt.wantOut, stdout.String()) + }) + } +} diff --git a/pkg/cmd/repo/rename/rename.go b/pkg/cmd/repo/rename/rename.go index dc4f69556..6a961067a 100644 --- a/pkg/cmd/repo/rename/rename.go +++ b/pkg/cmd/repo/rename/rename.go @@ -1,15 +1,12 @@ package rename import ( - "bytes" - "encoding/json" "errors" "fmt" "net/http" "github.com/AlecAivazis/survey/v2" "github.com/MakeNowJust/heredoc" - "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/context" "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/config" @@ -30,13 +27,6 @@ type RenameOptions struct { newRepoSelector string } -type renameRepo struct { - RepoHost string - RepoOwner string - RepoName string - Name string `json:"name,omitempty"` -} - func NewCmdRename(f *cmdutil.Factory, runf func(*RenameOptions) error) *cobra.Command { opts := &RenameOptions{ IO: f.IOStreams, @@ -46,15 +36,11 @@ func NewCmdRename(f *cmdutil.Factory, runf func(*RenameOptions) error) *cobra.Co } cmd := &cobra.Command{ - Use: "rename [] []", + Use: "rename []", Short: "Rename a repository", Long: heredoc.Doc(`Rename a GitHub repository - - With no argument, the repository for the current directory is renamed using a prompt - - With one argument, the repository of the current directory is renamed using the argument - - With '-R', and two arguments the given repository is replaced with the new name`), + + By default, renames the current repository otherwise rename the specified repository.`), Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { opts.BaseRepo = f.BaseRepo @@ -63,7 +49,7 @@ func NewCmdRename(f *cmdutil.Factory, runf func(*RenameOptions) error) *cobra.Co if len(args) > 0 { opts.newRepoSelector = args[0] } else if !opts.IO.CanPrompt() { - return &cmdutil.FlagError{Err: errors.New("could not prompt: proceed with a repo name")} + return &cmdutil.FlagError{Err: errors.New("could not prompt: new name required when not running interactively")} } if runf != nil { @@ -83,12 +69,9 @@ func renameRun(opts *RenameOptions) error { if err != nil { return err } - apiClient := api.NewClientFromHTTP(httpClient) - var input renameRepo var newRepo ghrepo.Interface var baseRemote *context.Remote - var remoteUpdateError error newRepoName := opts.newRepoSelector currRepo, err := opts.BaseRepo() @@ -99,7 +82,7 @@ func renameRun(opts *RenameOptions) error { if newRepoName == "" { err = prompt.SurveyAskOne( &survey.Input{ - Message: fmt.Sprintf("Rename %s to: ", currRepo.RepoOwner()+"/"+currRepo.RepoName()), + Message: fmt.Sprintf("Rename %s to: ", ghrepo.FullName(currRepo)), }, &newRepoName, ) @@ -108,21 +91,20 @@ func renameRun(opts *RenameOptions) error { } } - input = renameRepo{ - RepoHost: currRepo.RepoHost(), - RepoOwner: currRepo.RepoOwner(), - RepoName: currRepo.RepoName(), - Name: newRepoName, - } - newRepo = ghrepo.NewWithHost(currRepo.RepoOwner(), newRepoName, currRepo.RepoHost()) - err = runRename(apiClient, currRepo.RepoHost(), input) + err = runRename(httpClient, currRepo, newRepoName) if err != nil { - return fmt.Errorf("API called failed: %s, please check your parameters", err) + return fmt.Errorf("API called failed: %s", err) + } + + if opts.IO.IsStdoutTTY() { + cs := opts.IO.ColorScheme() + fmt.Fprintf(opts.IO.Out, "%s Renamed repository %s\n", cs.SuccessIcon(), ghrepo.FullName(newRepo)) } if !opts.HasRepoOverride { + cs := opts.IO.ColorScheme() cfg, err := opts.Config() if err != nil { return err @@ -132,30 +114,13 @@ func renameRun(opts *RenameOptions) error { remotes, _ := opts.Remotes() baseRemote, _ = remotes.FindByRepo(currRepo.RepoOwner(), currRepo.RepoName()) remoteURL := ghrepo.FormatRemoteURL(newRepo, protocol) - remoteUpdateError = git.UpdateRemoteURL(baseRemote.Name, remoteURL) - if remoteUpdateError != nil { - cs := opts.IO.ColorScheme() + err = git.UpdateRemoteURL(baseRemote.Name, remoteURL) + if err != nil { fmt.Fprintf(opts.IO.ErrOut, "%s warning: unable to update remote '%s' \n", cs.WarningIcon(), err) } - } - - if opts.IO.IsStdoutTTY() { - cs := opts.IO.ColorScheme() - fmt.Fprintf(opts.IO.Out, "%s Renamed repository %s\n", cs.SuccessIcon(), input.RepoOwner+"/"+input.Name) - if !opts.HasRepoOverride && remoteUpdateError == nil { + if opts.IO.IsStdoutTTY() { fmt.Fprintf(opts.IO.Out, "%s Updated the %q remote \n", cs.SuccessIcon(), baseRemote.Name) } } return nil } - -func runRename(apiClient *api.Client, hostname string, input renameRepo) error { - path := fmt.Sprintf("repos/%s/%s", input.RepoOwner, input.RepoName) - body := &bytes.Buffer{} - enc := json.NewEncoder(body) - if err := enc.Encode(input); err != nil { - return err - } - - return apiClient.REST(hostname, "PATCH", path, body, nil) -} diff --git a/pkg/cmd/repo/rename/rename_test.go b/pkg/cmd/repo/rename/rename_test.go index b95146b5c..1a2d507df 100644 --- a/pkg/cmd/repo/rename/rename_test.go +++ b/pkg/cmd/repo/rename/rename_test.go @@ -2,17 +2,10 @@ package rename import ( "bytes" - "net/http" "testing" - "github.com/cli/cli/v2/context" - "github.com/cli/cli/v2/git" - "github.com/cli/cli/v2/internal/config" - "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmdutil" - "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/pkg/prompt" "github.com/google/shlex" "github.com/stretchr/testify/assert" ) @@ -79,140 +72,3 @@ func TestNewCmdRename(t *testing.T) { }) } } - -func TestRenameRun(t *testing.T) { - testCases := []struct { - name string - opts RenameOptions - httpStubs func(*httpmock.Registry) - askStubs func(*prompt.AskStubber) - wantOut string - tty bool - prompt bool - }{ - { - name: "none argument", - wantOut: "✓ Renamed repository OWNER/NEW_REPO\n✓ Updated the \"origin\" remote \n", - askStubs: func(q *prompt.AskStubber) { - q.StubOne("NEW_REPO") - }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.REST("PATCH", "repos/OWNER/REPO"), - httpmock.StatusStringResponse(204, "{}")) - }, - tty: true, - }, - { - name: "owner repo change name prompt", - opts: RenameOptions{ - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - }, - }, - wantOut: "✓ Renamed repository OWNER/NEW_REPO\n✓ Updated the \"origin\" remote \n", - askStubs: func(q *prompt.AskStubber) { - q.StubOne("NEW_REPO") - }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.REST("PATCH", "repos/OWNER/REPO"), - httpmock.StatusStringResponse(204, "{}")) - }, - tty: true, - }, - { - name: "owner repo change name prompt no tty", - opts: RenameOptions{ - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - }, - }, - askStubs: func(q *prompt.AskStubber) { - q.StubOne("NEW_REPO") - }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.REST("PATCH", "repos/OWNER/REPO"), - httpmock.StatusStringResponse(204, "{}")) - }, - }, - { - name: "owner repo change name argument tty", - opts: RenameOptions{ - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - }, - newRepoSelector: "NEW_REPO", - }, - wantOut: "✓ Renamed repository OWNER/NEW_REPO\n✓ Updated the \"origin\" remote \n", - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.REST("PATCH", "repos/OWNER/REPO"), - httpmock.StatusStringResponse(204, "{}")) - }, - tty: true, - }, - { - name: "owner repo change name argument no tty", - opts: RenameOptions{ - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - }, - newRepoSelector: "REPO", - }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.REST("PATCH", "repos/OWNER/REPO"), - httpmock.StatusStringResponse(204, "{}")) - }, - }, - } - - for _, tt := range testCases { - q, teardown := prompt.InitAskStubber() - defer teardown() - if tt.askStubs != nil { - tt.askStubs(q) - } - - tt.opts.BaseRepo = func() (ghrepo.Interface, error) { - repo, _ := ghrepo.FromFullName("OWNER/REPO") - return repo, nil - } - - tt.opts.Config = func() (config.Config, error) { - return config.NewBlankConfig(), nil - } - - tt.opts.Remotes = func() (context.Remotes, error) { - r, _ := ghrepo.FromFullName("OWNER/REPO") - var remotes context.Remotes - remotes = append(remotes, &context.Remote{ - Remote: &git.Remote{Name: "origin"}, - Repo: r, - }) - return 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 - } - - io, _, stdout, _ := iostreams.Test() - io.SetStdinTTY(tt.tty) - io.SetStdoutTTY(tt.tty) - tt.opts.IO = io - - t.Run(tt.name, func(t *testing.T) { - defer reg.Verify(t) - err := renameRun(&tt.opts) - assert.NoError(t, err) - assert.Equal(t, tt.wantOut, stdout.String()) - }) - } -}