From 26da3dcc1b728b0f9220e5d75c0ea0e4842d31a5 Mon Sep 17 00:00:00 2001 From: Simon Alling Date: Thu, 13 Jan 2022 16:00:41 +0100 Subject: [PATCH] Add tests for `helm test` (ReleaseTesting) Signed-off-by: Simon Alling --- cmd/helm/helm.go | 1 + cmd/helm/helm_test.go | 18 ++-- cmd/helm/release_testing_test.go | 88 +++++++++++++++++++ cmd/helm/testdata/output/test-with-logs.txt | 14 +++ .../testdata/output/test-without-logs.txt | 11 +++ internal/test/test.go | 67 ++++++++++++++ pkg/action/action.go | 32 +++++++ pkg/action/action_test.go | 2 + pkg/action/release_testing.go | 17 +--- pkg/action/release_testing_test.go | 72 +++++++++++++++ pkg/release/hook.go | 3 + 11 files changed, 306 insertions(+), 19 deletions(-) create mode 100644 cmd/helm/testdata/output/test-with-logs.txt create mode 100644 cmd/helm/testdata/output/test-without-logs.txt create mode 100644 pkg/action/release_testing_test.go diff --git a/cmd/helm/helm.go b/cmd/helm/helm.go index 15b0d5c76..643d9c6ec 100644 --- a/cmd/helm/helm.go +++ b/cmd/helm/helm.go @@ -75,6 +75,7 @@ func main() { if err := actionConfig.Init(settings.RESTClientGetter(), settings.Namespace(), helmDriver, debug); err != nil { log.Fatal(err) } + actionConfig.HookLogGetter = actionConfig.GetHookLogFromRealCluster if helmDriver == "memory" { loadReleasesInMemory(actionConfig) } diff --git a/cmd/helm/helm_test.go b/cmd/helm/helm_test.go index 5e59c41ed..769f4e72f 100644 --- a/cmd/helm/helm_test.go +++ b/cmd/helm/helm_test.go @@ -46,6 +46,13 @@ func init() { } func runTestCmd(t *testing.T, tests []cmdTestCase) { + t.Helper() + runTestCmdWithCustomAssertion(t, tests, func(actualOutput, expectedFilename string) { + test.AssertGoldenString(t, actualOutput, expectedFilename) + }) +} + +func runTestCmdWithCustomAssertion(t *testing.T, tests []cmdTestCase, assertion func(actualOutput, expectedFilename string)) { t.Helper() for _, tt := range tests { for i := 0; i <= tt.repeat; i++ { @@ -64,7 +71,7 @@ func runTestCmd(t *testing.T, tests []cmdTestCase) { t.Errorf("expected error, got '%v'", err) } if tt.golden != "" { - test.AssertGoldenString(t, out, tt.golden) + assertion(out, tt.golden) } }) } @@ -109,10 +116,11 @@ func executeActionCommandStdinC(store *storage.Storage, in *os.File, cmd string) buf := new(bytes.Buffer) actionConfig := &action.Configuration{ - Releases: store, - KubeClient: &kubefake.PrintingKubeClient{Out: ioutil.Discard}, - Capabilities: chartutil.DefaultCapabilities, - Log: func(format string, v ...interface{}) {}, + Releases: store, + KubeClient: &kubefake.PrintingKubeClient{Out: ioutil.Discard}, + Capabilities: chartutil.DefaultCapabilities, + Log: func(format string, v ...interface{}) {}, + HookLogGetter: test.GetMockedHookLog, } root, err := newRootCmd(actionConfig, buf, args) diff --git a/cmd/helm/release_testing_test.go b/cmd/helm/release_testing_test.go index 680a9bd3e..4ee81945e 100644 --- a/cmd/helm/release_testing_test.go +++ b/cmd/helm/release_testing_test.go @@ -17,9 +17,97 @@ limitations under the License. package main import ( + "fmt" + "regexp" "testing" + "time" + + "helm.sh/helm/v3/internal/test" + "helm.sh/helm/v3/pkg/release" + helmtime "helm.sh/helm/v3/pkg/time" +) + +const ( + tableLinePattern = `^Last (Completed|Started):\s+(.+)$` + verbPosition = 1 // in the line patterns + timestampPosition = 2 // in the line patterns ) +type outputFormat struct { + linePattern regexp.Regexp + checkTime func(raw string) error +} + +func TestReleaseTesting(t *testing.T) { + mockReleases := []*release.Release{ + createMockRelease(), + } + + tableOutput := outputFormat{ + linePattern: *regexp.MustCompile(tableLinePattern), + checkTime: func(raw string) error { + _, err := helmtime.Parse(time.ANSIC, raw) // Layout/format must be the one actually used in the command output. + return err + }, + } + + tests := []cmdTestCase{ + { + name: "test without logs", + cmd: "test doge", + golden: "output/test-without-logs.txt", + rels: mockReleases, + }, + { + name: "test with logs", + cmd: "test doge --logs", + golden: "output/test-with-logs.txt", + rels: mockReleases, + }, + } + + runTestCmdWithCustomAssertion(t, tests, test.AssertGoldenStringWithCustomLineValidation(t, checkLineAs(tableOutput))) +} + +func checkLineAs(out outputFormat) func(expected, actual string) (bool, error) { + return func(expected, actual string) (bool, error) { + expectedMatch := out.linePattern.FindStringSubmatch(expected) + if expectedMatch != nil { + maybeTimestamp := expectedMatch[timestampPosition] + if out.checkTime(maybeTimestamp) == nil { + // This line requires special treatment. + actualMatch := out.linePattern.FindStringSubmatch(actual) + if actualMatch == nil { + return true, fmt.Errorf("expected to match %v", out.linePattern) + } + expectedVerb := expectedMatch[verbPosition] + actualVerb := actualMatch[verbPosition] + if actualVerb != expectedVerb { + return true, fmt.Errorf("expected '%s', but found '%s'", expectedVerb, actualVerb) + } + actualTimestamp := actualMatch[timestampPosition] + if err := out.checkTime(actualTimestamp); err != nil { + return true, fmt.Errorf("expected timestamp of same format, but found '%s' (%s)", actualTimestamp, err.Error()) + } + return true, nil // The actual line was identical to the expected one, modulo the point in time represented by the timestamp. + } + } + // This line does not require special treatment. + return false, nil + } +} + +func createMockRelease() *release.Release { + rel := release.Mock(&release.MockReleaseOptions{Name: "doge"}) + rel.Hooks[0] = &release.Hook{ + Name: "doge-test-pod", + Kind: "Pod", + Path: "doge-test-pod", + Events: []release.HookEvent{release.HookTest}, + } + return rel +} + func TestReleaseTestingCompletion(t *testing.T) { checkReleaseCompletion(t, "test", false) } diff --git a/cmd/helm/testdata/output/test-with-logs.txt b/cmd/helm/testdata/output/test-with-logs.txt new file mode 100644 index 000000000..33a900a3c --- /dev/null +++ b/cmd/helm/testdata/output/test-with-logs.txt @@ -0,0 +1,14 @@ +NAME: doge +LAST DEPLOYED: Fri Sep 2 22:04:05 1977 +NAMESPACE: default +STATUS: deployed +REVISION: 1 +TEST SUITE: doge-test-pod +Last Started: Fri Apr 16 13:37:00 2021 +Last Completed: Fri Apr 16 14:00:00 2021 +Phase: Succeeded +NOTES: +Some mock release notes! + +POD LOGS: doge-test-pod +example test pod log output diff --git a/cmd/helm/testdata/output/test-without-logs.txt b/cmd/helm/testdata/output/test-without-logs.txt new file mode 100644 index 000000000..f9a6f4914 --- /dev/null +++ b/cmd/helm/testdata/output/test-without-logs.txt @@ -0,0 +1,11 @@ +NAME: doge +LAST DEPLOYED: Fri Sep 2 22:04:05 1977 +NAMESPACE: default +STATUS: deployed +REVISION: 1 +TEST SUITE: doge-test-pod +Last Started: Fri Apr 16 13:37:00 2021 +Last Completed: Fri Apr 16 14:00:00 2021 +Phase: Succeeded +NOTES: +Some mock release notes! diff --git a/internal/test/test.go b/internal/test/test.go index 646037606..231c55f33 100644 --- a/internal/test/test.go +++ b/internal/test/test.go @@ -19,10 +19,15 @@ package test import ( "bytes" "flag" + "fmt" "io/ioutil" "path/filepath" + "strings" "github.com/pkg/errors" + "github.com/stretchr/testify/assert" + + "helm.sh/helm/v3/pkg/release" ) // UpdateGolden writes out the golden files with the latest values, rather than failing the test. @@ -30,6 +35,7 @@ var updateGolden = flag.Bool("update", false, "update golden files") // TestingT describes a testing object compatible with the critical functions from the testing.T type type TestingT interface { + Errorf(format string, args ...interface{}) Fatal(...interface{}) Fatalf(string, ...interface{}) HelperT @@ -69,6 +75,55 @@ func AssertGoldenFile(t TestingT, actualFileName string, expectedFilename string AssertGoldenBytes(t, actual, expectedFilename) } +// AssertGoldenStringWithCustomLineValidation asserts that the given string matches the contents of the given file, using the given function to check each line. +// It is useful when the output is expected to contain information that cannot be predicted, such as timestamps. +// +// The line validation function must return a pair of values representing, respectively, +// +// 1. whether the expected line is "special" or not, and +// +// 2. if the expected line is special, the validity of the actual line. +// +// "Not special" means that the actual line must be exactly equal to the expected line to be considered valid. +func AssertGoldenStringWithCustomLineValidation(t TestingT, checkLine func(expected, actual string) (bool, error)) func(actualOutput, expectedFilename string) { + t.Helper() + is := assert.New(t) + return func(actualOutput, expectedFilename string) { + expectedOutput, err := ioutil.ReadFile(path(expectedFilename)) + if err != nil { + t.Fatalf("%v", err) + } + expectedLines := lines(expectedOutput) + actualLines := lines([]byte(actualOutput)) + expectedLineCount := len(expectedLines) + actualLineCount := len(actualLines) + for i := 0; i < max(expectedLineCount, actualLineCount); i++ { + lineNumber := i + 1 + // We need to prevent index-out-of-range errors if the number of lines doesn't match between the expected and the actual output. + // But we cannot just use the empty string as a default value, because that's equivalent to downright ignoring trailing empty lines. + if lineNumber > expectedLineCount { + t.Errorf("Output should only have %d line(s), but has %d. Line %d is: %q", expectedLineCount, actualLineCount, lineNumber, actualLines[i]) + } else if lineNumber > actualLineCount { + t.Errorf("Output should have %d line(s), but has only %d. Line %d should have been: %q", expectedLineCount, actualLineCount, lineNumber, expectedLines[i]) + } else { + actualLine := actualLines[i] + expectedLine := expectedLines[i] + if isSpecialLine, err := checkLine(expectedLine, actualLine); isSpecialLine { + if err != nil { + t.Errorf("Unexpected content on line %d (%v): %s", lineNumber, err.Error(), actualLine) + } + } else { + is.Equal(expectedLine, actualLine, fmt.Sprintf("Line %d in the actual output does not match line %d in the expected output (%s).", lineNumber, lineNumber, expectedFilename)) + } + } + } + } +} + +func lines(raw []byte) []string { + return strings.Split(strings.TrimSuffix(string(normalize(raw)), "\n"), "\n") // We first remove the final newline (if any), so that e.g. the 2-line string "a\nb\n" is mapped to ["a", "b"] and not ["a", "b", ""]. +} + func path(filename string) string { if filepath.IsAbs(filename) { return filename @@ -93,6 +148,11 @@ func compare(actual []byte, filename string) error { return nil } +func GetMockedHookLog(rel *release.Release, hook *release.Hook) (*release.HookLog, error) { + hookLog := release.HookLog("example test pod log output") + return &hookLog, nil +} + func update(filename string, in []byte) error { if !*updateGolden { return nil @@ -103,3 +163,10 @@ func update(filename string, in []byte) error { func normalize(in []byte) []byte { return bytes.Replace(in, []byte("\r\n"), []byte("\n"), -1) } + +func max(x, y int) int { + if x > y { + return x + } + return y +} diff --git a/pkg/action/action.go b/pkg/action/action.go index deb3f65df..351eaf10d 100644 --- a/pkg/action/action.go +++ b/pkg/action/action.go @@ -18,7 +18,9 @@ package action import ( "bytes" + "context" "fmt" + "io" "os" "path" "path/filepath" @@ -26,6 +28,7 @@ import ( "strings" "github.com/pkg/errors" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/cli-runtime/pkg/genericclioptions" "k8s.io/client-go/discovery" @@ -95,8 +98,16 @@ type Configuration struct { Capabilities *chartutil.Capabilities Log func(string, ...interface{}) + + HookLogGetter HookLogGetter } +// A HookLogGetter gets the log of the given hook. +// +// The empty string represents the output from a pod that didn't print anything (i.e. the empty log). +// The nil pointer represents no log at all. +type HookLogGetter func(rel *release.Release, hook *release.Hook) (*release.HookLog, error) + // renderResources renders the templates in a chart // // TODO: This function is badly in need of a refactor. @@ -276,6 +287,27 @@ func (cfg *Configuration) getCapabilities() (*chartutil.Capabilities, error) { return cfg.Capabilities, nil } +// GetHookLogFromRealCluster gets the log from the pod associated with the given hook, which is expected to be a test hook. +func (cfg *Configuration) GetHookLogFromRealCluster(rel *release.Release, hook *release.Hook) (*release.HookLog, error) { + var nothing *release.HookLog + client, err := cfg.KubernetesClientSet() + if err != nil { + return nothing, errors.Wrapf(err, "unable to create Kubernetes client set to fetch pod logs") + } + req := client.CoreV1().Pods(rel.Namespace).GetLogs(hook.Name, &v1.PodLogOptions{}) + responseBody, err := req.Stream(context.Background()) + if err != nil { + return nothing, errors.Wrapf(err, "unable to get pod logs for %s", hook.Name) + } + stringBuilder := new(strings.Builder) + _, err = io.Copy(stringBuilder, responseBody) + if err != nil { + return nothing, errors.Wrapf(err, "unable to get pod logs for %s", hook.Name) + } + hookLog := release.HookLog(stringBuilder.String()) + return &hookLog, nil +} + // KubernetesClientSet creates a new kubernetes ClientSet based on the configuration func (cfg *Configuration) KubernetesClientSet() (kubernetes.Interface, error) { conf, err := cfg.RESTClientGetter.ToRESTConfig() diff --git a/pkg/action/action_test.go b/pkg/action/action_test.go index 190d00cae..06a145bc0 100644 --- a/pkg/action/action_test.go +++ b/pkg/action/action_test.go @@ -23,6 +23,7 @@ import ( fakeclientset "k8s.io/client-go/kubernetes/fake" + "helm.sh/helm/v3/internal/test" "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chartutil" kubefake "helm.sh/helm/v3/pkg/kube/fake" @@ -61,6 +62,7 @@ func actionConfigFixture(t *testing.T) *Configuration { t.Logf(format, v...) } }, + HookLogGetter: test.GetMockedHookLog, } } diff --git a/pkg/action/release_testing.go b/pkg/action/release_testing.go index ecaeaf59f..708822618 100644 --- a/pkg/action/release_testing.go +++ b/pkg/action/release_testing.go @@ -17,13 +17,11 @@ limitations under the License. package action import ( - "context" "fmt" "io" "time" "github.com/pkg/errors" - v1 "k8s.io/api/core/v1" "helm.sh/helm/v3/pkg/chartutil" "helm.sh/helm/v3/pkg/release" @@ -102,23 +100,14 @@ func (r *ReleaseTesting) Run(name string) (*release.Release, error) { // the given writer. These can be immediately output to the user or captured for // other uses func (r *ReleaseTesting) GetPodLogs(out io.Writer, rel *release.Release) error { - client, err := r.cfg.KubernetesClientSet() - if err != nil { - return errors.Wrap(err, "unable to get kubernetes client to fetch pod logs") - } - for _, h := range rel.Hooks { for _, e := range h.Events { if e == release.HookTest { - req := client.CoreV1().Pods(r.Namespace).GetLogs(h.Name, &v1.PodLogOptions{}) - logReader, err := req.Stream(context.Background()) + hookLog, err := r.cfg.HookLogGetter(rel, h) if err != nil { - return errors.Wrapf(err, "unable to get pod logs for %s", h.Name) + return err } - - fmt.Fprintf(out, "POD LOGS: %s\n", h.Name) - _, err = io.Copy(out, logReader) - fmt.Fprintln(out) + _, err = fmt.Fprintf(out, "POD LOGS: %s\n%s\n", h.Name, string(*hookLog)) if err != nil { return errors.Wrapf(err, "unable to write pod logs for %s", h.Name) } diff --git a/pkg/action/release_testing_test.go b/pkg/action/release_testing_test.go new file mode 100644 index 000000000..fb0f7bca0 --- /dev/null +++ b/pkg/action/release_testing_test.go @@ -0,0 +1,72 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package action + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "helm.sh/helm/v3/pkg/release" +) + +func releaseTestingAction(t *testing.T) *ReleaseTesting { + config := actionConfigFixture(t) + testAction := NewReleaseTesting(config) + return testAction +} + +func TestReleaseTesting(t *testing.T) { + is := assert.New(t) + + testAction := releaseTestingAction(t) + + rel := releaseStub() + testAction.cfg.Releases.Create(rel) + expectedConfigMapHook := release.Hook{ + Name: "test-cm", + Kind: "ConfigMap", + Path: "test-cm", + LastRun: release.HookExecution{ + Phase: "", + }, + } + expectedPodHook := release.Hook{ + Name: "finding-nemo", + Kind: "Pod", + Path: "finding-nemo", + LastRun: release.HookExecution{ + Phase: "Succeeded", + }, + } + res, err := testAction.Run(rel.Name) + is.NoError(err) + is.Equal("angry-panda", res.Name) + is.Len(res.Hooks, 2, "The action seems to have changed the number of hooks on the release.") + checkHook(t, expectedConfigMapHook, res.Hooks[0]) + checkHook(t, expectedPodHook, res.Hooks[1]) +} + +func checkHook(t *testing.T, expected release.Hook, actual *release.Hook) { + t.Helper() + is := assert.New(t) + is.Equal(expected.Name, actual.Name) + is.Equal(expected.Kind, actual.Kind) + is.Equal(expected.Path, actual.Path) + is.Equal(expected.LastRun.Phase, actual.LastRun.Phase) + // Cannot expect start and completion times because they cannot be predicted. +} diff --git a/pkg/release/hook.go b/pkg/release/hook.go index cb9955582..0ced35bef 100644 --- a/pkg/release/hook.go +++ b/pkg/release/hook.go @@ -78,6 +78,9 @@ type Hook struct { DeletePolicies []HookDeletePolicy `json:"delete_policies,omitempty"` } +// A HookLog represents a log associated with a hook. +type HookLog string + // A HookExecution records the result for the last execution of a hook for a given release. type HookExecution struct { // StartedAt indicates the date/time this hook was started