test(helm): remove usage of pointers in tests for /cmd/helm

Before this commit, test data was modified by test code, affecting
subsequent tests. Specifically, when working on a bug in the memory
driver, we noticed that whatever test case (no matter which one) was
placed first in '/cmd/helm/rollback_test.go' would always pass while the
others would fail.

Signed-off-by: Latiif <abdullatif.alshriaf@combitech.se>

cmd/helm/get_all_test.go

@@ -27,12 +27,12 @@ func TestGetCmd(t *testing.T) {
 		name:   "get all with a release",
 		cmd:    "get all thomas-guide",
 		golden: "output/get-release.txt",
-		rels:   []*release.Release{release.Mock(&release.MockReleaseOptions{Name: "thomas-guide"})},
+		rels:   []release.Release{release.Mock(&release.MockReleaseOptions{Name: "thomas-guide"})},
 	}, {
 		name:   "get all with a formatted release",
 		cmd:    "get all elevated-turkey --template {{.Release.Chart.Metadata.Version}}",
 		golden: "output/get-release-template.txt",
-		rels:   []*release.Release{release.Mock(&release.MockReleaseOptions{Name: "elevated-turkey"})},
+		rels:   []release.Release{release.Mock(&release.MockReleaseOptions{Name: "elevated-turkey"})},
 	}, {
 		name:      "get all requires release name arg",
 		cmd:       "get all",

cmd/helm/get_hooks_test.go

@@ -27,7 +27,7 @@ func TestGetHooks(t *testing.T) {
 		name:   "get hooks with release",
 		cmd:    "get hooks aeneas",
 		golden: "output/get-hooks.txt",
-		rels:   []*release.Release{release.Mock(&release.MockReleaseOptions{Name: "aeneas"})},
+		rels:   []release.Release{release.Mock(&release.MockReleaseOptions{Name: "aeneas"})},
 	}, {
 		name:      "get hooks without args",
 		cmd:       "get hooks",

cmd/helm/get_manifest_test.go

@@ -27,7 +27,7 @@ func TestGetManifest(t *testing.T) {
 		name:   "get manifest with release",
 		cmd:    "get manifest juno",
 		golden: "output/get-manifest.txt",
-		rels:   []*release.Release{release.Mock(&release.MockReleaseOptions{Name: "juno"})},
+		rels:   []release.Release{release.Mock(&release.MockReleaseOptions{Name: "juno"})},
 	}, {
 		name:      "get manifest without args",
 		cmd:       "get manifest",

cmd/helm/get_notes_test.go

@@ -27,7 +27,7 @@ func TestGetNotesCmd(t *testing.T) {
 		name:   "get notes of a deployed release",
 		cmd:    "get notes the-limerick",
 		golden: "output/get-notes.txt",
-		rels:   []*release.Release{release.Mock(&release.MockReleaseOptions{Name: "the-limerick"})},
+		rels:   []release.Release{release.Mock(&release.MockReleaseOptions{Name: "the-limerick"})},
 	}, {
 		name:      "get notes without args",
 		cmd:       "get notes",

cmd/helm/get_values_test.go

@@ -27,28 +27,28 @@ func TestGetValuesCmd(t *testing.T) {
 		name:   "get values with a release",
 		cmd:    "get values thomas-guide",
 		golden: "output/get-values.txt",
-		rels:   []*release.Release{release.Mock(&release.MockReleaseOptions{Name: "thomas-guide"})},
+		rels:   []release.Release{release.Mock(&release.MockReleaseOptions{Name: "thomas-guide"})},
 	}, {
 		name:      "get values requires release name arg",
 		cmd:       "get values",
 		golden:    "output/get-values-args.txt",
-		rels:      []*release.Release{release.Mock(&release.MockReleaseOptions{Name: "thomas-guide"})},
+		rels:      []release.Release{release.Mock(&release.MockReleaseOptions{Name: "thomas-guide"})},
 		wantError: true,
 	}, {
 		name:   "get values thomas-guide (all)",
 		cmd:    "get values thomas-guide --all",
 		golden: "output/get-values-all.txt",
-		rels:   []*release.Release{release.Mock(&release.MockReleaseOptions{Name: "thomas-guide"})},
+		rels:   []release.Release{release.Mock(&release.MockReleaseOptions{Name: "thomas-guide"})},
 	}, {
 		name:   "get values to json",
 		cmd:    "get values thomas-guide --output json",
 		golden: "output/values.json",
-		rels:   []*release.Release{release.Mock(&release.MockReleaseOptions{Name: "thomas-guide"})},
+		rels:   []release.Release{release.Mock(&release.MockReleaseOptions{Name: "thomas-guide"})},
 	}, {
 		name:   "get values to yaml",
 		cmd:    "get values thomas-guide --output yaml",
 		golden: "output/values.yaml",
-		rels:   []*release.Release{release.Mock(&release.MockReleaseOptions{Name: "thomas-guide"})},
+		rels:   []release.Release{release.Mock(&release.MockReleaseOptions{Name: "thomas-guide"})},
 	}}
 	runTestCmd(t, tests)
 }

cmd/helm/helm_test.go

@@ -52,8 +52,11 @@ func runTestCmd(t *testing.T, tests []cmdTestCase) {
 				defer resetEnv()()
 
 				storage := storageFixture()
-				for _, rel := range tt.rels {
+				for i := range tt.rels {
-					if err := storage.Create(rel); err != nil {
+					// Shouldn't do `for _, rel := range tt.rels`, because we pass the _address_ of `rel` to `storage.Create`.
+					// See: https://github.com/golang/go/wiki/CommonMistakes#using-reference-to-loop-iterator-variable
+					rel := tt.rels[i]
+					if err := storage.Create(&rel); err != nil {
 						t.Fatal(err)
 					}
 				}
@@ -77,8 +80,11 @@ func runTestActionCmd(t *testing.T, tests []cmdTestCase) {
 			defer resetEnv()()
 
 			store := storageFixture()
-			for _, rel := range tt.rels {
+			for i := range tt.rels {
-				store.Create(rel)
+				// Shouldn't do `for _, rel := range tt.rels`, because we pass the _address_ of `rel` to `store.Create`.
+				// See: https://github.com/golang/go/wiki/CommonMistakes#using-reference-to-loop-iterator-variable
+				rel := tt.rels[i]
+				store.Create(&rel)
 			}
 			_, out, err := executeActionCommandC(store, tt.cmd)
 			if (err != nil) != tt.wantError {
@@ -125,7 +131,7 @@ type cmdTestCase struct {
 	golden    string
 	wantError bool
 	// Rels are the available releases at the start of the test.
-	rels []*release.Release
+	rels []release.Release
 	// Number of repeats (in case a feature was previously flaky and the test checks
 	// it's now stably producing identical results). 0 means test is run exactly once.
 	repeat int

cmd/helm/history_test.go

@@ -23,7 +23,7 @@ import (
 )
 
 func TestHistoryCmd(t *testing.T) {
-	mk := func(name string, vers int, status release.Status) *release.Release {
+	mk := func(name string, vers int, status release.Status) release.Release {
 		return release.Mock(&release.MockReleaseOptions{
 			Name:    name,
 			Version: vers,
@@ -34,7 +34,7 @@ func TestHistoryCmd(t *testing.T) {
 	tests := []cmdTestCase{{
 		name: "get history for release",
 		cmd:  "history angry-bird",
-		rels: []*release.Release{
+		rels: []release.Release{
 			mk("angry-bird", 4, release.StatusDeployed),
 			mk("angry-bird", 3, release.StatusSuperseded),
 			mk("angry-bird", 2, release.StatusSuperseded),
@@ -44,7 +44,7 @@ func TestHistoryCmd(t *testing.T) {
 	}, {
 		name: "get history with max limit set",
 		cmd:  "history angry-bird --max 2",
-		rels: []*release.Release{
+		rels: []release.Release{
 			mk("angry-bird", 4, release.StatusDeployed),
 			mk("angry-bird", 3, release.StatusSuperseded),
 		},
@@ -52,7 +52,7 @@ func TestHistoryCmd(t *testing.T) {
 	}, {
 		name: "get history with yaml output format",
 		cmd:  "history angry-bird --output yaml",
-		rels: []*release.Release{
+		rels: []release.Release{
 			mk("angry-bird", 4, release.StatusDeployed),
 			mk("angry-bird", 3, release.StatusSuperseded),
 		},
@@ -60,7 +60,7 @@ func TestHistoryCmd(t *testing.T) {
 	}, {
 		name: "get history with json output format",
 		cmd:  "history angry-bird --output json",
-		rels: []*release.Release{
+		rels: []release.Release{
 			mk("angry-bird", 4, release.StatusDeployed),
 			mk("angry-bird", 3, release.StatusSuperseded),
 		},

cmd/helm/list_test.go

@@ -40,7 +40,7 @@ func TestListCmd(t *testing.T) {
 		},
 	}
 
-	releaseFixture := []*release.Release{
+	releaseFixture := []release.Release{
 		{
 			Name:      "starlord",
 			Version:   1,

cmd/helm/rollback_test.go

@@ -24,7 +24,7 @@ import (
 )
 
 func TestRollbackCmd(t *testing.T) {
-	rels := []*release.Release{
+	rels := []release.Release{
 		{
 			Name:    "funny-honey",
 			Info:    &release.Info{Status: release.StatusSuperseded},

cmd/helm/status_test.go

@@ -26,9 +26,9 @@ import (
 )
 
 func TestStatusCmd(t *testing.T) {
-	releasesMockWithStatus := func(info *release.Info, hooks ...*release.Hook) []*release.Release {
+	releasesMockWithStatus := func(info *release.Info, hooks ...*release.Hook) []release.Release {
 		info.LastDeployed = helmtime.Unix(1452902400, 0).UTC()
-		return []*release.Release{{
+		return []release.Release{{
 			Name:      "flummoxed-chickadee",
 			Namespace: "default",
 			Info:      info,

cmd/helm/uninstall_test.go

@@ -28,13 +28,13 @@ func TestUninstall(t *testing.T) {
 			name:   "basic uninstall",
 			cmd:    "uninstall aeneas",
 			golden: "output/uninstall.txt",
-			rels:   []*release.Release{release.Mock(&release.MockReleaseOptions{Name: "aeneas"})},
+			rels:   []release.Release{release.Mock(&release.MockReleaseOptions{Name: "aeneas"})},
 		},
 		{
 			name:   "multiple uninstall",
 			cmd:    "uninstall aeneas aeneas2",
 			golden: "output/uninstall-multiple.txt",
-			rels: []*release.Release{
+			rels: []release.Release{
 				release.Mock(&release.MockReleaseOptions{Name: "aeneas"}),
 				release.Mock(&release.MockReleaseOptions{Name: "aeneas2"}),
 			},
@@ -43,19 +43,19 @@ func TestUninstall(t *testing.T) {
 			name:   "uninstall with timeout",
 			cmd:    "uninstall aeneas --timeout 120s",
 			golden: "output/uninstall-timeout.txt",
-			rels:   []*release.Release{release.Mock(&release.MockReleaseOptions{Name: "aeneas"})},
+			rels:   []release.Release{release.Mock(&release.MockReleaseOptions{Name: "aeneas"})},
 		},
 		{
 			name:   "uninstall without hooks",
 			cmd:    "uninstall aeneas --no-hooks",
 			golden: "output/uninstall-no-hooks.txt",
-			rels:   []*release.Release{release.Mock(&release.MockReleaseOptions{Name: "aeneas"})},
+			rels:   []release.Release{release.Mock(&release.MockReleaseOptions{Name: "aeneas"})},
 		},
 		{
 			name:   "keep history",
 			cmd:    "uninstall aeneas --keep-history",
 			golden: "output/uninstall-keep-history.txt",
-			rels:   []*release.Release{release.Mock(&release.MockReleaseOptions{Name: "aeneas"})},
+			rels:   []release.Release{release.Mock(&release.MockReleaseOptions{Name: "aeneas"})},
 		},
 		{
 			name:      "uninstall without release",

cmd/helm/upgrade_test.go

@@ -79,7 +79,7 @@ func TestUpgradeCmd(t *testing.T) {
 	missingDepsPath := "testdata/testcharts/chart-missing-deps"
 	badDepsPath := "testdata/testcharts/chart-bad-requirements"
 
-	relMock := func(n string, v int, ch *chart.Chart) *release.Release {
+	relMock := func(n string, v int, ch *chart.Chart) release.Release {
 		return release.Mock(&release.MockReleaseOptions{Name: n, Version: v, Chart: ch})
 	}
 
@@ -88,43 +88,43 @@ func TestUpgradeCmd(t *testing.T) {
 			name:   "upgrade a release",
 			cmd:    fmt.Sprintf("upgrade funny-bunny '%s'", chartPath),
 			golden: "output/upgrade.txt",
-			rels:   []*release.Release{relMock("funny-bunny", 2, ch)},
+			rels:   []release.Release{relMock("funny-bunny", 2, ch)},
 		},
 		{
 			name:   "upgrade a release with timeout",
 			cmd:    fmt.Sprintf("upgrade funny-bunny --timeout 120s '%s'", chartPath),
 			golden: "output/upgrade-with-timeout.txt",
-			rels:   []*release.Release{relMock("funny-bunny", 3, ch2)},
+			rels:   []release.Release{relMock("funny-bunny", 3, ch2)},
 		},
 		{
 			name:   "upgrade a release with --reset-values",
 			cmd:    fmt.Sprintf("upgrade funny-bunny --reset-values '%s'", chartPath),
 			golden: "output/upgrade-with-reset-values.txt",
-			rels:   []*release.Release{relMock("funny-bunny", 4, ch2)},
+			rels:   []release.Release{relMock("funny-bunny", 4, ch2)},
 		},
 		{
 			name:   "upgrade a release with --reuse-values",
 			cmd:    fmt.Sprintf("upgrade funny-bunny --reuse-values '%s'", chartPath),
 			golden: "output/upgrade-with-reset-values2.txt",
-			rels:   []*release.Release{relMock("funny-bunny", 5, ch2)},
+			rels:   []release.Release{relMock("funny-bunny", 5, ch2)},
 		},
 		{
 			name:   "install a release with 'upgrade --install'",
 			cmd:    fmt.Sprintf("upgrade zany-bunny -i '%s'", chartPath),
 			golden: "output/upgrade-with-install.txt",
-			rels:   []*release.Release{relMock("zany-bunny", 1, ch)},
+			rels:   []release.Release{relMock("zany-bunny", 1, ch)},
 		},
 		{
 			name:   "install a release with 'upgrade --install' and timeout",
 			cmd:    fmt.Sprintf("upgrade crazy-bunny -i --timeout 120s '%s'", chartPath),
 			golden: "output/upgrade-with-install-timeout.txt",
-			rels:   []*release.Release{relMock("crazy-bunny", 1, ch)},
+			rels:   []release.Release{relMock("crazy-bunny", 1, ch)},
 		},
 		{
 			name:   "upgrade a release with wait",
 			cmd:    fmt.Sprintf("upgrade crazy-bunny --wait '%s'", chartPath),
 			golden: "output/upgrade-with-wait.txt",
-			rels:   []*release.Release{relMock("crazy-bunny", 2, ch2)},
+			rels:   []release.Release{relMock("crazy-bunny", 2, ch2)},
 		},
 		{
 			name:      "upgrade a release with missing dependencies",
@@ -150,7 +150,8 @@ func TestUpgradeWithValue(t *testing.T) {
 
 	store := storageFixture()
 
-	store.Create(relMock(releaseName, 3, ch))
+	releaseMock := relMock(releaseName, 3, ch)
+	store.Create(&releaseMock)
 
 	cmd := fmt.Sprintf("upgrade %s --set favoriteDrink=tea '%s'", releaseName, chartPath)
 	_, _, err := executeActionCommandC(store, cmd)
@@ -177,7 +178,8 @@ func TestUpgradeWithStringValue(t *testing.T) {
 
 	store := storageFixture()
 
-	store.Create(relMock(releaseName, 3, ch))
+	releaseMock := relMock(releaseName, 3, ch)
+	store.Create(&releaseMock)
 
 	cmd := fmt.Sprintf("upgrade %s --set-string favoriteDrink=coffee '%s'", releaseName, chartPath)
 	_, _, err := executeActionCommandC(store, cmd)
@@ -205,7 +207,8 @@ func TestUpgradeWithValuesFile(t *testing.T) {
 
 	store := storageFixture()
 
-	store.Create(relMock(releaseName, 3, ch))
+	releaseMock := relMock(releaseName, 3, ch)
+	store.Create(&releaseMock)
 
 	cmd := fmt.Sprintf("upgrade %s --values testdata/testcharts/upgradetest/values.yaml '%s'", releaseName, chartPath)
 	_, _, err := executeActionCommandC(store, cmd)
@@ -224,7 +227,7 @@ func TestUpgradeWithValuesFile(t *testing.T) {
 
 }
 
-func prepareMockRelease(releaseName string, t *testing.T) (func(n string, v int, ch *chart.Chart) *release.Release, *chart.Chart, string) {
+func prepareMockRelease(releaseName string, t *testing.T) (func(n string, v int, ch *chart.Chart) release.Release, *chart.Chart, string) {
 	tmpChart := ensure.TempDir(t)
 	configmapData, err := ioutil.ReadFile("testdata/testcharts/upgradetest/templates/configmap.yaml")
 	if err != nil {
@@ -252,7 +255,7 @@ func prepareMockRelease(releaseName string, t *testing.T) (func(n string, v int,
 		Chart: ch,
 	})
 
-	relMock := func(n string, v int, ch *chart.Chart) *release.Release {
+	relMock := func(n string, v int, ch *chart.Chart) release.Release {
 		return release.Mock(&release.MockReleaseOptions{Name: n, Version: v, Chart: ch})
 	}
 

pkg/release/mock.go

@@ -48,7 +48,7 @@ type MockReleaseOptions struct {
 }
 
 // Mock creates a mock release object based on options set by MockReleaseOptions. This function should typically not be used outside of testing.
-func Mock(opts *MockReleaseOptions) *Release {
+func Mock(opts *MockReleaseOptions) Release {
 	date := time.Unix(242085845, 0).UTC()
 
 	name := opts.Name
@@ -93,7 +93,7 @@ func Mock(opts *MockReleaseOptions) *Release {
 		Notes:         "Some mock release notes!",
 	}
 
-	return &Release{
+	return Release{
 		Name:      name,
 		Info:      info,
 		Chart:     ch,