From 2a462aef2ddc9be3cb7cdb8145300e7bee4a2629 Mon Sep 17 00:00:00 2001
From: Simon Alling <alling.simon@gmail.com>
Date: Tue, 1 Oct 2019 09:25:30 +0200
Subject: [PATCH] fix(pkg/chartutil): add tests according to feedback

The seemingly redundant `return filename, err` line is related to how
the name `err` is used throughout the function: there is a "global" (to
the function) `err` variable, as well as several locally block-scoped
ones. It took me hours to understand why my code did not work without
that line, but I decided not to clean up the `err` code in this commit.

Signed-off-by: Simon Alling <alling.simon@gmail.com>
---
 pkg/chartutil/save.go      |  1 +
 pkg/chartutil/save_test.go | 26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/pkg/chartutil/save.go b/pkg/chartutil/save.go
index cf2aad000..ca6eeaa19 100644
--- a/pkg/chartutil/save.go
+++ b/pkg/chartutil/save.go
@@ -133,6 +133,7 @@ func Save(c *chart.Chart, outDir string) (string, error) {
 
 	if err := writeTarContents(twriter, c, ""); err != nil {
 		rollback = true
+		return filename, err
 	}
 	return filename, err
 }
diff --git a/pkg/chartutil/save_test.go b/pkg/chartutil/save_test.go
index 377e89dcb..8941d0368 100644
--- a/pkg/chartutil/save_test.go
+++ b/pkg/chartutil/save_test.go
@@ -17,10 +17,12 @@ limitations under the License.
 package chartutil
 
 import (
+	"bytes"
 	"io/ioutil"
 	"os"
 	"path"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"testing"
 
@@ -46,7 +48,9 @@ func TestSave(t *testing.T) {
 				Files: []*chart.File{
 					{Name: "scheherazade/shahryar.txt", Data: []byte("1,001 Nights")},
 				},
+				Schema: []byte("{\n  \"title\": \"Values\"\n}"),
 			}
+			chartWithInvalidJSON := withSchema(*c, []byte("{"))
 
 			where, err := Save(c, dest)
 			if err != nil {
@@ -69,10 +73,32 @@ func TestSave(t *testing.T) {
 			if len(c2.Files) != 1 || c2.Files[0].Name != "scheherazade/shahryar.txt" {
 				t.Fatal("Files data did not match")
 			}
+
+			if !bytes.Equal(c.Schema, c2.Schema) {
+				indentation := 4
+				formattedExpected := Indent(indentation, string(c.Schema))
+				formattedActual := Indent(indentation, string(c2.Schema))
+				t.Fatalf("Schema data did not match.\nExpected:\n%s\nActual:\n%s", formattedExpected, formattedActual)
+			}
+			if _, err := Save(&chartWithInvalidJSON, dest); err == nil {
+				t.Fatalf("Invalid JSON was not caught while saving chart")
+			}
 		})
 	}
 }
 
+// Creates a copy with a different schema; does not modify anything.
+func withSchema(chart chart.Chart, schema []byte) chart.Chart {
+	chart.Schema = schema
+	return chart
+}
+
+func Indent(n int, text string) string {
+	startOfLine := regexp.MustCompile(`(?m)^`)
+	indentation := strings.Repeat(" ", n)
+	return startOfLine.ReplaceAllLiteralString(text, indentation)
+}
+
 func TestSaveDir(t *testing.T) {
 	tmp, err := ioutil.TempDir("", "helm-")
 	if err != nil {