Fix skaffold build templating output and add tests

cmd/skaffold/app/cmd/build.go

@@ -32,7 +32,12 @@ import (
 
 var (
 	quietFlag       bool
-	buildFormatFlag = flags.NewTemplateFlag("{{range .Builds}}{{.ImageName}} -> {{.Tag}}\n{{end}}", BuildOutput{})
+	buildFormatFlag = flags.NewTemplateFlag("{{.}}", BuildOutput{})
+)
+
+// For testing
+var (
+	createRunnerAndBuildFunc = createRunnerAndBuild
 )
 
 // NewCmdBuild describes the CLI command to build artifacts.
@@ -48,8 +53,8 @@ func NewCmdBuild(out io.Writer) *cobra.Command {
 	}
 	AddRunDevFlags(cmd)
 	cmd.Flags().StringArrayVarP(&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")
-	cmd.Flags().VarP(buildFormatFlag, "output", "o", buildFormatFlag.Usage())
+	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
 }
 
@@ -61,32 +66,22 @@ type BuildOutput struct {
 func runBuild(out io.Writer) error {
 	start := time.Now()
 	defer func() {
-		color.Default.Fprintln(out, "Complete in", time.Since(start))
+		if !quietFlag {
+			color.Default.Fprintln(out, "Complete in", time.Since(start))
+		}
 	}()
 
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 	catchCtrlC(cancel)
 
-	runner, config, err := newRunner(opts)
-	if err != nil {
-		return errors.Wrap(err, "creating runner")
-	}
-	defer runner.RPCServerShutdown()
-
 	buildOut := out
 	if quietFlag {
 		buildOut = ioutil.Discard
 	}
 
-	var targetArtifacts []*latest.Artifact
-	for _, artifact := range config.Build.Artifacts {
-		if runner.IsTargetImage(artifact) {
-			targetArtifacts = append(targetArtifacts, artifact)
-		}
-	}
+	bRes, err := createRunnerAndBuildFunc(ctx, buildOut)
 
-	bRes, err := runner.BuildAndTest(ctx, buildOut, targetArtifacts)
 	if err != nil {
 		return err
 	}
@@ -100,3 +95,18 @@ func runBuild(out io.Writer) error {
 
 	return nil
 }
+
+func createRunnerAndBuild(ctx context.Context, buildOut io.Writer) ([]build.Artifact, error) {
+	runner, config, err := newRunner(opts)
+	var targetArtifacts []*latest.Artifact
+	for _, artifact := range config.Build.Artifacts {
+		if runner.IsTargetImage(artifact) {
+			targetArtifacts = append(targetArtifacts, artifact)
+		}
+	}
+	if err != nil {
+		return nil, errors.Wrap(err, "creating runner")
+	}
+	defer runner.RPCServerShutdown()
+	return runner.BuildAndTest(ctx, buildOut, targetArtifacts)
+}

cmd/skaffold/app/cmd/build_test.go

@@ -0,0 +1,127 @@
+/*
+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 cmd
+
+import (
+	"bytes"
+	"context"
+	"errors"
+	"io"
+	"io/ioutil"
+	"testing"
+
+	"github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/flags"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build"
+	"github.com/GoogleContainerTools/skaffold/testutil"
+)
+
+func TestQuietFlag(t *testing.T) {
+	mockCreateRunner := func(c context.Context, buildOut io.Writer) ([]build.Artifact, error) {
+		return []build.Artifact{{
+			ImageName: "gcr.io/skaffold/example",
+			Tag:       "test",
+		}}, nil
+	}
+
+	orginalCreateRunner := createRunnerAndBuildFunc
+	defer func(f func(c context.Context, buildOut io.Writer) ([]build.Artifact, error)) {
+		createRunnerAndBuildFunc = f
+	}(orginalCreateRunner)
+	var tests = []struct {
+		description    string
+		template       string
+		expectedOutput []byte
+		mock           func(context.Context, io.Writer) ([]build.Artifact, error)
+		shouldErr      bool
+	}{
+		{
+			description:    "quiet flag print build images with no template",
+			expectedOutput: []byte("{[{gcr.io/skaffold/example test}]}"),
+			shouldErr:      false,
+			mock:           mockCreateRunner,
+		},
+		{
+			description:    "quiet flag print build images applies pattern specified in template ",
+			template:       "{{range .Builds}}{{.ImageName}} -> {{.Tag}}\n{{end}}",
+			expectedOutput: []byte("gcr.io/skaffold/example -> test\n"),
+			shouldErr:      false,
+			mock:           mockCreateRunner,
+		},
+		{
+			description:    "build errors out when incorrect template specified",
+			template:       "{{.Incorrect}}",
+			expectedOutput: nil,
+			shouldErr:      true,
+			mock:           mockCreateRunner,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.description, func(t *testing.T) {
+			quietFlag = true
+			defer func() { quietFlag = false }()
+			if test.template != "" {
+				buildFormatFlag = flags.NewTemplateFlag(test.template, BuildOutput{})
+			}
+			defer func() { buildFormatFlag = nil }()
+			createRunnerAndBuildFunc = test.mock
+			var output bytes.Buffer
+			err := runBuild(&output)
+			testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, string(test.expectedOutput), output.String())
+		})
+	}
+}
+
+func TestRunBuild(t *testing.T) {
+	errRunner := func(c context.Context, buildOut io.Writer) ([]build.Artifact, error) {
+		return nil, errors.New("some error")
+	}
+	mockCreateRunner := func(c context.Context, buildOut io.Writer) ([]build.Artifact, error) {
+		return []build.Artifact{{
+			ImageName: "gcr.io/skaffold/example",
+			Tag:       "test",
+		}}, nil
+	}
+	orginalCreateRunner := createRunnerAndBuildFunc
+	defer func(f func(c context.Context, buildOut io.Writer) ([]build.Artifact, error)) {
+		createRunnerAndBuildFunc = f
+	}(orginalCreateRunner)
+
+	var tests = []struct {
+		description string
+		mock        func(context.Context, io.Writer) ([]build.Artifact, error)
+		shouldErr   bool
+	}{
+		{
+			description: "build should return successfully when runner is successful.",
+			shouldErr:   false,
+			mock:        mockCreateRunner,
+		},
+		{
+			description: "build errors out when there was runner error.",
+			shouldErr:   true,
+			mock:        errRunner,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.description, func(t *testing.T) {
+			createRunnerAndBuildFunc = test.mock
+			err := runBuild(ioutil.Discard)
+			testutil.CheckError(t, test.shouldErr, err)
+		})
+	}
+}

docs/content/en/docs/references/cli/_index.md

@@ -70,10 +70,9 @@ Flags:
       --enable-rpc skaffold dev      Enable gRPC for exposing Skaffold events (true by default for skaffold dev)
   -f, --filename string              Filename or URL to the pipeline file (default "skaffold.yaml")
   -n, --namespace string             Run deployments in the specified namespace
-  -o, --output *flags.TemplateFlag   Format output with go-template. For full struct documentation, see https://godoc.org/github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/cmd#BuildOutput (default {{range .Builds}}{{.ImageName}} -> {{.Tag}}
-                                     {{end}})
+  -o, --output *flags.TemplateFlag   Used in conjuction with --quiet flag. Format output with go-template. For full struct documentation, see https://godoc.org/github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/cmd#BuildOutput (default {{.}})
   -p, --profile stringArray          Activate profiles by name
-  -q, --quiet                        Suppress the build output and print image built on success
+  -q, --quiet                        Suppress the build output and print image built on success. See --output to format output. 
       --rpc-http-port int            tcp port to expose event REST API over HTTP (default 50052)
       --rpc-port int                 tcp port to expose event API (default 50051)
       --skip-tests                   Whether to skip the tests after building