Skip to content

Commit 2dc9e48

Browse files
refactor(controller): reduce test boilerplate by defaulting existing objects
- pkg/cluster-handler/controller/multigrescluster/multigrescluster_controller_test.go: - Update test runners to default `existingObjects` to the standard set of templates (`coreTpl`, `cellTpl`, `shardTpl`) if `nil`. - Remove explicit `existingObjects` definitions from test cases where the default set is sufficient (e.g., `Create: MultiOrch Placement Defaulting`, `Create: MultiAdmin TemplateRef Only`). - This declutters the test table and focuses each test case on the specific overrides or logic being tested.
1 parent cd48659 commit 2dc9e48

File tree

1 file changed

+31
-6
lines changed

1 file changed

+31
-6
lines changed

pkg/cluster-handler/controller/multigrescluster/multigrescluster_controller_test.go

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ func TestMultigresClusterReconciler_Reconcile_Success(t *testing.T) {
156156
},
157157
},
158158
"Create: Full Cluster Creation with Templates": {
159-
existingObjects: []client.Object{coreTpl, cellTpl, shardTpl},
159+
// Using defaults
160160
validate: func(t testing.TB, c client.Client) {
161161
ctx := t.Context()
162162
updatedCluster := &multigresv1alpha1.MultigresCluster{}
@@ -180,8 +180,9 @@ func TestMultigresClusterReconciler_Reconcile_Success(t *testing.T) {
180180
},
181181
},
182182
"Create: Independent Templates (Topo vs Admin)": {
183+
// Using preReconcileUpdate instead of an explicit object to leverage defaults
183184
preReconcileUpdate: func(t testing.TB, c *multigresv1alpha1.MultigresCluster) {
184-
c.Spec.TemplateDefaults.CoreTemplate = ""
185+
c.Spec.TemplateDefaults.CoreTemplate = "" // clear default
185186
c.Spec.GlobalTopoServer.TemplateRef = "topo-core"
186187
c.Spec.MultiAdmin.TemplateRef = "admin-core"
187188
},
@@ -465,6 +466,8 @@ func TestMultigresClusterReconciler_Reconcile_Success(t *testing.T) {
465466
preReconcileUpdate: func(t testing.TB, c *multigresv1alpha1.MultigresCluster) {
466467
c.Spec.Databases = append(c.Spec.Databases, multigresv1alpha1.DatabaseConfig{Name: "db2", TableGroups: []multigresv1alpha1.TableGroupConfig{}})
467468
},
469+
// Here we insert the Cell WITH STATUS directly into existingObjects.
470+
// The fake client will respect this state.
468471
existingObjects: []client.Object{
469472
coreTpl, cellTpl, shardTpl,
470473
&multigresv1alpha1.Cell{
@@ -511,9 +514,15 @@ func TestMultigresClusterReconciler_Reconcile_Success(t *testing.T) {
511514
t.Run(name, func(t *testing.T) {
512515
t.Parallel()
513516

517+
// Default to all standard templates if existingObjects is nil
518+
objects := tc.existingObjects
519+
if objects == nil {
520+
objects = []client.Object{coreTpl, cellTpl, shardTpl}
521+
}
522+
514523
clientBuilder := fake.NewClientBuilder().
515524
WithScheme(scheme).
516-
WithObjects(tc.existingObjects...).
525+
WithObjects(objects...).
517526
WithStatusSubresource(&multigresv1alpha1.MultigresCluster{}, &multigresv1alpha1.Cell{}, &multigresv1alpha1.TableGroup{})
518527
baseClient := clientBuilder.Build()
519528

@@ -549,10 +558,15 @@ func TestMultigresClusterReconciler_Reconcile_Success(t *testing.T) {
549558
// Ensure DeletionTimestamp is set in the API if the test requires it.
550559
// client.Create strips this field, so we must invoke Delete() to re-apply it.
551560
if shouldDelete {
561+
// 1. Refresh object to avoid ResourceVersion conflict
562+
if err := baseClient.Get(t.Context(), types.NamespacedName{Name: cluster.Name, Namespace: cluster.Namespace}, cluster); err != nil {
563+
t.Fatalf("failed to refresh cluster before delete: %v", err)
564+
}
565+
// 2. Delete it
552566
if err := baseClient.Delete(t.Context(), cluster); err != nil {
553567
t.Fatalf("failed to set deletion timestamp: %v", err)
554568
}
555-
// Refresh our local object to match the client state
569+
// 3. Refresh again to ensure the controller sees the deletion timestamp
556570
if err := baseClient.Get(t.Context(), types.NamespacedName{Name: cluster.Name, Namespace: cluster.Namespace}, cluster); err != nil {
557571
t.Fatalf("failed to refresh cluster after deletion: %v", err)
558572
}
@@ -915,9 +929,15 @@ func TestMultigresClusterReconciler_Reconcile_Failure(t *testing.T) {
915929
t.Run(name, func(t *testing.T) {
916930
t.Parallel()
917931

932+
// Default to all standard templates if existingObjects is nil
933+
objects := tc.existingObjects
934+
if objects == nil {
935+
objects = []client.Object{coreTpl, cellTpl, shardTpl}
936+
}
937+
918938
clientBuilder := fake.NewClientBuilder().
919939
WithScheme(scheme).
920-
WithObjects(tc.existingObjects...).
940+
WithObjects(objects...).
921941
WithStatusSubresource(&multigresv1alpha1.MultigresCluster{}, &multigresv1alpha1.Cell{}, &multigresv1alpha1.TableGroup{})
922942
baseClient := clientBuilder.Build()
923943

@@ -956,10 +976,15 @@ func TestMultigresClusterReconciler_Reconcile_Failure(t *testing.T) {
956976
// Ensure DeletionTimestamp is set in the API if the test requires it.
957977
// client.Create strips this field, so we must invoke Delete() to re-apply it.
958978
if shouldDelete {
979+
// 1. Refresh object to avoid ResourceVersion conflict
980+
if err := baseClient.Get(t.Context(), types.NamespacedName{Name: cluster.Name, Namespace: cluster.Namespace}, cluster); err != nil {
981+
t.Fatalf("failed to refresh cluster before delete: %v", err)
982+
}
983+
// 2. Delete it
959984
if err := baseClient.Delete(t.Context(), cluster); err != nil {
960985
t.Fatalf("failed to set deletion timestamp: %v", err)
961986
}
962-
// Refresh our local object to match the client state
987+
// 3. Refresh again to ensure the controller sees the deletion timestamp
963988
if err := baseClient.Get(t.Context(), types.NamespacedName{Name: cluster.Name, Namespace: cluster.Namespace}, cluster); err != nil {
964989
t.Fatalf("failed to refresh cluster after deletion: %v", err)
965990
}

0 commit comments

Comments
 (0)