From 98862f7daf733bdd36ecee19fbd33ec2f2d940a9 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Wed, 20 Mar 2019 01:24:49 -0700 Subject: [PATCH] Fix skaffold build templating output and add tests --- cmd/skaffold/app/cmd/build.go | 50 +++++++----- cmd/skaffold/app/cmd/build_test.go | 118 +++++++++++++++++++++++++++++ 2 files changed, 148 insertions(+), 20 deletions(-) create mode 100644 cmd/skaffold/app/cmd/build_test.go diff --git a/cmd/skaffold/app/cmd/build.go b/cmd/skaffold/app/cmd/build.go index c3e7a0896ea..f5e77fce17f 100644 --- a/cmd/skaffold/app/cmd/build.go +++ b/cmd/skaffold/app/cmd/build.go @@ -35,6 +35,11 @@ var ( buildFormatFlag = flags.NewTemplateFlag("{{range .Builds}}{{.ImageName}} -> {{.Tag}}\n{{end}}", BuildOutput{}) ) +// For testing +var ( + createRunnerAndBuildFunc = createRunnerAndBuild +) + // NewCmdBuild describes the CLI command to build artifacts. func NewCmdBuild(out io.Writer) *cobra.Command { cmd := &cobra.Command{ @@ -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,18 @@ 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(buildOut) - bRes, err := runner.BuildAndTest(ctx, buildOut, targetArtifacts) if err != nil { return err } @@ -100,3 +91,22 @@ func runBuild(out io.Writer) error { return nil } + +func createRunnerAndBuild(buildOut io.Writer) ([]build.Artifact, error) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + catchCtrlC(cancel) + + 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) +} diff --git a/cmd/skaffold/app/cmd/build_test.go b/cmd/skaffold/app/cmd/build_test.go new file mode 100644 index 00000000000..8f74065eeb3 --- /dev/null +++ b/cmd/skaffold/app/cmd/build_test.go @@ -0,0 +1,118 @@ +/* +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" + "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(buildOut io.Writer) ([]build.Artifact, error) { + return []build.Artifact{{ + ImageName: "gcr.io/skaffold/example", + Tag: "test", + }}, nil + } + + orginalCreateRunner := createRunnerAndBuildFunc + defer func(c func(buildOut io.Writer) ([]build.Artifact, error)) { createRunnerAndBuildFunc = c }(orginalCreateRunner) + var tests = []struct { + name string + template string + expectedOutput []byte + mock func(io.Writer) ([]build.Artifact, error) + shdErr bool + }{ + { + name: "quiet flag print build images with no template", + expectedOutput: []byte("gcr.io/skaffold/example -> test\n"), + shdErr: false, + mock: mockCreateRunner, + }, + { + name: "quiet flag print build images applies pattern specified in template ", + template: "{{.}}", + expectedOutput: []byte("{[{gcr.io/skaffold/example test}]}"), + shdErr: false, + mock: mockCreateRunner, + }, + { + name: "build errors out when incorrect template specified", + template: "{{.Incorrect}}", + expectedOutput: nil, + shdErr: true, + mock: mockCreateRunner, + }, + } + + for _, test := range tests { + quietFlag = true + if test.template != "" { + buildFormatFlag = flags.NewTemplateFlag(test.template, BuildOutput{}) + } + createRunnerAndBuildFunc = test.mock + var output bytes.Buffer + err := runBuild(&output) + testutil.CheckErrorAndDeepEqual(t, test.shdErr, err, string(test.expectedOutput), output.String()) + } +} + +func TestRunBuild(t *testing.T) { + mockCreateRunner := func(buildOut io.Writer) ([]build.Artifact, error) { + return []build.Artifact{{ + ImageName: "gcr.io/skaffold/example", + Tag: "test", + }}, nil + } + errRunner := func(buildOut io.Writer) ([]build.Artifact, error) { + return nil, errors.New("some error") + } + + orginalCreateRunner := createRunnerAndBuildFunc + defer func(c func(buildOut io.Writer) ([]build.Artifact, error)) { createRunnerAndBuildFunc = c }(orginalCreateRunner) + + var tests = []struct { + name string + mock func(io.Writer) ([]build.Artifact, error) + shdErr bool + }{ + { + name: "buod should return successfully when runner is successful.", + shdErr: false, + mock: mockCreateRunner, + }, + { + name: "build errors out when there was runner error.", + shdErr: true, + mock: errRunner, + }, + } + for _, test := range tests { + createRunnerAndBuildFunc = test.mock + err := runBuild(ioutil.Discard) + testutil.CheckError(t, test.shdErr, err) + } + +}