From dd95e000763b78b1c6169f92bb9426ffe92ea1d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Wed, 7 Dec 2022 13:02:43 +0100 Subject: [PATCH] refactor: improve color management in cli test (#5609) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Charles-Edouard Brétéché Signed-off-by: Charles-Edouard Brétéché --- cmd/cli/kubectl-kyverno/test/output.go | 40 ++++++ cmd/cli/kubectl-kyverno/test/test_command.go | 126 +++---------------- 2 files changed, 60 insertions(+), 106 deletions(-) create mode 100644 cmd/cli/kubectl-kyverno/test/output.go diff --git a/cmd/cli/kubectl-kyverno/test/output.go b/cmd/cli/kubectl-kyverno/test/output.go new file mode 100644 index 0000000000..e8b429bf25 --- /dev/null +++ b/cmd/cli/kubectl-kyverno/test/output.go @@ -0,0 +1,40 @@ +package test + +import ( + "os" + + "github.com/fatih/color" + "github.com/kataras/tablewriter" + "github.com/lensesio/tableprinter" +) + +var ( + boldGreen = color.New(color.FgGreen).Add(color.Bold) + boldRed = color.New(color.FgRed).Add(color.Bold) + boldYellow = color.New(color.FgYellow).Add(color.Bold) + boldFgCyan = color.New(color.FgCyan).Add(color.Bold) +) + +func colorize(noColor bool, color *color.Color, format string, a ...interface{}) string { + if noColor { + return format + } + return color.Sprintf(format, a...) +} + +func newTablePrinter(noColor bool) *tableprinter.Printer { + printer := tableprinter.New(os.Stdout) + printer.BorderTop, printer.BorderBottom, printer.BorderLeft, printer.BorderRight = true, true, true, true + printer.CenterSeparator = "│" + printer.ColumnSeparator = "│" + printer.RowSeparator = "─" + printer.RowCharLimit = 300 + printer.RowLengthTitle = func(rowsLength int) bool { + return rowsLength > 10 + } + if !noColor { + printer.HeaderBgColor = tablewriter.BgBlackColor + printer.HeaderFgColor = tablewriter.FgGreenColor + } + return printer +} diff --git a/cmd/cli/kubectl-kyverno/test/test_command.go b/cmd/cli/kubectl-kyverno/test/test_command.go index d9372dcdd7..9c512af3e5 100644 --- a/cmd/cli/kubectl-kyverno/test/test_command.go +++ b/cmd/cli/kubectl-kyverno/test/test_command.go @@ -13,10 +13,8 @@ import ( "strings" "time" - "github.com/fatih/color" "github.com/go-git/go-billy/v5" "github.com/go-git/go-billy/v5/memfs" - "github.com/kataras/tablewriter" kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" "github.com/kyverno/kyverno/api/kyverno/v1beta1" policyreportv1alpha2 "github.com/kyverno/kyverno/api/policyreport/v1alpha2" @@ -32,7 +30,6 @@ import ( "github.com/kyverno/kyverno/pkg/openapi" policy2 "github.com/kyverno/kyverno/pkg/policy" gitutils "github.com/kyverno/kyverno/pkg/utils/git" - "github.com/lensesio/tableprinter" "github.com/spf13/cobra" "golang.org/x/exp/slices" corev1 "k8s.io/api/core/v1" @@ -377,7 +374,7 @@ func testCommandExecute(dirPath []string, fileName string, gitBranch string, tes fmt.Printf("\n") if rc.Fail > 0 && !failOnly { - printFailedTestResult() + printFailedTestResult(removeColor) os.Exit(1) } os.Exit(0) @@ -951,12 +948,8 @@ func applyPoliciesFromPath(fs billy.Filesystem, policyBytes []byte, isGit bool, } func printTestResult(resps map[string]policyreportv1alpha2.PolicyReportResult, testResults []api.TestResults, rc *resultCounts, failOnly, removeColor bool) error { - printer := tableprinter.New(os.Stdout) + printer := newTablePrinter(removeColor) table := []Table{} - boldGreen := color.New(color.FgGreen).Add(color.Bold) - boldRed := color.New(color.FgRed).Add(color.Bold) - boldYellow := color.New(color.FgYellow).Add(color.Bold) - boldFgCyan := color.New(color.FgCyan).Add(color.Bold) var countDeprecatedResource int testCount := 1 @@ -966,23 +959,14 @@ func printTestResult(resps map[string]policyreportv1alpha2.PolicyReportResult, t if v.Resources == nil { testCount++ } - if !removeColor { - res.Policy = boldFgCyan.Sprintf(v.Policy) - res.Rule = boldFgCyan.Sprintf(v.Rule) - } else { - res.Policy = v.Policy - res.Rule = v.Rule - } + res.Policy = colorize(removeColor, boldFgCyan, v.Policy) + res.Rule = colorize(removeColor, boldFgCyan, v.Rule) if v.Resources != nil { for _, resource := range v.Resources { res.ID = testCount testCount++ - if !removeColor { - res.Resource = boldFgCyan.Sprintf(v.Namespace) + "/" + boldFgCyan.Sprintf(v.Kind) + "/" + boldFgCyan.Sprintf(resource) - } else { - res.Resource = v.Namespace + "/" + v.Kind + "/" + resource - } + res.Resource = colorize(removeColor, boldFgCyan, v.Namespace) + "/" + colorize(removeColor, boldFgCyan, v.Kind) + "/" + colorize(removeColor, boldFgCyan, resource) var ruleNameInResultKey string if v.AutoGeneratedRule != "" { ruleNameInResultKey = fmt.Sprintf("%s-%s", v.AutoGeneratedRule, v.Rule) @@ -998,19 +982,10 @@ func printTestResult(resps map[string]policyreportv1alpha2.PolicyReportResult, t resultKey = fmt.Sprintf("%s-%s-%s-%s-%s-%s", ns, v.Policy, ruleNameInResultKey, v.Namespace, v.Kind, resource) } else if found { resultKey = fmt.Sprintf("%s-%s-%s-%s-%s", ns, v.Policy, ruleNameInResultKey, v.Kind, resource) - if !removeColor { - res.Policy = boldFgCyan.Sprintf(ns) + "/" + boldFgCyan.Sprintf(v.Policy) - res.Resource = boldFgCyan.Sprintf(v.Namespace) + "/" + boldFgCyan.Sprintf(v.Kind) + "/" + boldFgCyan.Sprintf(resource) - } else { - res.Policy = ns + "/" + v.Policy - res.Resource = v.Namespace + "/" + v.Kind + "/" + resource - } + res.Policy = colorize(removeColor, boldFgCyan, ns) + "/" + colorize(removeColor, boldFgCyan, v.Policy) + res.Resource = colorize(removeColor, boldFgCyan, v.Namespace) + "/" + colorize(removeColor, boldFgCyan, v.Kind) + "/" + colorize(removeColor, boldFgCyan, resource) } else if v.Namespace != "" { - if !removeColor { - res.Resource = boldFgCyan.Sprintf(v.Namespace) + "/" + boldFgCyan.Sprintf(v.Kind) + "/" + boldFgCyan.Sprintf(resource) - } else { - res.Resource = v.Namespace + "/" + v.Kind + "/" + resource - } + res.Resource = colorize(removeColor, boldFgCyan, v.Namespace) + "/" + colorize(removeColor, boldFgCyan, v.Kind) + "/" + colorize(removeColor, boldFgCyan, resource) resultKey = fmt.Sprintf("%s-%s-%s-%s-%s", v.Policy, ruleNameInResultKey, v.Namespace, v.Kind, resource) } @@ -1019,11 +994,7 @@ func printTestResult(resps map[string]policyreportv1alpha2.PolicyReportResult, t testRes = val } else { log.Log.V(2).Info("result not found", "key", resultKey) - if !removeColor { - res.Result = boldYellow.Sprintf("Not found") - } else { - res.Result = "Not found" - } + res.Result = colorize(removeColor, boldYellow, "Not found") rc.Fail++ table = append(table, *res) ftable = append(ftable, *res) @@ -1035,11 +1006,7 @@ func printTestResult(resps map[string]policyreportv1alpha2.PolicyReportResult, t } if testRes.Result == v.Result { - if !removeColor { - res.Result = boldGreen.Sprintf("Pass") - } else { - res.Result = "Pass" - } + res.Result = colorize(removeColor, boldGreen, "Pass") if testRes.Result == policyreportv1alpha2.StatusSkip { rc.Skip++ } else { @@ -1047,11 +1014,7 @@ func printTestResult(resps map[string]policyreportv1alpha2.PolicyReportResult, t } } else { log.Log.V(2).Info("result mismatch", "expected", v.Result, "received", testRes.Result, "key", resultKey) - if !removeColor { - res.Result = boldRed.Sprintf("Fail") - } else { - res.Result = "Fail" - } + res.Result = colorize(removeColor, boldRed, "Fail") rc.Fail++ ftable = append(ftable, *res) } @@ -1066,11 +1029,7 @@ func printTestResult(resps map[string]policyreportv1alpha2.PolicyReportResult, t } } else if v.Resource != "" { countDeprecatedResource++ - if !removeColor { - res.Resource = boldFgCyan.Sprintf(v.Namespace) + "/" + boldFgCyan.Sprintf(v.Kind) + "/" + boldFgCyan.Sprintf(v.Resource) - } else { - res.Resource = v.Namespace + "/" + v.Kind + "/" + v.Resource - } + res.Resource = colorize(removeColor, boldFgCyan, v.Namespace) + "/" + colorize(removeColor, boldFgCyan, v.Kind) + "/" + colorize(removeColor, boldFgCyan, v.Resource) var ruleNameInResultKey string if v.AutoGeneratedRule != "" { ruleNameInResultKey = fmt.Sprintf("%s-%s", v.AutoGeneratedRule, v.Rule) @@ -1086,19 +1045,10 @@ func printTestResult(resps map[string]policyreportv1alpha2.PolicyReportResult, t resultKey = fmt.Sprintf("%s-%s-%s-%s-%s-%s", ns, v.Policy, ruleNameInResultKey, v.Namespace, v.Kind, v.Resource) } else if found { resultKey = fmt.Sprintf("%s-%s-%s-%s-%s", ns, v.Policy, ruleNameInResultKey, v.Kind, v.Resource) - if !removeColor { - res.Policy = boldFgCyan.Sprintf(ns) + "/" + boldFgCyan.Sprintf(v.Policy) - res.Resource = boldFgCyan.Sprintf(v.Namespace) + "/" + boldFgCyan.Sprintf(v.Kind) + "/" + boldFgCyan.Sprintf(v.Resource) - } else { - res.Policy = ns + "/" + v.Policy - res.Resource = v.Namespace + "/" + v.Kind + "/" + v.Resource - } + res.Policy = colorize(removeColor, boldFgCyan, ns) + "/" + colorize(removeColor, boldFgCyan, v.Policy) + res.Resource = colorize(removeColor, boldFgCyan, v.Namespace) + "/" + colorize(removeColor, boldFgCyan, v.Kind) + "/" + colorize(removeColor, boldFgCyan, v.Resource) } else if v.Namespace != "" { - if !removeColor { - res.Resource = boldFgCyan.Sprintf(v.Namespace) + "/" + boldFgCyan.Sprintf(v.Kind) + "/" + boldFgCyan.Sprintf(v.Resource) - } else { - res.Resource = v.Namespace + "/" + v.Kind + "/" + v.Resource - } + res.Resource = colorize(removeColor, boldFgCyan, v.Namespace) + "/" + colorize(removeColor, boldFgCyan, v.Kind) + "/" + colorize(removeColor, boldFgCyan, v.Resource) resultKey = fmt.Sprintf("%s-%s-%s-%s-%s", v.Policy, ruleNameInResultKey, v.Namespace, v.Kind, v.Resource) } @@ -1107,11 +1057,7 @@ func printTestResult(resps map[string]policyreportv1alpha2.PolicyReportResult, t testRes = val } else { log.Log.V(2).Info("result not found", "key", resultKey) - if !removeColor { - res.Result = boldYellow.Sprintf("Not found") - } else { - res.Result = "Not found" - } + res.Result = colorize(removeColor, boldYellow, "Not found") rc.Fail++ table = append(table, *res) ftable = append(ftable, *res) @@ -1123,11 +1069,7 @@ func printTestResult(resps map[string]policyreportv1alpha2.PolicyReportResult, t } if testRes.Result == v.Result { - if !removeColor { - res.Result = boldGreen.Sprintf("Pass") - } else { - res.Result = "Pass" - } + res.Result = colorize(removeColor, boldGreen, "Pass") if testRes.Result == policyreportv1alpha2.StatusSkip { rc.Skip++ } else { @@ -1135,11 +1077,7 @@ func printTestResult(resps map[string]policyreportv1alpha2.PolicyReportResult, t } } else { log.Log.V(2).Info("result mismatch", "expected", v.Result, "received", testRes.Result, "key", resultKey) - if !removeColor { - res.Result = boldRed.Sprintf("Fail") - } else { - res.Result = "Fail" - } + res.Result = colorize(removeColor, boldRed, "Fail") rc.Fail++ ftable = append(ftable, *res) } @@ -1153,41 +1091,17 @@ func printTestResult(resps map[string]policyreportv1alpha2.PolicyReportResult, t } } } - - printer.BorderTop, printer.BorderBottom, printer.BorderLeft, printer.BorderRight = true, true, true, true - printer.CenterSeparator = "│" - printer.ColumnSeparator = "│" - printer.RowSeparator = "─" - printer.RowCharLimit = 300 - printer.RowLengthTitle = func(rowsLength int) bool { - return rowsLength > 10 - } - if !removeColor { - printer.HeaderBgColor = tablewriter.BgBlackColor - printer.HeaderFgColor = tablewriter.FgGreenColor - } fmt.Printf("\n") printer.Print(table) return nil } -func printFailedTestResult() { - printer := tableprinter.New(os.Stdout) +func printFailedTestResult(removeColor bool) { + printer := newTablePrinter(removeColor) for i, v := range ftable { v.ID = i + 1 } fmt.Printf("Aggregated Failed Test Cases : ") - printer.BorderTop, printer.BorderBottom, printer.BorderLeft, printer.BorderRight = true, true, true, true - printer.CenterSeparator = "│" - printer.ColumnSeparator = "│" - printer.RowSeparator = "─" - printer.RowCharLimit = 300 - printer.RowLengthTitle = func(rowsLength int) bool { - return rowsLength > 10 - } - - printer.HeaderBgColor = tablewriter.BgBlackColor - printer.HeaderFgColor = tablewriter.FgGreenColor fmt.Printf("\n") printer.Print(ftable) }