From f9afdc5c95429661359589edf24754cda2cda1aa Mon Sep 17 00:00:00 2001 From: Katrina Verey Date: Mon, 31 May 2021 10:57:41 -0700 Subject: [PATCH] Improve frameworktestutil usability with complex error messages --- kyaml/fn/framework/example_test.go | 22 +++++++++++++++-- .../frameworktestutil/frameworktestutil.go | 24 +++++++++++-------- kyaml/fn/framework/processors_test.go | 17 +++++++++++++ .../testdata/validation/error/errors.txt | 5 ++++ .../testdata/validation/error/input.yaml | 14 +++++++++++ 5 files changed, 70 insertions(+), 12 deletions(-) create mode 100644 kyaml/fn/framework/testdata/validation/error/errors.txt create mode 100644 kyaml/fn/framework/testdata/validation/error/input.yaml diff --git a/kyaml/fn/framework/example_test.go b/kyaml/fn/framework/example_test.go index 9b7e4d8da..14ed7015f 100644 --- a/kyaml/fn/framework/example_test.go +++ b/kyaml/fn/framework/example_test.go @@ -8,6 +8,7 @@ import ( "fmt" "log" "path/filepath" + "strings" "sigs.k8s.io/kustomize/kyaml/errors" "sigs.k8s.io/kustomize/kyaml/fn/framework" @@ -962,10 +963,27 @@ func (a *v1alpha1JavaSpringBoot) Default() error { } func (a *v1alpha1JavaSpringBoot) Validate() error { + var messages []string if a.Metadata.Name == "" { - return errors.Errorf("Name is required") + messages = append(messages, "name is required") } - return nil + if a.Spec.Replicas > 10 { + messages = append(messages, "replicas must be less than 10") + } + if !strings.HasSuffix(a.Spec.Domain, "example.com") { + messages = append(messages, "domain must be a subdomain of example.com") + } + if strings.HasSuffix(a.Spec.Image, ":latest") { + messages = append(messages, "image should not have latest tag") + } + if len(messages) == 0 { + return nil + } + errMsg := fmt.Sprintf("JavaSpringBoot had %d errors:\n", len(messages)) + for i, msg := range messages { + errMsg += fmt.Sprintf(" [%d] %s\n", i+1, msg) + } + return errors.Errorf(errMsg) } // ExampleVersionedAPIProcessor shows how to use the VersionedAPIProcessor and TemplateProcessor to diff --git a/kyaml/fn/framework/frameworktestutil/frameworktestutil.go b/kyaml/fn/framework/frameworktestutil/frameworktestutil.go index df73a050d..753e6f8a6 100644 --- a/kyaml/fn/framework/frameworktestutil/frameworktestutil.go +++ b/kyaml/fn/framework/frameworktestutil/frameworktestutil.go @@ -9,6 +9,7 @@ import ( "io/ioutil" "os" "path/filepath" + "regexp" "strings" "testing" @@ -44,8 +45,9 @@ type CommandResultsChecker struct { // nor ExpectedErrorFilename will be skipped. ExpectedOutputFilename string - // ExpectedErrorFilename is the file containing part of an expected error message - // Defaults to "error.yaml". Directories containing neither this file + // ExpectedErrorFilename is the file containing elements of an expected error message. + // Each line of the file will be treated as a regex that must match the actual error. + // Defaults to "errors.txt". Directories containing neither this file // nor ExpectedOutputFilename will be skipped. ExpectedErrorFilename string @@ -71,7 +73,7 @@ func (rc *CommandResultsChecker) Assert(t *testing.T) bool { rc.ExpectedOutputFilename = "expected.yaml" } if rc.ExpectedErrorFilename == "" { - rc.ExpectedErrorFilename = "error.yaml" + rc.ExpectedErrorFilename = "errors.txt" } if rc.InputFilenameGlob == "" { rc.InputFilenameGlob = "input*.yaml" @@ -187,8 +189,9 @@ type ProcessorResultsChecker struct { // nor ExpectedErrorFilename will be skipped. ExpectedOutputFilename string - // ExpectedErrorFilename is the file containing part of an expected error message - // Defaults to "error.yaml". Directories containing neither this file + // ExpectedErrorFilename is the file containing elements of an expected error message. + // Each line of the file will be treated as a regex that must match the actual error. + // Defaults to "errors.txt". Directories containing neither this file // nor ExpectedOutputFilename will be skipped. ExpectedErrorFilename string @@ -214,7 +217,7 @@ func (rc *ProcessorResultsChecker) Assert(t *testing.T) bool { rc.ExpectedOutputFilename = "expected.yaml" } if rc.ExpectedErrorFilename == "" { - rc.ExpectedErrorFilename = "error.yaml" + rc.ExpectedErrorFilename = "errors.txt" } err := filepath.Walk(rc.TestDataDirectory, func(path string, info os.FileInfo, err error) error { @@ -283,11 +286,12 @@ func (rc *ProcessorResultsChecker) compare(t *testing.T, path string) { // Compare the results if expectedError != "" { - // We expected an error, so make sure there was one and it matches + // We expected an error, so make sure there was one require.Error(t, err, actualOutput.String()) - require.Contains(t, - standardizeSpacing(err.Error()), - standardizeSpacing(expectedError), actualOutput.String()) + // Check that each expected line matches the output + for _, msg := range strings.Split(standardizeSpacing(expectedError), "\n") { + require.Regexp(t, regexp.MustCompile(msg), err.Error(), actualOutput.String()) + } } else { // We didn't expect an error, and the output should match require.NoError(t, err) diff --git a/kyaml/fn/framework/processors_test.go b/kyaml/fn/framework/processors_test.go index 950b725fa..554a15e47 100644 --- a/kyaml/fn/framework/processors_test.go +++ b/kyaml/fn/framework/processors_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "sigs.k8s.io/kustomize/kyaml/errors" + "sigs.k8s.io/kustomize/kyaml/fn/framework/frameworktestutil" "sigs.k8s.io/kustomize/kyaml/fn/framework/parser" "sigs.k8s.io/kustomize/kyaml/openapi" "sigs.k8s.io/kustomize/kyaml/yaml" @@ -589,3 +590,19 @@ items: }) require.Nil(t, found, "openAPI schema was not reset") } + +func TestTemplateProcessor_Validator(t *testing.T) { + // This test proves the Validate method is called when implemented + // and demonstrates the use of ProcessorResultsChecker's error matching + p := func() framework.ResourceListProcessor { + return &framework.VersionedAPIProcessor{FilterProvider: framework.GVKFilterMap{ + "JavaSpringBoot": { + "example.com/v1alpha1": &v1alpha1JavaSpringBoot{}, + }}} + } + c := frameworktestutil.ProcessorResultsChecker{ + TestDataDirectory: "testdata/validation", + Processor: p, + } + c.Assert(t) +} diff --git a/kyaml/fn/framework/testdata/validation/error/errors.txt b/kyaml/fn/framework/testdata/validation/error/errors.txt new file mode 100644 index 000000000..92f3c6a51 --- /dev/null +++ b/kyaml/fn/framework/testdata/validation/error/errors.txt @@ -0,0 +1,5 @@ +JavaSpringBoot had 4 errors: +\[\d\] replicas must be less than 10 +\[\d\] name is required +\[\d\] image should not have latest tag +\[\d\] domain must be a subdomain of example.com diff --git a/kyaml/fn/framework/testdata/validation/error/input.yaml b/kyaml/fn/framework/testdata/validation/error/input.yaml new file mode 100644 index 000000000..41052ae8d --- /dev/null +++ b/kyaml/fn/framework/testdata/validation/error/input.yaml @@ -0,0 +1,14 @@ +# Copyright 2021 The Kubernetes Authors. +# SPDX-License-Identifier: Apache-2.0 + +kind: ResourceList +apiVersion: config.kubernetes.io/v1alpha1 +functionConfig: + apiVersion: example.com/v1alpha1 + kind: JavaSpringBoot + metadata: + name: "" + spec: + replicas: 1000 + image: foo:latest + domain: bad