Merge pull request #352 from erikgb/fix-test-klog-new

Replace deprecated klog.New in tests with ktesting.NewTestContext
cert-manager · May 17, 2024 · 6821ea9 · 6821ea9
2 parents 2cb20f7 + 8d686f4
commit 6821ea9
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 21 deletions.
diff --git a/pkg/bundle/bundle_test.go b/pkg/bundle/bundle_test.go
@@ -32,7 +32,7 @@ import (
 	coreapplyconfig "k8s.io/client-go/applyconfigurations/core/v1"
 	v1 "k8s.io/client-go/applyconfigurations/meta/v1"
 	"k8s.io/client-go/tools/record"
-	"k8s.io/klog/v2/klogr"
+	"k8s.io/klog/v2/ktesting"
 	fakeclock "k8s.io/utils/clock/testing"
 	"k8s.io/utils/ptr"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -1302,13 +1302,14 @@ func Test_Reconcile(t *testing.T) {
 				resourcePatches []interface{}
 			)
 
+			log, ctx := ktesting.NewTestContext(t)
 			b := &bundle{
 				client:      fakeclient,
 				targetCache: fakeclient,
 				recorder:    fakerecorder,
 				clock:       fixedclock,
 				Options: Options{
-					Log:                  klogr.New(),
+					Log:                  log,
 					Namespace:            trustNamespace,
 					SecretTargetsEnabled: !test.disableSecretTargets,
 					FilterExpiredCerts:   true,
@@ -1325,7 +1326,7 @@ func Test_Reconcile(t *testing.T) {
 			if test.configureDefaultPackage {
 				b.defaultPackage = testDefaultPackage.Clone()
 			}
-			resp, result, err := b.reconcileBundle(context.TODO(), ctrl.Request{NamespacedName: types.NamespacedName{Name: bundleName}})
+			resp, result, err := b.reconcileBundle(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: bundleName}})
 			if (err != nil) != test.expError {
 				t.Errorf("unexpected error, exp=%t got=%v", test.expError, err)
 			}

diff --git a/pkg/bundle/sync_test.go b/pkg/bundle/sync_test.go
@@ -35,7 +35,7 @@ import (
 	coreapplyconfig "k8s.io/client-go/applyconfigurations/core/v1"
 	metav1applyconfig "k8s.io/client-go/applyconfigurations/meta/v1"
 	"k8s.io/client-go/tools/record"
-	"k8s.io/klog/v2/klogr"
+	"k8s.io/klog/v2/ktesting"
 	"k8s.io/utils/ptr"
 	fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/structured-merge-diff/fieldpath"
@@ -642,7 +642,8 @@ func Test_syncConfigMapTarget(t *testing.T) {
 				resolvedBundle.binaryData[pkcs12Key] = pkcs12Data
 			}
 
-			needsUpdate, err := b.syncConfigMapTarget(context.TODO(), klogr.New(), &trustapi.Bundle{
+			log, ctx := ktesting.NewTestContext(t)
+			needsUpdate, err := b.syncConfigMapTarget(ctx, log, &trustapi.Bundle{
 				ObjectMeta: metav1.ObjectMeta{Name: bundleName},
 				Spec:       spec,
 			}, bundleName, test.namespace.Name, resolvedBundle, test.shouldExist)
@@ -1261,7 +1262,8 @@ func Test_syncSecretTarget(t *testing.T) {
 				resolvedBundle.binaryData[pkcs12Key] = pkcs12Data
 			}
 
-			needsUpdate, err := b.syncSecretTarget(context.TODO(), klogr.New(), &trustapi.Bundle{
+			log, ctx := ktesting.NewTestContext(t)
+			needsUpdate, err := b.syncSecretTarget(ctx, log, &trustapi.Bundle{
 				ObjectMeta: metav1.ObjectMeta{Name: bundleName},
 				Spec:       spec,
 			}, bundleName, test.namespace.Name, resolvedBundle, test.shouldExist)

diff --git a/pkg/webhook/validation_test.go b/pkg/webhook/validation_test.go
@@ -17,15 +17,14 @@ limitations under the License.
 package webhook
 
 import (
-	"context"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/validation/field"
-	"k8s.io/klog/v2/klogr"
+	"k8s.io/klog/v2/ktesting"
 	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
@@ -395,8 +394,9 @@ func Test_validate(t *testing.T) {
 
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {
-			v := &validator{log: klogr.New()}
-			gotWarnings, gotErr := v.validate(context.TODO(), test.bundle)
+			log, ctx := ktesting.NewTestContext(t)
+			v := &validator{log: log}
+			gotWarnings, gotErr := v.validate(ctx, test.bundle)
 			if test.expErr == nil && gotErr != nil {
 				t.Errorf("got an unexpected error: %v", gotErr)
 			} else if test.expErr != nil && (gotErr == nil || *test.expErr != gotErr.Error()) {
@@ -447,8 +447,9 @@ func Test_validate_update(t *testing.T) {
 
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {
-			v := &validator{log: klogr.New()}
-			gotWarnings, gotErr := v.ValidateUpdate(context.TODO(), test.oldBundle, test.newBundle)
+			log, ctx := ktesting.NewTestContext(t)
+			v := &validator{log: log}
+			gotWarnings, gotErr := v.ValidateUpdate(ctx, test.oldBundle, test.newBundle)
 			if test.expErr == nil && gotErr != nil {
 				t.Errorf("got an unexpected error: %v", gotErr)
 			} else if test.expErr != nil && (gotErr == nil || *test.expErr != gotErr.Error()) {

diff --git a/test/smoke/suite_test.go b/test/smoke/suite_test.go
@@ -22,10 +22,11 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 
+	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/klog/v2/klogr"
+	"k8s.io/klog/v2/ktesting"
 	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -41,10 +42,23 @@ const (
 )
 
 var _ = Describe("Smoke", func() {
-	It("should create a bundle, sync to ConfigMap target, and then remove all configmap targets when deleted", func() {
-		ctx, cancel := context.WithCancel(context.Background())
-		defer cancel()
+	var (
+		ctx    context.Context
+		cancel func()
+
+		log logr.Logger
+	)
+
+	BeforeEach(func() {
+		log, ctx = ktesting.NewTestContext(GinkgoT())
+		ctx, cancel = context.WithCancel(ctx)
+	})
 
+	AfterEach(func() {
+		cancel()
+	})
+
+	It("should create a bundle, sync to ConfigMap target, and then remove all configmap targets when deleted", func() {
 		cl, err := client.New(cnf.RestConfig, client.Options{
 			Scheme: trustapi.GlobalScheme,
 		})
@@ -54,7 +68,7 @@ var _ = Describe("Smoke", func() {
 		testData := env.DefaultTrustData()
 
 		testBundle := env.NewTestBundleConfigMapTarget(ctx, cl, bundle.Options{
-			Log:       klogr.New(),
+			Log:       log,
 			Namespace: cnf.TrustNamespace,
 		}, testData)
 
@@ -65,9 +79,6 @@ var _ = Describe("Smoke", func() {
 	})
 
 	It("should create a bundle, sync to Secret target, and then remove all secret targets when deleted", func() {
-		ctx, cancel := context.WithCancel(context.Background())
-		defer cancel()
-
 		cl, err := client.New(cnf.RestConfig, client.Options{
 			Scheme: trustapi.GlobalScheme,
 		})
@@ -77,7 +88,7 @@ var _ = Describe("Smoke", func() {
 		testData := env.DefaultTrustData()
 
 		testBundle := env.NewTestBundleSecretTarget(ctx, cl, bundle.Options{
-			Log:       klogr.New(),
+			Log:       log,
 			Namespace: cnf.TrustNamespace,
 		}, testData)