Remove types deprecated in 0.4 (#237)

- `rtesting.TestLogger` -> use https://pkg.go.dev/github.com/go-logr/logr@v1.2.2/testing#NewTestLogger instead - `ReconcilerTestCase#Test` -> `ReconcilerTestCase#Run` - `SubReconcilerTestCase#Test` -> `SubReconcilerTestCase#Run` Signed-off-by: Scott Andrews <andrewssc@vmware.com>
vmware-labs · May 16, 2022 · 01ff7be · 01ff7be
1 parent 75bd54f
commit 01ff7be
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 79 deletions.
diff --git a/reconcilers/reconcilers.go b/reconcilers/reconcilers.go
@@ -381,6 +381,7 @@ func StashCastParentType(ctx context.Context, currentType client.Object) context
 	return context.WithValue(ctx, castParentTypeStashKey, currentType)
 }
 
+// RetrieveRequest returns the reconciler Request from the context, or empty if not found.
 func RetrieveRequest(ctx context.Context) ctrl.Request {
 	value := ctx.Value(requestStashKey)
 	if req, ok := value.(ctrl.Request); ok {
@@ -389,38 +390,45 @@ func RetrieveRequest(ctx context.Context) ctrl.Request {
 	return ctrl.Request{}
 }
 
-func RetrieveConfig(ctx context.Context) Config {
+// RetrieveConfig returns the Config from the context. An error is returned if not found.
+func RetrieveConfig(ctx context.Context) (Config, error) {
 	value := ctx.Value(configStashKey)
 	if config, ok := value.(Config); ok {
-		return config
+		return config, nil
 	}
-	return Config{}
+	return Config{}, fmt.Errorf("config must exist on the context. Check that the context is from a ParentReconciler or WithConfig")
 }
 
+// RetrieveConfigOrDie returns the Config from the context. Panics if not found.
 func RetrieveConfigOrDie(ctx context.Context) Config {
-	config := RetrieveConfig(ctx)
-	if config.IsEmpty() {
-		panic(fmt.Errorf("config must exist on the context. Check that the context is from a ParentReconciler or WithConfig"))
+	config, err := RetrieveConfig(ctx)
+	if err != nil {
+		panic(err)
 	}
 	return config
 }
 
-func RetrieveParentConfig(ctx context.Context) Config {
+// RetrieveParentConfig returns the Config from the context used to load the parent resource. An
+// error is returned if not found.
+func RetrieveParentConfig(ctx context.Context) (Config, error) {
 	value := ctx.Value(parentConfigStashKey)
 	if parentConfig, ok := value.(Config); ok {
-		return parentConfig
+		return parentConfig, nil
 	}
-	return Config{}
+	return Config{}, fmt.Errorf("parent config must exist on the context. Check that the context is from a ParentReconciler")
 }
 
+// RetrieveParentConfigOrDie returns the Config from the context used to load the parent resource.
+// Panics if not found.
 func RetrieveParentConfigOrDie(ctx context.Context) Config {
-	config := RetrieveParentConfig(ctx)
-	if config.IsEmpty() {
-		panic(fmt.Errorf("parent config must exist on the context. Check that the context is from a ParentReconciler"))
+	config, err := RetrieveParentConfig(ctx)
+	if err != nil {
+		panic(err)
 	}
 	return config
 }
 
+// RetrieveParentType returns the parent type object, or nil if not found.
 func RetrieveParentType(ctx context.Context) client.Object {
 	value := ctx.Value(parentTypeStashKey)
 	if parentType, ok := value.(client.Object); ok {
@@ -429,6 +437,7 @@ func RetrieveParentType(ctx context.Context) client.Object {
 	return nil
 }
 
+// RetrieveCastParentType returns the parent type object, or nil if not found.
 func RetrieveCastParentType(ctx context.Context) client.Object {
 	value := ctx.Value(castParentTypeStashKey)
 	if currentType, ok := value.(client.Object); ok {
@@ -1452,7 +1461,7 @@ func (r *WithConfig) SetupWithManager(ctx context.Context, mgr ctrl.Manager, bld
 	if err := r.validate(ctx); err != nil {
 		return err
 	}
-	c, err := r.Config(ctx, RetrieveConfig(ctx))
+	c, err := r.Config(ctx, RetrieveConfigOrDie(ctx))
 	if err != nil {
 		return err
 	}
@@ -1479,7 +1488,7 @@ func (r *WithConfig) Reconcile(ctx context.Context, parent client.Object) (ctrl.
 		WithName(r.Name)
 	ctx = logr.NewContext(ctx, log)
 
-	c, err := r.Config(ctx, RetrieveConfig(ctx))
+	c, err := r.Config(ctx, RetrieveConfigOrDie(ctx))
 	if err != nil {
 		return ctrl.Result{}, err
 	}
@@ -1600,7 +1609,7 @@ func ClearParentFinalizer(ctx context.Context, parent client.Object, finalizer s
 }
 
 func ensureParentFinalizer(ctx context.Context, parent client.Object, finalizer string, add bool) error {
-	config := RetrieveParentConfig(ctx)
+	config := RetrieveParentConfigOrDie(ctx)
 	if config.IsEmpty() {
 		panic(fmt.Errorf("parent config must exist on the context. Check that the context from a ParentReconciler"))
 	}

diff --git a/reconcilers/reconcilers_test.go b/reconcilers/reconcilers_test.go
@@ -74,10 +74,10 @@ func TestConfig_TrackAndGet(t *testing.T) {
 		},
 	}}
 
-	rts.Test(t, scheme, func(t *testing.T, rtc *rtesting.SubReconcilerTestCase, c reconcilers.Config) reconcilers.SubReconciler {
+	rts.Run(t, scheme, func(t *testing.T, rtc *rtesting.SubReconcilerTestCase, c reconcilers.Config) reconcilers.SubReconciler {
 		return &reconcilers.SyncReconciler{
 			Sync: func(ctx context.Context, parent *resources.TestResource) error {
-				c := reconcilers.RetrieveConfig(ctx)
+				c := reconcilers.RetrieveConfigOrDie(ctx)
 
 				cm := &corev1.ConfigMap{}
 				err := c.TrackAndGet(ctx, types.NamespacedName{Namespace: "track-namespace", Name: "track-name"}, cm)
@@ -127,7 +127,7 @@ func TestParentReconciler_NoStatus(t *testing.T) {
 			},
 		},
 	}}
-	rts.Test(t, scheme, func(t *testing.T, rtc *rtesting.ReconcilerTestCase, c reconcilers.Config) reconcile.Reconciler {
+	rts.Run(t, scheme, func(t *testing.T, rtc *rtesting.ReconcilerTestCase, c reconcilers.Config) reconcile.Reconciler {
 		return &reconcilers.ParentReconciler{
 			Type:       &resources.TestResourceNoStatus{},
 			Reconciler: rtc.Metadata["SubReconciler"].(func(*testing.T, reconcilers.Config) reconcilers.SubReconciler)(t, c),
@@ -168,7 +168,7 @@ func TestParentReconciler_EmptyStatus(t *testing.T) {
 			},
 		},
 	}}
-	rts.Test(t, scheme, func(t *testing.T, rtc *rtesting.ReconcilerTestCase, c reconcilers.Config) reconcile.Reconciler {
+	rts.Run(t, scheme, func(t *testing.T, rtc *rtesting.ReconcilerTestCase, c reconcilers.Config) reconcile.Reconciler {
 		return &reconcilers.ParentReconciler{
 			Type:       &resources.TestResourceEmptyStatus{},
 			Reconciler: rtc.Metadata["SubReconciler"].(func(*testing.T, reconcilers.Config) reconcilers.SubReconciler)(t, c),
@@ -309,7 +309,7 @@ func TestParentReconciler_NilableStatus(t *testing.T) {
 		ShouldErr: true,
 	}}
 
-	rts.Test(t, scheme, func(t *testing.T, rtc *rtesting.ReconcilerTestCase, c reconcilers.Config) reconcile.Reconciler {
+	rts.Run(t, scheme, func(t *testing.T, rtc *rtesting.ReconcilerTestCase, c reconcilers.Config) reconcile.Reconciler {
 		return &reconcilers.ParentReconciler{
 			Type:       &resources.TestResourceNilableStatus{},
 			Reconciler: rtc.Metadata["SubReconciler"].(func(*testing.T, reconcilers.Config) reconcilers.SubReconciler)(t, c),
@@ -544,10 +544,10 @@ func TestParentReconciler(t *testing.T) {
 			"SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler {
 				return &reconcilers.SyncReconciler{
 					Sync: func(ctx context.Context, parent *resources.TestResource) error {
-						if config := reconcilers.RetrieveConfig(ctx); config != c {
+						if config := reconcilers.RetrieveConfigOrDie(ctx); config != c {
 							t.Errorf("expected config in context, found %#v", config)
 						}
-						if parentConfig := reconcilers.RetrieveParentConfig(ctx); parentConfig != c {
+						if parentConfig := reconcilers.RetrieveParentConfigOrDie(ctx); parentConfig != c {
 							t.Errorf("expected parent config in context, found %#v", parentConfig)
 						}
 						return nil
@@ -578,7 +578,7 @@ func TestParentReconciler(t *testing.T) {
 		},
 	}}
 
-	rts.Test(t, scheme, func(t *testing.T, rtc *rtesting.ReconcilerTestCase, c reconcilers.Config) reconcile.Reconciler {
+	rts.Run(t, scheme, func(t *testing.T, rtc *rtesting.ReconcilerTestCase, c reconcilers.Config) reconcile.Reconciler {
 		return &reconcilers.ParentReconciler{
 			Type:       &resources.TestResource{},
 			Reconciler: rtc.Metadata["SubReconciler"].(func(*testing.T, reconcilers.Config) reconcilers.SubReconciler)(t, c),
@@ -770,7 +770,7 @@ func TestSyncReconciler(t *testing.T) {
 		ShouldErr: true,
 	}}
 
-	rts.Test(t, scheme, func(t *testing.T, rtc *rtesting.SubReconcilerTestCase, c reconcilers.Config) reconcilers.SubReconciler {
+	rts.Run(t, scheme, func(t *testing.T, rtc *rtesting.SubReconcilerTestCase, c reconcilers.Config) reconcilers.SubReconciler {
 		return rtc.Metadata["SubReconciler"].(func(*testing.T, reconcilers.Config) reconcilers.SubReconciler)(t, c)
 	})
 }
@@ -1706,7 +1706,7 @@ func TestChildReconciler(t *testing.T) {
 		ShouldErr: true,
 	}}
 
-	rts.Test(t, scheme, func(t *testing.T, rtc *rtesting.SubReconcilerTestCase, c reconcilers.Config) reconcilers.SubReconciler {
+	rts.Run(t, scheme, func(t *testing.T, rtc *rtesting.SubReconcilerTestCase, c reconcilers.Config) reconcilers.SubReconciler {
 		return rtc.Metadata["SubReconciler"].(func(*testing.T, reconcilers.Config) reconcilers.SubReconciler)(t, c)
 	})
 }
@@ -1951,7 +1951,7 @@ func TestSequence(t *testing.T) {
 		ExpectedResult: ctrl.Result{RequeueAfter: 1 * time.Minute},
 	}}
 
-	rts.Test(t, scheme, func(t *testing.T, rtc *rtesting.SubReconcilerTestCase, c reconcilers.Config) reconcilers.SubReconciler {
+	rts.Run(t, scheme, func(t *testing.T, rtc *rtesting.SubReconcilerTestCase, c reconcilers.Config) reconcilers.SubReconciler {
 		return rtc.Metadata["SubReconciler"].(func(*testing.T, reconcilers.Config) reconcilers.SubReconciler)(t, c)
 	})
 }
@@ -2149,7 +2149,7 @@ func TestCastParent(t *testing.T) {
 		ShouldErr: true,
 	}}
 
-	rts.Test(t, scheme, func(t *testing.T, rtc *rtesting.SubReconcilerTestCase, c reconcilers.Config) reconcilers.SubReconciler {
+	rts.Run(t, scheme, func(t *testing.T, rtc *rtesting.SubReconcilerTestCase, c reconcilers.Config) reconcilers.SubReconciler {
 		return rtc.Metadata["SubReconciler"].(func(*testing.T, reconcilers.Config) reconcilers.SubReconciler)(t, c)
 	})
 }
@@ -2183,8 +2183,8 @@ func TestWithConfig(t *testing.T) {
 					},
 					Reconciler: &reconcilers.SyncReconciler{
 						Sync: func(ctx context.Context, parent *resources.TestResource) error {
-							ac := reconcilers.RetrieveConfig(ctx)
-							apc := reconcilers.RetrieveParentConfig(ctx)
+							ac := reconcilers.RetrieveConfigOrDie(ctx)
+							apc := reconcilers.RetrieveParentConfigOrDie(ctx)
 
 							if ac != c {
 								t.Errorf("unexpected config")
@@ -2206,7 +2206,7 @@ func TestWithConfig(t *testing.T) {
 		},
 	}}
 
-	rts.Test(t, scheme, func(t *testing.T, rtc *rtesting.SubReconcilerTestCase, c reconcilers.Config) reconcilers.SubReconciler {
+	rts.Run(t, scheme, func(t *testing.T, rtc *rtesting.SubReconcilerTestCase, c reconcilers.Config) reconcilers.SubReconciler {
 		return rtc.Metadata["SubReconciler"].(func(*testing.T, reconcilers.Config) reconcilers.SubReconciler)(t, c)
 	})
 }
@@ -2350,7 +2350,7 @@ func TestWithFinalizer(t *testing.T) {
 		},
 	}}
 
-	rts.Test(t, scheme, func(t *testing.T, rtc *rtesting.SubReconcilerTestCase, c reconcilers.Config) reconcilers.SubReconciler {
+	rts.Run(t, scheme, func(t *testing.T, rtc *rtesting.SubReconcilerTestCase, c reconcilers.Config) reconcilers.SubReconciler {
 		var syncErr, finalizeErr error
 		if err, ok := rtc.Metadata["SyncError"]; ok {
 			syncErr = err.(error)

diff --git a/testing/logger.go b/testing/logger.go
diff --git a/testing/reconciler.go b/testing/reconciler.go
@@ -95,13 +95,6 @@ type VerifyFunc func(t *testing.T, result controllerruntime.Result, err error)
 // ReconcilerTestSuite represents a list of reconciler test cases.
 type ReconcilerTestSuite []ReconcilerTestCase
 
-// Deprecated: Use Run instead
-// Test executes the test case.
-func (tc *ReconcilerTestCase) Test(t *testing.T, scheme *runtime.Scheme, factory ReconcilerFactory) {
-	t.Helper()
-	tc.Run(t, scheme, factory)
-}
-
 // Run executes the test case.
 func (tc *ReconcilerTestCase) Run(t *testing.T, scheme *runtime.Scheme, factory ReconcilerFactory) {
 	t.Helper()
@@ -350,13 +343,6 @@ var (
 	SafeDeployDiff = cmpopts.IgnoreUnexported(resource.Quantity{})
 )
 
-// Deprecated: Use Run instead
-// Test executes the reconciler test suite.
-func (ts ReconcilerTestSuite) Test(t *testing.T, scheme *runtime.Scheme, factory ReconcilerFactory) {
-	t.Helper()
-	ts.Run(t, scheme, factory)
-}
-
 // Run executes the reconciler test suite.
 func (ts ReconcilerTestSuite) Run(t *testing.T, scheme *runtime.Scheme, factory ReconcilerFactory) {
 	t.Helper()
@@ -374,7 +360,7 @@ func (ts ReconcilerTestSuite) Run(t *testing.T, scheme *runtime.Scheme, factory
 	for _, test := range testsToExecute {
 		t.Run(test.Name, func(t *testing.T) {
 			t.Helper()
-			test.Test(t, scheme, factory)
+			test.Run(t, scheme, factory)
 		})
 	}
 	if len(focused) > 0 {

diff --git a/testing/subreconciler.go b/testing/subreconciler.go
@@ -93,13 +93,6 @@ type SubReconcilerTestCase struct {
 // SubReconcilerTestSuite represents a list of subreconciler test cases.
 type SubReconcilerTestSuite []SubReconcilerTestCase
 
-// Deprecated: Use Run instead
-// Test executes the test case.
-func (tc *SubReconcilerTestCase) Test(t *testing.T, scheme *runtime.Scheme, factory SubReconcilerFactory) {
-	t.Helper()
-	tc.Run(t, scheme, factory)
-}
-
 // Run executes the test case.
 func (tc *SubReconcilerTestCase) Run(t *testing.T, scheme *runtime.Scheme, factory SubReconcilerFactory) {
 	t.Helper()
@@ -330,13 +323,6 @@ func (tc *SubReconcilerTestCase) Run(t *testing.T, scheme *runtime.Scheme, facto
 	}
 }
 
-// Deprecated: Use Run instead
-// Test executes the subreconciler test suite.
-func (ts SubReconcilerTestSuite) Test(t *testing.T, scheme *runtime.Scheme, factory SubReconcilerFactory) {
-	t.Helper()
-	ts.Run(t, scheme, factory)
-}
-
 // Run executes the subreconciler test suite.
 func (ts SubReconcilerTestSuite) Run(t *testing.T, scheme *runtime.Scheme, factory SubReconcilerFactory) {
 	t.Helper()