Repository: doitintl/kube-no-trouble (go)

Issue:
Add config option to allow output to specific file, e.g. `--output-file`
This will allow for better integration in situations like #302, without requiring a shell in the container image.

Pull request:
Add support for configuring output file
This PR adds support for `-O` flag to configure output to a file to main binary and all the existing printers (json, text).

Related changes introduced:
- Increased test coverage for the common code and existing printers in the `printer` package
- Refactored Main tests to pass arguments via env variable, this was required to maintain consistency between runs (tmp dir)


Fixes #304

_Sonar CLoud complains about test func naming with underscores (which are generated by Goland and questionably a standard for unexported func tests in Go) and some repeated strings all in tests. I plan to ignore it... in the longer term, we should really get to enabling some linters in a way that makes sense), the SC is getting annoying._

Patch:
diff --git a/README.md b/README.md
index 6c43661..86662c4 100644
--- a/README.md
+++ b/README.md
@@ -87,9 +87,9 @@ Usage of ./kubent:
   -k, --kubeconfig string               path to the kubeconfig file
   -l, --log-level string                set log level (trace, debug, info, warn, error, fatal, panic, disabled) (default "info")
   -o, --output string                   output format - [text|json] (default "text")
+  -O, --output-file string        output file, use - for stdout (default "-")
   -t, --target-version string           target K8s version in SemVer format (autodetected by default)
   -v, --version                         prints the version of kubent and exits
-
 ```
 - *`--additional-annotation`*
   Check additional annotations for the last applied configuration. This can be useful if a resource was applied
diff --git a/cmd/kubent/main.go b/cmd/kubent/main.go
index 10426d2..c6b57cf 100644
--- a/cmd/kubent/main.go
+++ b/cmd/kubent/main.go
@@ -155,14 +155,9 @@ func main() {
 		log.Fatal().Err(err).Str("name", "Rego").Msg("Failed to filter results")
 	}
 
-	printer, err := printer.NewPrinter(config.Output)
+	err = outputResults(results, config.Output, config.OutputFile)
 	if err != nil {
-		log.Fatal().Err(err).Msg("Failed to create printer")
-	}
-
-	err = printer.Print(results)
-	if err != nil {
-		log.Fatal().Err(err).Msg("Failed to print results")
+		log.Fatal().Err(err).Msgf("Failed to output results")
 	}
 
 	if config.ExitError && len(results) > 0 {
@@ -172,3 +167,18 @@ func main() {
 	}
 	os.Exit(exitCode)
 }
+
+func outputResults(results []judge.Result, outputType string, outputFile string) error {
+	printer, err := printer.NewPrinter(outputType, outputFile)
+	if err != nil {
+		return fmt.Errorf("failed to create printer: %v", err)
+	}
+	defer printer.Close()
+
+	err = printer.Print(results)
+	if err != nil {
+		return fmt.Errorf("failed to print results: %v", err)
+	}
+
+	return nil
+}
diff --git a/pkg/config/config.go b/pkg/config/config.go
index c4ae2ed..dffd490 100644
--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@@ -1,7 +1,11 @@
 package config
 
 import (
+	"errors"
 	"fmt"
+	"io/fs"
+	"os"
+	"path/filepath"
 	"strings"
 	"unicode"
 
@@ -24,6 +28,7 @@ type Config struct {
 	Kubeconfig            string
 	LogLevel              ZeroLogLevel
 	Output                string
+	OutputFile            string
 	TargetVersion         *judge.Version
 	KubentVersion         bool
 }
@@ -45,6 +50,7 @@ func NewFromFlags() (*Config, error) {
 	flag.StringSliceVarP(&config.Filenames, "filename", "f", []string{}, "manifests to check, use - for stdin")
 	flag.StringVarP(&config.Kubeconfig, "kubeconfig", "k", "", "path to the kubeconfig file")
 	flag.StringVarP(&config.Output, "output", "o", "text", "output format - [text|json]")
+	flag.StringVarP(&config.OutputFile, "output-file", "O", "-", "output file, use - for stdout")
 	flag.VarP(&config.LogLevel, "log-level", "l", "set log level (trace, debug, info, warn, error, fatal, panic, disabled)")
 	flag.VarP(config.TargetVersion, "target-version", "t", "target K8s version in SemVer format (autodetected by default)")
 
@@ -54,6 +60,10 @@ func NewFromFlags() (*Config, error) {
 		return nil, fmt.Errorf("failed to validate argument output: %w", err)
 	}
 
+	if err := validateOutputFile(config.OutputFile); err != nil {
+		return nil, fmt.Errorf("failed to validate argument output-file: %w", err)
+	}
+
 	if err := validateAdditionalResources(config.AdditionalKinds); err != nil {
 		return nil, fmt.Errorf("failed to validate arguments: %w", err)
 	}
@@ -83,3 +93,20 @@ func validateAdditionalResources(resources []string) error {
 	}
 	return nil
 }
+
+// validateOutputFile checks if output file name is valid and if the
+// destination directory exists
+func validateOutputFile(outputFileName string) error {
+	if outputFileName == "" {
+		return fmt.Errorf("output file name can't be empty (use - for stdout)")
+	}
+
+	if outputFileName != "-" {
+		dir := filepath.Dir(outputFileName)
+		if _, err := os.Stat(dir); errors.Is(err, fs.ErrNotExist) {
+			return fmt.Errorf("output directory %s does not exist", dir)
+		}
+	}
+
+	return nil
+}
diff --git a/pkg/printer/json.go b/pkg/printer/json.go
index a17881f..e51756d 100644
--- a/pkg/printer/json.go
+++ b/pkg/printer/json.go
@@ -1,7 +1,7 @@
 package printer
 
 import (
-	"bytes"
+	"bufio"
 	"encoding/json"
 	"fmt"
 
@@ -9,30 +9,37 @@ import (
 )
 
 type jsonPrinter struct {
+	*commonPrinter
 }
 
-func newJSONPrinter() Printer {
-	return &jsonPrinter{}
-}
-
-func (c *jsonPrinter) populateOutput(results []judge.Result) (string, error) {
-	buffer := new(bytes.Buffer)
-	encoder := json.NewEncoder(buffer)
-	encoder.SetIndent("", "\t")
-
-	err := encoder.Encode(results)
+// newJSONPrinter creates new JSON printer that prints to given output file
+func newJSONPrinter(outputFileName string) (Printer, error) {
+	cp, err := newCommonPrinter(outputFileName)
 	if err != nil {
-		return "", err
+		return nil, fmt.Errorf("failed to create new common printer: %w", err)
 	}
-	return buffer.String(), nil
+	return &jsonPrinter{
+		commonPrinter: cp,
+	}, nil
 }
 
+// Close will free resources used by the printer
+func (c *jsonPrinter) Close() error {
+	return c.commonPrinter.Close()
+}
+
+// Print will print results in text format
 func (c *jsonPrinter) Print(results []judge.Result) error {
-	output, err := c.populateOutput(results)
+	writer := bufio.NewWriter(c.commonPrinter.outputFile)
+	defer writer.Flush()
+
+	encoder := json.NewEncoder(writer)
+	encoder.SetIndent("", "\t")
+
+	err := encoder.Encode(results)
 	if err != nil {
 		return err
 	}
-	fmt.Printf("%s", output)
 
 	return nil
 }
diff --git a/pkg/printer/printer.go b/pkg/printer/printer.go
index 3837244..6b10252 100644
--- a/pkg/printer/printer.go
+++ b/pkg/printer/printer.go
@@ -2,31 +2,75 @@ package printer
 
 import (
 	"fmt"
+	"os"
 
 	"github.com/doitintl/kube-no-trouble/pkg/judge"
 )
 
-var printers = map[string]func() Printer{
+var printers = map[string]func(string) (Printer, error){
 	"json": newJSONPrinter,
 	"text": newTextPrinter,
 }
 
 type Printer interface {
 	Print([]judge.Result) error
+	Close() error
 }
 
-func NewPrinter(choice string) (Printer, error) {
+type commonPrinter struct {
+	outputFile *os.File
+}
+
+// newCommonPrinter creates new printer that prints to given output file
+func newCommonPrinter(outputFileName string) (*commonPrinter, error) {
+	outputFile, err := ensureOutputFileExists(outputFileName)
+	if err != nil {
+		return nil, fmt.Errorf("failed to ensure output device: %v", err)
+	}
+
+	return &commonPrinter{
+		outputFile: outputFile,
+	}, nil
+}
+
+// NewPrinter creates new printer of given type that prints to given output file
+func NewPrinter(choice string, outputFileName string) (Printer, error) {
 	printer, err := ParsePrinter(choice)
 	if err != nil {
 		return nil, err
 	}
-	return printer(), nil
+	return printer(outputFileName)
+}
+
+// Close will free resources used by the printer
+func (c *commonPrinter) Close() error {
+	if c.outputFile.Name() != os.Stdout.Name() {
+		if err := c.outputFile.Close(); err != nil {
+			return fmt.Errorf("failed to close output file: %w", err)
+		}
+	}
+
+	return nil
 }
 
-func ParsePrinter(choice string) (func() Printer, error) {
+func ParsePrinter(choice string) (func(string) (Printer, error), error) {
 	printer, exists := printers[choice]
 	if !exists {
 		return nil, fmt.Errorf("unknown printer type %s", choice)
 	}
 	return printer, nil
 }
+
+// ensureOutputFileExists will open file for writing, or create one if non-existent
+func ensureOutputFileExists(fileName string) (*os.File, error) {
+	if fileName == "-" {
+		return os.Stdout, nil
+	}
+
+	of, err := os.OpenFile(fileName, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create/open output file %s: %w", fileName, err)
+	}
+
+	return of, nil
+}
diff --git a/pkg/printer/text.go b/pkg/printer/text.go
index e9cf7a5..f613cc7 100644
--- a/pkg/printer/text.go
+++ b/pkg/printer/text.go
@@ -2,7 +2,6 @@ package printer
 
 import (
 	"fmt"
-	"os"
 	"sort"
 	"strings"
 	"text/tabwriter"
@@ -11,10 +10,24 @@ import (
 )
 
 type textPrinter struct {
+	*commonPrinter
 }
 
-func newTextPrinter() Printer {
-	return &textPrinter{}
+// newTextPrinter creates new text printer that prints to given output file
+func newTextPrinter(outputFileName string) (Printer, error) {
+	cp, err := newCommonPrinter(outputFileName)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create new common printer: %w", err)
+	}
+
+	return &textPrinter{
+		commonPrinter: cp,
+	}, nil
+}
+
+// Close will free resources used by the printer
+func (c *textPrinter) Close() error {
+	return c.commonPrinter.Close()
 }
 
 func (c *textPrinter) Print(results []judge.Result) error {
@@ -33,7 +46,7 @@ func (c *textPrinter) Print(results []judge.Result) error {
 	})
 
 	ruleSet := ""
-	w := tabwriter.NewWriter(os.Stdout, 10, 0, 3, ' ', 0)
+	w := tabwriter.NewWriter(c.commonPrinter.outputFile, 10, 0, 3, ' ', 0)
 
 	for _, r := range results {
 		if ruleSet != r.RuleSet {


Test patch:
diff --git a/cmd/kubent/main_test.go b/cmd/kubent/main_test.go
index 2505fe0..997032b 100644
--- a/cmd/kubent/main_test.go
+++ b/cmd/kubent/main_test.go
@@ -1,11 +1,15 @@
 package main
 
 import (
+	"bytes"
+	"encoding/base64"
+	"encoding/json"
 	"errors"
+	"io/ioutil"
 	"os"
 	"os/exec"
 	"path/filepath"
-	"strconv"
+	"strings"
 	"testing"
 
 	"github.com/doitintl/kube-no-trouble/pkg/collector"
@@ -99,45 +103,59 @@ func TestStoreCollectorError(t *testing.T) {
 }
 
 func TestMainExitCodes(t *testing.T) {
+	tmpDir, err := ioutil.TempDir(os.TempDir(), "kubent-tests-")
+	if err != nil {
+		t.Fatalf("failed to create temp dir for testing: %v", err)
+	}
+	defer os.RemoveAll(tmpDir)
+
 	expectedJsonOutput, _ := os.ReadFile(filepath.Join(FIXTURES_DIR, "expected-json-output.json"))
 	helm2FlagDisabled := "--helm2=false"
 	helm3FlagDisabled := "--helm3=false"
 	clusterFlagDisabled := "--cluster=false"
 	testCases := []struct {
-		name     string
-		args     []string // file list
-		expected int      // number of manifests
-		stdout   string   // stdout
+		name        string
+		args        []string // file list
+		expected    int      // expected exit code
+		stdout      string   // expected stdout
+		outFileName string
 	}{
-		{"success", []string{clusterFlagDisabled, helm2FlagDisabled, helm3FlagDisabled}, 0, ""},
-		{"errorBadFlag", []string{"-c=not-boolean"}, 2, ""},
-		{"successFound", []string{"-o=json", clusterFlagDisabled, helm2FlagDisabled, helm3FlagDisabled, "-f=" + filepath.Join(FIXTURES_DIR, "deployment-v1beta1.yaml")}, 0, string(expectedJsonOutput)},
-		{"exitErrorFlagNone", []string{clusterFlagDisabled, helm2FlagDisabled, helm3FlagDisabled, "-e"}, 0, ""},
-		{"exitErrorFlagFound", []string{clusterFlagDisabled, helm2FlagDisabled, helm3FlagDisabled, "-e", "-f=" + filepath.Join(FIXTURES_DIR, "deployment-v1beta1.yaml")}, 200, ""},
-		{"version short flag set", []string{"-v"}, 0, ""},
-		{"version long flag set", []string{"--version"}, 0, ""},
-		{"empty text output", []string{}, 0, ""},
-		{"empty json output", []string{"-o=json"}, 0, "[]\n"},
+		{"success", []string{clusterFlagDisabled, helm2FlagDisabled, helm3FlagDisabled}, 0, "", ""},
+		{"errorBadFlag", []string{"-c=not-boolean"}, 2, "", ""},
+		{"successFound", []string{"-o=json", clusterFlagDisabled, helm2FlagDisabled, helm3FlagDisabled, "-f=" + filepath.Join(FIXTURES_DIR, "deployment-v1beta1.yaml")}, 0, string(expectedJsonOutput), ""},
+		{"exitErrorFlagNone", []string{clusterFlagDisabled, helm2FlagDisabled, helm3FlagDisabled, "-e"}, 0, "", ""},
+		{"exitErrorFlagFound", []string{clusterFlagDisabled, helm2FlagDisabled, helm3FlagDisabled, "-e", "-f=" + filepath.Join(FIXTURES_DIR, "deployment-v1beta1.yaml")}, 200, "", ""},
+		{"version short flag set", []string{"-v"}, 0, "", ""},
+		{"version long flag set", []string{"--version"}, 0, "", ""},
+		{"empty text output", []string{clusterFlagDisabled, helm2FlagDisabled, helm3FlagDisabled}, 0, "", ""},
+		{"empty json output", []string{"-o=json", clusterFlagDisabled, helm2FlagDisabled, helm3FlagDisabled}, 0, "[]\n", ""},
+		{"json-file", []string{"-o=json", clusterFlagDisabled, helm2FlagDisabled, helm3FlagDisabled, "-f=" + filepath.Join(FIXTURES_DIR, "deployment-v1beta1.yaml")}, 0, "", filepath.Join(tmpDir, "json-file.out")},
+		{"text-file", []string{"-o=json", clusterFlagDisabled, helm2FlagDisabled, helm3FlagDisabled, "-f=" + filepath.Join(FIXTURES_DIR, "deployment-v1beta1.yaml")}, 0, "", filepath.Join(tmpDir, "text-file.out")},
+		{"json-stdout", []string{"-o=json", clusterFlagDisabled, helm2FlagDisabled, helm3FlagDisabled, "-f=" + filepath.Join(FIXTURES_DIR, "deployment-v1beta1.yaml")}, 0, string(expectedJsonOutput), "-"},
+		{"error-bad-file", []string{clusterFlagDisabled, helm2FlagDisabled, helm3FlagDisabled}, 1, "", "/this/dir/is/unlikely/to/exist"},
 	}
 
 	if os.Getenv("TEST_EXIT_CODE") == "1" {
-		tc, err := strconv.Atoi(os.Getenv("TEST_CASE"))
-		if err != nil {
-			t.Errorf("failed to determine the test case num (TEST_CASE env var): %v", err)
-		}
-
-		os.Args = []string{os.Args[0]}
-		os.Args = append(os.Args, testCases[tc].args...)
+		args := []string{}
+		decodeBase64(&args, os.Getenv("TEST_ARGS"))
+		os.Args = append(os.Args[:1], args...)
 		main()
 		return
 	}
 
-	for i, tc := range testCases {
+	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			var ee *exec.ExitError
 
+			if tc.outFileName != "" {
+				tc.args = append(tc.args, "-O="+tc.outFileName)
+			}
+			base64Args, _ := encodeBase64(tc.args)
+
 			cmd := exec.Command(os.Args[0], "-test.run=TestMainExitCodes")
-			cmd.Env = append(os.Environ(), "TEST_EXIT_CODE=1", "TEST_CASE="+strconv.Itoa(i))
+			cmd.Env = append(os.Environ(),
+				"TEST_EXIT_CODE=1",
+				"TEST_ARGS="+base64Args)
 			out, err := cmd.Output()
 
 			if tc.expected == 0 && err != nil {
@@ -158,6 +176,13 @@ func TestMainExitCodes(t *testing.T) {
 			if tc.expected == 0 && err == nil && tc.stdout != string(out) {
 				t.Fatalf("expected to get stdout as %s, instead got %s", tc.stdout, out)
 			}
+
+			if tc.expected == 0 && err == nil && tc.outFileName != "" && tc.outFileName != "-" {
+				if fs, err := os.Stat(tc.outFileName); err != nil || fs.Size() == 0 {
+					t.Fatalf("expected non-empty outputdile: %s, got error: %v", tc.outFileName, err)
+				}
+			}
+
 		})
 	}
 }
@@ -211,3 +236,51 @@ func TestGetServerVersion(t *testing.T) {
 		t.Errorf("Expected %s version to be detected, instead got: %s", fakeVersion.String(), version.String())
 	}
 }
+
+func encodeBase64(args []string) (string, error) {
+	var buf bytes.Buffer
+	encoder := base64.NewEncoder(base64.StdEncoding, &buf)
+	err := json.NewEncoder(encoder).Encode(args)
+	if err != nil {
+		return "", err
+	}
+	encoder.Close()
+	return buf.String(), nil
+}
+
+func decodeBase64(dst *[]string, encoded string) error {
+	r := strings.NewReader(encoded)
+	base64Dec := base64.NewDecoder(base64.StdEncoding, r)
+	jsonDec := json.NewDecoder(base64Dec)
+
+	return jsonDec.Decode(dst)
+}
+
+func Test_outputResults(t *testing.T) {
+	testVersion, _ := judge.NewVersion("4.5.6")
+	testResults := []judge.Result{{"name", "ns", "kind",
+		"1.2.3", "rs", "rep", testVersion}}
+
+	type args struct {
+		results    []judge.Result
+		outputType string
+		outputFile string
+	}
+	tests := []struct {
+		name    string
+		args    args
+		wantErr bool
+	}{
+		{"good", args{testResults, "text", "-"}, false},
+		{"bad-new-printer-type", args{testResults, "unknown", "-"}, true},
+		{"bad-new-printer-file", args{testResults, "text", "/unlikely/to/exist/dir"}, true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if err := outputResults(tt.args.results, tt.args.outputType, tt.args.outputFile); (err != nil) != tt.wantErr {
+				t.Errorf("unexpected error - got: %v, wantErr: %v", err, tt.wantErr)
+			}
+		})
+	}
+}
diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go
index 559d122..654bac0 100644
--- a/pkg/config/config_test.go
+++ b/pkg/config/config_test.go
@@ -161,3 +161,24 @@ func TestContext(t *testing.T) {
 		}
 	}
 }
+
+func Test_validateOutputFile(t *testing.T) {
+	tests := []struct {
+		name    string
+		arg     string
+		wantErr bool
+	}{
+		{"empty", "", true},
+		{"does-not-exist", "/this/directory/is/unlikely/to/exist", true},
+		{"relative", "my.log", false},
+		{"absolute", "/my.log", false},
+		{"stdout", "-", false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if err := validateOutputFile(tt.arg); (err != nil) != tt.wantErr {
+				t.Errorf("expected error = %v, got %v instead", err, tt.wantErr)
+			}
+		})
+	}
+}
diff --git a/pkg/printer/json_test.go b/pkg/printer/json_test.go
index 77ab233..cee1f70 100644
--- a/pkg/printer/json_test.go
+++ b/pkg/printer/json_test.go
@@ -2,25 +2,101 @@ package printer
 
 import (
 	"encoding/json"
+	"io/ioutil"
+	"os"
+	"reflect"
 	"testing"
 
 	"github.com/doitintl/kube-no-trouble/pkg/judge"
 )
 
-func TestJsonPopulateOutput(t *testing.T) {
-	printer := &jsonPrinter{}
-	output, err := printer.populateOutput(testInput)
+func Test_newJSONPrinter(t *testing.T) {
+	tmpFile, err := ioutil.TempFile(os.TempDir(), "kubent-tests-")
+	if err != nil {
+		t.Fatalf("failed to create temp dir for testing: %v", err)
+	}
+	defer os.Remove(tmpFile.Name())
+
+	tests := []struct {
+		name           string
+		outputFileName string
+		wantErr        bool
+	}{
+		{"good-stdout", "-", false},
+		{"good-file", tmpFile.Name(), false},
+		{"bad-empty", "", true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := newJSONPrinter(tt.outputFileName)
+			if (err != nil) != tt.wantErr {
+				t.Fatalf("unexpected error: %v, expected error: %v", err, tt.wantErr)
+			}
+
+			if err != nil && got != nil {
+				t.Errorf("expected nil in case of an error, got %v", got)
+			}
+		})
+	}
+}
+
+func Test_jsonPrinter_Print(t *testing.T) {
+	tmpFile, err := ioutil.TempFile(os.TempDir(), "kubent-tests-")
+	if err != nil {
+		t.Fatalf("failed to create temp dir for testing: %v", err)
+	}
+	defer os.Remove(tmpFile.Name())
 
+	c := &jsonPrinter{
+		commonPrinter: &commonPrinter{tmpFile},
+	}
+
+	version, _ := judge.NewVersion("1.2.3")
+	results := []judge.Result{{"Name", "Namespace", "Kind", "1.2.3", "Test", "4.5.6", version}}
+
+	if err := c.Print(results); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	tmpFile.Seek(0, 0)
+
+	var readResults []judge.Result
+	readBytes, err := ioutil.ReadAll(tmpFile)
 	if err != nil {
-		t.Fatal(err)
+		t.Fatalf("unexpected error reading back the file: %v", err)
 	}
+	if err := json.Unmarshal(readBytes, &readResults); err != nil {
+		t.Fatalf("unexpected error unmarshalling the previously written file: %v", err)
+	}
+	if !reflect.DeepEqual(readResults, results) {
+		t.Fatalf("written and read result do not seem to be equal")
+	}
+}
 
-	var j []judge.Result
-	err = json.Unmarshal([]byte(output), &j)
+func Test_jsonPrinter_Close(t *testing.T) {
+	tmpFile, err := ioutil.TempFile(os.TempDir(), "kubent-tests-")
 	if err != nil {
-		t.Fatal(err)
+		t.Fatalf("failed to create temp dir for testing: %v", err)
+	}
+	defer os.Remove(tmpFile.Name())
+
+	tests := []struct {
+		name       string
+		outputFile *os.File
+		wantErr    bool
+	}{
+		{"good-file", tmpFile, false},
+		{"bad-closed-file", tmpFile, true},
 	}
-	if len(j) != len(testInput) {
-		t.Error("wrong number of results")
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			c := &jsonPrinter{
+				commonPrinter: &commonPrinter{tt.outputFile},
+			}
+			if err := c.Close(); (err != nil) != tt.wantErr {
+				t.Errorf("Unexpected error - got: %v, expected error: %v", err, tt.wantErr)
+			}
+		})
 	}
 }
diff --git a/pkg/printer/printer_test.go b/pkg/printer/printer_test.go
index 28fff15..d530452 100644
--- a/pkg/printer/printer_test.go
+++ b/pkg/printer/printer_test.go
@@ -1,24 +1,173 @@
 package printer
 
 import (
+	"io/ioutil"
+	"os"
+	"reflect"
 	"testing"
 )
 
-func TestParsePrinters(t *testing.T) {
+func TestParsePrinter(t *testing.T) {
 	for k, v := range printers {
-		printer, err := NewPrinter(k)
+		p, err := ParsePrinter(k)
 		if err != nil {
-			t.Fatal(err)
+			t.Fatalf("failed to parse printer %s: %v", k, err)
 		}
-		if printer != v() {
-			t.Fatal("Should be a valid printer")
+
+		if reflect.ValueOf(p).Pointer() != reflect.ValueOf(v).Pointer() {
+			t.Fatalf("expected to get function %p, got %p instead", p, p)
 		}
 	}
 }
 
-func TestInvalidStringForParsePrinters(t *testing.T) {
-	_, err := NewPrinter("BAD")
+func TestParsePrinterInvalid(t *testing.T) {
+	_, err := ParsePrinter("BAD")
 	if err == nil {
-		t.Fatal(err)
+		t.Fatalf("expected ParsePrinter to fail with unimplemented type")
+	}
+}
+
+func TestNewPrinter(t *testing.T) {
+	tmpFile, err := ioutil.TempFile(os.TempDir(), "kubent-tests-")
+	if err != nil {
+		t.Fatalf("failed to create temp dir for testing: %v", err)
+	}
+	defer os.Remove(tmpFile.Name())
+
+	tests := []struct {
+		name              string
+		argChoice         string
+		argOutputFileName string
+		wantErr           bool
+		want              Printer
+	}{
+		{"good", "json", "-", false, &jsonPrinter{&commonPrinter{}}},
+		{"good-file", "json", tmpFile.Name(), false, &jsonPrinter{&commonPrinter{}}},
+		{"invalid", "xxx", "", true, nil},
+		{"invalid-out", "json", "/not/likely/to/exist", true, nil},
+		{"empty", "", "-", true, nil},
+		{"empty-out", "json", "", true, nil},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := NewPrinter(tt.argChoice, tt.argOutputFileName)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("unexpected error - got: %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if reflect.TypeOf(got) != reflect.TypeOf(tt.want) {
+				t.Errorf("unexpected result - got: %v, want: %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func Test_newCommonPrinter(t *testing.T) {
+	tmpFile, err := ioutil.TempFile(os.TempDir(), "kubent-tests-")
+	if err != nil {
+		t.Fatalf("failed to create temp dir for testing: %v", err)
+	}
+	defer os.Remove(tmpFile.Name())
+
+	tests := []struct {
+		name              string
+		argOutputFileName string
+		wantErr           bool
+	}{
+		{"good-file", tmpFile.Name(), false},
+		{"good-stdout", "-", false},
+		{"bad-empty", "", true},
+		{"bad-path", "/this/is/unlikely/to/exist", true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := newCommonPrinter(tt.argOutputFileName)
+			if (err != nil) != tt.wantErr {
+				t.Fatalf("unexpected error - got: %v, wantErr: %v", err, tt.wantErr)
+			}
+
+			if err != nil && got != nil {
+				t.Fatalf("expected nil return in case of an error, got %v", got)
+			}
+
+			if !tt.wantErr {
+				defer got.Close()
+				if (tt.argOutputFileName != "-" && got.outputFile.Name() != tt.argOutputFileName) ||
+					tt.argOutputFileName == "-" && got.outputFile.Name() != os.Stdout.Name() {
+					t.Errorf("unexpected file name- got: %s, want: %s", got.outputFile.Name(), tt.argOutputFileName)
+				}
+			}
+		})
+	}
+}
+
+func Test_ensureOutputFileExists(t *testing.T) {
+	tmpFile, err := ioutil.TempFile(os.TempDir(), "kubent-tests-")
+	if err != nil {
+		t.Fatalf("failed to create temp dir for testing: %v", err)
+	}
+
+	tests := []struct {
+		name         string
+		argsFileName string
+		wantErr      bool
+	}{
+		{"stdout", "-", false},
+		{"path", tmpFile.Name(), false},
+		{"bad-dir", "/unlikely/to/exist/directory/my.log", true},
+		{"bad-empty", "", true},
+	}
+	defer os.Remove(tmpFile.Name())
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := ensureOutputFileExists(tt.argsFileName)
+			if (err != nil) != tt.wantErr {
+				t.Fatalf("unexpected error - got: %v, wantErr: %v", err, tt.wantErr)
+			}
+
+			if tt.wantErr {
+				if got != nil {
+					t.Fatalf("expected nil return in case of an error, got %v", got)
+				}
+			} else {
+				if got == nil {
+					t.Fatalf("unexpected nil return, got %v", got)
+				}
+				if got.Name() != tt.argsFileName && tt.argsFileName != "-" ||
+					tt.argsFileName == "-" && got.Name() != os.Stdout.Name() {
+					t.Fatalf("expected os.File with Name: %s, got: %s", tt.argsFileName, got.Name())
+				}
+			}
+		})
+	}
+}
+
+func Test_commonPrinter_Close(t *testing.T) {
+	tmpFile, err := ioutil.TempFile(os.TempDir(), "kubent-tests-")
+	if err != nil {
+		t.Fatalf("failed to create temp dir for testing: %v", err)
+	}
+	defer os.Remove(tmpFile.Name())
+
+	tests := []struct {
+		name       string
+		outputFile *os.File
+		wantErr    bool
+	}{
+		{"good-file", tmpFile, false},
+		{"good-stdout", os.Stdout, false},
+		{"bad-closed-file", tmpFile, true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			c := &commonPrinter{
+				outputFile: tt.outputFile,
+			}
+			if err := c.Close(); (err != nil) != tt.wantErr {
+				t.Errorf("Unexpected error - got: %v, expected error: %v", err, tt.wantErr)
+			}
+		})
 	}
 }
diff --git a/pkg/printer/text_test.go b/pkg/printer/text_test.go
new file mode 100644
index 0000000..640ad47
--- /dev/null
+++ b/pkg/printer/text_test.go
@@ -0,0 +1,90 @@
+package printer
+
+import (
+	"github.com/doitintl/kube-no-trouble/pkg/judge"
+	"io/ioutil"
+	"os"
+	"testing"
+)
+
+func Test_newTextPrinter(t *testing.T) {
+	tmpFile, err := ioutil.TempFile(os.TempDir(), "kubent-tests-")
+	if err != nil {
+		t.Fatalf("failed to create temp dir for testing: %v", err)
+	}
+	defer os.Remove(tmpFile.Name())
+
+	tests := []struct {
+		name           string
+		outputFileName string
+		wantErr        bool
+	}{
+		{"good-stdout", "-", false},
+		{"good-file", tmpFile.Name(), false},
+		{"bad-empty", "", true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := newTextPrinter(tt.outputFileName)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("Unexpected error %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+
+			if err != nil && got != nil {
+				t.Errorf("expected nil in case of an error, got %v", got)
+			}
+		})
+	}
+}
+
+func Test_textPrinter_Print(t *testing.T) {
+	tmpFile, err := ioutil.TempFile(os.TempDir(), "kubent-tests-")
+	if err != nil {
+		t.Fatalf("failed to create temp dir for testing: %v", err)
+	}
+	defer os.Remove(tmpFile.Name())
+
+	tp := &textPrinter{
+		commonPrinter: &commonPrinter{tmpFile},
+	}
+
+	version, _ := judge.NewVersion("1.2.3")
+	results := []judge.Result{{"Name", "Namespace", "Kind", "1.2.3", "Test", "4.5.6", version}}
+
+	if err := tp.Print(results); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	fi, _ := tmpFile.Stat()
+	if fi.Size() == 0 {
+		t.Fatalf("expected non-zero size output file: %v", err)
+	}
+}
+
+func Test_textPrinter_Close(t *testing.T) {
+	tmpFile, err := ioutil.TempFile(os.TempDir(), "kubent-tests-")
+	if err != nil {
+		t.Fatalf("failed to create temp dir for testing: %v", err)
+	}
+	defer os.Remove(tmpFile.Name())
+
+	tests := []struct {
+		name       string
+		outputFile *os.File
+		wantErr    bool
+	}{
+		{"good-file", tmpFile, false},
+		{"bad-closed-file", tmpFile, true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			c := &textPrinter{
+				commonPrinter: &commonPrinter{tt.outputFile},
+			}
+			if err := c.Close(); (err != nil) != tt.wantErr {
+				t.Errorf("Unexpected error - got: %v, expected error: %v", err, tt.wantErr)
+			}
+		})
+	}
+}