Aggregate and display failures at the bottom of output with minor code refactoring.

pkg/http/client.go

@@ -11,6 +11,7 @@ import (
 	"strings"
 
 	"github.com/dustin/go-humanize"
+	"github.com/pkg/errors"
 
 	"github.com/kyverno/kuttl/pkg/version"
 )
@@ -40,21 +41,20 @@ func (c *Client) GetByteBuffer(url string) (*bytes.Buffer, error) {
 
 	resp, err := c.Get(url)
 	if err != nil {
-		return buf, err
+		return buf, errors.Wrapf(err, "failed to fetch %s", url)
 	}
+	defer resp.Body.Close()
+
 	if resp.StatusCode != 200 {
 		return buf, fmt.Errorf("failed to fetch %s : %s", url, resp.Status)
 	}
 
 	_, err = io.Copy(buf, resp.Body)
 	if err != nil {
-		return nil, err
-	}
-	err = resp.Body.Close()
-	if err != nil {
-		return nil, fmt.Errorf("error when closing the response body %s", err)
+		return nil, errors.Wrap(err, "error while copying response body")
 	}
-	return buf, err
+
+	return buf, nil
 }
 
 // DownloadFile expects a url with a file and will save that file to the path provided preserving the file name.
@@ -69,7 +69,7 @@ func (c *Client) Download(url string, path string) error {
 	// Get the data
 	resp, err := c.Get(url)
 	if err != nil {
-		return err
+		return errors.Wrap(err, "failed to perform HTTP GET")
 	}
 	defer resp.Body.Close()
 
@@ -79,22 +79,22 @@ func (c *Client) Download(url string, path string) error {
 	}
 	// captures errors other than does not exist
 	if err != nil && !os.IsNotExist(err) {
-		return err
+		return errors.Wrap(err, "error checking file existence")
 	}
 
 	// Create the file with .tmp extension, so that we won't overwrite a
 	// file until it's downloaded fully
 	out, err := os.Create(path + ".tmp")
 	if err != nil {
-		return err
+		return errors.Wrap(err, "error creating temporary file")
 	}
 	defer out.Close()
 
 	// Create our bytes counter and pass it to be used alongside our writer
 	counter := &writeCounter{Name: filepath.Base(path)}
 	_, err = io.Copy(out, io.TeeReader(resp.Body, counter))
 	if err != nil {
-		return err
+		return errors.Wrap(err, "error while copying response body to file")
 	}
 
 	// The progress use the same line so print a new line once it's finished downloading
@@ -103,7 +103,7 @@ func (c *Client) Download(url string, path string) error {
 	// Rename the tmp file back to the original file
 	err = os.Rename(path+".tmp", path)
 	if err != nil {
-		return err
+		return errors.Wrap(err, "error renaming temporary file")
 	}
 
 	return nil

pkg/http/http.go

@@ -10,7 +10,7 @@ import (
 	testutils "github.com/kyverno/kuttl/pkg/test/utils"
 )
 
-// IsURL returns true if string is an URL
+// IsURL returns true if string is a URL
 func IsURL(str string) bool {
 	u, err := url.Parse(str)
 	return err == nil && u.Scheme != "" && u.Host != ""

pkg/test/assert.go

@@ -14,14 +14,9 @@ import (
 
 // Assert checks all provided assert files against a namespace.  Upon assert failure, it prints the failures and returns an error
 func Assert(namespace string, timeout int, assertFiles ...string) error {
-	var objects []client.Object
-
-	for _, file := range assertFiles {
-		o, err := ObjectsFromPath(file, "")
-		if err != nil {
-			return err
-		}
-		objects = append(objects, o...)
+	objects, err := loadObjectsFromFiles(assertFiles...)
+	if err != nil {
+		return err
 	}
 
 	// feels like the wrong abstraction, need to do some refactoring
@@ -31,20 +26,7 @@ func Assert(namespace string, timeout int, assertFiles ...string) error {
 		DiscoveryClient: DiscoveryClient,
 	}
 
-	var testErrors []error
-	for i := 0; i < timeout; i++ {
-		// start fresh
-		testErrors = []error{}
-		for _, expected := range objects {
-			testErrors = append(testErrors, s.CheckResource(expected, namespace)...)
-		}
-
-		if len(testErrors) == 0 {
-			break
-		}
-
-		time.Sleep(time.Second)
-	}
+	testErrors := waitForObjects(s, objects, namespace, timeout)
 
 	if len(testErrors) == 0 {
 		fmt.Printf("assert is valid\n")
@@ -59,14 +41,9 @@ func Assert(namespace string, timeout int, assertFiles ...string) error {
 
 // Errors checks all provided errors files against a namespace.  Upon assert failure, it prints the failures and returns an error
 func Errors(namespace string, timeout int, errorFiles ...string) error {
-	var objects []client.Object
-
-	for _, file := range errorFiles {
-		o, err := ObjectsFromPath(file, "")
-		if err != nil {
-			return err
-		}
-		objects = append(objects, o...)
+	objects, err := loadObjectsFromFiles(errorFiles...)
+	if err != nil {
+		return err
 	}
 
 	// feels like the wrong abstraction, need to do some refactoring
@@ -76,22 +53,7 @@ func Errors(namespace string, timeout int, errorFiles ...string) error {
 		DiscoveryClient: DiscoveryClient,
 	}
 
-	var testErrors []error
-	for i := 0; i < timeout; i++ {
-		// start fresh
-		testErrors = []error{}
-		for _, expected := range objects {
-			if err := s.CheckResourceAbsent(expected, namespace); err != nil {
-				testErrors = append(testErrors, err)
-			}
-		}
-
-		if len(testErrors) == 0 {
-			break
-		}
-
-		time.Sleep(time.Second)
-	}
+	testErrors := waitForObjects(s, objects, namespace, timeout)
 
 	if len(testErrors) == 0 {
 		fmt.Printf("error assert is valid\n")
@@ -104,6 +66,7 @@ func Errors(namespace string, timeout int, errorFiles ...string) error {
 	return errors.New("error asserts not valid")
 }
 
+// Client returns a Kubernetes client.
 func Client(forceNew bool) (client.Client, error) {
 	cfg, err := config.GetConfig()
 	if err != nil {
@@ -118,6 +81,7 @@ func Client(forceNew bool) (client.Client, error) {
 	return client, nil
 }
 
+// DiscoveryClient returns a Kubernetes discovery client.
 func DiscoveryClient() (discovery.DiscoveryInterface, error) {
 	cfg, err := config.GetConfig()
 	if err != nil {
@@ -129,3 +93,35 @@ func DiscoveryClient() (discovery.DiscoveryInterface, error) {
 	}
 	return dclient, nil
 }
+
+// LoadObjectsFromFiles loads Kubernetes objects from YAML files and returns them.
+func loadObjectsFromFiles(files ...string) ([]client.Object, error) {
+	var objects []client.Object
+	for _, file := range files {
+		o, err := ObjectsFromPath(file, "")
+		if err != nil {
+			return nil, err
+		}
+		objects = append(objects, o...)
+	}
+	return objects, nil
+}
+
+// waitForObjects waits for specific Kubernetes objects' presence/absence in a namespace.
+func waitForObjects(s *Step, objects []client.Object, namespace string, timeout int) []error {
+	var testErrors []error
+	for i := 0; i < timeout; i++ {
+		// start fresh
+		testErrors = []error{}
+		for _, expected := range objects {
+			testErrors = append(testErrors, s.CheckResource(expected, namespace)...)
+		}
+
+		if len(testErrors) == 0 {
+			return nil
+		}
+
+		time.Sleep(time.Second)
+	}
+	return testErrors
+}

pkg/test/harness.go

@@ -489,7 +489,7 @@ func (h *Harness) Run() {
 // Setup spins up the test env based on configuration
 // It can be used to start env which can than be modified prior to running tests, otherwise use Run().
 func (h *Harness) Setup() {
-	rand.Seed(time.Now().UTC().UnixNano())
+	rand.New(rand.NewSource(time.Now().UTC().UnixNano()))
 	h.report = report.NewSuiteCollection(h.TestSuite.Name)
 	h.T.Log("starting setup")
 
@@ -612,6 +612,16 @@ func (h *Harness) Stop() {
 
 		h.kind = nil
 	}
+
+	h.T.Logf("Total test failures: %d", h.report.Failures)
+	for _, j := range h.report.Testsuite {
+		h.T.Logf("Test suite %s: %d failures", j.Name, j.Failures)
+		for _, v := range j.Testcase {
+			if v.Failure != nil {
+				h.T.Logf("test %s failed: %s", v.Name, v.Failure.Message)
+			}
+		}
+	}
 }
 
 // wraps Test.Fatal in order to clean up harness

pkg/test/kind.go

@@ -92,6 +92,7 @@ func (k *kind) Stop() error {
 	return k.Provider.Delete(k.context, k.explicitPath)
 }
 
+// loadContainer loads a Docker container image onto a KIND node.
 func loadContainer(docker testutils.DockerClient, node nodes.Node, container string) error {
 	image, err := docker.ImageSave(context.TODO(), []string{container})
 	if err != nil {
@@ -105,7 +106,5 @@ func loadContainer(docker testutils.DockerClient, node nodes.Node, container str
 
 // IsMinVersion checks if pass ver is the min required kind version
 func IsMinVersion(ver string) bool {
-	minVersion := "kind.sigs.k8s.io/v1alpha4"
-	comp := version.CompareKubeAwareVersionStrings(minVersion, ver)
-	return comp != -1
+	return version.CompareKubeAwareVersionStrings("kind.sigs.k8s.io/v1alpha4", ver) != -1
 }

pkg/test/step.go

@@ -173,6 +173,7 @@ func (s *Step) DeleteExisting(namespace string) error {
 	})
 }
 
+// doApply performs the actual object creation/update and cleanup.
 func doApply(test *testing.T, skipDelete bool, logger testutils.Logger, timeout int, dClient discovery.DiscoveryInterface, cl client.Client, obj client.Object, namespace string) error {
 	_, _, err := testutils.Namespaced(dClient, obj, namespace)
 	if err != nil {
@@ -265,6 +266,7 @@ func (s *Step) GetTimeout() int {
 	return timeout
 }
 
+// list returns a list of unstructured objects of a given GVK and namespace.
 func list(cl client.Client, gvk schema.GroupVersionKind, namespace string) ([]unstructured.Unstructured, error) {
 	list := unstructured.UnstructuredList{}
 	list.SetGroupVersionKind(gvk)
@@ -680,6 +682,7 @@ func cleanPath(path, dir string) string {
 	return filepath.Join(dir, path)
 }
 
+// hasTimeoutErr checks if the error list contains a context.DeadlineExceeded error.
 func hasTimeoutErr(err []error) bool {
 	for i := range err {
 		if errors.Is(err[i], context.DeadlineExceeded) {

pkg/test/utils/logger.go

@@ -54,7 +54,7 @@ func (t *TestLogger) WithPrefix(prefix string) Logger {
 
 // Write implements the io.Writer interface.
 // Logs each line written to it, buffers incomplete lines until the next Write() call.
-func (t *TestLogger) Write(p []byte) (n int, err error) {
+func (t *TestLogger) Write(p []byte) (int, error) {
 	t.buffer = append(t.buffer, p...)
 
 	splitBuf := bytes.Split(t.buffer, []byte{'\n'})

pkg/test/utils/subset.go

@@ -69,7 +69,7 @@ func IsSubset(expected, actual interface{}) error {
 			if !actualValue.IsValid() {
 				return &SubsetError{
 					path:    []string{iter.Key().String()},
-					message: "key is missing from map",
+					message: fmt.Sprintf("key '%s' is missing from map", iter.Key().String()),
 				}
 			}
 

pkg/test/utils/testing.go

@@ -21,25 +21,19 @@ func RunTests(testName string, testToRun string, parallelism int, testFunc func(
 	testing.Init()
 
 	// Set the verbose test flag to true since we are not using the regular go test CLI.
-	if err := flag.Set("test.v", "true"); err != nil {
-		panic(err)
-	}
+	setFlag("test.v", "true")
 
 	// Set the -run flag on the Go test harness.
 	// See the go test documentation: https://golang.org/pkg/cmd/go/internal/test/
 	if testToRun != "" {
-		if err := flag.Set("test.run", fmt.Sprintf("//%s", testToRun)); err != nil {
-			panic(err)
-		}
+		setFlag("test.run", fmt.Sprintf("//%s", testToRun))
 	}
 
 	parallelismStr := "8"
 	if parallelism != 0 {
 		parallelismStr = fmt.Sprintf("%d", parallelism)
 	}
-	if err := flag.Set("test.parallel", parallelismStr); err != nil {
-		panic(err)
-	}
+	setFlag("test.parallel", parallelismStr)
 
 	os.Exit(testing.MainStart(&testDeps{}, []testing.InternalTest{
 		{
@@ -49,6 +43,14 @@ func RunTests(testName string, testToRun string, parallelism int, testFunc func(
 	}, nil, nil, nil).Run())
 }
 
+// setFlag is a utility function that sets the value of a command-line flag using the flag package.
+// If an error occurs while setting the flag, the function panics with the provided error.
+func setFlag(name, value string) {
+	if err := flag.Set(name, value); err != nil {
+		panic(err)
+	}
+}
+
 // testDeps implements the testDeps interface for MainStart.
 type testDeps struct{}
 
@@ -79,7 +81,11 @@ func (testDeps) StopCPUProfile() {
 }
 
 func (testDeps) WriteProfileTo(name string, w io.Writer, debug int) error {
-	return pprof.Lookup(name).WriteTo(w, debug)
+	prof := pprof.Lookup(name)
+	if prof == nil {
+		return fmt.Errorf("profile %q not found", name)
+	}
+	return prof.WriteTo(w, debug)
 }
 
 func (testDeps) ImportPath() string {