diff --git a/cmd/helm/get_all_test.go b/cmd/helm/get_all_test.go index 0b026fca4..278b4015b 100644 --- a/cmd/helm/get_all_test.go +++ b/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", diff --git a/cmd/helm/get_hooks_test.go b/cmd/helm/get_hooks_test.go index f843f7d59..bdc51d29f 100644 --- a/cmd/helm/get_hooks_test.go +++ b/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", diff --git a/cmd/helm/get_manifest_test.go b/cmd/helm/get_manifest_test.go index be54d4a5b..69099eae5 100644 --- a/cmd/helm/get_manifest_test.go +++ b/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", diff --git a/cmd/helm/get_notes_test.go b/cmd/helm/get_notes_test.go index a3ddea9a7..876e270fb 100644 --- a/cmd/helm/get_notes_test.go +++ b/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", diff --git a/cmd/helm/get_values_test.go b/cmd/helm/get_values_test.go index da3cc9e39..632e3393e 100644 --- a/cmd/helm/get_values_test.go +++ b/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) } diff --git a/cmd/helm/helm_test.go b/cmd/helm/helm_test.go index a08720e7a..70b9db76e 100644 --- a/cmd/helm/helm_test.go +++ b/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 { - if err := storage.Create(rel); err != nil { + for i := range tt.rels { + // 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 { - store.Create(rel) + for i := range tt.rels { + // 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 diff --git a/cmd/helm/history_test.go b/cmd/helm/history_test.go index 3e750cefc..73185c2c6 100644 --- a/cmd/helm/history_test.go +++ b/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), }, diff --git a/cmd/helm/list_test.go b/cmd/helm/list_test.go index b5833fd72..b6506499e 100644 --- a/cmd/helm/list_test.go +++ b/cmd/helm/list_test.go @@ -40,7 +40,7 @@ func TestListCmd(t *testing.T) { }, } - releaseFixture := []*release.Release{ + releaseFixture := []release.Release{ { Name: "starlord", Version: 1, diff --git a/cmd/helm/rollback_test.go b/cmd/helm/rollback_test.go index fdc627b5f..8c6b7e899 100644 --- a/cmd/helm/rollback_test.go +++ b/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}, diff --git a/cmd/helm/status_test.go b/cmd/helm/status_test.go index 91a008e5a..f17048fe6 100644 --- a/cmd/helm/status_test.go +++ b/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, diff --git a/cmd/helm/uninstall_test.go b/cmd/helm/uninstall_test.go index a34934077..d22da00ef 100644 --- a/cmd/helm/uninstall_test.go +++ b/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", diff --git a/cmd/helm/upgrade_test.go b/cmd/helm/upgrade_test.go index bd1ccec35..a1b483ee7 100644 --- a/cmd/helm/upgrade_test.go +++ b/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}) } diff --git a/pkg/release/mock.go b/pkg/release/mock.go index 3abbd574b..24a62fcd3 100644 --- a/pkg/release/mock.go +++ b/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,