Merge pull request #2128 from dgageot/more-tests

More tests

cmd/skaffold/app/cmd/build.go

@@ -26,6 +26,7 @@ import (
 	"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/config"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
@@ -53,26 +54,20 @@ func NewCmdBuild(out io.Writer) *cobra.Command {
 			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)
+		NoArgs(cancelWithCtrlC(context.Background(), doBuild))
 }
 
-func doBuild(out io.Writer) error {
-	start := time.Now()
-	defer func() {
-		if !quietFlag {
-			color.Default.Fprintln(out, "Complete in", time.Since(start))
-		}
-	}()
-
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
-	catchCtrlC(cancel)
-
+func doBuild(ctx context.Context, out io.Writer) error {
 	buildOut := out
 	if quietFlag {
 		buildOut = ioutil.Discard
 	}
 
+	start := time.Now()
+	defer func() {
+		color.Default.Fprintln(buildOut, "Complete in", time.Since(start))
+	}()
+
 	bRes, err := createRunnerAndBuildFunc(ctx, buildOut)
 	if err != nil {
 		return err
@@ -94,11 +89,18 @@ func createRunnerAndBuild(ctx context.Context, buildOut io.Writer) ([]build.Arti
 		return nil, errors.Wrap(err, "creating runner")
 	}
 	defer runner.RPCServerShutdown()
+
+	return runner.BuildAndTest(ctx, buildOut, targetArtifacts(opts, config))
+}
+
+func targetArtifacts(opts *config.SkaffoldOptions, cfg *latest.SkaffoldConfig) []*latest.Artifact {
 	var targetArtifacts []*latest.Artifact
-	for _, artifact := range config.Build.Artifacts {
-		if runner.IsTargetImage(artifact) {
+
+	for _, artifact := range cfg.Build.Artifacts {
+		if opts.IsTargetImage(artifact) {
 			targetArtifacts = append(targetArtifacts, artifact)
 		}
 	}
-	return runner.BuildAndTest(ctx, buildOut, targetArtifacts)
+
+	return targetArtifacts
 }

cmd/skaffold/app/cmd/build_test.go

@@ -80,7 +80,8 @@ func TestQuietFlag(t *testing.T) {
 			}
 
 			var output bytes.Buffer
-			err := doBuild(&output)
+
+			err := doBuild(context.Background(), &output)
 
 			testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, string(test.expectedOutput), output.String())
 		})
@@ -119,7 +120,7 @@ func TestRunBuild(t *testing.T) {
 			restore := testutil.Override(t, &createRunnerAndBuildFunc, test.mock)
 			defer restore()
 
-			err := doBuild(ioutil.Discard)
+			err := doBuild(context.Background(), ioutil.Discard)
 
 			testutil.CheckError(t, test.shouldErr, err)
 		})

cmd/skaffold/app/cmd/debug.go

@@ -17,6 +17,7 @@ limitations under the License.
 package cmd
 
 import (
+	"context"
 	"io"
 
 	"github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/cmd/commands"
@@ -35,10 +36,10 @@ func NewCmdDebug(out io.Writer) *cobra.Command {
 			AddRunDevFlags(f)
 			AddDevDebugFlags(f)
 		}).
-		NoArgs(doDebug)
+		NoArgs(cancelWithCtrlC(context.Background(), doDebug))
 }
 
-func doDebug(out io.Writer) error {
+func doDebug(ctx context.Context, out io.Writer) error {
 	// HACK: disable watcher to prevent redeploying changed containers during debugging
 	// TODO: enable file-sync but avoid redeploys of artifacts being debugged
 	if len(opts.TargetImages) == 0 {
@@ -47,5 +48,5 @@ func doDebug(out io.Writer) error {
 
 	deploy.AddManifestTransform(debugging.ApplyDebuggingTransforms)
 
-	return doDev(out)
+	return doDev(ctx, out)
 }

cmd/skaffold/app/cmd/delete.go

@@ -21,7 +21,8 @@ import (
 	"io"
 
 	"github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/cmd/commands"
-	"github.com/pkg/errors"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 )
@@ -34,19 +35,11 @@ func NewCmdDelete(out io.Writer) *cobra.Command {
 		WithFlags(func(f *pflag.FlagSet) {
 			AddRunCommonFlags(f)
 		}).
-		NoArgs(doDelete)
+		NoArgs(cancelWithCtrlC(context.Background(), doDelete))
 }
 
-func doDelete(out io.Writer) error {
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
-	catchCtrlC(cancel)
-
-	runner, _, err := newRunner(opts)
-	if err != nil {
-		return errors.Wrap(err, "creating runner")
-	}
-	defer runner.RPCServerShutdown()
-
-	return runner.Deployer.Cleanup(ctx, out)
+func doDelete(ctx context.Context, out io.Writer) error {
+	return withRunner(func(r *runner.SkaffoldRunner, _ *latest.SkaffoldConfig) error {
+		return r.Deployer.Cleanup(ctx, out)
+	})
 }

cmd/skaffold/app/cmd/deploy.go

@@ -23,7 +23,8 @@ import (
 	"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/pkg/errors"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 )
@@ -45,22 +46,15 @@ E.g. build.out created by running skaffold build --quiet {{json .}} > build.out`
 			AddRunDevFlags(f)
 			AddRunDeployFlags(f)
 		}).
-		NoArgs(doDeploy)
+		NoArgs(cancelWithCtrlC(context.Background(), doDeploy))
 }
 
-func doDeploy(out io.Writer) error {
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
-	catchCtrlC(cancel)
-	runner, _, err := newRunner(opts)
-	if err != nil {
-		return errors.Wrap(err, "creating runner")
-	}
-	defer runner.RPCServerShutdown()
+func doDeploy(ctx context.Context, out io.Writer) error {
+	return withRunner(func(r *runner.SkaffoldRunner, _ *latest.SkaffoldConfig) error {
+		// If the BuildArtifacts contains an image in the preBuilt list,
+		// use image from BuildArtifacts instead
+		deployArtifacts := build.MergeWithPreviousBuilds(buildOutputFile.BuildArtifacts(), preBuiltImages.Artifacts())
 
-	// If the BuildArtifacts contains an image in the preBuilt list,
-	// use image from BuildArtifacts instead
-	deployArtifacts := build.MergeWithPreviousBuilds(buildOutputFile.BuildArtifacts(), preBuiltImages.Artifacts())
-
-	return runner.Deploy(ctx, out, deployArtifacts)
+		return r.Deploy(ctx, out, deployArtifacts)
+	})
 }

cmd/skaffold/app/cmd/dev.go

@@ -40,15 +40,11 @@ func NewCmdDev(out io.Writer) *cobra.Command {
 			AddRunDevFlags(f)
 			AddDevDebugFlags(f)
 		}).
-		NoArgs(doDev)
+		NoArgs(cancelWithCtrlC(context.Background(), doDev))
 }
 
-func doDev(out io.Writer) error {
+func doDev(ctx context.Context, out io.Writer) error {
 	opts.EnableRPC = true
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
-
-	catchCtrlC(cancel)
 
 	cleanup := func() {}
 	if opts.Cleanup {

cmd/skaffold/app/cmd/diagnose.go

@@ -22,6 +22,8 @@ import (
 
 	"github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/cmd/commands"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/color"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/version"
 
 	"github.com/pkg/errors"
@@ -43,25 +45,22 @@ func NewCmdDiagnose(out io.Writer) *cobra.Command {
 }
 
 func doDiagnose(out io.Writer) error {
-	runner, config, err := newRunner(opts)
-	if err != nil {
-		return errors.Wrap(err, "creating runner")
-	}
+	return withRunner(func(r *runner.SkaffoldRunner, config *latest.SkaffoldConfig) error {
+		fmt.Fprintln(out, "Skaffold version:", version.Get().GitCommit)
+		fmt.Fprintln(out, "Configuration version:", config.APIVersion)
+		fmt.Fprintln(out, "Number of artifacts:", len(config.Build.Artifacts))
 
-	fmt.Fprintln(out, "Skaffold version:", version.Get().GitCommit)
-	fmt.Fprintln(out, "Configuration version:", config.APIVersion)
-	fmt.Fprintln(out, "Number of artifacts:", len(config.Build.Artifacts))
+		if err := r.DiagnoseArtifacts(out); err != nil {
+			return errors.Wrap(err, "running diagnostic on artifacts")
+		}
 
-	if err := runner.DiagnoseArtifacts(out); err != nil {
-		return errors.Wrap(err, "running diagnostic on artifacts")
-	}
+		color.Blue.Fprintln(out, "\nConfiguration")
+		buf, err := yaml.Marshal(config)
+		if err != nil {
+			return errors.Wrap(err, "marshalling configuration")
+		}
+		out.Write(buf)
 
-	color.Blue.Fprintln(out, "\nConfiguration")
-	buf, err := yaml.Marshal(config)
-	if err != nil {
-		return errors.Wrap(err, "marshalling configuration")
-	}
-	out.Write(buf)
-
-	return nil
+		return nil
+	})
 }

cmd/skaffold/app/cmd/run.go

@@ -22,7 +22,8 @@ import (
 
 	"github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/cmd/commands"
 	"github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/tips"
-	"github.com/pkg/errors"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 )
@@ -37,24 +38,16 @@ func NewCmdRun(out io.Writer) *cobra.Command {
 			AddRunDevFlags(f)
 			AddRunDeployFlags(f)
 		}).
-		NoArgs(doRun)
+		NoArgs(cancelWithCtrlC(context.Background(), doRun))
 }
 
-func doRun(out io.Writer) error {
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
-	catchCtrlC(cancel)
+func doRun(ctx context.Context, out io.Writer) error {
+	return withRunner(func(r *runner.SkaffoldRunner, config *latest.SkaffoldConfig) error {
+		err := r.Run(ctx, out, config.Build.Artifacts)
+		if err == nil {
+			tips.PrintForRun(out, opts)
+		}
 
-	runner, config, err := newRunner(opts)
-	if err != nil {
-		return errors.Wrap(err, "creating runner")
-	}
-	defer runner.RPCServerShutdown()
-
-	err = runner.Run(ctx, out, config.Build.Artifacts)
-	if err == nil {
-		tips.PrintForRun(out, opts)
-	}
-
-	return err
+		return err
+	})
 }

cmd/skaffold/app/cmd/runner.go

@@ -33,6 +33,16 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
+func withRunner(action func(*runner.SkaffoldRunner, *latest.SkaffoldConfig) error) error {
+	runner, config, err := newRunner(opts)
+	if err != nil {
+		return errors.Wrap(err, "creating runner")
+	}
+	defer runner.RPCServerShutdown()
+
+	return action(runner, config)
+}
+
 // newRunner creates a SkaffoldRunner and returns the SkaffoldConfig associated with it.
 func newRunner(opts *config.SkaffoldOptions) (*runner.SkaffoldRunner, *latest.SkaffoldConfig, error) {
 	parsed, err := schema.ParseConfig(opts.ConfigurationFile, true)

cmd/skaffold/app/cmd/signals.go

@@ -18,11 +18,22 @@ package cmd
 
 import (
 	"context"
+	"io"
 	"os"
 	"os/signal"
 	"syscall"
 )
 
+func cancelWithCtrlC(ctx context.Context, action func(context.Context, io.Writer) error) func(io.Writer) error {
+	return func(out io.Writer) error {
+		ctx, cancel := context.WithCancel(ctx)
+		defer cancel()
+
+		catchCtrlC(cancel)
+		return action(ctx, out)
+	}
+}
+
 func catchCtrlC(cancel context.CancelFunc) {
 	signals := make(chan os.Signal, 1)
 	signal.Notify(signals,