From 5994a316be2e6f2881ba17e160f8ab3a76661192 Mon Sep 17 00:00:00 2001 From: nocturnalastro Date: Fri, 9 Aug 2024 10:04:36 +0100 Subject: [PATCH 1/3] Detect tests that panic --- pkg/compare/compare_test.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/pkg/compare/compare_test.go b/pkg/compare/compare_test.go index c04d7a63..cdbdbaf7 100644 --- a/pkg/compare/compare_test.go +++ b/pkg/compare/compare_test.go @@ -249,6 +249,8 @@ func matchErrorRegexCheck(msg string) Check { } } +const ExpectedPanic = "Expected Error Test Case" + // TestCompareRun ensures that Run command calls the right actions // and returns the expected error. func TestCompareRun(t *testing.T) { @@ -345,10 +347,13 @@ func TestCompareRun(t *testing.T) { cmdutil.BehaviorOnFatal(func(str string, code int) { errorStr := fmt.Sprintf("%s\nerror code:%d\n", testutils.RemoveInconsistentInfo(t, str), code) test.checks.Err.check(t, test, mode, errorStr) - panic("Expected Error Test Case") + panic(ExpectedPanic) }) defer func() { - _ = recover() + r := recover() + if s, ok := r.(string); r != nil && (!ok || s != ExpectedPanic) { + t.Fatalf("test paniced: %v", r) + } test.checks.Out.check(t, test, mode, testutils.RemoveInconsistentInfo(t, out.String())) }() cmd.Run(cmd, []string{}) From 435f1d92f852e0ea8d3ef3b5ebfca207c620a3c7 Mon Sep 17 00:00:00 2001 From: nocturnalastro Date: Fri, 9 Aug 2024 10:06:01 +0100 Subject: [PATCH 2/3] Stop failing metadata collection from causing a panic --- pkg/compare/parsing.go | 9 ++++++--- .../localerr.golden | 2 +- pkg/compare/testdata/YAMLOutput/localout.golden | 6 +++--- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/pkg/compare/parsing.go b/pkg/compare/parsing.go index 2e94b411..8d122ae4 100644 --- a/pkg/compare/parsing.go +++ b/pkg/compare/parsing.go @@ -132,7 +132,10 @@ func (rf ReferenceTemplate) Exec(params map[string]any) (*unstructured.Unstructu content := buf.Bytes() err = yaml.Unmarshal(bytes.ReplaceAll(content, []byte(noValue), []byte("")), &data) if err != nil { - return nil, fmt.Errorf("template: %s isn't a yaml file after injection. yaml unmarshal error: %w. The Template After Execution: %s", rf.Name(), err, string(content)) + return nil, fmt.Errorf( + "template: %s isn't a yaml file after injection. yaml unmarshal error: %w. The Template After Execution: %s", + rf.Name(), err, string(content), + ) } return &unstructured.Unstructured{Object: data}, nil } @@ -272,13 +275,13 @@ func parseTemplates(templateReference []*ReferenceTemplate, functionTemplates [] temp.Template = parsedTemp temp.metadata, err = temp.Exec(map[string]any{}) // Extract Metadata if err != nil { - errs = append(errs, err) + errs = append(errs, fmt.Errorf("failed to parse template %s with empty data: %w", temp.Path, err)) } err = temp.ValidateFieldsToOmit(ref.FieldsToOmit) if err != nil { errs = append(errs, err) } - if temp.metadata.GetKind() == "" { + if temp.metadata != nil && temp.metadata.GetKind() == "" { errs = append(errs, fmt.Errorf("template missing kind: %s", temp.Path)) } } diff --git a/pkg/compare/testdata/TemplateIsntYAMLAfterExecutionWithEmptyMap/localerr.golden b/pkg/compare/testdata/TemplateIsntYAMLAfterExecutionWithEmptyMap/localerr.golden index 753f7df7..4a22b31d 100644 --- a/pkg/compare/testdata/TemplateIsntYAMLAfterExecutionWithEmptyMap/localerr.golden +++ b/pkg/compare/testdata/TemplateIsntYAMLAfterExecutionWithEmptyMap/localerr.golden @@ -1,2 +1,2 @@ -error: failed to constuct template: template: apps.v1.DaemonSet.kube-system.kindnet.yaml:5:61: executing "apps.v1.DaemonSet.kube-system.kindnet.yaml" at : error calling len: reflect: call of reflect.Value.Type on zero Value +error: failed to parse template apps.v1.DaemonSet.kube-system.kindnet.yaml with empty data: failed to constuct template: template: apps.v1.DaemonSet.kube-system.kindnet.yaml:5:61: executing "apps.v1.DaemonSet.kube-system.kindnet.yaml" at : error calling len: reflect: call of reflect.Value.Type on zero Value error code:2 diff --git a/pkg/compare/testdata/YAMLOutput/localout.golden b/pkg/compare/testdata/YAMLOutput/localout.golden index bd293828..7a4e02e9 100644 --- a/pkg/compare/testdata/YAMLOutput/localout.golden +++ b/pkg/compare/testdata/YAMLOutput/localout.golden @@ -3,9 +3,9 @@ Diffs: CorrelatedTemplate: deploymentDashboard.yaml DiffOutput: "diff -u -N TEMP/apps-v1_deployment_kubernetes-dashboard_kubernetes-dashboard TEMP/apps-v1_deployment_kubernetes-dashboard_kubernetes-dashboard\n--- - TEMP/apps-v1_deployment_kubernetes-dashboard_kubernetes-dashboard\tDATE\n+++ TEMP/apps-v1_deployment_kubernetes-dashboard_kubernetes-dashboard\tDATE\n@@ -14,7 +14,7 @@\n template:\n metadata:\n labels:\n- k8s-app: - kubernetes-dashboard\n+ k8s-app: kubernetes-dashboard-diff\n spec:\n - \ containers:\n - args:\n" + TEMP/apps-v1_deployment_kubernetes-dashboard_kubernetes-dashboard\tDATE\n+++ TEMP/apps-v1_deployment_kubernetes-dashboard_kubernetes-dashboard\tDATE\n@@ -14,7 +14,7 @@\n template:\n metadata:\n labels:\n- + \ k8s-app: kubernetes-dashboard\n+ k8s-app: kubernetes-dashboard-diff\n + \ spec:\n containers:\n - args:\n" Summary: NumDiffCRs: 1 NumMissing: 1 From 6c924ad3c6f459a5a1aa7b130d847545b7a457d6 Mon Sep 17 00:00:00 2001 From: nocturnalastro Date: Fri, 9 Aug 2024 10:25:07 +0100 Subject: [PATCH 3/3] Add check for unused error files --- pkg/compare/compare_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/pkg/compare/compare_test.go b/pkg/compare/compare_test.go index cdbdbaf7..3e23a423 100644 --- a/pkg/compare/compare_test.go +++ b/pkg/compare/compare_test.go @@ -79,6 +79,13 @@ func (c Check) getPath(test Test, mode Mode) string { return path.Join(test.getTestDir(), string(mode.crSource)+c.suffix) } +func (c Check) hasErrorFile(test Test, mode Mode) bool { + if _, err := os.Stat(c.getPath(test, mode)); errors.Is(err, os.ErrNotExist) { + return false + } + return true +} + func (c Check) check(t *testing.T, test Test, mode Mode, value string) { switch c.checkType { case matchFile: @@ -344,16 +351,23 @@ func TestCompareRun(t *testing.T) { IOStream, _, out, _ := genericiooptions.NewTestIOStreams() klog.SetOutputBySeverity("INFO", out) cmd := getCommand(t, &test, i, tf, &IOStream) // nolint:gosec + + hasCheckedError := false cmdutil.BehaviorOnFatal(func(str string, code int) { errorStr := fmt.Sprintf("%s\nerror code:%d\n", testutils.RemoveInconsistentInfo(t, str), code) test.checks.Err.check(t, test, mode, errorStr) + hasCheckedError = true panic(ExpectedPanic) }) + defer func() { r := recover() if s, ok := r.(string); r != nil && (!ok || s != ExpectedPanic) { t.Fatalf("test paniced: %v", r) } + if !hasCheckedError && test.checks.Err.hasErrorFile(test, mode) { + t.Fatalf("Unchecked error file %s", test.checks.Err.getPath(test, mode)) + } test.checks.Out.check(t, test, mode, testutils.RemoveInconsistentInfo(t, out.String())) }() cmd.Run(cmd, []string{})