Improvements to kyaml fn framework

This commit creates a new version of the alpha configuration functions framework. Goals include:
- Make it easy to build multi-version APIs with the framework (not previously facilitated at all).
- Simplify the framework's APIs where redundant configuration options exist (leaving the most powerful, replacing others with helpers to maintain usability they provided).
- Make the Framework's APIs more consistent (e.g. between the various template types, usage of kio.Filter, field names)
- Decouple responsibilities (e.g. command creation, resource list processing, generation of templating functions).
- Make the framework even more powerfully pluggable (e.g. any kio.Filter can be a selector, and the selector the framework provides is itself a filter built from reusable abstractions).
- Improve documentation.
- Make container patches merge fields (notably list fields like `env`) correctly.

kyaml/fn/framework/frameworktestutil/frameworktestutil.go

@@ -14,13 +14,17 @@ import (
 
 	"github.com/spf13/cobra"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"sigs.k8s.io/kustomize/kyaml/fn/framework"
+	"sigs.k8s.io/kustomize/kyaml/kio"
 )
 
-// ResultsChecker tests a function by running it with predefined inputs and comparing
+// CommandResultsChecker tests a function by running it with predefined inputs and comparing
 // the outputs to expected results.
-type ResultsChecker struct {
+type CommandResultsChecker struct {
 	// TestDataDirectory is the directory containing the testdata subdirectories.
-	// ResultsChecker will recurse into each test directory and run the Command
+	// CommandResultsChecker will recurse into each test directory and run the Command
 	// if the directory contains both the ConfigInputFilename and at least one
 	// of ExpectedOutputFilname or ExpectedErrorFilename.
 	// Defaults to "testdata"
@@ -37,12 +41,12 @@ type ResultsChecker struct {
 
 	// ExpectedOutputFilename is the file with the expected output of the function
 	// Defaults to "expected.yaml".  Directories containing neither this file
-	// nore ExpectedErrorFilename will be skipped.
+	// 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
-	// nore ExpectedOutputFilname will be skipped.
+	// nor ExpectedOutputFilename will be skipped.
 	ExpectedErrorFilename string
 
 	// Command provides the function to run.
@@ -51,10 +55,12 @@ type ResultsChecker struct {
 	// UpdateExpectedFromActual if set to true will write the actual results to the
 	// expected testdata files.  This is useful for updating test data.
 	UpdateExpectedFromActual bool
+
+	testsCasesRun int
 }
 
 // Assert asserts the results for functions
-func (rc ResultsChecker) Assert(t *testing.T) bool {
+func (rc *CommandResultsChecker) Assert(t *testing.T) bool {
 	if rc.TestDataDirectory == "" {
 		rc.TestDataDirectory = "testdata"
 	}
@@ -71,10 +77,8 @@ func (rc ResultsChecker) Assert(t *testing.T) bool {
 		rc.InputFilenameGlob = "input*.yaml"
 	}
 
-	_ = filepath.Walk(rc.TestDataDirectory, func(path string, info os.FileInfo, err error) error {
-		if err != nil {
-			t.FailNow()
-		}
+	err := filepath.Walk(rc.TestDataDirectory, func(path string, info os.FileInfo, err error) error {
+		require.NoError(t, err)
 		if !info.IsDir() {
 			// skip non-directories
 			return nil
@@ -82,21 +86,21 @@ func (rc ResultsChecker) Assert(t *testing.T) bool {
 		rc.compare(t, path)
 		return nil
 	})
+	require.NoError(t, err)
+
+	require.NotZero(t, rc.testsCasesRun, "No complete test cases found in %s", rc.TestDataDirectory)
 
 	return true
 }
 
-func (rc ResultsChecker) compare(t *testing.T, path string) {
+func (rc *CommandResultsChecker) compare(t *testing.T, path string) {
 	// cd into the directory so we can test functions that refer
 	// local files by relative paths
 	d, err := os.Getwd()
-	if !assert.NoError(t, err) {
-		t.FailNow()
-	}
-	defer func() { _ = os.Chdir(d) }()
-	if !assert.NoError(t, os.Chdir(path)) {
-		t.FailNow()
-	}
+	require.NoError(t, err)
+
+	defer func() { require.NoError(t, os.Chdir(d)) }()
+	require.NoError(t, os.Chdir(path))
 
 	// make sure this directory contains test data
 	_, err = os.Stat(rc.ConfigInputFilename)
@@ -106,22 +110,19 @@ func (rc ResultsChecker) compare(t *testing.T, path string) {
 	}
 	args := []string{rc.ConfigInputFilename}
 
-	expectedOutput, expectedError := rc.getExpected(t)
+	expectedOutput, expectedError := getExpected(t, rc.ExpectedOutputFilename, rc.ExpectedErrorFilename)
 	if expectedError == "" && expectedOutput == "" {
 		// missing expected
 		return
 	}
-	if !assert.NoError(t, err) {
-		t.FailNow()
-	}
+	require.NoError(t, err)
 
 	// run the test
 	t.Run(path, func(t *testing.T) {
+		rc.testsCasesRun += 1
 		if rc.InputFilenameGlob != "" {
 			inputs, err := filepath.Glob(rc.InputFilenameGlob)
-			if !assert.NoError(t, err) {
-				t.FailNow()
-			}
+			require.NoError(t, err)
 			args = append(args, inputs...)
 		}
 
@@ -133,64 +134,196 @@ func (rc ResultsChecker) compare(t *testing.T, path string) {
 
 		err = cmd.Execute()
 
-		// Compae the results
-		if expectedError != "" && !assert.Error(t, err, actualOutput.String()) {
-			if !rc.UpdateExpectedFromActual {
-				t.FailNow()
+		// Update the fixtures if configured to
+		if rc.UpdateExpectedFromActual {
+			if actualError.String() != "" {
+				assert.NoError(t, ioutil.WriteFile(rc.ExpectedErrorFilename, actualError.Bytes(), 0600))
 			}
+			if actualOutput.String() != "" {
+				assert.NoError(t, ioutil.WriteFile(rc.ExpectedOutputFilename, actualOutput.Bytes(), 0600))
+			}
+			return
 		}
-		if expectedError == "" && !assert.NoError(t, err, actualError.String()) {
-			if !rc.UpdateExpectedFromActual {
-				t.FailNow()
-			}
+
+		// Compare the results
+		if expectedError != "" {
+			// We expected an error, so make sure there was one and it matches
+			require.Error(t, err, actualOutput.String())
+			require.Contains(t,
+				standardizeSpacing(actualError.String()),
+				standardizeSpacing(expectedError), actualOutput.String())
+		} else {
+			// We didn't expect an error, and the output should match
+			require.NoError(t, err, actualError.String())
+			require.Equal(t,
+				standardizeSpacing(expectedOutput),
+				standardizeSpacing(actualOutput.String()), actualError.String())
 		}
-		if !assert.Equal(t,
-			strings.TrimSpace(expectedOutput),
-			strings.TrimSpace(actualOutput.String()), actualError.String()) {
-			if !rc.UpdateExpectedFromActual {
-				t.FailNow()
-			}
-			// update test results
-			assert.NoError(t, ioutil.WriteFile(rc.ExpectedOutputFilename, actualOutput.Bytes(), 0600))
+	})
+}
+
+func standardizeSpacing(s string) string {
+	// remove extra whitespace and convert Windows line endings
+	return strings.ReplaceAll(strings.TrimSpace(s), "\r\n", "\n")
+}
+
+// ProcessorResultsChecker tests a function by running it with predefined inputs and comparing
+// the outputs to expected results.
+type ProcessorResultsChecker struct {
+	// TestDataDirectory is the directory containing the testdata subdirectories.
+	// CommandResultsChecker will recurse into each test directory and run the Processor
+	// if the directory contains both the InputFilename and at least one
+	// of ExpectedOutputFilename or ExpectedErrorFilename.
+	// Defaults to "testdata"
+	TestDataDirectory string
+
+	// InputFilename is the name of the file containing the ResourceList input.
+	// Directories without this file will be skipped.
+	// Defaults to "input.yaml"
+	InputFilename string
+
+	// ExpectedOutputFilename is the file with the expected output of the function
+	// Defaults to "expected.yaml".  Directories containing neither this file
+	// 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
+	// nor ExpectedOutputFilename will be skipped.
+	ExpectedErrorFilename string
+
+	// Processor returns a ResourceListProcessor to run.
+	Processor func() framework.ResourceListProcessor
+
+	// UpdateExpectedFromActual if set to true will write the actual results to the
+	// expected testdata files.  This is useful for updating test data.
+	UpdateExpectedFromActual bool
+
+	testsCasesRun int
+}
+
+// Assert asserts the results for functions
+func (rc *ProcessorResultsChecker) Assert(t *testing.T) bool {
+	if rc.TestDataDirectory == "" {
+		rc.TestDataDirectory = "testdata"
+	}
+	if rc.InputFilename == "" {
+		rc.InputFilename = "input.yaml"
+	}
+	if rc.ExpectedOutputFilename == "" {
+		rc.ExpectedOutputFilename = "expected.yaml"
+	}
+	if rc.ExpectedErrorFilename == "" {
+		rc.ExpectedErrorFilename = "error.yaml"
+	}
+
+	err := filepath.Walk(rc.TestDataDirectory, func(path string, info os.FileInfo, err error) error {
+		require.NoError(t, err)
+		if !info.IsDir() {
+			// skip non-directories
+			return nil
 		}
-		if !assert.Contains(t,
-			strings.TrimSpace(actualError.String()),
-			strings.TrimSpace(expectedError), actualOutput.String()) {
-			if !rc.UpdateExpectedFromActual {
-				t.FailNow()
+		rc.compare(t, path)
+		return nil
+	})
+	require.NoError(t, err)
+
+	require.NotZero(t, rc.testsCasesRun, "No complete test cases found in %s", rc.TestDataDirectory)
+
+	return true
+}
+
+func (rc *ProcessorResultsChecker) compare(t *testing.T, path string) {
+	// cd into the directory so we can test functions that refer
+	// local files by relative paths
+	d, err := os.Getwd()
+	require.NoError(t, err)
+
+	defer func() { require.NoError(t, os.Chdir(d)) }()
+	require.NoError(t, os.Chdir(path))
+
+	// make sure this directory contains test data
+	_, err = os.Stat(rc.InputFilename)
+	if os.IsNotExist(err) {
+		// missing input
+		return
+	}
+	require.NoError(t, err)
+
+	expectedOutput, expectedError := getExpected(t, rc.ExpectedOutputFilename, rc.ExpectedErrorFilename)
+	if expectedError == "" && expectedOutput == "" {
+		// missing expected
+		return
+	}
+
+	// run the test
+	t.Run(path, func(t *testing.T) {
+		rc.testsCasesRun += 1
+		actualOutput := bytes.NewBuffer([]byte{})
+		rlBytes, err := ioutil.ReadFile(rc.InputFilename)
+		require.NoError(t, err)
+
+		rw := kio.ByteReadWriter{
+			Reader: bytes.NewBuffer(rlBytes),
+			Writer: actualOutput,
+		}
+
+		err = framework.Execute(rc.Processor(), &rw)
+
+		// Update the fixtures if configured to
+		if rc.UpdateExpectedFromActual {
+			if err != nil {
+				require.NoError(t, ioutil.WriteFile(rc.ExpectedErrorFilename, []byte(err.Error()), 0600))
 			}
-			// update test results
-			assert.NoError(t, ioutil.WriteFile(rc.ExpectedErrorFilename, actualError.Bytes(), 0600))
+			if len(actualOutput.String()) > 0 {
+				require.NoError(t, ioutil.WriteFile(rc.ExpectedOutputFilename, actualOutput.Bytes(), 0600))
+			}
+			return
+		}
+
+		// Compare the results
+		if expectedError != "" {
+			// We expected an error, so make sure there was one and it matches
+			require.Error(t, err, actualOutput.String())
+			require.Contains(t,
+				standardizeSpacing(err.Error()),
+				standardizeSpacing(expectedError), actualOutput.String())
+		} else {
+			// We didn't expect an error, and the output should match
+			require.NoError(t, err)
+			require.Equal(t,
+				standardizeSpacing(expectedOutput),
+				standardizeSpacing(actualOutput.String()))
 		}
 	})
 }
 
 // getExpected reads the expected results and error files
-func (rc ResultsChecker) getExpected(t *testing.T) (string, string) {
+func getExpected(t *testing.T, expectedOutFilename, expectedErrFilename string) (string, string) {
 	// read the expected results
 	var expectedOutput, expectedError string
-	if rc.ExpectedOutputFilename != "" {
-		_, err := os.Stat(rc.ExpectedOutputFilename)
+	if expectedOutFilename != "" {
+		_, err := os.Stat(expectedOutFilename)
 		if !os.IsNotExist(err) && err != nil {
 			t.FailNow()
 		}
 		if err == nil {
 			// only read the file if it exists
-			b, err := ioutil.ReadFile(rc.ExpectedOutputFilename)
+			b, err := ioutil.ReadFile(expectedOutFilename)
 			if !assert.NoError(t, err) {
 				t.FailNow()
 			}
 			expectedOutput = string(b)
 		}
 	}
-	if rc.ExpectedErrorFilename != "" {
-		_, err := os.Stat(rc.ExpectedErrorFilename)
+	if expectedErrFilename != "" {
+		_, err := os.Stat(expectedErrFilename)
 		if !os.IsNotExist(err) && err != nil {
 			t.FailNow()
 		}
 		if err == nil {
 			// only read the file if it exists
-			b, err := ioutil.ReadFile(rc.ExpectedErrorFilename)
+			b, err := ioutil.ReadFile(expectedErrFilename)
 			if !assert.NoError(t, err) {
 				t.FailNow()
 			}