Remove duplication around cobra code.

cmd/skaffold/app/cmd/build.go

@@ -22,12 +22,14 @@ import (
 	"io/ioutil"
 	"time"
 
+	"github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/cmd/commands"
 	"github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/flags"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/color"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
 )
 
 var (
@@ -42,22 +44,19 @@ var (
 
 // NewCmdBuild describes the CLI command to build artifacts.
 func NewCmdBuild(out io.Writer) *cobra.Command {
-	cmd := &cobra.Command{
-		Use:   "build",
-		Short: "Builds the artifacts",
-		Args:  cobra.NoArgs,
-		RunE: func(cmd *cobra.Command, args []string) error {
-			return runBuild(out)
-		},
-	}
-	AddRunDevFlags(cmd)
-	cmd.Flags().StringSliceVarP(&opts.TargetImages, "build-image", "b", nil, "Choose which artifacts to build. Artifacts with image names that contain the expression will be built only. Default is to build sources for all artifacts")
-	cmd.Flags().BoolVarP(&quietFlag, "quiet", "q", false, "Suppress the build output and print image built on success. See --output to format output.")
-	cmd.Flags().VarP(buildFormatFlag, "output", "o", "Used in conjuction with --quiet flag. "+buildFormatFlag.Usage())
-	return cmd
+	return commands.
+		New(out).
+		WithDescription("build", "Builds the artifacts").
+		WithFlags(func(f *pflag.FlagSet) {
+			AddRunDevFlags(f)
+			f.StringSliceVarP(&opts.TargetImages, "build-image", "b", nil, "Choose which artifacts to build. Artifacts with image names that contain the expression will be built only. Default is to build sources for all artifacts")
+			f.BoolVarP(&quietFlag, "quiet", "q", false, "Suppress the build output and print image built on success. See --output to format output.")
+			f.VarP(buildFormatFlag, "output", "o", "Used in conjuction with --quiet flag. "+buildFormatFlag.Usage())
+		}).
+		NoArgs(doBuild)
 }
 
-func runBuild(out io.Writer) error {
+func doBuild(out io.Writer) error {
 	start := time.Now()
 	defer func() {
 		if !quietFlag {
@@ -75,7 +74,6 @@ func runBuild(out io.Writer) error {
 	}
 
 	bRes, err := createRunnerAndBuildFunc(ctx, buildOut)
-
 	if err != nil {
 		return err
 	}

cmd/skaffold/app/cmd/build_test.go

@@ -80,7 +80,8 @@ func TestQuietFlag(t *testing.T) {
 			createRunnerAndBuildFunc = test.mock
 
 			var output bytes.Buffer
-			err := runBuild(&output)
+			err := doBuild(&output)
+
 			testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, string(test.expectedOutput), output.String())
 		})
 	}
@@ -118,7 +119,8 @@ func TestRunBuild(t *testing.T) {
 			defer func(f func(context.Context, io.Writer) ([]build.Artifact, error)) { createRunnerAndBuildFunc = f }(createRunnerAndBuildFunc)
 			createRunnerAndBuildFunc = test.mock
 
-			err := runBuild(ioutil.Discard)
+			err := doBuild(ioutil.Discard)
+
 			testutil.CheckError(t, test.shouldErr, err)
 		})
 	}

cmd/skaffold/app/cmd/cmd.go

@@ -149,38 +149,38 @@ func FlagToEnvVarName(f *pflag.Flag) string {
 	return fmt.Sprintf("SKAFFOLD_%s", strings.Replace(strings.ToUpper(f.Name), "-", "_", -1))
 }
 
-func AddRunCommonFlags(cmd *cobra.Command) {
-	cmd.Flags().BoolVar(&opts.EnableRPC, "enable-rpc", false, "Enable gRPC for exposing Skaffold events (true by default for `skaffold dev`)")
-	cmd.Flags().IntVar(&opts.RPCPort, "rpc-port", constants.DefaultRPCPort, "tcp port to expose event API")
-	cmd.Flags().IntVar(&opts.RPCHTTPPort, "rpc-http-port", constants.DefaultRPCHTTPPort, "tcp port to expose event REST API over HTTP")
-	cmd.Flags().StringVarP(&opts.ConfigurationFile, "filename", "f", "skaffold.yaml", "Filename or URL to the pipeline file")
-	cmd.Flags().BoolVar(&opts.Notification, "toot", false, "Emit a terminal beep after the deploy is complete")
-	cmd.Flags().StringSliceVarP(&opts.Profiles, "profile", "p", nil, "Activate profiles by name")
-	cmd.Flags().StringVarP(&opts.Namespace, "namespace", "n", "", "Run deployments in the specified namespace")
-	cmd.Flags().StringVarP(&opts.DefaultRepo, "default-repo", "d", "", "Default repository value (overrides global config)")
-	cmd.Flags().BoolVar(&opts.NoPrune, "no-prune", false, "Skip removing images and containers built by Skaffold")
-	cmd.Flags().BoolVar(&opts.NoPruneChildren, "no-prune-children", false, "Skip removing layers reused by Skaffold")
-	cmd.Flags().StringSliceVar(&opts.InsecureRegistries, "insecure-registry", nil, "Target registries for built images which are not secure")
+func AddRunCommonFlags(f *pflag.FlagSet) {
+	f.BoolVar(&opts.EnableRPC, "enable-rpc", false, "Enable gRPC for exposing Skaffold events (true by default for `skaffold dev`)")
+	f.IntVar(&opts.RPCPort, "rpc-port", constants.DefaultRPCPort, "tcp port to expose event API")
+	f.IntVar(&opts.RPCHTTPPort, "rpc-http-port", constants.DefaultRPCHTTPPort, "tcp port to expose event REST API over HTTP")
+	f.StringVarP(&opts.ConfigurationFile, "filename", "f", "skaffold.yaml", "Filename or URL to the pipeline file")
+	f.BoolVar(&opts.Notification, "toot", false, "Emit a terminal beep after the deploy is complete")
+	f.StringSliceVarP(&opts.Profiles, "profile", "p", nil, "Activate profiles by name")
+	f.StringVarP(&opts.Namespace, "namespace", "n", "", "Run deployments in the specified namespace")
+	f.StringVarP(&opts.DefaultRepo, "default-repo", "d", "", "Default repository value (overrides global config)")
+	f.BoolVar(&opts.NoPrune, "no-prune", false, "Skip removing images and containers built by Skaffold")
+	f.BoolVar(&opts.NoPruneChildren, "no-prune-children", false, "Skip removing layers reused by Skaffold")
+	f.StringSliceVar(&opts.InsecureRegistries, "insecure-registry", nil, "Target registries for built images which are not secure")
 }
 
-func AddRunDeployFlags(cmd *cobra.Command) {
-	cmd.Flags().BoolVar(&opts.Tail, "tail", false, "Stream logs from deployed objects")
-	cmd.Flags().BoolVar(&opts.Force, "force", false, "Recreate kubernetes resources if necessary for deployment (default: false, warning: might cause downtime!)")
-	cmd.Flags().StringSliceVarP(&opts.CustomLabels, "label", "l", nil, "Add custom labels to deployed objects. Set multiple times for multiple labels.")
+func AddRunDeployFlags(f *pflag.FlagSet) {
+	f.BoolVar(&opts.Tail, "tail", false, "Stream logs from deployed objects")
+	f.BoolVar(&opts.Force, "force", false, "Recreate kubernetes resources if necessary for deployment (default: false, warning: might cause downtime!)")
+	f.StringSliceVarP(&opts.CustomLabels, "label", "l", nil, "Add custom labels to deployed objects. Set multiple times for multiple labels.")
 }
 
-func AddRunDevFlags(cmd *cobra.Command) {
-	AddRunCommonFlags(cmd)
-	cmd.Flags().BoolVar(&opts.SkipTests, "skip-tests", false, "Whether to skip the tests after building")
-	cmd.Flags().BoolVar(&opts.CacheArtifacts, "cache-artifacts", false, "Set to true to enable caching of artifacts.")
-	cmd.Flags().StringVarP(&opts.CacheFile, "cache-file", "", "", "Specify the location of the cache file (default $HOME/.skaffold/cache)")
+func AddRunDevFlags(f *pflag.FlagSet) {
+	AddRunCommonFlags(f)
+	f.BoolVar(&opts.SkipTests, "skip-tests", false, "Whether to skip the tests after building")
+	f.BoolVar(&opts.CacheArtifacts, "cache-artifacts", false, "Set to true to enable caching of artifacts.")
+	f.StringVarP(&opts.CacheFile, "cache-file", "", "", "Specify the location of the cache file (default $HOME/.skaffold/cache)")
 }
 
-func AddDevDebugFlags(cmd *cobra.Command) {
-	cmd.Flags().BoolVar(&opts.TailDev, "tail", true, "Stream logs from deployed objects")
-	cmd.Flags().BoolVar(&opts.Cleanup, "cleanup", true, "Delete deployments after dev mode is interrupted")
-	cmd.Flags().BoolVar(&opts.PortForward, "port-forward", false, "Port-forward exposed container ports within pods")
-	cmd.Flags().StringSliceVarP(&opts.CustomLabels, "label", "l", nil, "Add custom labels to deployed objects. Set multiple times for multiple labels")
+func AddDevDebugFlags(f *pflag.FlagSet) {
+	f.BoolVar(&opts.TailDev, "tail", true, "Stream logs from deployed objects")
+	f.BoolVar(&opts.Cleanup, "cleanup", true, "Delete deployments after dev mode is interrupted")
+	f.BoolVar(&opts.PortForward, "port-forward", false, "Port-forward exposed container ports within pods")
+	f.StringSliceVarP(&opts.CustomLabels, "label", "l", nil, "Add custom labels to deployed objects. Set multiple times for multiple labels")
 }
 
 func SetUpLogs(out io.Writer, level string) error {

cmd/skaffold/app/cmd/commands/commands.go

@@ -0,0 +1,76 @@
+/*
+Copyright 2019 The Skaffold Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package commands
+
+import (
+	"io"
+
+	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
+)
+
+type CmdBuilder interface {
+	WithDescription(use, description string) CmdBuilder
+	WithLongDescription(use, description, long string) CmdBuilder
+	WithFlags(adder func(*pflag.FlagSet)) CmdBuilder
+	ExactArgs(argCount int, action func(io.Writer, []string) error) *cobra.Command
+	NoArgs(action func(io.Writer) error) *cobra.Command
+}
+
+type cmdBuilder struct {
+	out io.Writer
+	cmd cobra.Command
+}
+
+func New(out io.Writer) CmdBuilder {
+	return &cmdBuilder{
+		out: out,
+		cmd: cobra.Command{},
+	}
+}
+
+func (c *cmdBuilder) WithDescription(use, description string) CmdBuilder {
+	c.cmd.Use = use
+	c.cmd.Short = description
+	return c
+}
+
+func (c *cmdBuilder) WithLongDescription(use, description, long string) CmdBuilder {
+	c.cmd.Long = long
+	return c.WithDescription(use, description)
+}
+
+func (c *cmdBuilder) WithFlags(adder func(*pflag.FlagSet)) CmdBuilder {
+	adder(c.cmd.Flags())
+	return c
+}
+
+func (c *cmdBuilder) ExactArgs(argCount int, action func(io.Writer, []string) error) *cobra.Command {
+	c.cmd.Args = cobra.ExactArgs(argCount)
+	c.cmd.RunE = func(cmd *cobra.Command, args []string) error {
+		return action(c.out, args)
+	}
+	return &c.cmd
+}
+
+func (c *cmdBuilder) NoArgs(action func(io.Writer) error) *cobra.Command {
+	c.cmd.Args = cobra.NoArgs
+	c.cmd.RunE = func(cmd *cobra.Command, _ []string) error {
+		return action(c.out)
+	}
+	return &c.cmd
+}

cmd/skaffold/app/cmd/commands/commands_test.go

@@ -0,0 +1,80 @@
+/*
+Copyright 2019 The Skaffold Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package commands
+
+import (
+	"bytes"
+	"errors"
+	"fmt"
+	"io"
+	"testing"
+
+	"github.com/GoogleContainerTools/skaffold/testutil"
+)
+
+func TestNewCommandDescription(t *testing.T) {
+	cmd := New(nil).WithDescription("help", "prints help").NoArgs(nil)
+
+	testutil.CheckDeepEqual(t, "help", cmd.Use)
+	testutil.CheckDeepEqual(t, "prints help", cmd.Short)
+}
+
+func TestNewCommandLongDescription(t *testing.T) {
+	cmd := New(nil).WithLongDescription("help", "prints help", "long description").NoArgs(nil)
+
+	testutil.CheckDeepEqual(t, "help", cmd.Use)
+	testutil.CheckDeepEqual(t, "prints help", cmd.Short)
+	testutil.CheckDeepEqual(t, "long description", cmd.Long)
+}
+
+func TestNewCommandNoArgs(t *testing.T) {
+	cmd := New(nil).NoArgs(nil)
+
+	testutil.CheckError(t, false, cmd.Args(cmd, []string{}))
+	testutil.CheckError(t, true, cmd.Args(cmd, []string{"extract arg"}))
+}
+
+func TestNewCommandExactArgs(t *testing.T) {
+	cmd := New(nil).ExactArgs(1, nil)
+
+	testutil.CheckError(t, true, cmd.Args(cmd, []string{}))
+	testutil.CheckError(t, false, cmd.Args(cmd, []string{"valid"}))
+	testutil.CheckError(t, true, cmd.Args(cmd, []string{"valid", "extra"}))
+}
+
+func TestNewCommandError(t *testing.T) {
+	cmd := New(nil).NoArgs(func(out io.Writer) error {
+		return errors.New("expected error")
+	})
+
+	err := cmd.RunE(nil, nil)
+
+	testutil.CheckErrorAndDeepEqual(t, true, err, "expected error", err.Error())
+}
+
+func TestNewCommandOutput(t *testing.T) {
+	var buf bytes.Buffer
+
+	cmd := New(&buf).ExactArgs(1, func(out io.Writer, args []string) error {
+		fmt.Fprintf(out, "test output: %v\n", args)
+		return nil
+	})
+
+	err := cmd.RunE(nil, []string{"arg1"})
+
+	testutil.CheckErrorAndDeepEqual(t, false, err, "test output: [arg1]\n", buf.String())
+}

cmd/skaffold/app/cmd/config/flags.go

@@ -16,22 +16,16 @@ limitations under the License.
 
 package config
 
-import (
-	"github.com/spf13/cobra"
-)
+import "github.com/spf13/pflag"
 
 var configFile, kubecontext string
 var showAll, global bool
 
-func AddConfigFlags(cmd *cobra.Command) {
-	cmd.Flags().StringVarP(&configFile, "config", "c", "", "Path to Skaffold config")
-	cmd.Flags().StringVarP(&kubecontext, "kube-context", "k", "", "Kubectl context to set values against")
+func AddConfigFlags(f *pflag.FlagSet) {
+	f.StringVarP(&configFile, "config", "c", "", "Path to Skaffold config")
+	f.StringVarP(&kubecontext, "kube-context", "k", "", "Kubectl context to set values against")
 }
 
-func AddListFlags(cmd *cobra.Command) {
-	cmd.Flags().BoolVarP(&showAll, "all", "a", false, "Show values for all kubecontexts")
-}
-
-func AddSetFlags(cmd *cobra.Command) {
-	cmd.Flags().BoolVarP(&global, "global", "g", false, "Set value for global config")
+func AddSetFlags(f *pflag.FlagSet) {
+	f.BoolVarP(&global, "global", "g", false, "Set value for global config")
 }

cmd/skaffold/app/cmd/config/list.go

@@ -20,26 +20,25 @@ import (
 	"fmt"
 	"io"
 
+	"github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/cmd/commands"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
 	yaml "gopkg.in/yaml.v2"
 )
 
 func NewCmdList(out io.Writer) *cobra.Command {
-	cmd := &cobra.Command{
-		Use:   "list",
-		Short: "List all values set in the global Skaffold config",
-		Args:  cobra.ExactArgs(0),
-		RunE: func(cmd *cobra.Command, args []string) error {
-			return runList(out)
-		},
-	}
-	AddConfigFlags(cmd)
-	AddListFlags(cmd)
-	return cmd
+	return commands.
+		New(out).
+		WithDescription("list", "List all values set in the global Skaffold config").
+		WithFlags(func(f *pflag.FlagSet) {
+			f.BoolVarP(&showAll, "all", "a", false, "Show values for all kubecontexts")
+			AddConfigFlags(f)
+		}).
+		NoArgs(doList)
 }
 
-func runList(out io.Writer) error {
+func doList(out io.Writer) error {
 	var configYaml []byte
 	if showAll {
 		cfg, err := readConfig()