Skip to content

Commit

Permalink
modify comparison by returning panic error
Browse files Browse the repository at this point in the history
fix gci lint error
  • Loading branch information
DiptoChakrabarty committed Jun 29, 2023
1 parent de1fbbc commit 14b7314
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
24 changes: 13 additions & 11 deletions util/conditions/compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
15 changes: 8 additions & 7 deletions util/conditions/compare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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))
})
}
Expand Down
40 changes: 32 additions & 8 deletions util/patch/patch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
})
Expand Down Expand Up @@ -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())
})
Expand Down Expand Up @@ -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())
})
Expand Down Expand Up @@ -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())
})
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand Down

0 comments on commit 14b7314

Please sign in to comment.