Revert "minor code refactoring and some improved error msgs"

This reverts commit f5ac481.
Reverting this commit as i want this branch free from other changes and only contain the validation changes.

Signed-off-by: AdiAkhileshSingh15 <adiakhilesh.singh.cse21@itbhu.ac.in>

pkg/http/client.go

@@ -11,7 +11,6 @@ import (
 	"strings"
 
 	"github.com/dustin/go-humanize"
-	"github.com/pkg/errors"
 
 	"github.com/kyverno/kuttl/pkg/version"
 )
@@ -41,20 +40,21 @@ func (c *Client) GetByteBuffer(url string) (*bytes.Buffer, error) {
 
 	resp, err := c.Get(url)
 	if err != nil {
-		return buf, errors.Wrapf(err, "failed to fetch %s", url)
+		return buf, err
 	}
-	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, errors.Wrap(err, "error while copying response body")
+		return nil, err
+	}
+	err = resp.Body.Close()
+	if err != nil {
+		return nil, fmt.Errorf("error when closing the response body %s", err)
 	}
-
-	return buf, nil
+	return buf, err
 }
 
 // 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 errors.Wrap(err, "failed to perform HTTP GET")
+		return err
 	}
 	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 errors.Wrap(err, "error checking file existence")
+		return err
 	}
 
 	// 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 errors.Wrap(err, "error creating temporary file")
+		return err
 	}
 	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 errors.Wrap(err, "error while copying response body to file")
+		return err
 	}
 
 	// 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 errors.Wrap(err, "error renaming temporary file")
+		return err
 	}
 
 	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 a URL
+// IsURL returns true if string is an URL
 func IsURL(str string) bool {
 	u, err := url.Parse(str)
 	return err == nil && u.Scheme != "" && u.Host != ""

pkg/test/assert.go

@@ -14,9 +14,14 @@ 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 {
-	objects, err := loadObjectsFromFiles(assertFiles...)
-	if err != nil {
-		return err
+	var objects []client.Object
+
+	for _, file := range assertFiles {
+		o, err := ObjectsFromPath(file, "")
+		if err != nil {
+			return err
+		}
+		objects = append(objects, o...)
 	}
 
 	// feels like the wrong abstraction, need to do some refactoring
@@ -26,7 +31,20 @@ func Assert(namespace string, timeout int, assertFiles ...string) error {
 		DiscoveryClient: DiscoveryClient,
 	}
 
-	testErrors := waitForObjects(s, objects, namespace, timeout)
+	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)
+	}
 
 	if len(testErrors) == 0 {
 		fmt.Printf("assert is valid\n")
@@ -41,9 +59,14 @@ 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 {
-	objects, err := loadObjectsFromFiles(errorFiles...)
-	if err != nil {
-		return err
+	var objects []client.Object
+
+	for _, file := range errorFiles {
+		o, err := ObjectsFromPath(file, "")
+		if err != nil {
+			return err
+		}
+		objects = append(objects, o...)
 	}
 
 	// feels like the wrong abstraction, need to do some refactoring
@@ -53,7 +76,22 @@ func Errors(namespace string, timeout int, errorFiles ...string) error {
 		DiscoveryClient: DiscoveryClient,
 	}
 
-	testErrors := waitForObjects(s, objects, namespace, timeout)
+	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)
+	}
 
 	if len(testErrors) == 0 {
 		fmt.Printf("error assert is valid\n")
@@ -66,7 +104,6 @@ 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 {
@@ -81,7 +118,6 @@ 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 {
@@ -93,35 +129,3 @@ 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.New(rand.NewSource(time.Now().UTC().UnixNano()))
+	rand.Seed(time.Now().UTC().UnixNano())
 	h.report = report.NewSuiteCollection(h.TestSuite.Name)
 	h.T.Log("starting setup")
 

pkg/test/kind.go

@@ -92,7 +92,6 @@ 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 {
@@ -106,5 +105,7 @@ 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 {
-	return version.CompareKubeAwareVersionStrings("kind.sigs.k8s.io/v1alpha4", ver) != -1
+	minVersion := "kind.sigs.k8s.io/v1alpha4"
+	comp := version.CompareKubeAwareVersionStrings(minVersion, ver)
+	return comp != -1
 }

pkg/test/step.go

@@ -174,7 +174,6 @@ 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 {
@@ -267,7 +266,6 @@ 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)
@@ -688,7 +686,6 @@ 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) (int, error) {
+func (t *TestLogger) Write(p []byte) (n int, err 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: fmt.Sprintf("key '%s' is missing from map", iter.Key().String()),
+					message: "key is missing from map",
 				}
 			}
 

pkg/test/utils/testing.go

@@ -21,19 +21,25 @@ 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.
-	setFlag("test.v", "true")
+	if err := flag.Set("test.v", "true"); err != nil {
+		panic(err)
+	}
 
 	// Set the -run flag on the Go test harness.
 	// See the go test documentation: https://golang.org/pkg/cmd/go/internal/test/
 	if testToRun != "" {
-		setFlag("test.run", fmt.Sprintf("//%s", testToRun))
+		if err := flag.Set("test.run", fmt.Sprintf("//%s", testToRun)); err != nil {
+			panic(err)
+		}
 	}
 
 	parallelismStr := "8"
 	if parallelism != 0 {
 		parallelismStr = fmt.Sprintf("%d", parallelism)
 	}
-	setFlag("test.parallel", parallelismStr)
+	if err := flag.Set("test.parallel", parallelismStr); err != nil {
+		panic(err)
+	}
 
 	os.Exit(testing.MainStart(&testDeps{}, []testing.InternalTest{
 		{
@@ -43,14 +49,6 @@ 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{}
 
@@ -81,11 +79,7 @@ func (testDeps) StopCPUProfile() {
 }
 
 func (testDeps) WriteProfileTo(name string, w io.Writer, debug int) error {
-	prof := pprof.Lookup(name)
-	if prof == nil {
-		return fmt.Errorf("profile %q not found", name)
-	}
-	return prof.WriteTo(w, debug)
+	return pprof.Lookup(name).WriteTo(w, debug)
 }
 
 func (testDeps) ImportPath() string {