From bff8b3007a85aed6206beb079257b5d44fb367fc Mon Sep 17 00:00:00 2001 From: Gowtham Munukutla Date: Thu, 18 Feb 2021 18:45:43 +0530 Subject: [PATCH] Add test cases and improve errors with color schemes --- pkg/cmd/gist/edit/edit.go | 130 ++++++++++++++++----------- pkg/cmd/gist/edit/edit_test.go | 160 +++++++++++++++++++++++++++++++-- 2 files changed, 233 insertions(+), 57 deletions(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index 988b7c141..b430f0d97 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -5,6 +5,13 @@ import ( "encoding/json" "errors" "fmt" + "io/ioutil" + "net/http" + "os" + "path/filepath" + "sort" + "strings" + "github.com/AlecAivazis/survey/v2" "github.com/cli/cli/api" "github.com/cli/cli/internal/config" @@ -15,12 +22,6 @@ import ( "github.com/cli/cli/pkg/prompt" "github.com/cli/cli/pkg/surveyext" "github.com/spf13/cobra" - "io/ioutil" - "net/http" - "os" - "path/filepath" - "sort" - "strings" ) type EditOptions struct { @@ -28,8 +29,8 @@ type EditOptions struct { HttpClient func() (*http.Client, error) Config func() (config.Config, error) - Edit func(string, string, string, *iostreams.IOStreams) (string, error) - + Edit func(string, string, string, *iostreams.IOStreams) (string, error) + Add func(string, string, *iostreams.IOStreams) (string, error) Selector string EditFilename string AddFilename string @@ -47,6 +48,13 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman defaultContent, io.In, io.Out, io.ErrOut, nil) }, + Add: func(editorCmd, filename string, io *iostreams.IOStreams) (string, error) { + return surveyext.Edit( + editorCmd, + "*."+filename, + "", + io.In, io.Out, io.ErrOut, nil) + }, } cmd := &cobra.Command{ @@ -104,60 +112,34 @@ func editRun(opts *EditOptions) error { filesToUpdate := map[string]string{} addFilename := opts.AddFilename - fileExists := false + cs := opts.IO.ColorScheme() if addFilename != "" { - //Add files to an existing gist - if fi, err := os.Stat(addFilename); err != nil { - return err - } else { - switch mode := fi.Mode(); { - case mode.IsDir(): - return fmt.Errorf("found directory %s", addFilename) - case mode.IsRegular(): - fileExists = true - } - } - - wd, err := os.Getwd() + //Add files to an existing gist. Aborts when file is already present + // in the directory but the user chooses not to proceed + filenamePath, fileExists, userAbort, err := processFiles(addFilename) if err != nil { - return err - } - - m, err := filepath.Glob(wd + "/[^.]*.*") - if err != nil { - return err + return fmt.Errorf("%s %s", cs.Red("!"), err) } filesToAdd := map[string]*shared.GistFile{} - for _, f := range m { - if addFilename == filepath.Base(f) { - choice := false - err := prompt.Confirm("File found in this directory. Proceed?", &choice) - if err != nil { - return err - } - - if choice { - fileExists = true - } - break - } + if userAbort { + return nil } + filename := filepath.Base(filenamePath) + if fileExists { - content, err := ioutil.ReadFile(addFilename) + content, err := ioutil.ReadFile(filenamePath) if err != nil { - return fmt.Errorf("failed to read file %s: %w", addFilename, err) + return fmt.Errorf("%s failed to read file %s: %w", cs.FailureIcon(), addFilename, err) } if string(content) == "" { - return fmt.Errorf("Contents can't be empty") + return fmt.Errorf("%s Contents can't be empty", cs.FailureIcon()) } - filename := filepath.Base(addFilename) - filesToAdd[filename] = &shared.GistFile{ Filename: filename, Content: string(content), @@ -169,17 +151,17 @@ func editRun(opts *EditOptions) error { return err } - text, err := opts.Edit(editorCommand, addFilename, "", opts.IO) + text, err := opts.Add(editorCommand, filename, opts.IO) if err != nil { return err } if text == "" { - return fmt.Errorf("Contents can't be empty") + return fmt.Errorf("%s Contents can't be empty", cs.Red("!")) } - filesToAdd[addFilename] = &shared.GistFile{ - Filename: addFilename, + filesToAdd[filename] = &shared.GistFile{ + Filename: filename, Content: text, } @@ -190,6 +172,11 @@ func editRun(opts *EditOptions) error { if err != nil { return err } + + completionMessage := filename + " added to gist" + + fmt.Fprintf(opts.IO.Out, "%s %s\n", cs.SuccessIconWithColor(cs.Green), completionMessage) + } else { for { filename := opts.EditFilename @@ -315,3 +302,46 @@ func updateGist(apiClient *api.Client, hostname string, gist *shared.Gist) error return nil } + +func processFiles(filename string) (string, bool, bool, error) { + fileExists := false + + if fi, err := os.Stat(filename); err != nil { + } else { + switch mode := fi.Mode(); { + case mode.IsDir(): + return "", false, false, fmt.Errorf("found directory %s" , filename) + case mode.IsRegular(): + fileExists = true + } + } + + wd, err := os.Getwd() + if err != nil { + return "", false, false, err + } + + m, err := filepath.Glob(wd + "/[^.]*.*") + if err != nil { + return "", false, false, err + } + + for _, f := range m { + if filename == filepath.Base(f) { + choice := false + err := prompt.Confirm("File found in this directory. Proceed?", &choice) + if err != nil { + return "", false, false, err + } + + if choice { + fileExists = true + } else { + return "", true, true, nil + } + break + } + } + + return filename, fileExists, false, nil +} diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index 59156dec6..6f0df8040 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -17,6 +17,32 @@ import ( "github.com/stretchr/testify/assert" ) +const ( + fixtureFile = "../fixture.txt" + nonExistentFile = "../file.txt" +) + +func Test_processFiles(t *testing.T) { + filePath, fileExists, userAbort, err := processFiles(fixtureFile) + if err != nil { + t.Fatalf("unexpected error processing files: %s", err) + } + + assert.Equal(t, "../fixture.txt", filePath) + assert.Equal(t, true, fileExists) + assert.Equal(t, false, userAbort) + + filePath, fileExists, userAbort, err = processFiles(nonExistentFile) + if err != nil { + t.Fatalf("unexpected error processing files: %s", err) + } + + assert.Equal(t, "../file.txt", filePath) + assert.Equal(t, false, fileExists) + assert.Equal(t, false, userAbort) + +} + func TestNewCmdEdit(t *testing.T) { tests := []struct { name string @@ -34,15 +60,15 @@ func TestNewCmdEdit(t *testing.T) { name: "filename", cli: "123 --filename cool.md", wants: EditOptions{ - Selector: "123", + Selector: "123", EditFilename: "cool.md", }, }, { name: "add", - cli: "123 --add cool.md", + cli: "123 --add cool.md", wants: EditOptions{ - Selector: "123", + Selector: "123", AddFilename: "cool.md", }, }, @@ -225,10 +251,126 @@ func Test_editRun(t *testing.T) { gist: &shared.Gist{ ID: "1234", Files: map[string]*shared.GistFile{ - "foo.txt": { - Filename: "foo.txt", - Content: "bwhiizzzbwhuiiizzzz", - Type: "text/plain", + "sample.txt": { + Filename: "sample.txt", + Content: "bwhiizzzbwhuiiizzzz", + Type: "text/plain", + }, + }, + Owner: &shared.GistOwner{Login: "octocat"}, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.REST("POST", "gists/1234"), + httpmock.StatusStringResponse(201, "{}")) + }, + opts: &EditOptions{ + AddFilename: "foo.txt", + }, + wantParams: map[string]interface{}{ + "description": "", + "updated_at": "0001-01-01T00:00:00Z", + "public": false, + "files": map[string]interface{}{ + "foo.txt": map[string]interface{}{ + "content": "new content to existing gist", + "filename": "foo.txt", + }, + }, + }, + }, + { + name: "add file to existing gist with absolute path", + gist: &shared.Gist{ + ID: "1234", + Files: map[string]*shared.GistFile{ + "sample.txt": { + Filename: "sample.txt", + Content: "bwhiizzzbwhuiiizzzz", + Type: "text/plain", + }, + }, + Owner: &shared.GistOwner{Login: "octocat"}, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.REST("POST", "gists/1234"), + httpmock.StatusStringResponse(201, "{}")) + }, + opts: &EditOptions{ + AddFilename: "/Users/octocat/foo.txt", + }, + wantParams: map[string]interface{}{ + "description": "", + "updated_at": "0001-01-01T00:00:00Z", + "public": false, + "files": map[string]interface{}{ + "foo.txt": map[string]interface{}{ + "content": "new content to existing gist", + "filename": "foo.txt", + }, + }, + }, + }, + { + name: "add file to existing gist with relative path", + gist: &shared.Gist{ + ID: "1234", + Files: map[string]*shared.GistFile{ + "sample.txt": { + Filename: "sample.txt", + Content: "bwhiizzzbwhuiiizzzz", + Type: "text/plain", + }, + }, + Owner: &shared.GistOwner{Login: "octocat"}, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.REST("POST", "gists/1234"), + httpmock.StatusStringResponse(201, "{}")) + }, + opts: &EditOptions{ + AddFilename: "../foo.txt", + }, + wantParams: map[string]interface{}{ + "description": "", + "updated_at": "0001-01-01T00:00:00Z", + "public": false, + "files": map[string]interface{}{ + "foo.txt": map[string]interface{}{ + "content": "new content to existing gist", + "filename": "foo.txt", + }, + }, + }, + }, + { + name: "add file to existing gist in same directory", + + gist: &shared.Gist{ + ID: "1234", + Files: map[string]*shared.GistFile{ + "sample.txt": { + Filename: "sample.txt", + Content: "bwhiizzzbwhuiiizzzz", + Type: "text/plain", + }, + }, + Owner: &shared.GistOwner{Login: "octocat"}, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.REST("POST", "gists/1234"), + httpmock.StatusStringResponse(201, "{}")) + }, + opts: &EditOptions{ + AddFilename: "foo.txt", + }, + wantParams: map[string]interface{}{ + "description": "", + "updated_at": "0001-01-01T00:00:00Z", + "public": false, + "files": map[string]interface{}{ + "foo.txt": map[string]interface{}{ + "content": "new content to existing gist", + "filename": "foo.txt", }, }, }, @@ -265,6 +407,10 @@ func Test_editRun(t *testing.T) { return "new file content", nil } + tt.opts.Add = func(_, _ string, _ *iostreams.IOStreams) (string, error) { + return "new content to existing gist", nil + } + tt.opts.HttpClient = func() (*http.Client, error) { return &http.Client{Transport: reg}, nil }