Skip to content

Commit b2a2970

Browse files
committed
operator: Replace reflect.DeepEqual with assert.Equal in tests
`reflect.DeepEqual` produces unhelpful output on failure: it reports only that two values differ, not which fields. This PR replaces it with testify's `assert.Equal` in operator test code, which prints a field-level diff. Two cases that didn't map to `assert.Equal`: * `operator/api/metrics_test.go`: `testMetric` is an error-returning helper comparing `map[string]string`, so I used the reflect-free `maps.Equal`. * `operator/auth/spire/client_test.go`: one call compared error strings, so a plain != was enough. Part of cilium#40562 Signed-off-by: Hadrien Patte <hadrien.patte@datadoghq.com>
1 parent 79af469 commit b2a2970

11 files changed

Lines changed: 42 additions & 77 deletions

File tree

operator/api/metrics_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,11 @@ import (
1111
"encoding/json"
1212
"encoding/pem"
1313
"fmt"
14+
"maps"
1415
"math/big"
1516
"net/http"
1617
"net/http/httptest"
1718
"os"
18-
"reflect"
1919
"testing"
2020
"time"
2121

@@ -293,7 +293,7 @@ func testMetric(metrics []models.Metric, name string, value float64, labels map[
293293
if metric.Value != value {
294294
return fmt.Errorf("expected value %f for %q, got %f", value, name, metric.Value)
295295
}
296-
if !reflect.DeepEqual(metric.Labels, labels) {
296+
if !maps.Equal(metric.Labels, labels) {
297297
return fmt.Errorf("expected labels map %v for %q, got %v", labels, name, metric.Labels)
298298
}
299299
return nil

operator/auth/spire/client_test.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,12 @@ package spire
66
import (
77
"context"
88
"fmt"
9-
"reflect"
109
"testing"
1110

1211
"github.com/cilium/hive/hivetest"
1312
entryv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/entry/v1"
1413
"github.com/spiffe/spire-api-sdk/proto/spire/api/types"
14+
"github.com/stretchr/testify/assert"
1515
"github.com/stretchr/testify/require"
1616
"google.golang.org/grpc"
1717
"google.golang.org/grpc/codes"
@@ -522,13 +522,11 @@ func Test_resolvedK8sService(t *testing.T) {
522522
for _, tt := range tests {
523523
t.Run(tt.name, func(t *testing.T) {
524524
got, err := resolvedK8sService(t.Context(), tt.args.client, tt.args.address)
525-
if tt.wantedErr != nil && (err == nil || !reflect.DeepEqual(err.Error(), tt.wantedErr.Error())) {
525+
if tt.wantedErr != nil && (err == nil || err.Error() != tt.wantedErr.Error()) {
526526
t.Errorf("resolvedK8sService() error = %v, wantErr %v", err, tt.wantedErr)
527527
return
528528
}
529-
if !reflect.DeepEqual(got, tt.want) {
530-
t.Errorf("resolvedK8sService() got = %v, want %v", got, tt.want)
531-
}
529+
assert.Equal(t, tt.want, got)
532530
})
533531
}
534532
}

operator/pkg/ciliumidentity/cache_test.go

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,8 @@
44
package ciliumidentity
55

66
import (
7-
"fmt"
87
"log/slog"
98
"os"
10-
"reflect"
119
"sync"
1210
"testing"
1311

@@ -55,7 +53,7 @@ func TestCIDState(t *testing.T) {
5553
},
5654
}
5755

58-
assert.NoError(t, validateCIDState(state, expectedState), "cid 1 added")
56+
validateCIDState(t, state, expectedState, "cid 1 added")
5957

6058
state.Upsert("2", k2)
6159
expectedState = &CIDState{
@@ -72,7 +70,7 @@ func TestCIDState(t *testing.T) {
7270
},
7371
}
7472

75-
assert.NoError(t, validateCIDState(state, expectedState), "cid 2 added")
73+
validateCIDState(t, state, expectedState, "cid 2 added")
7674

7775
state.Upsert("3", k3)
7876
expectedState = &CIDState{
@@ -89,7 +87,7 @@ func TestCIDState(t *testing.T) {
8987
},
9088
}
9189

92-
assert.NoError(t, validateCIDState(state, expectedState), "cid 3 added - duplicate")
90+
validateCIDState(t, state, expectedState, "cid 3 added - duplicate")
9391
})
9492

9593
t.Run("Lookup CID state", func(t *testing.T) {
@@ -124,7 +122,7 @@ func TestCIDState(t *testing.T) {
124122
},
125123
}
126124

127-
assert.NoError(t, validateCIDState(state, expectedState), "cid 2 removed")
125+
validateCIDState(t, state, expectedState, "cid 2 removed")
128126

129127
_, exists := state.LookupByID("2")
130128
assert.False(t, exists, "cid 2 LookupByID - not found")
@@ -139,7 +137,7 @@ func TestCIDState(t *testing.T) {
139137
},
140138
},
141139
}
142-
assert.NoError(t, validateCIDState(state, expectedState), "cid 3 removed")
140+
validateCIDState(t, state, expectedState, "cid 3 removed")
143141
})
144142
}
145143

@@ -175,16 +173,10 @@ func TestCIDStateThreadSafety(t *testing.T) {
175173
wg.Wait()
176174
}
177175

178-
func validateCIDState(state, expectedState *CIDState) error {
179-
if !reflect.DeepEqual(state.idToLabels, expectedState.idToLabels) {
180-
return fmt.Errorf("failed to validate the state, expected idToLabels %v, got %v", expectedState.idToLabels, state.idToLabels)
181-
}
182-
183-
if !reflect.DeepEqual(state.labelsToID, expectedState.labelsToID) {
184-
return fmt.Errorf("failed to validate the state, expected labelsToID %v, got %v", expectedState.labelsToID, state.labelsToID)
185-
}
186-
187-
return nil
176+
func validateCIDState(t *testing.T, state, expectedState *CIDState, msg string) {
177+
t.Helper()
178+
assert.Equal(t, expectedState.idToLabels, state.idToLabels, msg)
179+
assert.Equal(t, expectedState.labelsToID, state.labelsToID, msg)
188180
}
189181

190182
func TestCIDUsageInPods(t *testing.T) {

operator/pkg/ciliumidentity/namespace_test.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,10 @@
44
package ciliumidentity
55

66
import (
7-
"reflect"
87
"testing"
98

9+
"github.com/stretchr/testify/assert"
10+
1011
ciliumio "github.com/cilium/cilium/pkg/k8s/apis/cilium.io"
1112
slimcorev1 "github.com/cilium/cilium/pkg/k8s/slim/k8s/api/core/v1"
1213
slim_metav1 "github.com/cilium/cilium/pkg/k8s/slim/k8s/apis/meta/v1"
@@ -64,9 +65,7 @@ func TestGetNamespaceLabels(t *testing.T) {
6465
t.Run(tc.desc, func(t *testing.T) {
6566
result := getNamespaceLabels(tc.namespace)
6667

67-
if !reflect.DeepEqual(result, tc.expected) {
68-
t.Errorf("Expected %v, but got %v", tc.expected, result)
69-
}
68+
assert.Equal(t, tc.expected, result)
7069
})
7170
}
7271
}

operator/pkg/ciliumidentity/reconciler_test.go

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,13 @@ package ciliumidentity
66
import (
77
"context"
88
"fmt"
9-
"reflect"
109
"testing"
1110
"time"
1211

1312
"github.com/cilium/hive/cell"
1413
"github.com/cilium/hive/hivetest"
1514
"github.com/google/go-cmp/cmp"
15+
"github.com/stretchr/testify/assert"
1616
"github.com/stretchr/testify/require"
1717
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1818
"k8s.io/apimachinery/pkg/runtime"
@@ -232,12 +232,8 @@ func TestReconcileCID(t *testing.T) {
232232
t.Errorf("Unexpected error during reconciliation: %v", err)
233233
}
234234

235-
if !reflect.DeepEqual(createCID, tc.expectedCreate) {
236-
t.Errorf("Unexpected createCID result: got %v, want %v", createCID, tc.expectedCreate)
237-
}
238-
if !reflect.DeepEqual(updateCID, tc.expectedUpdate) {
239-
t.Errorf("Unexpected updateCID result: got %v, want %v", updateCID, tc.expectedUpdate)
240-
}
235+
assert.Equal(t, tc.expectedCreate, createCID)
236+
assert.Equal(t, tc.expectedUpdate, updateCID)
241237
if deleteCIDName != tc.expectedDelete {
242238
t.Errorf("Unexpected deleteCIDName result: got %v, want %v", deleteCIDName, tc.expectedDelete)
243239
}

operator/pkg/gateway-api/indexers/grpcroute_test.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@ package indexers
55

66
import (
77
"log/slog"
8-
"reflect"
98
"slices"
109
"testing"
1110

11+
"github.com/stretchr/testify/assert"
1212
corev1 "k8s.io/api/core/v1"
1313
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1414
"k8s.io/utils/ptr"
@@ -355,9 +355,8 @@ func Test_IndexGRPCRouteByGammaService(t *testing.T) {
355355
t.Run(tt.name, func(t *testing.T) {
356356
parentIndexFunc := IndexGRPCRouteByGammaService
357357

358-
if got := parentIndexFunc(tt.args.obj); !reflect.DeepEqual(got, tt.want) {
359-
t.Errorf("IndexGRPCRouteByGammaService() = %#v, want %#v", got, tt.want)
360-
}
358+
got := parentIndexFunc(tt.args.obj)
359+
assert.Equal(t, tt.want, got)
361360
})
362361
}
363362
}

operator/pkg/gateway-api/indexers/httproute_test.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@ package indexers
55

66
import (
77
"log/slog"
8-
"reflect"
98
"slices"
109
"testing"
1110

11+
"github.com/stretchr/testify/assert"
1212
corev1 "k8s.io/api/core/v1"
1313
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1414
"k8s.io/utils/ptr"
@@ -475,9 +475,8 @@ func Test_IndexHTTPRouteByGammaService(t *testing.T) {
475475
t.Run(tt.name, func(t *testing.T) {
476476
parentIndexFunc := IndexHTTPRouteByGammaService
477477

478-
if got := parentIndexFunc(tt.args.obj); !reflect.DeepEqual(got, tt.want) {
479-
t.Errorf("getGammaHTTPRouteParentIndexFunc() = %#v, want %#v", got, tt.want)
480-
}
478+
got := parentIndexFunc(tt.args.obj)
479+
assert.Equal(t, tt.want, got)
481480
})
482481
}
483482
}

operator/pkg/gateway-api/indexers/listenerset_test.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@
44
package indexers
55

66
import (
7-
"reflect"
87
"testing"
98

9+
"github.com/stretchr/testify/assert"
1010
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1111
"k8s.io/utils/ptr"
1212
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -55,9 +55,7 @@ func TestIndexListenerSetByGateway(t *testing.T) {
5555
for _, tt := range tests {
5656
t.Run(tt.name, func(t *testing.T) {
5757
got := IndexListenerSetByGateway(tt.obj)
58-
if !reflect.DeepEqual(got, tt.want) {
59-
t.Errorf("IndexListenerSetByGateway() = %v, want %v", got, tt.want)
60-
}
58+
assert.Equal(t, tt.want, got)
6159
})
6260
}
6361
}

operator/pkg/ingress/annotations/annotations_test.go

Lines changed: 8 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@
44
package annotations
55

66
import (
7-
"reflect"
87
"testing"
98
"time"
109

10+
"github.com/stretchr/testify/assert"
1111
networkingv1 "k8s.io/api/networking/v1"
1212
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1313
"k8s.io/utils/ptr"
@@ -130,9 +130,7 @@ func TestGetAnnotationServiceExternalTrafficPolicy(t *testing.T) {
130130
t.Errorf("GetAnnotationServiceExternalTrafficPolicy() error = %v, wantErr %v", err, tt.wantErr)
131131
return
132132
}
133-
if !reflect.DeepEqual(got, tt.want) {
134-
t.Errorf("GetAnnotationServiceExternalTrafficPolicy() got = %v, want %v", got, tt.want)
135-
}
133+
assert.Equal(t, tt.want, got)
136134
})
137135
}
138136
}
@@ -192,9 +190,7 @@ func TestGetAnnotationRequestTimeout(t *testing.T) {
192190
return
193191
}
194192

195-
if !reflect.DeepEqual(got, tt.want) {
196-
t.Errorf("GetAnnotationRequestTimeout() got = %v, want %v", got, tt.want)
197-
}
193+
assert.Equal(t, tt.want, got)
198194
})
199195
}
200196
}
@@ -252,9 +248,7 @@ func TestGetAnnotationSecureNodePort(t *testing.T) {
252248
t.Errorf("GetAnnotationSecureNodePort() error = %v, wantErr %v", err, tt.wantErr)
253249
return
254250
}
255-
if !reflect.DeepEqual(got, tt.want) {
256-
t.Errorf("GetAnnotationSecureNodePort() got = %v, want %v", got, tt.want)
257-
}
251+
assert.Equal(t, tt.want, got)
258252
})
259253
}
260254
}
@@ -311,9 +305,7 @@ func TestGetAnnotationInsecureNodePort(t *testing.T) {
311305
t.Errorf("GetAnnotationSecureNodePort() error = %v, wantErr %v", err, tt.wantErr)
312306
return
313307
}
314-
if !reflect.DeepEqual(got, tt.want) {
315-
t.Errorf("GetAnnotationSecureNodePort() got = %v, want %v", got, tt.want)
316-
}
308+
assert.Equal(t, tt.want, got)
317309
})
318310
}
319311
}
@@ -371,9 +363,7 @@ func TestGetAnnotationHostListenerPort(t *testing.T) {
371363
t.Errorf("GetAnnotationHostListenerPort() error = %v, wantErr %v", err, tt.wantErr)
372364
return
373365
}
374-
if !reflect.DeepEqual(got, tt.want) {
375-
t.Errorf("GetAnnotationHostListenerPort() got = %v, want %v", got, tt.want)
376-
}
366+
assert.Equal(t, tt.want, got)
377367
})
378368
}
379369
}
@@ -464,9 +454,7 @@ func TestGetAnnotationSSLPassthrough(t *testing.T) {
464454
t.Run(tt.name, func(t *testing.T) {
465455
got := GetAnnotationTLSPassthroughEnabled(tt.args.ingress)
466456

467-
if !reflect.DeepEqual(got, tt.want) {
468-
t.Errorf("GetAnnotationSecureNodePort() got = %v, want %v", got, tt.want)
469-
}
457+
assert.Equal(t, tt.want, got)
470458
})
471459
}
472460
}
@@ -557,9 +545,7 @@ func TestGetAnnotationEnforceHTTPSEnabled(t *testing.T) {
557545
for _, tt := range tests {
558546
t.Run(tt.name, func(t *testing.T) {
559547
got := GetAnnotationForceHTTPSEnabled(tt.args.ingress)
560-
if !reflect.DeepEqual(got, tt.want) {
561-
t.Errorf("GetAnnotationForceHTTPSEnabled() got = %v, want %v", got, tt.want)
562-
}
548+
assert.Equal(t, tt.want, got)
563549
})
564550
}
565551
}

operator/pkg/kvstore/locksweeper/locksweeper_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,10 @@
44
package locksweeper
55

66
import (
7-
"reflect"
87
"testing"
98

9+
"github.com/stretchr/testify/assert"
10+
1011
"github.com/cilium/cilium/pkg/kvstore"
1112
)
1213

@@ -88,9 +89,8 @@ func Test_getOldestLeases(t *testing.T) {
8889
}
8990
for _, tt := range tests {
9091
t.Run(tt.name, func(t *testing.T) {
91-
if got := getOldestLeases(tt.args.m); !reflect.DeepEqual(got, tt.want) {
92-
t.Errorf("getOldestLeases() = %v, want %v", got, tt.want)
93-
}
92+
got := getOldestLeases(tt.args.m)
93+
assert.Equal(t, tt.want, got)
9494
})
9595
}
9696
}

0 commit comments

Comments
 (0)