diff --git a/exp/addons/api/v1alpha3/clusterresourcesetbinding_types_test.go b/exp/addons/api/v1alpha3/clusterresourcesetbinding_types_test.go index c36895fd0eba..44df1d5990a9 100644 --- a/exp/addons/api/v1alpha3/clusterresourcesetbinding_types_test.go +++ b/exp/addons/api/v1alpha3/clusterresourcesetbinding_types_test.go @@ -147,7 +147,7 @@ func TestSetResourceBinding(t *testing.T) { tt.resourceSetBinding.SetBinding(tt.resourceBinding) exist := false for _, b := range tt.resourceSetBinding.Resources { - if diff := cmp.Diff(b.ResourceRef, tt.resourceBinding.ResourceRef); diff == "" { + if cmp.Equal(b.ResourceRef, tt.resourceBinding.ResourceRef) { gs.Expect(tt.resourceBinding.Applied).To(BeEquivalentTo(b.Applied)) exist = true } diff --git a/exp/addons/api/v1alpha4/clusterresourcesetbinding_types_test.go b/exp/addons/api/v1alpha4/clusterresourcesetbinding_types_test.go index c83651841df3..31fa5ca0b5c7 100644 --- a/exp/addons/api/v1alpha4/clusterresourcesetbinding_types_test.go +++ b/exp/addons/api/v1alpha4/clusterresourcesetbinding_types_test.go @@ -147,7 +147,7 @@ func TestSetResourceBinding(t *testing.T) { tt.resourceSetBinding.SetBinding(tt.resourceBinding) exist := false for _, b := range tt.resourceSetBinding.Resources { - if diff := cmp.Diff(b.ResourceRef, tt.resourceBinding.ResourceRef); diff == "" { + if cmp.Equal(b.ResourceRef, tt.resourceBinding.ResourceRef) { gs.Expect(tt.resourceBinding.Applied).To(BeEquivalentTo(b.Applied)) exist = true } diff --git a/exp/addons/api/v1beta1/clusterresourcesetbinding_types_test.go b/exp/addons/api/v1beta1/clusterresourcesetbinding_types_test.go index f550f2232f3d..f80f5eea3aa8 100644 --- a/exp/addons/api/v1beta1/clusterresourcesetbinding_types_test.go +++ b/exp/addons/api/v1beta1/clusterresourcesetbinding_types_test.go @@ -207,7 +207,7 @@ func TestSetResourceBinding(t *testing.T) { tt.resourceSetBinding.SetBinding(tt.resourceBinding) exist := false for _, b := range tt.resourceSetBinding.Resources { - if diff := cmp.Diff(b.ResourceRef, tt.resourceBinding.ResourceRef); diff == "" { + if cmp.Equal(b.ResourceRef, tt.resourceBinding.ResourceRef) { gs.Expect(tt.resourceBinding.Applied).To(BeEquivalentTo(b.Applied)) exist = true } diff --git a/util/conditions/compare.go b/util/conditions/compare.go index 9e93567ae606..d3ebb0b387af 100644 --- a/util/conditions/compare.go +++ b/util/conditions/compare.go @@ -18,23 +18,25 @@ package conditions import ( "fmt" - "log" "github.com/google/go-cmp/cmp" ) -func Check(actual interface{}, expected interface{}) (result bool, err error) { - defer func() error { +// Check compares the actual and expected values provided using the +// cmp package and handles panic errors in case of comparison. +func Check(actual interface{}, expected interface{}) (result bool, diff string, err error) { + result = true + defer func() { if recover() != nil { - log.Printf("panic occured got %v expected %v", actual, expected) - err = fmt.Errorf("panic occured got %v expected %v", actual, expected) + err = fmt.Errorf("panic occurred got %v expected %v", actual, expected) } - return nil }() - diff := cmp.Diff(actual, expected) - if diff == "" { - return true, err + diff = cmp.Diff(actual, expected) + if err != nil { + return !result, "", err } - log.Printf("mismatch of objects actual %v expected %v diff %v", actual, expected, diff) - return false, err + if diff != "" { + return !result, diff, nil + } + return result, diff, nil } diff --git a/util/conditions/compare_test.go b/util/conditions/compare_test.go index c23de02541da..cd7f6c306b9e 100644 --- a/util/conditions/compare_test.go +++ b/util/conditions/compare_test.go @@ -21,7 +21,8 @@ import ( . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + unstructv1 "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" ) @@ -154,7 +155,7 @@ func TestCompareConditions(t *testing.T) { }, { name: "with similar unstructed object", - actual: &unstructured.Unstructured{ + actual: &unstructv1.Unstructured{ Object: map[string]interface{}{ "kind": "GenericBootstrapConfig", "apiVersion": "bootstrap.cluster.x-k8s.io/v1beta1", @@ -168,7 +169,7 @@ func TestCompareConditions(t *testing.T) { }, }, }, - expected: &unstructured.Unstructured{ + expected: &unstructv1.Unstructured{ Object: map[string]interface{}{ "kind": "GenericBootstrapConfig", "apiVersion": "bootstrap.cluster.x-k8s.io/v1beta1", @@ -186,7 +187,7 @@ func TestCompareConditions(t *testing.T) { }, { name: "with unsimilar dataSecretName type object", - actual: &unstructured.Unstructured{ + actual: &unstructv1.Unstructured{ Object: map[string]interface{}{ "kind": "GenericBootstrapConfig", "apiVersion": "bootstrap.cluster.x-k8s.io/v1beta1", @@ -200,7 +201,7 @@ func TestCompareConditions(t *testing.T) { }, }, }, - expected: &unstructured.Unstructured{ + expected: &unstructv1.Unstructured{ Object: map[string]interface{}{ "kind": "GenericBootstrapConfig", "apiVersion": "bootstrap.cluster.x-k8s.io/v1beta1", @@ -218,7 +219,7 @@ func TestCompareConditions(t *testing.T) { }, { name: "with unsimilar types", - actual: &unstructured.Unstructured{ + actual: &unstructv1.Unstructured{ Object: map[string]interface{}{ "kind": "GenericBootstrapConfig", "apiVersion": "bootstrap.cluster.x-k8s.io/v1beta1", @@ -254,7 +255,7 @@ func TestCompareConditions(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { g := NewWithT(t) - result, _ := Check(tc.actual, tc.expected) + result, _, _ := Check(tc.actual, tc.expected) g.Expect(result).To(Equal(tc.expectMatch)) }) } diff --git a/util/patch/patch_test.go b/util/patch/patch_test.go index 2fdfa9e494d3..43027dbf603d 100644 --- a/util/patch/patch_test.go +++ b/util/patch/patch_test.go @@ -107,7 +107,10 @@ func TestPatchHelper(t *testing.T) { if err := env.Get(ctx, key, objAfter); err != nil { return false } - result, _ := conditions.Check(obj.GetOwnerReferences(), objAfter.GetOwnerReferences()) + result, diff, _ := conditions.Check(obj.GetOwnerReferences(), objAfter.GetOwnerReferences()) + if diff != "" { + t.Logf("mismatch of objects actual %v expected %v diff %v", obj.GetOwnerReferences(), objAfter.GetOwnerReferences(), diff) + } return result }, timeout).Should(BeTrue()) }) @@ -542,7 +545,10 @@ func TestPatchHelper(t *testing.T) { if err := env.Get(ctx, key, objAfter); err != nil { return false } - result, _ := conditions.Check(obj.Finalizers, objAfter.Finalizers) + result, diff, _ := conditions.Check(obj.Finalizers, objAfter.Finalizers) + if diff != "" { + t.Logf("mismatch of objects actual %v expected %v diff %v", obj.Finalizers, objAfter.Finalizers, diff) + } return result }, timeout).Should(BeTrue()) }) @@ -626,7 +632,10 @@ func TestPatchHelper(t *testing.T) { if err := env.Get(ctx, key, objAfter); err != nil { return false } - result, _ := conditions.Check(obj.Spec.InfrastructureRef, objAfter.Spec.InfrastructureRef) + result, diff, _ := conditions.Check(obj.Spec.InfrastructureRef, objAfter.Spec.InfrastructureRef) + if diff != "" { + t.Logf("mismatch of objects actual %v expected %v diff %v", obj.Spec.InfrastructureRef, objAfter.Spec.InfrastructureRef, diff) + } return objAfter.Spec.Paused && result }, timeout).Should(BeTrue()) }) @@ -665,7 +674,10 @@ func TestPatchHelper(t *testing.T) { if err := env.Get(ctx, key, objAfter); err != nil { return false } - result, _ := conditions.Check(objAfter.Status, obj.Status) + result, diff, _ := conditions.Check(objAfter.Status, obj.Status) + if diff != "" { + t.Logf("mismatch of objects actual %v expected %v diff %v", obj.Status, objAfter.Status, diff) + } return result }, timeout).Should(BeTrue()) }) @@ -715,7 +727,10 @@ func TestPatchHelper(t *testing.T) { if err := env.Get(ctx, key, objAfter); err != nil { return false } - result, _ := conditions.Check(obj.Spec, objAfter.Spec) + result, diff, _ := conditions.Check(obj.Spec, objAfter.Spec) + if diff != "" { + t.Logf("mismatch of objects actual %v expected %v diff %v", obj.Spec, objAfter.Spec, diff) + } return obj.Status.InfrastructureReady == objAfter.Status.InfrastructureReady && conditions.IsTrue(objAfter, clusterv1.ReadyCondition) && result @@ -773,7 +788,10 @@ func TestPatchHelper(t *testing.T) { if err := env.Get(ctx, key, objAfter); err != nil { return false } - result, _ := conditions.Check(obj.Spec, objAfter.Spec) + result, diff, _ := conditions.Check(obj.Spec, objAfter.Spec) + if diff != "" { + t.Logf("mismatch of objects actual %v expected %v diff %v", obj.Spec, objAfter.Spec, diff) + } return result && obj.GetGeneration() == objAfter.Status.ObservedGeneration }, timeout).Should(BeTrue()) @@ -822,8 +840,14 @@ func TestPatchHelper(t *testing.T) { if err := env.Get(ctx, key, objAfter); err != nil { return false } - specResult, _ := conditions.Check(obj.Spec, objAfter.Spec) - statusResult, _ := conditions.Check(obj.Status, objAfter.Status) + specResult, diff, _ := conditions.Check(obj.Spec, objAfter.Spec) + if diff != "" { + t.Logf("mismatch of objects actual %v expected %v diff %v", obj.Spec, objAfter.Spec, diff) + } + statusResult, diff, _ := conditions.Check(obj.Status, objAfter.Status) + if diff != "" { + t.Logf("mismatch of objects actual %v expected %v diff %v", obj.Status, objAfter.Status, diff) + } return specResult && statusResult && obj.GetGeneration() == objAfter.Status.ObservedGeneration }, timeout).Should(BeTrue())