ref(action): remove io.Writers, return string instead

Signed-off-by: Matthew Fisher <matt.fisher@microsoft.com>

cmd/helm/pull.go

@@ -17,6 +17,7 @@ limitations under the License.
 package main
 
 import (
+	"fmt"
 	"io"
 
 	"github.com/spf13/cobra"
@@ -42,7 +43,6 @@ result in an error, and the chart will not be saved locally.
 
 func newPullCmd(out io.Writer) *cobra.Command {
 	client := action.NewPull()
-	client.Out = out
 
 	cmd := &cobra.Command{
 		Use:     "pull [chart URL | repo/chartname] [...]",
@@ -58,9 +58,11 @@ func newPullCmd(out io.Writer) *cobra.Command {
 			}
 
 			for i := 0; i < len(args); i++ {
-				if err := client.Run(args[i]); err != nil {
+				output, err := client.Run(args[i])
+				if err != nil {
 					return err
 				}
+				fmt.Fprint(out, output)
 			}
 			return nil
 		},

cmd/helm/show.go

@@ -17,6 +17,7 @@ limitations under the License.
 package main
 
 import (
+	"fmt"
 	"io"
 
 	"github.com/spf13/cobra"
@@ -48,7 +49,7 @@ of the README file
 `
 
 func newShowCmd(out io.Writer) *cobra.Command {
-	client := action.NewShow(out, action.ShowAll)
+	client := action.NewShow(action.ShowAll)
 
 	showCommand := &cobra.Command{
 		Use:     "show [CHART]",
@@ -61,7 +62,12 @@ func newShowCmd(out io.Writer) *cobra.Command {
 			if err != nil {
 				return err
 			}
-			return client.Run(cp)
+			output, err := client.Run(cp)
+			if err != nil {
+				return err
+			}
+			fmt.Fprint(out, output)
+			return nil
 		},
 	}
 
@@ -76,7 +82,12 @@ func newShowCmd(out io.Writer) *cobra.Command {
 			if err != nil {
 				return err
 			}
-			return client.Run(cp)
+			output, err := client.Run(cp)
+			if err != nil {
+				return err
+			}
+			fmt.Fprint(out, output)
+			return nil
 		},
 	}
 
@@ -91,7 +102,12 @@ func newShowCmd(out io.Writer) *cobra.Command {
 			if err != nil {
 				return err
 			}
-			return client.Run(cp)
+			output, err := client.Run(cp)
+			if err != nil {
+				return err
+			}
+			fmt.Fprint(out, output)
+			return nil
 		},
 	}
 
@@ -106,7 +122,12 @@ func newShowCmd(out io.Writer) *cobra.Command {
 			if err != nil {
 				return err
 			}
-			return client.Run(cp)
+			output, err := client.Run(cp)
+			if err != nil {
+				return err
+			}
+			fmt.Fprint(out, output)
+			return nil
 		},
 	}
 

pkg/action/pull.go

@@ -18,10 +18,10 @@ package action
 
 import (
 	"fmt"
-	"io"
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"github.com/pkg/errors"
 	"github.com/spf13/pflag"
@@ -39,7 +39,6 @@ import (
 type Pull struct {
 	ChartPathOptions
 
-	Out      io.Writer       // TODO: refactor this out of pkg/action
 	Settings cli.EnvSettings // TODO: refactor this out of pkg/action
 
 	Devel       bool
@@ -55,10 +54,12 @@ func NewPull() *Pull {
 }
 
 // Run executes 'helm pull' against the given release.
-func (p *Pull) Run(chartRef string) error {
+func (p *Pull) Run(chartRef string) (string, error) {
+	var out strings.Builder
+
 	c := downloader.ChartDownloader{
 		HelmHome: p.Settings.Home,
-		Out:      p.Out,
+		Out:      &out,
 		Keyring:  p.Keyring,
 		Verify:   downloader.VerifyNever,
 		Getters:  getter.All(p.Settings),
@@ -79,7 +80,7 @@ func (p *Pull) Run(chartRef string) error {
 		var err error
 		dest, err = ioutil.TempDir("", "helm-")
 		if err != nil {
-			return errors.Wrap(err, "failed to untar")
+			return out.String(), errors.Wrap(err, "failed to untar")
 		}
 		defer os.RemoveAll(dest)
 	}
@@ -87,18 +88,18 @@ func (p *Pull) Run(chartRef string) error {
 	if p.RepoURL != "" {
 		chartURL, err := repo.FindChartInAuthRepoURL(p.RepoURL, p.Username, p.Password, chartRef, p.Version, p.CertFile, p.KeyFile, p.CaFile, getter.All(p.Settings))
 		if err != nil {
-			return err
+			return out.String(), err
 		}
 		chartRef = chartURL
 	}
 
 	saved, v, err := c.DownloadTo(chartRef, p.Version, dest)
 	if err != nil {
-		return err
+		return out.String(), err
 	}
 
 	if p.Verify {
-		fmt.Fprintf(p.Out, "Verification: %v\n", v)
+		fmt.Fprintf(&out, "Verification: %v\n", v)
 	}
 
 	// After verification, untar the chart into the requested directory.
@@ -109,16 +110,16 @@ func (p *Pull) Run(chartRef string) error {
 		}
 		if fi, err := os.Stat(ud); err != nil {
 			if err := os.MkdirAll(ud, 0755); err != nil {
-				return errors.Wrap(err, "failed to untar (mkdir)")
+				return out.String(), errors.Wrap(err, "failed to untar (mkdir)")
 			}
 
 		} else if !fi.IsDir() {
-			return errors.Errorf("failed to untar: %s is not a directory", ud)
+			return out.String(), errors.Errorf("failed to untar: %s is not a directory", ud)
 		}
 
-		return chartutil.ExpandFile(ud, saved)
+		return out.String(), chartutil.ExpandFile(ud, saved)
 	}
-	return nil
+	return out.String(), nil
 }
 
 func (p *Pull) AddFlags(f *pflag.FlagSet) {

pkg/action/show.go

@@ -18,7 +18,6 @@ package action
 
 import (
 	"fmt"
-	"io"
 	"strings"
 
 	"github.com/ghodss/yaml"
@@ -63,15 +62,13 @@ func ParseShowOutputFormat(s string) (out ShowOutputFormat, err error) {
 //
 // It provides the implementation of 'helm show' and its respective subcommands.
 type Show struct {
-	Out          io.Writer
 	OutputFormat ShowOutputFormat
 	ChartPathOptions
 }
 
 // NewShow creates a new Show object with the given configuration.
-func NewShow(out io.Writer, output ShowOutputFormat) *Show {
+func NewShow(output ShowOutputFormat) *Show {
 	return &Show{
-		Out:          out,
 		OutputFormat: output,
 	}
 }
@@ -81,42 +78,43 @@ func (s *Show) AddFlags(f *pflag.FlagSet) {
 }
 
 // Run executes 'helm show' against the given release.
-func (s *Show) Run(chartpath string) error {
+func (s *Show) Run(chartpath string) (string, error) {
+	var out strings.Builder
 	chrt, err := loader.Load(chartpath)
 	if err != nil {
-		return err
+		return "", err
 	}
 	cf, err := yaml.Marshal(chrt.Metadata)
 	if err != nil {
-		return err
+		return "", err
 	}
 
 	if s.OutputFormat == ShowChart || s.OutputFormat == ShowAll {
-		fmt.Fprintln(s.Out, string(cf))
+		fmt.Fprintf(&out, "%s\n", cf)
 	}
 
 	if (s.OutputFormat == ShowValues || s.OutputFormat == ShowAll) && chrt.Values != nil {
 		if s.OutputFormat == ShowAll {
-			fmt.Fprintln(s.Out, "---")
+			fmt.Fprintln(&out, "---")
 		}
 		b, err := yaml.Marshal(chrt.Values)
 		if err != nil {
-			return err
+			return "", err
 		}
-		fmt.Fprintln(s.Out, string(b))
+		fmt.Fprintf(&out, "%s\n", b)
 	}
 
 	if s.OutputFormat == ShowReadme || s.OutputFormat == ShowAll {
 		if s.OutputFormat == ShowAll {
-			fmt.Fprintln(s.Out, "---")
+			fmt.Fprintln(&out, "---")
 		}
 		readme := findReadme(chrt.Files)
 		if readme == nil {
-			return nil
+			return out.String(), nil
 		}
-		fmt.Fprintln(s.Out, string(readme.Data))
+		fmt.Fprintf(&out, "%s\n", readme.Data)
 	}
-	return nil
+	return out.String(), nil
 }
 
 func findReadme(files []*chart.File) (file *chart.File) {

pkg/action/show_test.go

@@ -17,18 +17,18 @@ limitations under the License.
 package action
 
 import (
-	"bytes"
 	"io/ioutil"
 	"strings"
 	"testing"
 )
 
 func TestShow(t *testing.T) {
-	b := bytes.NewBuffer(nil)
+	client := NewShow(ShowAll)
 
-	client := NewShow(b, ShowAll)
-
-	client.Run("../../cmd/helm/testdata/testcharts/alpine")
+	output, err := client.Run("../../cmd/helm/testdata/testcharts/alpine")
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	// Load the data from the textfixture directly.
 	cdata, err := ioutil.ReadFile("../../cmd/helm/testdata/testcharts/alpine/Chart.yaml")
@@ -43,7 +43,7 @@ func TestShow(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	parts := strings.SplitN(b.String(), "---", 3)
+	parts := strings.SplitN(output, "---", 3)
 	if len(parts) != 3 {
 		t.Fatalf("Expected 2 parts, got %d", len(parts))
 	}
@@ -64,11 +64,13 @@ func TestShow(t *testing.T) {
 	}
 
 	// Regression tests for missing values. See issue #1024.
-	b.Reset()
 	client.OutputFormat = ShowValues
-	client.Run("../../cmd/helm/testdata/testcharts/novals")
+	output, err = client.Run("../../cmd/helm/testdata/testcharts/novals")
+	if err != nil {
+		t.Fatal(err)
+	}
 
-	if b.Len() != 0 {
-		t.Errorf("expected empty values buffer, got %q", b.String())
+	if len(output) != 0 {
+		t.Errorf("expected empty values buffer, got %s", output)
 	}
 }