start passing io.Writer for ignoring output in tests

Signed-off-by: ezgidemirel <ezgidemirel91@gmail.com>
crossplane · Feb 2, 2024 · 45628bb · 45628bb
1 parent 13448ed
commit 45628bb
Show file tree

Hide file tree

Showing 6 changed files with 44 additions and 15 deletions.
diff --git a/cmd/crank/beta/validate/cache.go b/cmd/crank/beta/validate/cache.go
@@ -17,7 +17,6 @@ limitations under the License.
 package validate
 
 import (
-	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
@@ -26,6 +25,7 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 
 	"github.com/crossplane/crossplane-runtime/pkg/errors"
+	"github.com/crossplane/crossplane-runtime/pkg/logging"
 )
 
 // Cache defines an interface for caching schemas
@@ -41,6 +41,7 @@ type Cache interface {
 type LocalCache struct {
 	fs       afero.Fs
 	cacheDir string
+	logger   logging.Logger
 }
 
 // Store stores the schemas in the directory
@@ -84,7 +85,8 @@ func (c *LocalCache) Init() error {
 
 // Flush removes the cache directory
 func (c *LocalCache) Flush() error {
-	fmt.Println("flushing cache directory: ", c.cacheDir)
+	c.logger.Info("flushing cache directory: ", c.cacheDir)
+
 	return c.fs.RemoveAll(c.cacheDir)
 }
 

diff --git a/cmd/crank/beta/validate/cache_test.go b/cmd/crank/beta/validate/cache_test.go
@@ -1,6 +1,7 @@
 package validate
 
 import (
+	"bytes"
 	"path/filepath"
 	"testing"
 
@@ -79,10 +80,12 @@ func TestLocalCacheFlush(t *testing.T) {
 		},
 	}
 	for name, tc := range cases {
+		w := &bytes.Buffer{}
 		t.Run(name, func(t *testing.T) {
 			c := &LocalCache{
 				fs:       fs,
 				cacheDir: "testdata/cache",
+				w:        w,
 			}
 
 			err := c.Flush()

diff --git a/cmd/crank/beta/validate/cmd.go b/cmd/crank/beta/validate/cmd.go
@@ -81,7 +81,7 @@ func (c *Cmd) AfterApply() error {
 }
 
 // Run validate.
-func (c *Cmd) Run(_ *kong.Context, _ logging.Logger) error { //nolint:gocyclo // stdin check makes it over the top
+func (c *Cmd) Run(k *kong.Context, logger logging.Logger) error { //nolint:gocyclo // stdin check makes it over the top
 	if c.Resources == "-" && c.Extensions == "-" {
 		return errors.New("cannot use stdin for both extensions and resources")
 	}
@@ -117,7 +117,7 @@ func (c *Cmd) Run(_ *kong.Context, _ logging.Logger) error { //nolint:gocyclo //
 		c.CacheDir = filepath.Join(currentPath, c.CacheDir)
 	}
 
-	m := NewManager(c.CacheDir, c.fs)
+	m := NewManager(c.CacheDir, c.fs, logger)
 
 	// Convert XRDs/CRDs to CRDs and add package dependencies
 	if err := m.PrepExtensions(extensions); err != nil {
@@ -130,7 +130,7 @@ func (c *Cmd) Run(_ *kong.Context, _ logging.Logger) error { //nolint:gocyclo //
 	}
 
 	// Validate resources against schemas
-	if err := SchemaValidation(resources, m.crds, c.SkipSuccessResults); err != nil {
+	if err := SchemaValidation(resources, m.crds, c.SkipSuccessResults, k.Stdout); err != nil {
 		return errors.Wrapf(err, "cannot validate resources")
 	}
 

diff --git a/cmd/crank/beta/validate/manager.go b/cmd/crank/beta/validate/manager.go
@@ -18,6 +18,7 @@ package validate
 
 import (
 	"fmt"
+	"github.com/crossplane/crossplane-runtime/pkg/logging"
 
 	"github.com/spf13/afero"
 	apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
@@ -47,22 +48,25 @@ const (
 type Manager struct {
 	fetcher ImageFetcher
 	cache   Cache
+	logger  logging.Logger
 
 	crds  []*apiextv1.CustomResourceDefinition
 	deps  map[string]bool // One level dependency images
 	confs map[string]bool // Configuration images
 }
 
 // NewManager returns a new Manager
-func NewManager(cacheDir string, fs afero.Fs) *Manager {
+func NewManager(cacheDir string, fs afero.Fs, logger logging.Logger) *Manager {
 	m := &Manager{}
 
 	m.cache = &LocalCache{
 		fs:       fs,
 		cacheDir: cacheDir,
+		logger:   logger,
 	}
 
 	m.fetcher = &Fetcher{}
+	m.logger = logger
 
 	m.crds = make([]*apiextv1.CustomResourceDefinition, 0)
 	m.deps = make(map[string]bool)
@@ -218,7 +222,7 @@ func (m *Manager) cacheDependencies() error {
 			continue
 		}
 
-		fmt.Printf("package schemas does not exist, downloading: %s\n", image)
+		m.logger.Info("package schemas does not exist, downloading: ", image)
 
 		layer, err := m.fetcher.FetchBaseLayer(image)
 		if err != nil {

diff --git a/cmd/crank/beta/validate/validate.go b/cmd/crank/beta/validate/validate.go
@@ -19,6 +19,7 @@ package validate
 import (
 	"context"
 	"fmt"
+	"io"
 
 	ext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
 	extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
@@ -34,6 +35,11 @@ import (
 	"github.com/crossplane/crossplane/internal/controller/apiextensions/composite"
 )
 
+const (
+	errWriteOutput    = "cannot write output"
+	errFlushTabWriter = "cannot flush tab writer"
+)
+
 func newValidatorsAndStructurals(crds []*extv1.CustomResourceDefinition) (map[runtimeschema.GroupVersionKind][]*validation.SchemaValidator, map[runtimeschema.GroupVersionKind]*schema.Structural, error) {
 	validators := map[runtimeschema.GroupVersionKind][]*validation.SchemaValidator{}
 	structurals := map[runtimeschema.GroupVersionKind]*schema.Structural{}
@@ -86,7 +92,7 @@ func newValidatorsAndStructurals(crds []*extv1.CustomResourceDefinition) (map[ru
 }
 
 // SchemaValidation validates the resources against the given CRDs
-func SchemaValidation(resources []*unstructured.Unstructured, crds []*extv1.CustomResourceDefinition, skipSuccessLogs bool) error {
+func SchemaValidation(resources []*unstructured.Unstructured, crds []*extv1.CustomResourceDefinition, skipSuccessLogs bool, w io.Writer) error { //nolint:gocyclo // printing the output increases the cyclomatic complexity a little bit
 	schemaValidators, structurals, err := newValidatorsAndStructurals(crds)
 	if err != nil {
 		return errors.Wrap(err, "cannot create schema validators")
@@ -99,7 +105,10 @@ func SchemaValidation(resources []*unstructured.Unstructured, crds []*extv1.Cust
 		sv, ok := schemaValidators[gvk]
 		if !ok {
 			missingSchemas++
-			fmt.Println("[!] could not find CRD/XRD for: " + r.GroupVersionKind().String())
+			if _, err := fmt.Fprintf(w, "[!] could not find CRD/XRD for: %s\n", r.GroupVersionKind().String()); err != nil {
+				return errors.Wrap(err, errWriteOutput)
+			}
+
 			continue
 		}
 
@@ -109,7 +118,9 @@ func SchemaValidation(resources []*unstructured.Unstructured, crds []*extv1.Cust
 			re := validation.ValidateCustomResource(nil, r, *v)
 			for _, e := range re {
 				rf++
-				fmt.Printf("[x] schema validation error %s, %s : %s\n", r.GroupVersionKind().String(), getResourceName(r), e.Error())
+				if _, err := fmt.Fprintf(w, "[x] schema validation error %s, %s : %s\n", r.GroupVersionKind().String(), getResourceName(r), e.Error()); err != nil {
+					return errors.Wrap(err, errWriteOutput)
+				}
 			}
 
 			s := structurals[gvk] // if we have a schema validator, we should also have a structural
@@ -118,18 +129,24 @@ func SchemaValidation(resources []*unstructured.Unstructured, crds []*extv1.Cust
 			re, _ = celValidator.Validate(context.TODO(), nil, s, resources[i].Object, nil, celconfig.PerCallLimit)
 			for _, e := range re {
 				rf++
-				fmt.Printf("[x] CEL validation error %s, %s : %s\n", r.GroupVersionKind().String(), getResourceName(r), e.Error())
+				if _, err := fmt.Fprintf(w, "[x] CEL validation error %s, %s : %s\n", r.GroupVersionKind().String(), getResourceName(r), e.Error()); err != nil {
+					return errors.Wrap(err, errWriteOutput)
+				}
 			}
 
 			if rf == 0 && !skipSuccessLogs {
-				fmt.Printf("[✓] %s, %s validated successfully\n", r.GroupVersionKind().String(), getResourceName(r))
+				if _, err := fmt.Fprintf(w, "[✓] %s, %s validated successfully\n", r.GroupVersionKind().String(), getResourceName(r)); err != nil {
+					return errors.Wrap(err, errWriteOutput)
+				}
 			} else {
 				failure++
 			}
 		}
 	}
 
-	fmt.Printf("Total %d resources: %d missing schemas, %d success cases, %d failure cases\n", len(resources), missingSchemas, len(resources)-failure-missingSchemas, failure)
+	if _, err := fmt.Fprintf(w, "Total %d resources: %d missing schemas, %d success cases, %d failure cases\n", len(resources), missingSchemas, len(resources)-failure-missingSchemas, failure); err != nil {
+		return errors.Wrap(err, errWriteOutput)
+	}
 
 	if failure > 0 {
 		return errors.New("could not validate all resources")

diff --git a/cmd/crank/beta/validate/validate_test.go b/cmd/crank/beta/validate/validate_test.go
@@ -17,6 +17,7 @@ limitations under the License.
 package validate
 
 import (
+	"bytes"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -1153,7 +1154,8 @@ func TestConvertToCRDs(t *testing.T) {
 
 	for name, tc := range cases {
 		t.Run(name, func(t *testing.T) {
-			m := NewManager("", nil)
+			w := &bytes.Buffer{}
+			m := NewManager("", nil, w)
 			err := m.PrepExtensions(tc.args.schemas)
 
 			if diff := cmp.Diff(tc.want.crd, m.crds); diff != "" {
@@ -1300,7 +1302,8 @@ func TestValidateResources(t *testing.T) {
 	}
 	for name, tc := range cases {
 		t.Run(name, func(t *testing.T) {
-			got := SchemaValidation(tc.args.resources, tc.args.crds, false)
+			w := &bytes.Buffer{}
+			got := SchemaValidation(tc.args.resources, tc.args.crds, false, w)
 
 			if diff := cmp.Diff(tc.want.err, got, test.EquateErrors()); diff != "" {
 				t.Errorf("%s\nvalidateResources(...): -want error, +got error:\n%s", tc.reason, diff)