Merge pull request #1179 from shajmakh/gocmp-replace

OCPBUGS-49407: pkg: use go-cmp only in tests
openshift-kni · Feb 18, 2025 · a58dcdd · a58dcdd
2 parents 7147d3d + 8481677
commit a58dcdd
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 31 deletions.
diff --git a/pkg/status/status.go b/pkg/status/status.go
@@ -18,11 +18,9 @@ package status
 
 import (
 	"errors"
+	"reflect"
 	"time"
 
-	"github.com/google/go-cmp/cmp"
-	"github.com/google/go-cmp/cmp/cmpopts"
-
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	nropv1 "github.com/openshift-kni/numaresources-operator/api/v1"
@@ -51,12 +49,13 @@ const (
 )
 
 func IsUpdatedNUMAResourcesOperator(oldStatus, newStatus *nropv1.NUMAResourcesOperatorStatus) bool {
-	options := []cmp.Option{
-		cmpopts.IgnoreFields(metav1.Condition{}, "LastTransitionTime"),
-		cmpopts.IgnoreFields(metav1.Condition{}, "ObservedGeneration"),
-	}
+	os := oldStatus.DeepCopy()
+	ns := newStatus.DeepCopy()
+
+	resetIncomparableConditionFields(os.Conditions)
+	resetIncomparableConditionFields(ns.Conditions)
 
-	return !cmp.Equal(newStatus, oldStatus, options...)
+	return !reflect.DeepEqual(os, ns)
 }
 
 // UpdateConditions compute new conditions based on arguments, and then compare with given current conditions.
@@ -65,12 +64,13 @@ func IsUpdatedNUMAResourcesOperator(oldStatus, newStatus *nropv1.NUMAResourcesOp
 func UpdateConditions(currentConditions []metav1.Condition, condition string, reason string, message string) ([]metav1.Condition, bool) {
 	conditions := NewConditions(condition, reason, message)
 
-	options := []cmp.Option{
-		cmpopts.IgnoreFields(metav1.Condition{}, "LastTransitionTime"),
-		cmpopts.IgnoreFields(metav1.Condition{}, "ObservedGeneration"),
-	}
+	cond := clone(conditions)
+	curCond := clone(currentConditions)
+
+	resetIncomparableConditionFields(cond)
+	resetIncomparableConditionFields(curCond)
 
-	if cmp.Equal(conditions, currentConditions, options...) {
+	if reflect.DeepEqual(cond, curCond) {
 		return currentConditions, false
 	}
 	return conditions, true
@@ -151,3 +151,16 @@ func MessageFromError(err error) string {
 	}
 	return unwErr.Error()
 }
+
+func resetIncomparableConditionFields(conditions []metav1.Condition) {
+	for idx := range conditions {
+		conditions[idx].LastTransitionTime = metav1.Time{}
+		conditions[idx].ObservedGeneration = 0
+	}
+}
+
+func clone(conditions []metav1.Condition) []metav1.Condition {
+	var c = make([]metav1.Condition, len(conditions))
+	copy(c, conditions)
+	return c
+}
diff --git a/pkg/status/status_test.go b/pkg/status/status_test.go
@@ -72,7 +72,7 @@ func TestFindCondition(t *testing.T) {
 	}
 }
 
-func TestUpdate(t *testing.T) {
+func TestUpdateConditions(t *testing.T) {
 	err := nropv1.AddToScheme(scheme.Scheme)
 	if err != nil {
 		t.Errorf("nropv1.AddToScheme() failed with: %v", err)
@@ -81,7 +81,12 @@ func TestUpdate(t *testing.T) {
 	nro := testobjs.NewNUMAResourcesOperator("test-nro")
 	fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(nro).Build()
 
-	nro.Status.Conditions, _ = UpdateConditions(nro.Status.Conditions, ConditionProgressing, "testReason", "test message")
+	var ok bool
+	nro.Status.Conditions, ok = UpdateConditions(nro.Status.Conditions, ConditionProgressing, "testReason", "test message")
+	if !ok {
+		t.Errorf("Update did not change status, but it should")
+	}
+
 	err = fakeClient.Update(context.TODO(), nro)
 	if err != nil {
 		t.Errorf("Update() failed with: %v", err)
@@ -98,24 +103,9 @@ func TestUpdate(t *testing.T) {
 	if progressingCondition.Status != metav1.ConditionTrue {
 		t.Errorf("Update() failed to set correct status, expected: %q, got: %q", metav1.ConditionTrue, progressingCondition.Status)
 	}
-}
-
-func TestUpdateIfNeeded(t *testing.T) {
-	err := nropv1.AddToScheme(scheme.Scheme)
-	if err != nil {
-		t.Errorf("nropv1.AddToScheme() failed with: %v", err)
-	}
-
-	nro := testobjs.NewNUMAResourcesOperator("test-nro")
-
-	var ok bool
-	nro.Status.Conditions, ok = UpdateConditions(nro.Status.Conditions, ConditionAvailable, "", "")
-	if !ok {
-		t.Errorf("Update did not change status, but it should")
-	}
 
 	// same status twice in a row. We should not overwrite identical status to save transactions.
-	_, ok = UpdateConditions(nro.Status.Conditions, ConditionAvailable, "", "")
+	_, ok = UpdateConditions(nro.Status.Conditions, ConditionProgressing, "testReason", "test message")
 	if ok {
 		t.Errorf("Update did change status, but it should not")
 	}