Make frameworktestutil assertions configurable

Also refactor to share common code between results checkers

kyaml/fn/framework/frameworktestutil/frameworktestutil.go

@@ -16,21 +16,53 @@ 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"
 )
 
-// CommandResultsChecker tests a function by running it with predefined inputs and comparing
+const (
-// the outputs to expected results.
+	DefaultTestDataDirectory   = "testdata"
+	DefaultConfigInputFilename = "config.yaml"
+	DefaultInputFilename       = "input.yaml"
+	DefaultInputFilenameGlob   = "input*.yaml"
+	DefaultOutputFilename      = "expected.yaml"
+	DefaultErrorFilename       = "errors.txt"
+)
+
+// CommandResultsChecker tests a command-wrapped function by running it with predefined inputs
+// and comparing the outputs to expected results.
 type CommandResultsChecker struct {
 	// TestDataDirectory is the directory containing the testdata subdirectories.
 	// CommandResultsChecker will recurse into each test directory and run the Command
-	// if the directory contains both the ConfigInputFilename and at least one
+	// if the directory contains at least one of ExpectedOutputFilename or ExpectedErrorFilename.
-	// of ExpectedOutputFilname or ExpectedErrorFilename.
 	// Defaults to "testdata"
 	TestDataDirectory 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 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
+
+	// UpdateExpectedFromActual if set to true will write the actual results to the
+	// expected testdata files.  This is useful for updating test data.
+	UpdateExpectedFromActual bool
+
+	// OutputAssertionFunc allows you to swap out the logic used to compare the expected output
+	// from the fixture file to the actual output.
+	// By default, it performs a string comparison after normalizing whitespace.
+	OutputAssertionFunc AssertionFunc
+
+	// ErrorAssertionFunc allows you to swap out the logic used to compare the expected error
+	// message from the fixture file to the actual error message.
+	// By default, it interprets each line of the fixture as a regex that the actual error must match.
+	ErrorAssertionFunc AssertionFunc
+
 	// ConfigInputFilename is the name of the config file provided as the first
 	// argument to the function.  Directories without this file will be skipped.
 	// Defaults to "config.yaml"
@@ -40,150 +72,61 @@ type CommandResultsChecker struct {
 	// Defaults to "input*.yaml"
 	InputFilenameGlob 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 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
-
 	// Command provides the function to run.
 	Command func() *cobra.Command
-
-	// 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
+// Assert runs the command with the input provided in each valid test directory
+// and verifies that the actual output and error match the fixtures in the directory.
 func (rc *CommandResultsChecker) Assert(t *testing.T) bool {
-	if rc.TestDataDirectory == "" {
-		rc.TestDataDirectory = "testdata"
-	}
 	if rc.ConfigInputFilename == "" {
-		rc.ConfigInputFilename = "config.yaml"
+		rc.ConfigInputFilename = DefaultConfigInputFilename
-	}
-	if rc.ExpectedOutputFilename == "" {
-		rc.ExpectedOutputFilename = "expected.yaml"
-	}
-	if rc.ExpectedErrorFilename == "" {
-		rc.ExpectedErrorFilename = "errors.txt"
 	}
 	if rc.InputFilenameGlob == "" {
-		rc.InputFilenameGlob = "input*.yaml"
+		rc.InputFilenameGlob = DefaultInputFilenameGlob
 	}
 
-	err := filepath.Walk(rc.TestDataDirectory, func(path string, info os.FileInfo, err error) error {
+	checker := newResultsChecker(
-		require.NoError(t, err)
+		rc.TestDataDirectory, rc.ExpectedOutputFilename, rc.ExpectedErrorFilename,
-		if !info.IsDir() {
+		rc.OutputAssertionFunc, rc.ErrorAssertionFunc,
-			// skip non-directories
+		rc.UpdateExpectedFromActual,
-			return nil
+	)
+	checker.assert(t, func() (string, string) {
+		_, err := os.Stat(rc.ConfigInputFilename)
+		if os.IsNotExist(err) {
+			t.Errorf("Test case is missing FunctionConfig input file (default: %s)", DefaultConfigInputFilename)
 		}
-		rc.compare(t, path)
+		require.NoError(t, err)
-		return nil
+		args := []string{rc.ConfigInputFilename}
-	})
-	require.NoError(t, err)
 
-	require.NotZero(t, rc.testsCasesRun, "No complete test cases found in %s", rc.TestDataDirectory)
-
-	return true
-}
-
-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()
-	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)
-	if os.IsNotExist(err) {
-		// missing input
-		return
-	}
-	args := []string{rc.ConfigInputFilename}
-
-	expectedOutput, expectedError := getExpected(t, rc.ExpectedOutputFilename, rc.ExpectedErrorFilename)
-	if expectedError == "" && expectedOutput == "" && !rc.UpdateExpectedFromActual {
-		// missing expected and UpdateExpectedFromActual == false, return and skip this test
-		return
-	}
-	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)
 			require.NoError(t, err)
 			args = append(args, inputs...)
 		}
 
-		var actualOutput, actualError bytes.Buffer
+		var stdOut, stdErr bytes.Buffer
 		cmd := rc.Command()
 		cmd.SetArgs(args)
-		cmd.SetOut(&actualOutput)
+		cmd.SetOut(&stdOut)
-		cmd.SetErr(&actualError)
+		cmd.SetErr(&stdErr)
 
 		err = cmd.Execute()
-
+		return stdOut.String(), stdErr.String()
-		// 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))
-			}
-			t.Skip("Updated fixtures for test case")
-		}
-
-		// 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())
-		}
 	})
+
+	return true
 }
 
-func standardizeSpacing(s string) string {
+// ProcessorResultsChecker tests a processor function by running it with predefined inputs
-	// remove extra whitespace and convert Windows line endings
+// and comparing the outputs to expected results.
-	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
+	// ProcessorResultsChecker will recurse into each test directory and run the Command
-	// if the directory contains both the InputFilename and at least one
+	// if the directory contains at least one of ExpectedOutputFilename or ExpectedErrorFilename.
-	// 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.
@@ -195,73 +138,48 @@ type ProcessorResultsChecker struct {
 	// 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
+	// InputFilename is the name of the file containing the ResourceList input.
+	// Directories without this file will be skipped.
+	// Defaults to "input.yaml"
+	InputFilename string
+
+	// OutputAssertionFunc allows you to swap out the logic used to compare the expected output
+	// from the fixture file to the actual output.
+	// By default, it performs a string comparison after normalizing whitespace.
+	OutputAssertionFunc AssertionFunc
+
+	// ErrorAssertionFunc allows you to swap out the logic used to compare the expected error
+	// message from the fixture file to the actual error message.
+	// By default, it interprets each line of the fixture as a regex that the actual error must match.
+	ErrorAssertionFunc AssertionFunc
+
+	// Processor returns a ResourceListProcessor to run.
+	Processor func() framework.ResourceListProcessor
 }
 
-// Assert asserts the results for functions
+// Assert runs the processor with the input provided in each valid test directory
+// and verifies that the actual output and error match the fixtures in the directory.
 func (rc *ProcessorResultsChecker) Assert(t *testing.T) bool {
-	if rc.TestDataDirectory == "" {
-		rc.TestDataDirectory = "testdata"
-	}
 	if rc.InputFilename == "" {
-		rc.InputFilename = "input.yaml"
+		rc.InputFilename = DefaultInputFilename
-	}
-	if rc.ExpectedOutputFilename == "" {
-		rc.ExpectedOutputFilename = "expected.yaml"
-	}
-	if rc.ExpectedErrorFilename == "" {
-		rc.ExpectedErrorFilename = "errors.txt"
 	}
 
-	err := filepath.Walk(rc.TestDataDirectory, func(path string, info os.FileInfo, err error) error {
+	checker := newResultsChecker(
-		require.NoError(t, err)
+		rc.TestDataDirectory, rc.ExpectedOutputFilename, rc.ExpectedErrorFilename,
-		if !info.IsDir() {
+		rc.OutputAssertionFunc, rc.ErrorAssertionFunc,
-			// skip non-directories
+		rc.UpdateExpectedFromActual,
-			return nil
+	)
+	checker.assert(t, func() (string, string) {
+		_, err := os.Stat(rc.InputFilename)
+		if os.IsNotExist(err) {
+			t.Error("Test case is missing input file")
 		}
-		rc.compare(t, path)
+		require.NoError(t, err)
-		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 == "" && !rc.UpdateExpectedFromActual {
-		// missing expected and UpdateExpectedFromActual == false, return and skip this test
-		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)
@@ -270,64 +188,170 @@ func (rc *ProcessorResultsChecker) compare(t *testing.T, path string) {
 			Reader: bytes.NewBuffer(rlBytes),
 			Writer: actualOutput,
 		}
-
 		err = framework.Execute(rc.Processor(), &rw)
+		if err != nil {
+			require.NotEmptyf(t, err.Error(), "processor returned error with empty message")
+			return actualOutput.String(), err.Error()
+		}
+		return actualOutput.String(), ""
+	})
+	return true
+}
 
-		// Update the fixtures if configured to
+type AssertionFunc func(t *testing.T, expected string, actual string)
-		if rc.UpdateExpectedFromActual {
+
-			if err != nil {
+// RequireEachLineMatches is an AssertionFunc that treats each line of expected string
-				require.NoError(t, ioutil.WriteFile(rc.ExpectedErrorFilename, []byte(err.Error()), 0600))
+// as a regex that must match the actual string.
+func RequireEachLineMatches(t *testing.T, expected, actual string) {
+	// Check that each expected line matches the output
+	actual = standardizeSpacing(actual)
+	for _, msg := range strings.Split(standardizeSpacing(expected), "\n") {
+		require.Regexp(t, regexp.MustCompile(msg), actual)
+	}
+}
+
+// RequireStrippedStringsEqual is an AssertionFunc that does a simple string comparison
+// of expected and actual after normalizing whitespace.
+func RequireStrippedStringsEqual(t *testing.T, expected, actual string) {
+	require.Equal(t,
+		standardizeSpacing(expected),
+		standardizeSpacing(actual))
+}
+
+func standardizeSpacing(s string) string {
+	// remove extra whitespace and convert Windows line endings
+	return strings.ReplaceAll(strings.TrimSpace(s), "\r\n", "\n")
+}
+
+// resultsChecker implements the core logic shared by all results checking types.
+type resultsChecker struct {
+	testDataDirectory        string
+	expectedOutputFilename   string
+	expectedErrorFilename    string
+	updateExpectedFromActual bool
+	outputAssertionFunc      AssertionFunc
+	errorAssertionFunc       AssertionFunc
+
+	testsCasesRun int
+}
+
+func newResultsChecker(testDataDir string, outputFilename string, errorFilename string,
+	outputAsserter AssertionFunc, errorAsserter AssertionFunc,
+	updateFixtures bool) *resultsChecker {
+	rc := resultsChecker{
+		testDataDirectory:        testDataDir,
+		expectedOutputFilename:   outputFilename,
+		expectedErrorFilename:    errorFilename,
+		updateExpectedFromActual: updateFixtures,
+		outputAssertionFunc:      outputAsserter,
+		errorAssertionFunc:       errorAsserter,
+	}
+	if rc.testDataDirectory == "" {
+		rc.testDataDirectory = DefaultTestDataDirectory
+	}
+	if rc.expectedOutputFilename == "" {
+		rc.expectedOutputFilename = DefaultOutputFilename
+	}
+	if rc.expectedErrorFilename == "" {
+		rc.expectedErrorFilename = DefaultErrorFilename
+	}
+	if rc.outputAssertionFunc == nil {
+		rc.outputAssertionFunc = RequireStrippedStringsEqual
+	}
+	if rc.errorAssertionFunc == nil {
+		rc.errorAssertionFunc = RequireEachLineMatches
+	}
+	return &rc
+}
+
+// assert traverses TestDataDirectory to find test cases, calls getResult to invoke the function
+// under test in each directory, and then runs assertions on the returned output and error results.
+// It triggers a test failure if no valid test directories were found.
+func (rc *resultsChecker) assert(t *testing.T, getResult func() (string, string)) {
+	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 len(actualOutput.String()) > 0 {
+			rc.runDirectoryTestCase(t, path, getResult)
-				require.NoError(t, ioutil.WriteFile(rc.ExpectedOutputFilename, actualOutput.Bytes(), 0600))
+			return nil
+		})
+	require.NoError(t, err)
+	require.NotZero(t, rc.testsCasesRun, "No complete test cases found in %s", rc.testDataDirectory)
+}
+
+func (rc *resultsChecker) runDirectoryTestCase(t *testing.T, path string, getResult func() (string, 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))
+
+	expectedOutput, expectedError := rc.readAssertionFiles(t)
+	if expectedError == "" && expectedOutput == "" && !rc.updateExpectedFromActual {
+		// not a test directory: missing expectations and updateExpectedFromActual == false
+		return
+	}
+
+	// run the test
+	t.Run(path, func(t *testing.T) {
+		rc.testsCasesRun += 1
+		actualOutput, actualError := getResult()
+
+		// Configured to update the assertion files instead of comparing them
+		if rc.updateExpectedFromActual {
+			if actualError != "" {
+				require.NoError(t, ioutil.WriteFile(rc.expectedErrorFilename, []byte(actualError), 0600))
 			}
-			return
+			if len(actualOutput) > 0 {
+				require.NoError(t, ioutil.WriteFile(rc.expectedOutputFilename, []byte(actualOutput), 0600))
+			}
+			t.Skip("Updated fixtures for test case")
 		}
 
-		// Compare the results
+		// Compare the results to the assertion files
 		if expectedError != "" {
 			// We expected an error, so make sure there was one
-			require.Error(t, err, actualOutput.String())
+			require.NotEmptyf(t, actualError, "test expected an error but message was empty, and output was:\n%s", actualOutput)
-			// Check that each expected line matches the output
+			rc.errorAssertionFunc(t, expectedError, actualError)
-			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)
+			require.Emptyf(t, actualError, "test expected no error but got an error message, and output was:\n%s", actualOutput)
-			require.Equal(t,
+			rc.outputAssertionFunc(t, expectedOutput, actualOutput)
-				standardizeSpacing(expectedOutput),
-				standardizeSpacing(actualOutput.String()))
 		}
 	})
 }
 
-// getExpected reads the expected results and error files
+// readAssertionFiles reads the expected results and error files
-func getExpected(t *testing.T, expectedOutFilename, expectedErrFilename string) (string, string) {
+func (rc *resultsChecker) readAssertionFiles(t *testing.T) (string, string) {
 	// read the expected results
 	var expectedOutput, expectedError string
-	if expectedOutFilename != "" {
+	if rc.expectedOutputFilename != "" {
-		_, err := os.Stat(expectedOutFilename)
+		_, err := os.Stat(rc.expectedOutputFilename)
 		if !os.IsNotExist(err) && err != nil {
 			t.FailNow()
 		}
 		if err == nil {
 			// only read the file if it exists
-			b, err := ioutil.ReadFile(expectedOutFilename)
+			b, err := ioutil.ReadFile(rc.expectedOutputFilename)
 			if !assert.NoError(t, err) {
 				t.FailNow()
 			}
 			expectedOutput = string(b)
 		}
 	}
-	if expectedErrFilename != "" {
+	if rc.expectedErrorFilename != "" {
-		_, err := os.Stat(expectedErrFilename)
+		_, err := os.Stat(rc.expectedErrorFilename)
 		if !os.IsNotExist(err) && err != nil {
 			t.FailNow()
 		}
 		if err == nil {
 			// only read the file if it exists
-			b, err := ioutil.ReadFile(expectedErrFilename)
+			b, err := ioutil.ReadFile(rc.expectedErrorFilename)
 			if !assert.NoError(t, err) {
 				t.FailNow()
 			}