Skip to content

Commit e08df36

Browse files
refactor(controller): standardize tablegroup controller tests
- pkg/cluster-handler/controller/tablegroup/tablegroup_controller_test.go: - Update `setupFixtures` to accept `testing.TB` and mark it as a test helper for better failure reporting. - Switch to injecting `TableGroup` objects via the fake client builder's `existingObjects` rather than manually creating them inside the test loop. This simplifies the test setup and ensures consistent client state initialization. - Replace `cmp.Diff` with standard equality checks (`!=`) for simple scalar field assertions (strings, ints, bools) to align with Go idioms. - Adopt short variable declarations (`finalClient :=`) and remove unnecessary blank lines before error checks to adhere to codebase style guidelines.
1 parent 3694e40 commit e08df36

File tree

1 file changed

+37
-47
lines changed

1 file changed

+37
-47
lines changed

pkg/cluster-handler/controller/tablegroup/tablegroup_controller_test.go

Lines changed: 37 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"fmt"
66
"testing"
77

8-
"github.com/google/go-cmp/cmp"
98
apierrors "k8s.io/apimachinery/pkg/api/errors"
109
"k8s.io/apimachinery/pkg/api/meta"
1110
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -20,7 +19,9 @@ import (
2019
"github.com/numtide/multigres-operator/pkg/testutil"
2120
)
2221

23-
func setupFixtures() (*multigresv1alpha1.TableGroup, string, string, string, string, string) {
22+
func setupFixtures(t testing.TB) (*multigresv1alpha1.TableGroup, string, string, string, string, string) {
23+
t.Helper()
24+
2425
tgName := "test-tg"
2526
namespace := "default"
2627
clusterName := "test-cluster"
@@ -72,7 +73,7 @@ func TestTableGroupReconciler_Reconcile_Success(t *testing.T) {
7273
scheme := runtime.NewScheme()
7374
_ = multigresv1alpha1.AddToScheme(scheme)
7475

75-
baseTG, tgName, namespace, clusterName, dbName, tgLabelName := setupFixtures()
76+
baseTG, tgName, namespace, clusterName, dbName, tgLabelName := setupFixtures(t)
7677

7778
tests := map[string]struct {
7879
tableGroup *multigresv1alpha1.TableGroup
@@ -91,8 +92,8 @@ func TestTableGroupReconciler_Reconcile_Success(t *testing.T) {
9192
if err := c.Get(ctx, types.NamespacedName{Name: shardNameFull, Namespace: namespace}, shard); err != nil {
9293
t.Fatalf("Shard %s not created: %v", shardNameFull, err)
9394
}
94-
if diff := cmp.Diff(dbName, shard.Spec.DatabaseName); diff != "" {
95-
t.Errorf("Shard DB name mismatch (-want +got):\n%s", diff)
95+
if got, want := shard.Spec.DatabaseName, dbName; got != want {
96+
t.Errorf("Shard DB name mismatch got %q, want %q", got, want)
9697
}
9798
},
9899
},
@@ -160,8 +161,8 @@ func TestTableGroupReconciler_Reconcile_Success(t *testing.T) {
160161
if err := c.Get(t.Context(), types.NamespacedName{Name: tgName, Namespace: namespace}, updatedTG); err != nil {
161162
t.Fatalf("failed to get tablegroup: %v", err)
162163
}
163-
if diff := cmp.Diff(int32(1), updatedTG.Status.ReadyShards); diff != "" {
164-
t.Errorf("ReadyShards mismatch (-want +got):\n%s", diff)
164+
if got, want := updatedTG.Status.ReadyShards, int32(1); got != want {
165+
t.Errorf("ReadyShards mismatch got %d, want %d", got, want)
165166
}
166167
},
167168
},
@@ -187,11 +188,11 @@ func TestTableGroupReconciler_Reconcile_Success(t *testing.T) {
187188
if err := c.Get(t.Context(), types.NamespacedName{Name: tgName, Namespace: namespace}, updatedTG); err != nil {
188189
t.Fatalf("failed to get tablegroup: %v", err)
189190
}
190-
if diff := cmp.Diff(int32(0), updatedTG.Status.ReadyShards); diff != "" {
191-
t.Errorf("ReadyShards mismatch (-want +got):\n%s", diff)
191+
if got, want := updatedTG.Status.ReadyShards, int32(0); got != want {
192+
t.Errorf("ReadyShards mismatch got %d, want %d", got, want)
192193
}
193-
if diff := cmp.Diff(false, meta.IsStatusConditionTrue(updatedTG.Status.Conditions, "Available")); diff != "" {
194-
t.Errorf("TableGroup Available condition mismatch (-want +got):\n%s", diff)
194+
if meta.IsStatusConditionTrue(updatedTG.Status.Conditions, "Available") {
195+
t.Error("TableGroup should NOT be Available")
195196
}
196197
},
197198
},
@@ -206,8 +207,8 @@ func TestTableGroupReconciler_Reconcile_Success(t *testing.T) {
206207
if err := c.Get(t.Context(), types.NamespacedName{Name: tgName, Namespace: namespace}, updatedTG); err != nil {
207208
t.Fatalf("failed to get tablegroup: %v", err)
208209
}
209-
if diff := cmp.Diff(true, meta.IsStatusConditionTrue(updatedTG.Status.Conditions, "Available")); diff != "" {
210-
t.Errorf("Zero shard TableGroup Available condition mismatch (-want +got):\n%s", diff)
210+
if !meta.IsStatusConditionTrue(updatedTG.Status.Conditions, "Available") {
211+
t.Error("Zero shard TableGroup should be Available")
211212
}
212213
},
213214
},
@@ -222,28 +223,23 @@ func TestTableGroupReconciler_Reconcile_Success(t *testing.T) {
222223
t.Run(name, func(t *testing.T) {
223224
t.Parallel()
224225

225-
clientBuilder := fake.NewClientBuilder().
226-
WithScheme(scheme).
227-
WithObjects(tc.existingObjects...).
228-
WithStatusSubresource(&multigresv1alpha1.TableGroup{}, &multigresv1alpha1.Shard{})
229-
baseClient := clientBuilder.Build()
230-
231226
// Apply pre-reconcile updates if defined
232227
if tc.preReconcileUpdate != nil {
233228
tc.preReconcileUpdate(t, tc.tableGroup)
234229
}
235230

236-
// Initialize client state
231+
objects := tc.existingObjects
232+
// Inject TableGroup if creation is not skipped
237233
if !tc.skipCreate {
238-
check := &multigresv1alpha1.TableGroup{}
239-
err := baseClient.Get(t.Context(), types.NamespacedName{Name: tc.tableGroup.Name, Namespace: tc.tableGroup.Namespace}, check)
240-
if apierrors.IsNotFound(err) {
241-
if err := baseClient.Create(t.Context(), tc.tableGroup); err != nil {
242-
t.Fatalf("failed to create initial tablegroup: %v", err)
243-
}
244-
}
234+
objects = append(objects, tc.tableGroup)
245235
}
246236

237+
clientBuilder := fake.NewClientBuilder().
238+
WithScheme(scheme).
239+
WithObjects(objects...).
240+
WithStatusSubresource(&multigresv1alpha1.TableGroup{}, &multigresv1alpha1.Shard{})
241+
baseClient := clientBuilder.Build()
242+
247243
reconciler := &TableGroupReconciler{
248244
Client: baseClient,
249245
Scheme: scheme,
@@ -257,7 +253,6 @@ func TestTableGroupReconciler_Reconcile_Success(t *testing.T) {
257253
}
258254

259255
_, err := reconciler.Reconcile(t.Context(), req)
260-
261256
if err != nil {
262257
t.Errorf("Unexpected error from Reconcile: %v", err)
263258
}
@@ -275,7 +270,7 @@ func TestTableGroupReconciler_Reconcile_Failure(t *testing.T) {
275270
scheme := runtime.NewScheme()
276271
_ = multigresv1alpha1.AddToScheme(scheme)
277272

278-
baseTG, tgName, namespace, clusterName, dbName, tgLabelName := setupFixtures()
273+
baseTG, tgName, namespace, clusterName, dbName, tgLabelName := setupFixtures(t)
279274
errBoom := errors.New("boom")
280275

281276
tests := map[string]struct {
@@ -344,31 +339,27 @@ func TestTableGroupReconciler_Reconcile_Failure(t *testing.T) {
344339
t.Run(name, func(t *testing.T) {
345340
t.Parallel()
346341

342+
// Apply pre-reconcile updates if defined
343+
if tc.preReconcileUpdate != nil {
344+
tc.preReconcileUpdate(t, tc.tableGroup)
345+
}
346+
347+
objects := tc.existingObjects
348+
// Default behavior: create the TableGroup unless getting it is set to fail (which simulates Not Found or error)
349+
// For failure tests, usually the object exists so the code can proceed to the failing step.
350+
objects = append(objects, tc.tableGroup)
351+
347352
clientBuilder := fake.NewClientBuilder().
348353
WithScheme(scheme).
349-
WithObjects(tc.existingObjects...).
354+
WithObjects(objects...).
350355
WithStatusSubresource(&multigresv1alpha1.TableGroup{}, &multigresv1alpha1.Shard{})
351356
baseClient := clientBuilder.Build()
352357

353-
var finalClient client.Client
354-
finalClient = baseClient
358+
finalClient := client.Client(baseClient)
355359
if tc.failureConfig != nil {
356360
finalClient = testutil.NewFakeClientWithFailures(baseClient, tc.failureConfig)
357361
}
358362

359-
// Apply pre-reconcile updates if defined
360-
if tc.preReconcileUpdate != nil {
361-
tc.preReconcileUpdate(t, tc.tableGroup)
362-
}
363-
364-
check := &multigresv1alpha1.TableGroup{}
365-
err := baseClient.Get(t.Context(), types.NamespacedName{Name: tc.tableGroup.Name, Namespace: tc.tableGroup.Namespace}, check)
366-
if apierrors.IsNotFound(err) {
367-
if err := baseClient.Create(t.Context(), tc.tableGroup); err != nil {
368-
t.Fatalf("failed to create initial tablegroup: %v", err)
369-
}
370-
}
371-
372363
reconciler := &TableGroupReconciler{
373364
Client: finalClient,
374365
Scheme: scheme,
@@ -381,8 +372,7 @@ func TestTableGroupReconciler_Reconcile_Failure(t *testing.T) {
381372
},
382373
}
383374

384-
_, err = reconciler.Reconcile(t.Context(), req)
385-
375+
_, err := reconciler.Reconcile(t.Context(), req)
386376
if err == nil {
387377
t.Error("Expected error from Reconcile, got nil")
388378
}

0 commit comments

Comments
 (0)