Skip to content

Commit bff8b30

Browse files
g14ag14a
committed
Add test cases and improve errors with color schemes
1 parent 2983887 commit bff8b30

File tree

2 files changed

+233
-57
lines changed

2 files changed

+233
-57
lines changed

‎pkg/cmd/gist/edit/edit.go‎

Lines changed: 80 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,13 @@ import (
55
"encoding/json"
66
"errors"
77
"fmt"
8+
"io/ioutil"
9+
"net/http"
10+
"os"
11+
"path/filepath"
12+
"sort"
13+
"strings"
14+
815
"github.com/AlecAivazis/survey/v2"
916
"github.com/cli/cli/api"
1017
"github.com/cli/cli/internal/config"
@@ -15,21 +22,15 @@ import (
1522
"github.com/cli/cli/pkg/prompt"
1623
"github.com/cli/cli/pkg/surveyext"
1724
"github.com/spf13/cobra"
18-
"io/ioutil"
19-
"net/http"
20-
"os"
21-
"path/filepath"
22-
"sort"
23-
"strings"
2425
)
2526

2627
type EditOptions struct {
2728
IO *iostreams.IOStreams
2829
HttpClient func() (*http.Client, error)
2930
Config func() (config.Config, error)
3031

31-
Edit func(string, string, string, *iostreams.IOStreams) (string, error)
32-
32+
Edit func(string, string, string, *iostreams.IOStreams) (string, error)
33+
Add func(string, string, *iostreams.IOStreams) (string, error)
3334
Selector string
3435
EditFilename string
3536
AddFilename string
@@ -47,6 +48,13 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman
4748
defaultContent,
4849
io.In, io.Out, io.ErrOut, nil)
4950
},
51+
Add: func(editorCmd, filename string, io *iostreams.IOStreams) (string, error) {
52+
return surveyext.Edit(
53+
editorCmd,
54+
"*."+filename,
55+
"",
56+
io.In, io.Out, io.ErrOut, nil)
57+
},
5058
}
5159

5260
cmd := &cobra.Command{
@@ -104,60 +112,34 @@ func editRun(opts *EditOptions) error {
104112
filesToUpdate := map[string]string{}
105113

106114
addFilename := opts.AddFilename
107-
fileExists := false
115+
cs := opts.IO.ColorScheme()
108116

109117
if addFilename != "" {
110-
//Add files to an existing gist
111-
if fi, err := os.Stat(addFilename); err != nil {
112-
return err
113-
} else {
114-
switch mode := fi.Mode(); {
115-
case mode.IsDir():
116-
return fmt.Errorf("found directory %s", addFilename)
117-
case mode.IsRegular():
118-
fileExists = true
119-
}
120-
}
121-
122-
wd, err := os.Getwd()
123-
if err != nil {
124-
return err
125-
}
126-
127-
m, err := filepath.Glob(wd + "/[^.]*.*")
118+
//Add files to an existing gist. Aborts when file is already present
119+
// in the directory but the user chooses not to proceed
120+
filenamePath, fileExists, userAbort, err := processFiles(addFilename)
128121
if err != nil {
129-
return err
122+
return fmt.Errorf("%s %s", cs.Red("!"), err)
130123
}
131124

132125
filesToAdd := map[string]*shared.GistFile{}
133126

134-
for _, f := range m {
135-
if addFilename == filepath.Base(f) {
136-
choice := false
137-
err := prompt.Confirm("File found in this directory. Proceed?", &choice)
138-
if err != nil {
139-
return err
140-
}
141-
142-
if choice {
143-
fileExists = true
144-
}
145-
break
146-
}
127+
if userAbort {
128+
return nil
147129
}
148130

131+
filename := filepath.Base(filenamePath)
132+
149133
if fileExists {
150-
content, err := ioutil.ReadFile(addFilename)
134+
content, err := ioutil.ReadFile(filenamePath)
151135
if err != nil {
152-
return fmt.Errorf("failed to read file %s: %w", addFilename, err)
136+
return fmt.Errorf("%s failed to read file %s: %w", cs.FailureIcon(), addFilename, err)
153137
}
154138

155139
if string(content) == "" {
156-
return fmt.Errorf("Contents can't be empty")
140+
return fmt.Errorf("%s Contents can't be empty", cs.FailureIcon())
157141
}
158142

159-
filename := filepath.Base(addFilename)
160-
161143
filesToAdd[filename] = &shared.GistFile{
162144
Filename: filename,
163145
Content: string(content),
@@ -169,17 +151,17 @@ func editRun(opts *EditOptions) error {
169151
return err
170152
}
171153

172-
text, err := opts.Edit(editorCommand, addFilename, "", opts.IO)
154+
text, err := opts.Add(editorCommand, filename, opts.IO)
173155
if err != nil {
174156
return err
175157
}
176158

177159
if text == "" {
178-
return fmt.Errorf("Contents can't be empty")
160+
return fmt.Errorf("%s Contents can't be empty", cs.Red("!"))
179161
}
180162

181-
filesToAdd[addFilename] = &shared.GistFile{
182-
Filename: addFilename,
163+
filesToAdd[filename] = &shared.GistFile{
164+
Filename: filename,
183165
Content: text,
184166
}
185167

@@ -190,6 +172,11 @@ func editRun(opts *EditOptions) error {
190172
if err != nil {
191173
return err
192174
}
175+
176+
completionMessage := filename + " added to gist"
177+
178+
fmt.Fprintf(opts.IO.Out, "%s %s\n", cs.SuccessIconWithColor(cs.Green), completionMessage)
179+
193180
} else {
194181
for {
195182
filename := opts.EditFilename
@@ -315,3 +302,46 @@ func updateGist(apiClient *api.Client, hostname string, gist *shared.Gist) error
315302

316303
return nil
317304
}
305+
306+
func processFiles(filename string) (string, bool, bool, error) {
307+
fileExists := false
308+
309+
if fi, err := os.Stat(filename); err != nil {
310+
} else {
311+
switch mode := fi.Mode(); {
312+
case mode.IsDir():
313+
return "", false, false, fmt.Errorf("found directory %s" , filename)
314+
case mode.IsRegular():
315+
fileExists = true
316+
}
317+
}
318+
319+
wd, err := os.Getwd()
320+
if err != nil {
321+
return "", false, false, err
322+
}
323+
324+
m, err := filepath.Glob(wd + "/[^.]*.*")
325+
if err != nil {
326+
return "", false, false, err
327+
}
328+
329+
for _, f := range m {
330+
if filename == filepath.Base(f) {
331+
choice := false
332+
err := prompt.Confirm("File found in this directory. Proceed?", &choice)
333+
if err != nil {
334+
return "", false, false, err
335+
}
336+
337+
if choice {
338+
fileExists = true
339+
} else {
340+
return "", true, true, nil
341+
}
342+
break
343+
}
344+
}
345+
346+
return filename, fileExists, false, nil
347+
}

‎pkg/cmd/gist/edit/edit_test.go‎

Lines changed: 153 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,32 @@ import (
1717
"github.com/stretchr/testify/assert"
1818
)
1919

20+
const (
21+
fixtureFile = "../fixture.txt"
22+
nonExistentFile = "../file.txt"
23+
)
24+
25+
func Test_processFiles(t *testing.T) {
26+
filePath, fileExists, userAbort, err := processFiles(fixtureFile)
27+
if err != nil {
28+
t.Fatalf("unexpected error processing files: %s", err)
29+
}
30+
31+
assert.Equal(t, "../fixture.txt", filePath)
32+
assert.Equal(t, true, fileExists)
33+
assert.Equal(t, false, userAbort)
34+
35+
filePath, fileExists, userAbort, err = processFiles(nonExistentFile)
36+
if err != nil {
37+
t.Fatalf("unexpected error processing files: %s", err)
38+
}
39+
40+
assert.Equal(t, "../file.txt", filePath)
41+
assert.Equal(t, false, fileExists)
42+
assert.Equal(t, false, userAbort)
43+
44+
}
45+
2046
func TestNewCmdEdit(t *testing.T) {
2147
tests := []struct {
2248
name string
@@ -34,15 +60,15 @@ func TestNewCmdEdit(t *testing.T) {
3460
name: "filename",
3561
cli: "123 --filename cool.md",
3662
wants: EditOptions{
37-
Selector: "123",
63+
Selector: "123",
3864
EditFilename: "cool.md",
3965
},
4066
},
4167
{
4268
name: "add",
43-
cli: "123 --add cool.md",
69+
cli: "123 --add cool.md",
4470
wants: EditOptions{
45-
Selector: "123",
71+
Selector: "123",
4672
AddFilename: "cool.md",
4773
},
4874
},
@@ -225,10 +251,126 @@ func Test_editRun(t *testing.T) {
225251
gist: &shared.Gist{
226252
ID: "1234",
227253
Files: map[string]*shared.GistFile{
228-
"foo.txt": {
229-
Filename: "foo.txt",
230-
Content: "bwhiizzzbwhuiiizzzz",
231-
Type: "text/plain",
254+
"sample.txt": {
255+
Filename: "sample.txt",
256+
Content: "bwhiizzzbwhuiiizzzz",
257+
Type: "text/plain",
258+
},
259+
},
260+
Owner: &shared.GistOwner{Login: "octocat"},
261+
},
262+
httpStubs: func(reg *httpmock.Registry) {
263+
reg.Register(httpmock.REST("POST", "gists/1234"),
264+
httpmock.StatusStringResponse(201, "{}"))
265+
},
266+
opts: &EditOptions{
267+
AddFilename: "foo.txt",
268+
},
269+
wantParams: map[string]interface{}{
270+
"description": "",
271+
"updated_at": "0001-01-01T00:00:00Z",
272+
"public": false,
273+
"files": map[string]interface{}{
274+
"foo.txt": map[string]interface{}{
275+
"content": "new content to existing gist",
276+
"filename": "foo.txt",
277+
},
278+
},
279+
},
280+
},
281+
{
282+
name: "add file to existing gist with absolute path",
283+
gist: &shared.Gist{
284+
ID: "1234",
285+
Files: map[string]*shared.GistFile{
286+
"sample.txt": {
287+
Filename: "sample.txt",
288+
Content: "bwhiizzzbwhuiiizzzz",
289+
Type: "text/plain",
290+
},
291+
},
292+
Owner: &shared.GistOwner{Login: "octocat"},
293+
},
294+
httpStubs: func(reg *httpmock.Registry) {
295+
reg.Register(httpmock.REST("POST", "gists/1234"),
296+
httpmock.StatusStringResponse(201, "{}"))
297+
},
298+
opts: &EditOptions{
299+
AddFilename: "/Users/octocat/foo.txt",
300+
},
301+
wantParams: map[string]interface{}{
302+
"description": "",
303+
"updated_at": "0001-01-01T00:00:00Z",
304+
"public": false,
305+
"files": map[string]interface{}{
306+
"foo.txt": map[string]interface{}{
307+
"content": "new content to existing gist",
308+
"filename": "foo.txt",
309+
},
310+
},
311+
},
312+
},
313+
{
314+
name: "add file to existing gist with relative path",
315+
gist: &shared.Gist{
316+
ID: "1234",
317+
Files: map[string]*shared.GistFile{
318+
"sample.txt": {
319+
Filename: "sample.txt",
320+
Content: "bwhiizzzbwhuiiizzzz",
321+
Type: "text/plain",
322+
},
323+
},
324+
Owner: &shared.GistOwner{Login: "octocat"},
325+
},
326+
httpStubs: func(reg *httpmock.Registry) {
327+
reg.Register(httpmock.REST("POST", "gists/1234"),
328+
httpmock.StatusStringResponse(201, "{}"))
329+
},
330+
opts: &EditOptions{
331+
AddFilename: "../foo.txt",
332+
},
333+
wantParams: map[string]interface{}{
334+
"description": "",
335+
"updated_at": "0001-01-01T00:00:00Z",
336+
"public": false,
337+
"files": map[string]interface{}{
338+
"foo.txt": map[string]interface{}{
339+
"content": "new content to existing gist",
340+
"filename": "foo.txt",
341+
},
342+
},
343+
},
344+
},
345+
{
346+
name: "add file to existing gist in same directory",
347+
348+
gist: &shared.Gist{
349+
ID: "1234",
350+
Files: map[string]*shared.GistFile{
351+
"sample.txt": {
352+
Filename: "sample.txt",
353+
Content: "bwhiizzzbwhuiiizzzz",
354+
Type: "text/plain",
355+
},
356+
},
357+
Owner: &shared.GistOwner{Login: "octocat"},
358+
},
359+
httpStubs: func(reg *httpmock.Registry) {
360+
reg.Register(httpmock.REST("POST", "gists/1234"),
361+
httpmock.StatusStringResponse(201, "{}"))
362+
},
363+
opts: &EditOptions{
364+
AddFilename: "foo.txt",
365+
},
366+
wantParams: map[string]interface{}{
367+
"description": "",
368+
"updated_at": "0001-01-01T00:00:00Z",
369+
"public": false,
370+
"files": map[string]interface{}{
371+
"foo.txt": map[string]interface{}{
372+
"content": "new content to existing gist",
373+
"filename": "foo.txt",
232374
},
233375
},
234376
},
@@ -265,6 +407,10 @@ func Test_editRun(t *testing.T) {
265407
return "new file content", nil
266408
}
267409

410+
tt.opts.Add = func(_, _ string, _ *iostreams.IOStreams) (string, error) {
411+
return "new content to existing gist", nil
412+
}
413+
268414
tt.opts.HttpClient = func() (*http.Client, error) {
269415
return &http.Client{Transport: reg}, nil
270416
}

0 commit comments

Comments
 (0)