Merge pull request #3578 from g14a/fix/empty-gist-contents

Add validation to gists if contents are empty

pkg/cmd/gist/create/create.go

@@ -151,7 +151,13 @@ func createRun(opts *CreateOptions) error {
 		var httpError api.HTTPError
 		if errors.As(err, &httpError) {
 			if httpError.OAuthScopes != "" && !strings.Contains(httpError.OAuthScopes, "gist") {
-				return fmt.Errorf("This command requires the 'gist' OAuth scope.\nPlease re-authenticate by doing `gh config set -h github.com oauth_token ''` and running the command again.")
+				return fmt.Errorf("This command requires the 'gist' OAuth scope.\nPlease re-authenticate with:  gh auth refresh -h %s -s gist", host)
+			}
+			if httpError.StatusCode == http.StatusUnprocessableEntity {
+				if detectEmptyFiles(files) {
+					fmt.Fprintf(errOut, "%s Failed to create gist: %s\n", cs.FailureIcon(), "a gist file cannot be blank")
+					return cmdutil.SilentError
+				}
 			}
 		}
 		return fmt.Errorf("%s Failed to create gist: %w", cs.Red("X"), err)
@@ -266,3 +272,12 @@ func createGist(client *http.Client, hostname, description string, public bool,
 
 	return &result, nil
 }
+
+func detectEmptyFiles(files map[string]*shared.GistFile) bool {
+	for _, file := range files {
+		if strings.TrimSpace(file.Content) == "" {
+			return true
+		}
+	}
+	return false
+}

pkg/cmd/gist/create/create_test.go

@@ -5,9 +5,11 @@ import (
 	"encoding/json"
 	"io/ioutil"
 	"net/http"
+	"path"
 	"strings"
 	"testing"
 
+	"github.com/MakeNowJust/heredoc"
 	"github.com/cli/cli/internal/config"
 	"github.com/cli/cli/internal/run"
 	"github.com/cli/cli/pkg/cmd/gist/shared"
@@ -18,10 +20,6 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-const (
-	fixtureFile = "../fixture.txt"
-)
-
 func Test_processFiles(t *testing.T) {
 	fakeStdin := strings.NewReader("hey cool how is it going")
 	files, err := processFiles(ioutil.NopCloser(fakeStdin), "", []string{"-"})
@@ -164,15 +162,22 @@ func TestNewCmdCreate(t *testing.T) {
 }
 
 func Test_createRun(t *testing.T) {
+	tempDir := t.TempDir()
+	fixtureFile := path.Join(tempDir, "fixture.txt")
+	assert.NoError(t, ioutil.WriteFile(fixtureFile, []byte("{}"), 0644))
+	emptyFile := path.Join(tempDir, "empty.txt")
+	assert.NoError(t, ioutil.WriteFile(emptyFile, []byte(" \t\n"), 0644))
+
 	tests := []struct {
-		name       string
-		opts       *CreateOptions
-		stdin      string
-		wantOut    string
-		wantStderr string
-		wantParams map[string]interface{}
-		wantErr    bool
-		wantBrowse string
+		name           string
+		opts           *CreateOptions
+		stdin          string
+		wantOut        string
+		wantStderr     string
+		wantParams     map[string]interface{}
+		wantErr        bool
+		wantBrowse     string
+		responseStatus int
 	}{
 		{
 			name: "public",
@@ -193,6 +198,7 @@ func Test_createRun(t *testing.T) {
 					},
 				},
 			},
+			responseStatus: http.StatusOK,
 		},
 		{
 			name: "with description",
@@ -213,6 +219,7 @@ func Test_createRun(t *testing.T) {
 					},
 				},
 			},
+			responseStatus: http.StatusOK,
 		},
 		{
 			name: "multiple files",
@@ -236,6 +243,28 @@ func Test_createRun(t *testing.T) {
 					},
 				},
 			},
+			responseStatus: http.StatusOK,
+		},
+		{
+			name: "file with empty content",
+			opts: &CreateOptions{
+				Filenames: []string{emptyFile},
+			},
+			wantOut: "",
+			wantStderr: heredoc.Doc(`
+				- Creating gist empty.txt
+				X Failed to create gist: a gist file cannot be blank
+			`),
+			wantErr: true,
+			wantParams: map[string]interface{}{
+				"description": "",
+				"updated_at":  "0001-01-01T00:00:00Z",
+				"public":      false,
+				"files": map[string]interface{}{
+					"empty.txt": map[string]interface{}{"content": " \t\n"},
+				},
+			},
+			responseStatus: http.StatusUnprocessableEntity,
 		},
 		{
 			name: "stdin arg",
@@ -256,6 +285,7 @@ func Test_createRun(t *testing.T) {
 					},
 				},
 			},
+			responseStatus: http.StatusOK,
 		},
 		{
 			name: "web arg",
@@ -277,14 +307,22 @@ func Test_createRun(t *testing.T) {
 					},
 				},
 			},
+			responseStatus: http.StatusOK,
 		},
 	}
 	for _, tt := range tests {
 		reg := &httpmock.Registry{}
-		reg.Register(httpmock.REST("POST", "gists"),
-			httpmock.JSONResponse(struct {
-				Html_url string
-			}{"https://gist.github.com/aa5a315d61ae9438b18d"}))
+		if tt.responseStatus == http.StatusOK {
+			reg.Register(
+				httpmock.REST("POST", "gists"),
+				httpmock.StringResponse(`{
+					"html_url": "https://gist.github.com/aa5a315d61ae9438b18d"
+				}`))
+		} else {
+			reg.Register(
+				httpmock.REST("POST", "gists"),
+				httpmock.StatusStringResponse(tt.responseStatus, "{}"))
+		}
 
 		mockClient := func() (*http.Client, error) {
 			return &http.Client{Transport: reg}, nil
@@ -325,6 +363,32 @@ func Test_createRun(t *testing.T) {
 	}
 }
 
+func Test_detectEmptyFiles(t *testing.T) {
+	tests := []struct {
+		content     string
+		isEmptyFile bool
+	}{
+		{
+			content:     "{}",
+			isEmptyFile: false,
+		},
+		{
+			content:     "\n\t",
+			isEmptyFile: true,
+		},
+	}
+
+	for _, tt := range tests {
+		files := map[string]*shared.GistFile{}
+		files["file"] = &shared.GistFile{
+			Content: tt.content,
+		}
+
+		isEmptyFile := detectEmptyFiles(files)
+		assert.Equal(t, tt.isEmptyFile, isEmptyFile)
+	}
+}
+
 func Test_CreateRun_reauth(t *testing.T) {
 	reg := &httpmock.Registry{}
 	reg.Register(httpmock.REST("POST", "gists"), func(req *http.Request) (*http.Response, error) {
@@ -332,33 +396,24 @@ func Test_CreateRun_reauth(t *testing.T) {
 			StatusCode: 404,
 			Request:    req,
 			Header: map[string][]string{
-				"X-Oauth-Scopes": {"coolScope"},
+				"X-Oauth-Scopes": {"repo, read:org"},
 			},
 			Body: ioutil.NopCloser(bytes.NewBufferString("oh no")),
 		}, nil
 	})
 
-	mockClient := func() (*http.Client, error) {
-		return &http.Client{Transport: reg}, nil
-	}
-
 	io, _, _, _ := iostreams.Test()
 
 	opts := &CreateOptions{
-		IO:         io,
-		HttpClient: mockClient,
+		IO: io,
+		HttpClient: func() (*http.Client, error) {
+			return &http.Client{Transport: reg}, nil
+		},
 		Config: func() (config.Config, error) {
 			return config.NewBlankConfig(), nil
 		},
-		Filenames: []string{fixtureFile},
 	}
 
 	err := createRun(opts)
-	if err == nil {
-		t.Fatalf("expected oauth error")
-	}
-
-	if !strings.Contains(err.Error(), "Please re-authenticate") {
-		t.Errorf("got unexpected error: %s", err)
-	}
+	assert.EqualError(t, err, "This command requires the 'gist' OAuth scope.\nPlease re-authenticate with:  gh auth refresh -h github.com -s gist")
 }

pkg/cmd/gist/fixture.txt

@@ -1 +0,0 @@
-{}

pkg/cmd/gist/shared/shared.go

@@ -20,7 +20,7 @@ type GistFile struct {
 	Filename string `json:"filename,omitempty"`
 	Type     string `json:"type,omitempty"`
 	Language string `json:"language,omitempty"`
-	Content  string `json:"content,omitempty"`
+	Content  string `json:"content"`
 }
 
 type GistOwner struct {