refactor: output flag code duplication (datreeio#582)

* refactor: extract output flag validation to a single function * refactor: replace if_else with switch statement * refactor: a small refactor * test: add test case for empty output option
Segun76 · Apr 21, 2022 · 4ac1680 · 4ac1680
1 parent 52dd306
commit 4ac1680
Show file tree

Hide file tree

Showing 6 changed files with 48 additions and 23 deletions.
diff --git a/bl/evaluation/output_flag.go b/bl/evaluation/output_flag.go
@@ -0,0 +1,24 @@
+package evaluation
+
+var FormattedOutputOptions = []string{"yaml", "json", "xml", "JUnit"}
+var InteractiveOutputOptions = []string{"", "simple"}
+var ValidOutputOptions = append(FormattedOutputOptions, InteractiveOutputOptions...)
+var ExplicitOptionOptions = []string{"simple", "yaml", "json", "xml", "JUnit"}
+
+func IsValidOutputOption(option string) bool {
+	for _, validOption := range ValidOutputOptions {
+		if option == validOption {
+			return true
+		}
+	}
+	return false
+}
+
+func IsFormattedOutputOption(option string) bool {
+	for _, formattedOption := range FormattedOutputOptions {
+		if option == formattedOption {
+			return true
+		}
+	}
+	return false
+}
diff --git a/bl/evaluation/printer.go b/bl/evaluation/printer.go
@@ -51,7 +51,7 @@ type textOutputData struct {
 }
 
 func PrintResults(resultsData *PrintResultsData) error {
-	if resultsData.OutputFormat == "json" || resultsData.OutputFormat == "yaml" || resultsData.OutputFormat == "xml" || resultsData.OutputFormat == "JUnit" {
+	if IsFormattedOutputOption(resultsData.OutputFormat) {
 		nonInteractiveEvaluationResults := resultsData.Results.NonInteractiveEvaluationResults
 		if nonInteractiveEvaluationResults == nil {
 			nonInteractiveEvaluationResults = &NonInteractiveEvaluationResults{}
@@ -70,15 +70,16 @@ func PrintResults(resultsData *PrintResultsData) error {
 			K8sValidationResults:  resultsData.InvalidK8sFiles,
 		}
 
-		if resultsData.OutputFormat == "json" {
+		switch resultsData.OutputFormat {
+		case "json":
 			return jsonOutput(&formattedOutput)
-		} else if resultsData.OutputFormat == "yaml" {
+		case "yaml":
 			return yamlOutput(&formattedOutput)
-		} else if resultsData.OutputFormat == "xml" {
+		case "xml":
 			return xmlOutput(&formattedOutput)
-		} else if resultsData.OutputFormat == "JUnit" {
+		case "JUnit":
 			return jUnitOutput(&formattedOutput)
-		} else {
+		default:
 			panic(errors.New("invalid output format"))
 		}
 	} else {

diff --git a/bl/evaluation/printer_test.go b/bl/evaluation/printer_test.go
@@ -92,20 +92,20 @@ func TestCustomOutputs(t *testing.T) {
 	formattedOutput := createFormattedOutput()
 	expectedOutputs := getExpectedOutputs()
 
-	jsonStdout := readOutput(t, "json", formattedOutput)
+	jsonStdout := readOutput("json", formattedOutput)
 	assert.Equal(t, expectedOutputs.json, jsonStdout)
 
-	yamlStdout := readOutput(t, "yaml", formattedOutput)
+	yamlStdout := readOutput("yaml", formattedOutput)
 	assert.Equal(t, expectedOutputs.yaml, yamlStdout)
 
-	xmlStdout := readOutput(t, "xml", formattedOutput)
+	xmlStdout := readOutput("xml", formattedOutput)
 	assert.Equal(t, expectedOutputs.xml, xmlStdout)
 
-	JUnitStdout := readOutput(t, "JUnit", formattedOutput)
+	JUnitStdout := readOutput("JUnit", formattedOutput)
 	assert.Equal(t, expectedOutputs.JUnit, JUnitStdout)
 }
 
-func readOutput(t *testing.T, outputFormat string, formattedOutput FormattedOutput) string {
+func readOutput(outputFormat string, formattedOutput FormattedOutput) string {
 	reader, writer, err := os.Pipe()
 	if err != nil {
 		panic(err)
@@ -130,7 +130,9 @@ func readOutput(t *testing.T, outputFormat string, formattedOutput FormattedOutp
 		xmlOutput(&formattedOutput)
 	case outputFormat == "JUnit":
 		err := jUnitOutput(&formattedOutput)
-		assert.Equal(t, err, nil)
+		if err != nil {
+			panic("unexpected error in printer_test: " + err.Error())
+		}
 	}
 
 	writer.Close()

diff --git a/cmd/publish/main.go b/cmd/publish/main.go
@@ -2,6 +2,7 @@ package publish
 
 import (
 	"fmt"
+	"github.com/datreeio/datree/bl/evaluation"
 
 	"github.com/datreeio/datree/bl/files"
 	"github.com/datreeio/datree/bl/messager"
@@ -58,7 +59,7 @@ func New(ctx *PublishCommandContext) *cobra.Command {
 		},
 		PreRunE: func(cmd *cobra.Command, args []string) error {
 			outputFlag, _ := cmd.Flags().GetString("output")
-			if (outputFlag != "json") && (outputFlag != "yaml") && (outputFlag != "xml") && (outputFlag != "JUnit") {
+			if !evaluation.IsFormattedOutputOption(outputFlag) {
 
 				messages := ctx.Messager.LoadVersionMessages(ctx.CliVersion)
 				for msg := range messages {

diff --git a/cmd/test/main.go b/cmd/test/main.go
@@ -70,12 +70,9 @@ func NewTestCommandFlags() *TestCommandFlags {
 func (flags *TestCommandFlags) Validate() error {
 	outputValue := flags.Output
 
-	if outputValue != "" {
-		if (outputValue != "simple") && (outputValue != "json") && (outputValue != "yaml") && (outputValue != "xml") && (outputValue != "JUnit") {
-
-			return fmt.Errorf("Invalid --output option - %q\n"+
-				"Valid output values are - simple, yaml, json, xml, JUnit\n", outputValue)
-		}
+	if !evaluation.IsValidOutputOption(outputValue) {
+		return fmt.Errorf("Invalid --output option - %q\n"+
+			"Valid output values are - "+strings.Join(evaluation.ExplicitOptionOptions, ", ")+"\n", outputValue)
 	}
 
 	err := validateK8sVersionFormatIfProvided(flags.K8sVersion)
@@ -140,7 +137,7 @@ type TestCommandContext struct {
 
 func LoadVersionMessages(ctx *TestCommandContext, args []string, cmd *cobra.Command) error {
 	outputFlag, _ := cmd.Flags().GetString("output")
-	if (outputFlag != "json") && (outputFlag != "yaml") && (outputFlag != "xml") && (outputFlag != "JUnit") {
+	if !evaluation.IsFormattedOutputOption(outputFlag) {
 
 		messages := ctx.Messager.LoadVersionMessages(ctx.CliVersion)
 		for msg := range messages {
@@ -401,7 +398,7 @@ type EvaluationResultData struct {
 }
 
 func evaluate(ctx *TestCommandContext, filesPaths []string, prerunData *TestCommandData) (EvaluationResultData, error) {
-	isInteractiveMode := (prerunData.Output != "json") && (prerunData.Output != "yaml") && (prerunData.Output != "xml") && (prerunData.Output != "JUnit")
+	isInteractiveMode := !evaluation.IsFormattedOutputOption(prerunData.Output)
 
 	var _spinner *spinner.Spinner
 	if isInteractiveMode && prerunData.Output != "simple" {

diff --git a/cmd/test/main_test.go b/cmd/test/main_test.go
@@ -814,15 +814,15 @@ func executeTestCommand(ctx *TestCommandContext, args []string) error {
 
 func test_testCommand_output_flags_validation(t *testing.T, ctx *TestCommandContext) {
 
-	validOutputValues := [5]string{"simple", "json", "yaml", "xml", "JUnit"}
+	validOutputValues := [...]string{"", "simple", "json", "yaml", "xml", "JUnit"}
 
 	for _, value := range validOutputValues {
 		flags := TestCommandFlags{Output: value}
 		err := flags.Validate()
 		assert.NoError(t, err)
 	}
 
-	values := []string{"Simple", "Json", "Yaml", "Xml", "junit", "invalid", "113", "true"}
+	values := []string{" ", "Simple", "Json", "Yaml", "Xml", "junit", "invalid", "113", "true"}
 
 	for _, value := range values {
 		err := executeTestCommand(ctx, []string{"8/*", "--output=" + value})