Merge pull request #55 from vedala/empty-value-wrong-error

Check and report error on empty option values

Author: josh-padnick

main.go

@@ -117,7 +117,11 @@ func runFetchWrapper(c *cli.Context) {
 
 // Run the fetch program
 func runFetch(c *cli.Context) error {
-	options := parseOptions(c)
+	options, err := parseOptions(c)
+	if err != nil {
+        return err
+    }
+
 	if err := validateOptions(options); err != nil {
 		return err
 	}
@@ -188,7 +192,7 @@ func runFetch(c *cli.Context) error {
 	return nil
 }
 
-func parseOptions(c *cli.Context) FetchOptions {
+func parseOptions(c *cli.Context) (FetchOptions, error) {
 	localDownloadPath := c.Args().First()
 	sourcePaths := c.StringSlice(OPTION_SOURCE_PATH)
 	assetChecksums := c.StringSlice(OPTION_RELEASE_ASSET_CHECKSUM)
@@ -202,6 +206,36 @@ func parseOptions(c *cli.Context) FetchOptions {
 		localDownloadPath = c.Args().Get(1)
 	}
 
+    optionsList := []string{
+        OPTION_REPO,
+        OPTION_TAG,
+        OPTION_COMMIT,
+        OPTION_BRANCH,
+        OPTION_GITHUB_TOKEN,
+        OPTION_SOURCE_PATH,
+        OPTION_RELEASE_ASSET,
+        OPTION_RELEASE_ASSET_CHECKSUM,
+        OPTION_RELEASE_ASSET_CHECKSUM_ALGO,
+        OPTION_GITHUB_API_VERSION,
+    }
+
+    for _, option := range optionsList {
+        switch option {
+        case OPTION_SOURCE_PATH, OPTION_RELEASE_ASSET_CHECKSUM:
+            if c.IsSet(option) {
+                for _, slice := range c.StringSlice(option) {
+                    if slice == "" {
+                        return FetchOptions{}, fmt.Errorf("You specified the --%s flag but did not provide any value.", option)
+                    }
+                }
+            }
+        default:
+            if c.IsSet(option) && c.String(option) == "" {
+                return FetchOptions{}, fmt.Errorf("You specified the --%s flag but did not provide any value.", option)
+            }
+        }
+    }
+
 	for _, assetChecksum := range assetChecksums {
 		assetChecksumMap[assetChecksum] = true
 	}
@@ -218,7 +252,7 @@ func parseOptions(c *cli.Context) FetchOptions {
 		ReleaseAssetChecksumAlgo: c.String(OPTION_RELEASE_ASSET_CHECKSUM_ALGO),
 		LocalDownloadPath:        localDownloadPath,
 		GithubApiVersion:         c.String(OPTION_GITHUB_API_VERSION),
-	}
+	}, nil
 }
 
 func validateOptions(options FetchOptions) error {

main_test.go

@@ -4,6 +4,8 @@ import (
 	"fmt"
 	"os"
 	"testing"
+
+	cli "gopkg.in/urfave/cli.v1"
 )
 
 // Expect to download 2 assets:
@@ -58,3 +60,81 @@ func TestInvalidReleaseAssetsRegex(t *testing.T) {
 		t.Fatalf("Expected error for invalid regex")
 	}
 }
+
+func TestEmptyOptionValues(t *testing.T) {
+	app := cli.NewApp()
+	app.Name = "main_test"
+	app.Flags = []cli.Flag{
+		cli.StringFlag{
+			Name:  OPTION_REPO,
+		},
+		cli.StringFlag{
+			Name:  OPTION_COMMIT,
+		},
+		cli.StringFlag{
+			Name:  OPTION_BRANCH,
+		},
+		cli.StringFlag{
+			Name:  OPTION_TAG,
+		},
+		cli.StringFlag{
+			Name:   OPTION_GITHUB_TOKEN,
+			EnvVar: ENV_VAR_GITHUB_TOKEN,
+		},
+		cli.StringSliceFlag{
+			Name:  OPTION_SOURCE_PATH,
+		},
+		cli.StringFlag{
+			Name:  OPTION_RELEASE_ASSET,
+		},
+		cli.StringSliceFlag{
+			Name:  OPTION_RELEASE_ASSET_CHECKSUM,
+		},
+		cli.StringFlag{
+			Name:  OPTION_RELEASE_ASSET_CHECKSUM_ALGO,
+		},
+		cli.StringFlag{
+			Name:  OPTION_GITHUB_API_VERSION,
+			Value: "v3",
+		},
+	}
+
+    app.Action = func(c *cli.Context) error {
+        _, err := parseOptions(c)
+        return err
+    }
+
+    var args []string
+    var expected string
+    var err error
+
+    optionsList := []string{
+        OPTION_REPO,
+        OPTION_COMMIT,
+        OPTION_BRANCH,
+        OPTION_TAG,
+        OPTION_GITHUB_TOKEN,
+        OPTION_SOURCE_PATH,
+        OPTION_RELEASE_ASSET,
+        OPTION_RELEASE_ASSET_CHECKSUM,
+        OPTION_RELEASE_ASSET_CHECKSUM_ALGO,
+        OPTION_GITHUB_API_VERSION,
+    }
+
+    for _, option := range optionsList {
+        args = os.Args[0:1]
+        dashedOption := "--" + option
+        emptyOption := dashedOption + "="
+        args = append(args, emptyOption)
+
+        err = app.Run(args)
+        expected = fmt.Sprintf("You specified the %s flag but did not provide any value.", dashedOption)
+        if (err != nil) && (err.Error() != expected) { 
+            t.Fatalf("Expected '%s' but received '%s'", expected, err.Error())
+        }
+
+        if err == nil {
+            t.Fatalf("Expected '%s' but received nothing", expected)
+        }
+    }
+}