Improve frameworktestutil usability with complex error messages

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,11 +963,28 @@ 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")
 	}
+	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
 // build functions that implement complex multi-version APIs that require defaulting and validation.

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
+	// ExpectedErrorFilename is the file containing elements of an expected error message.
-	// Defaults to "error.yaml".  Directories containing neither this file
+	// 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
+	// ExpectedErrorFilename is the file containing elements of an expected error message.
-	// Defaults to "error.yaml".  Directories containing neither this file
+	// 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,
+			// Check that each expected line matches the output
-				standardizeSpacing(err.Error()),
+			for _, msg := range strings.Split(standardizeSpacing(expectedError), "\n") {
-				standardizeSpacing(expectedError), actualOutput.String())
+				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)

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)
+}

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

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