From 48e58cbd996301ab40fa3582afe559aca4266517 Mon Sep 17 00:00:00 2001
From: sujeet01 <phadtare.sujeet@gmail.com>
Date: Fri, 22 Nov 2024 06:04:24 +0530
Subject: [PATCH] Refactor tests to ensure proper cache sync using custom
 context

---
 broker/bucketbroker/server/bucket_create_test.go    |  6 +++---
 broker/bucketbroker/server/bucket_delete_test.go    | 10 +++++-----
 broker/bucketbroker/server/bucket_list_test.go      |  4 +++-
 broker/bucketbroker/server/bucketclass_list_test.go |  5 +++--
 broker/bucketbroker/server/event_list_test.go       | 13 ++++++-------
 broker/bucketbroker/server/server.go                | 13 +------------
 broker/bucketbroker/server/server_suite_test.go     |  4 ++--
 broker/volumebroker/server/event_list_test.go       |  6 +++---
 broker/volumebroker/server/server_suite_test.go     |  4 ++--
 broker/volumebroker/server/volume_create_test.go    |  6 +++---
 broker/volumebroker/server/volume_delete_test.go    | 10 +++++-----
 broker/volumebroker/server/volume_expand_test.go    | 10 +++++-----
 broker/volumebroker/server/volume_list_test.go      |  4 +++-
 13 files changed, 44 insertions(+), 51 deletions(-)

diff --git a/broker/bucketbroker/server/bucket_create_test.go b/broker/bucketbroker/server/bucket_create_test.go
index 77ad3983c..2366a5f65 100644
--- a/broker/bucketbroker/server/bucket_create_test.go
+++ b/broker/bucketbroker/server/bucket_create_test.go
@@ -10,7 +10,7 @@ import (
 	iri "github.com/ironcore-dev/ironcore/iri/apis/bucket/v1alpha1"
 	irimeta "github.com/ironcore-dev/ironcore/iri/apis/meta/v1alpha1"
 	bucketpoolletv1alpha1 "github.com/ironcore-dev/ironcore/poollet/bucketpoollet/api/v1alpha1"
-
+	. "github.com/ironcore-dev/ironcore/utils/testing"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 
@@ -18,7 +18,8 @@ import (
 )
 
 var _ = Describe("CreateBucket", func() {
-	ns, _, srv := SetupTest()
+	ctx := SetupContext()
+	ns, _, srv := SetupTest(ctx)
 	bucketClass := SetupBucketClass("250Mi", "1500")
 
 	It("should correctly create a bucket", func(ctx SpecContext) {
@@ -35,7 +36,6 @@ var _ = Describe("CreateBucket", func() {
 				},
 			},
 		})
-
 		Expect(err).NotTo(HaveOccurred())
 		Expect(res).NotTo(BeNil())
 
diff --git a/broker/bucketbroker/server/bucket_delete_test.go b/broker/bucketbroker/server/bucket_delete_test.go
index 424ce7754..08edd38d9 100644
--- a/broker/bucketbroker/server/bucket_delete_test.go
+++ b/broker/bucketbroker/server/bucket_delete_test.go
@@ -8,15 +8,16 @@ import (
 	iri "github.com/ironcore-dev/ironcore/iri/apis/bucket/v1alpha1"
 	irimeta "github.com/ironcore-dev/ironcore/iri/apis/meta/v1alpha1"
 	bucketpoolletv1alpha1 "github.com/ironcore-dev/ironcore/poollet/bucketpoollet/api/v1alpha1"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	"sigs.k8s.io/controller-runtime/pkg/client"
-
+	. "github.com/ironcore-dev/ironcore/utils/testing"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 var _ = Describe("DeleteBucket", func() {
-	ns, _, srv := SetupTest()
+	ctx := SetupContext()
+	ns, _, srv := SetupTest(ctx)
 	bucketClass := SetupBucketClass("250Mi", "1500")
 
 	It("should correctly delete a bucket", func(ctx SpecContext) {
@@ -33,7 +34,6 @@ var _ = Describe("DeleteBucket", func() {
 				},
 			},
 		})
-
 		Expect(err).NotTo(HaveOccurred())
 		Expect(createRes).NotTo(BeNil())
 
diff --git a/broker/bucketbroker/server/bucket_list_test.go b/broker/bucketbroker/server/bucket_list_test.go
index 8b333930f..14a59caa2 100644
--- a/broker/bucketbroker/server/bucket_list_test.go
+++ b/broker/bucketbroker/server/bucket_list_test.go
@@ -7,12 +7,14 @@ import (
 	iri "github.com/ironcore-dev/ironcore/iri/apis/bucket/v1alpha1"
 	irimeta "github.com/ironcore-dev/ironcore/iri/apis/meta/v1alpha1"
 	bucketpoolletv1alpha1 "github.com/ironcore-dev/ironcore/poollet/bucketpoollet/api/v1alpha1"
+	. "github.com/ironcore-dev/ironcore/utils/testing"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 )
 
 var _ = Describe("ListBuckets", func() {
-	_, _, srv := SetupTest()
+	ctx := SetupContext()
+	_, _, srv := SetupTest(ctx)
 	bucketClass := SetupBucketClass("250Mi", "1500")
 
 	It("should correctly list buckets", func(ctx SpecContext) {
diff --git a/broker/bucketbroker/server/bucketclass_list_test.go b/broker/bucketbroker/server/bucketclass_list_test.go
index 3161a27aa..f27e101ef 100644
--- a/broker/bucketbroker/server/bucketclass_list_test.go
+++ b/broker/bucketbroker/server/bucketclass_list_test.go
@@ -5,14 +5,15 @@ package server_test
 
 import (
 	iri "github.com/ironcore-dev/ironcore/iri/apis/bucket/v1alpha1"
+	. "github.com/ironcore-dev/ironcore/utils/testing"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	corev1 "k8s.io/api/core/v1"
 )
 
 var _ = Describe("ListBucketClasses", func() {
-
-	_, bucketPool, srv := SetupTest()
+	ctx := SetupContext()
+	_, bucketPool, srv := SetupTest(ctx)
 	bucketClass1 := SetupBucketClass("100Mi", "100")
 	bucketClass2 := SetupBucketClass("200Mi", "200")
 
diff --git a/broker/bucketbroker/server/event_list_test.go b/broker/bucketbroker/server/event_list_test.go
index 7d8ddda6d..0a17ef24d 100644
--- a/broker/bucketbroker/server/event_list_test.go
+++ b/broker/bucketbroker/server/event_list_test.go
@@ -11,21 +11,21 @@ import (
 	"github.com/ironcore-dev/ironcore/iri/apis/event/v1alpha1"
 	irimeta "github.com/ironcore-dev/ironcore/iri/apis/meta/v1alpha1"
 	bucketpoolletv1alpha1 "github.com/ironcore-dev/ironcore/poollet/bucketpoollet/api/v1alpha1"
-
+	. "github.com/ironcore-dev/ironcore/utils/testing"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/client-go/kubernetes/scheme"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-
-	. "github.com/onsi/ginkgo/v2"
-	. "github.com/onsi/gomega"
 )
 
 var _ = Describe("ListEvents", func() {
-	ns, _, srv := SetupTest()
+	ctx := SetupContext()
+	ns, _, srv := SetupTest(ctx)
 	bucketClass := SetupBucketClass("250Mi", "1500")
 
-	FIt("should correctly list events", func(ctx SpecContext) {
+	It("should correctly list events", func(ctx SpecContext) {
 		Expect(storagev1alpha1.AddToScheme(scheme.Scheme)).To(Succeed())
 		k8sManager, err := ctrl.NewManager(cfg, ctrl.Options{
 			Scheme: scheme.Scheme,
@@ -45,7 +45,6 @@ var _ = Describe("ListEvents", func() {
 				},
 			},
 		})
-
 		Expect(err).NotTo(HaveOccurred())
 		Expect(res).NotTo(BeNil())
 
diff --git a/broker/bucketbroker/server/server.go b/broker/bucketbroker/server/server.go
index c3a4d9bf3..6d6a8c206 100644
--- a/broker/bucketbroker/server/server.go
+++ b/broker/bucketbroker/server/server.go
@@ -33,7 +33,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
-	"sigs.k8s.io/controller-runtime/pkg/log"
 )
 
 var scheme = runtime.NewScheme()
@@ -112,10 +111,8 @@ var _ iri.BucketRuntimeServer = (*Server)(nil)
 //+kubebuilder:rbac:groups=storage.ironcore.dev,resources=buckets,verbs=get;list;watch;create;update;patch;delete
 
 func New(ctx context.Context, cfg *rest.Config, opts Options) (*Server, error) {
-	log := log.FromContext(ctx)
-	log.Info("entered server.New")
 	setOptionsDefaults(&opts)
-	log.Info("create cache")
+
 	readCache, err := cache.New(cfg, cache.Options{
 		Scheme: scheme,
 		DefaultNamespaces: map[string]cache.Config{
@@ -126,22 +123,15 @@ func New(ctx context.Context, cfg *rest.Config, opts Options) (*Server, error) {
 		return nil, fmt.Errorf("error creating cache: %w", err)
 	}
 
-	log.Info("start cache")
 	go func() {
 		if err := readCache.Start(ctx); err != nil {
-			log.Info("error starting cache")
 			fmt.Printf("Error starting cache: %v\n", err)
 		}
-		log.Info("started cache")
 	}()
-
-	log.Info("syncing cache")
 	if !readCache.WaitForCacheSync(ctx) {
-		log.Info("timeout waiting for cache sync")
 		return nil, fmt.Errorf("failed to sync cache")
 	}
 
-	log.Info("creating client")
 	c, err := client.New(cfg, client.Options{
 		Scheme: scheme,
 		Cache: &client.CacheOptions{
@@ -153,7 +143,6 @@ func New(ctx context.Context, cfg *rest.Config, opts Options) (*Server, error) {
 		return nil, fmt.Errorf("error creating client: %w", err)
 	}
 
-	log.Info("returning server obj")
 	return &Server{
 		client:             c,
 		namespace:          opts.Namespace,
diff --git a/broker/bucketbroker/server/server_suite_test.go b/broker/bucketbroker/server/server_suite_test.go
index a75f5c050..2be489e0c 100644
--- a/broker/bucketbroker/server/server_suite_test.go
+++ b/broker/bucketbroker/server/server_suite_test.go
@@ -98,14 +98,14 @@ var _ = BeforeSuite(func() {
 	Expect(utilsenvtest.WaitUntilAPIServicesReadyWithTimeout(apiServiceTimeout, testEnvExt, k8sClient, scheme.Scheme)).To(Succeed())
 })
 
-func SetupTest() (*corev1.Namespace, *storagev1alpha1.BucketPool, *server.Server) {
+func SetupTest(ctx context.Context) (*corev1.Namespace, *storagev1alpha1.BucketPool, *server.Server) {
 	var (
 		ns         = &corev1.Namespace{}
 		srv        = &server.Server{}
 		bucketPool = &storagev1alpha1.BucketPool{}
 	)
 
-	BeforeEach(func(ctx SpecContext) {
+	BeforeEach(func() {
 		*ns = corev1.Namespace{
 			ObjectMeta: metav1.ObjectMeta{
 				GenerateName: "test-ns-",
diff --git a/broker/volumebroker/server/event_list_test.go b/broker/volumebroker/server/event_list_test.go
index fb93cd3db..bd6e363a0 100644
--- a/broker/volumebroker/server/event_list_test.go
+++ b/broker/volumebroker/server/event_list_test.go
@@ -12,9 +12,9 @@ import (
 
 	irimeta "github.com/ironcore-dev/ironcore/iri/apis/meta/v1alpha1"
 	volumepoolletv1alpha1 "github.com/ironcore-dev/ironcore/poollet/volumepoollet/api/v1alpha1"
+	. "github.com/ironcore-dev/ironcore/utils/testing"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
-
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/client-go/kubernetes/scheme"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -22,7 +22,8 @@ import (
 )
 
 var _ = Describe("ListEvents", func() {
-	ns, srv := SetupTest()
+	ctx := SetupContext()
+	ns, srv := SetupTest(ctx)
 	volumeClass := SetupVolumeClass()
 
 	It("should correctly list events", func(ctx SpecContext) {
@@ -49,7 +50,6 @@ var _ = Describe("ListEvents", func() {
 				},
 			},
 		})
-
 		Expect(err).NotTo(HaveOccurred())
 		Expect(res).NotTo(BeNil())
 
diff --git a/broker/volumebroker/server/server_suite_test.go b/broker/volumebroker/server/server_suite_test.go
index c46de17f7..755924586 100644
--- a/broker/volumebroker/server/server_suite_test.go
+++ b/broker/volumebroker/server/server_suite_test.go
@@ -99,14 +99,14 @@ var _ = BeforeSuite(func() {
 	Expect(utilsenvtest.WaitUntilAPIServicesReadyWithTimeout(apiServiceTimeout, testEnvExt, k8sClient, scheme.Scheme)).To(Succeed())
 })
 
-func SetupTest() (*corev1.Namespace, *server.Server) {
+func SetupTest(ctx context.Context) (*corev1.Namespace, *server.Server) {
 	var (
 		ns         = &corev1.Namespace{}
 		srv        = &server.Server{}
 		volumePool = &storagev1alpha1.VolumePool{}
 	)
 
-	BeforeEach(func(ctx SpecContext) {
+	BeforeEach(func() {
 		*ns = corev1.Namespace{
 			ObjectMeta: metav1.ObjectMeta{
 				GenerateName: "test-ns-",
diff --git a/broker/volumebroker/server/volume_create_test.go b/broker/volumebroker/server/volume_create_test.go
index 90e2aaf23..89bbfe53b 100644
--- a/broker/volumebroker/server/volume_create_test.go
+++ b/broker/volumebroker/server/volume_create_test.go
@@ -10,7 +10,7 @@ import (
 	irimeta "github.com/ironcore-dev/ironcore/iri/apis/meta/v1alpha1"
 	iri "github.com/ironcore-dev/ironcore/iri/apis/volume/v1alpha1"
 	volumepoolletv1alpha1 "github.com/ironcore-dev/ironcore/poollet/volumepoollet/api/v1alpha1"
-
+	. "github.com/ironcore-dev/ironcore/utils/testing"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 
@@ -18,7 +18,8 @@ import (
 )
 
 var _ = Describe("CreateVolume", func() {
-	ns, srv := SetupTest()
+	ctx := SetupContext()
+	ns, srv := SetupTest(ctx)
 	volumeClass := SetupVolumeClass()
 
 	It("should correctly create a volume", func(ctx SpecContext) {
@@ -38,7 +39,6 @@ var _ = Describe("CreateVolume", func() {
 				},
 			},
 		})
-
 		Expect(err).NotTo(HaveOccurred())
 		Expect(res).NotTo(BeNil())
 
diff --git a/broker/volumebroker/server/volume_delete_test.go b/broker/volumebroker/server/volume_delete_test.go
index bc7f6cb94..bbaac7aca 100644
--- a/broker/volumebroker/server/volume_delete_test.go
+++ b/broker/volumebroker/server/volume_delete_test.go
@@ -8,15 +8,16 @@ import (
 	irimeta "github.com/ironcore-dev/ironcore/iri/apis/meta/v1alpha1"
 	iri "github.com/ironcore-dev/ironcore/iri/apis/volume/v1alpha1"
 	volumepoolletv1alpha1 "github.com/ironcore-dev/ironcore/poollet/volumepoollet/api/v1alpha1"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	"sigs.k8s.io/controller-runtime/pkg/client"
-
+	. "github.com/ironcore-dev/ironcore/utils/testing"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 var _ = Describe("DeleteVolume", func() {
-	ns, srv := SetupTest()
+	ctx := SetupContext()
+	ns, srv := SetupTest(ctx)
 	volumeClass := SetupVolumeClass()
 
 	It("should correctly delete a volume", func(ctx SpecContext) {
@@ -36,7 +37,6 @@ var _ = Describe("DeleteVolume", func() {
 				},
 			},
 		})
-
 		Expect(err).NotTo(HaveOccurred())
 		Expect(createRes).NotTo(BeNil())
 
diff --git a/broker/volumebroker/server/volume_expand_test.go b/broker/volumebroker/server/volume_expand_test.go
index f7ffc695d..6ddf4ef59 100644
--- a/broker/volumebroker/server/volume_expand_test.go
+++ b/broker/volumebroker/server/volume_expand_test.go
@@ -9,16 +9,17 @@ import (
 	irimeta "github.com/ironcore-dev/ironcore/iri/apis/meta/v1alpha1"
 	iri "github.com/ironcore-dev/ironcore/iri/apis/volume/v1alpha1"
 	volumepoolletv1alpha1 "github.com/ironcore-dev/ironcore/poollet/volumepoollet/api/v1alpha1"
+	. "github.com/ironcore-dev/ironcore/utils/testing"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-
-	. "github.com/onsi/ginkgo/v2"
-	. "github.com/onsi/gomega"
 )
 
 var _ = Describe("ExpandVolume", func() {
-	ns, srv := SetupTest()
+	ctx := SetupContext()
+	ns, srv := SetupTest(ctx)
 
 	It("should correctly Expand a volume", func(ctx SpecContext) {
 		By("creating a volume class with resize policy expandonly")
@@ -32,7 +33,6 @@ var _ = Describe("ExpandVolume", func() {
 			},
 			ResizePolicy: storagev1alpha1.ResizePolicyExpandOnly,
 		}
-
 		Expect(k8sClient.Create(ctx, volumeClass)).To(Succeed(), "failed to create test volume class")
 		DeferCleanup(k8sClient.Delete, volumeClass)
 
diff --git a/broker/volumebroker/server/volume_list_test.go b/broker/volumebroker/server/volume_list_test.go
index 4a840e1c6..6975560a7 100644
--- a/broker/volumebroker/server/volume_list_test.go
+++ b/broker/volumebroker/server/volume_list_test.go
@@ -7,12 +7,14 @@ import (
 	irimeta "github.com/ironcore-dev/ironcore/iri/apis/meta/v1alpha1"
 	iri "github.com/ironcore-dev/ironcore/iri/apis/volume/v1alpha1"
 	volumepoolletv1alpha1 "github.com/ironcore-dev/ironcore/poollet/volumepoollet/api/v1alpha1"
+	. "github.com/ironcore-dev/ironcore/utils/testing"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 )
 
 var _ = Describe("ListVolumes", func() {
-	_, srv := SetupTest()
+	ctx := SetupContext()
+	_, srv := SetupTest(ctx)
 	volumeClass := SetupVolumeClass()
 
 	It("should correctly list volumes", func(ctx SpecContext) {