internal/helm: tweak and test chart build summary

This makes the string less verbose and deals with the safe handling
of some edge-case build states.

Signed-off-by: Hidde Beydals <hello@hidde.co>

Author: hiddeco

internal/helm/chart/builder.go

@@ -138,8 +138,8 @@ type Build struct {
 
 // Summary returns a human-readable summary of the Build.
 func (b *Build) Summary() string {
-	if b == nil {
-		return "no chart build"
+	if b == nil || b.Name == "" || b.Version == "" {
+		return "No chart build."
 	}
 
 	var s strings.Builder
@@ -148,25 +148,26 @@ func (b *Build) Summary() string {
 	if b.Packaged {
 		action = "Packaged"
 	}
-	s.WriteString(fmt.Sprintf("%s '%s' chart with version '%s'.", action, b.Name, b.Version))
+	s.WriteString(fmt.Sprintf("%s '%s' chart with version '%s'", action, b.Name, b.Version))
 
-	if b.Packaged && b.ResolvedDependencies > 0 {
-		s.WriteString(fmt.Sprintf(" Resolved %d dependencies before packaging.", b.ResolvedDependencies))
+	if b.Packaged && len(b.ValueFiles) > 0 {
+		s.WriteString(fmt.Sprintf(", with merged value files %v", b.ValueFiles))
 	}
 
-	if len(b.ValueFiles) > 0 {
-		s.WriteString(fmt.Sprintf(" Merged %v value files into default chart values.", b.ValueFiles))
+	if b.Packaged && b.ResolvedDependencies > 0 {
+		s.WriteString(fmt.Sprintf(", resolving %d dependencies before packaging", b.ResolvedDependencies))
 	}
 
+	s.WriteString(".")
 	return s.String()
 }
 
 // String returns the Path of the Build.
 func (b *Build) String() string {
-	if b != nil {
-		return b.Path
+	if b == nil {
+		return ""
 	}
-	return ""
+	return b.Path
 }
 
 // packageToPath attempts to package the given chart to the out filepath.

internal/helm/chart/builder_test.go

@@ -25,8 +25,90 @@ import (
 
 	. "github.com/onsi/gomega"
 	"helm.sh/helm/v3/pkg/chart/loader"
+	"helm.sh/helm/v3/pkg/chartutil"
 )
 
+func TestBuildOptions_GetValueFiles(t *testing.T) {
+	tests := []struct {
+		name       string
+		valueFiles []string
+		want       []string
+	}{
+		{
+			name:       "Default values.yaml",
+			valueFiles: []string{chartutil.ValuesfileName},
+			want:       nil,
+		},
+		{
+			name:       "Value files",
+			valueFiles: []string{chartutil.ValuesfileName, "foo.yaml"},
+			want:       []string{chartutil.ValuesfileName, "foo.yaml"},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			o := BuildOptions{ValueFiles: tt.valueFiles}
+			g.Expect(o.GetValueFiles()).To(Equal(tt.want))
+		})
+	}
+}
+
+func TestChartBuildResult_Summary(t *testing.T) {
+	tests := []struct {
+		name  string
+		build *Build
+		want  string
+	}{
+		{
+			name: "Simple",
+			build: &Build{
+				Name:    "chart",
+				Version: "1.2.3-rc.1+bd6bf40",
+			},
+			want: "Fetched 'chart' chart with version '1.2.3-rc.1+bd6bf40'.",
+		},
+		{
+			name: "With value files",
+			build: &Build{
+				Name:       "chart",
+				Version:    "arbitrary-version",
+				Packaged:   true,
+				ValueFiles: []string{"a.yaml", "b.yaml"},
+			},
+			want: "Packaged 'chart' chart with version 'arbitrary-version', with merged value files [a.yaml b.yaml].",
+		},
+		{
+			name: "With dependencies",
+			build: &Build{
+				Name:                 "chart",
+				Version:              "arbitrary-version",
+				Packaged:             true,
+				ResolvedDependencies: 5,
+			},
+			want: "Packaged 'chart' chart with version 'arbitrary-version', resolving 5 dependencies before packaging.",
+		},
+		{
+			name:  "Empty build",
+			build: &Build{},
+			want:  "No chart build.",
+		},
+		{
+			name:  "Nil build",
+			build: nil,
+			want:  "No chart build.",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			g.Expect(tt.build.Summary()).To(Equal(tt.want))
+		})
+	}
+}
+
 func TestChartBuildResult_String(t *testing.T) {
 	g := NewWithT(t)
 

internal/helm/repository/chart_repository.go

@@ -287,8 +287,6 @@ func (r *ChartRepository) CacheIndex() (string, error) {
 // LoadFromCache if it does not HasIndex.
 // If it not HasCacheFile, a cache attempt is made using CacheIndex
 // before continuing to load.
-// It returns a boolean indicating if it cached the index before
-// loading, or an error.
 func (r *ChartRepository) StrategicallyLoadIndex() (err error) {
 	if r.HasIndex() {
 		return