test(base): migrate runtime_test.go to Ginkgo/Gomega (#5602)

* test(base): migrate runtime_test.go to Ginkgo/Gomega

Signed-off-by: Harsh <harshmastic@gmail.com>

* fix: address Copilot review comments

- Add base_internal_suite_test.go for package base tests
- Use DeferCleanup with proper env var save/restore
- Fix os.Setenv to preserve previous values

Signed-off-by: Harsh <harshmastic@gmail.com>

* fix: resolve dual Ginkgo suite conflict and flaky test

- Remove base_internal_suite_test.go (duplicate suite causing 'RunSpecs more than once')
- Convert base_suite_test.go to internal package (package base)
- Update runtime_conventions_test.go to use internal package
- Fix flaky PermitSync test by setting explicit syncRetryDuration before assertions

Signed-off-by: Harsh <harshmastic@gmail.com>

---------

Signed-off-by: Harsh <harshmastic@gmail.com>

Author: hxrshxz

pkg/ddc/base/base_suite_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package base_test
+package base
 
 import (
 	"testing"

pkg/ddc/base/runtime_conventions_test.go

@@ -14,11 +14,10 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package base_test
+package base
 
 import (
 	"github.com/fluid-cloudnative/fluid/pkg/common"
-	"github.com/fluid-cloudnative/fluid/pkg/ddc/base"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 )
@@ -28,7 +27,7 @@ const testNamespace = "default"
 var _ = Describe("RuntimeInfo.GetWorkerStatefulsetName", func() {
 	DescribeTable("returns correct statefulset name",
 		func(runtimeName, runtimeType, suffix string) {
-			info, err := base.BuildRuntimeInfo(runtimeName, testNamespace, runtimeType)
+			info, err := BuildRuntimeInfo(runtimeName, testNamespace, runtimeType)
 			Expect(err).NotTo(HaveOccurred())
 			Expect(info.GetWorkerStatefulsetName()).To(Equal(runtimeName + suffix))
 		},

pkg/ddc/base/runtime_test.go

@@ -17,10 +17,12 @@ limitations under the License.
 package base
 
 import (
-	"reflect"
-	"testing"
+	"os"
 	"time"
 
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+
 	"github.com/fluid-cloudnative/fluid/api/v1alpha1"
 	"github.com/fluid-cloudnative/fluid/pkg/common"
 	cruntime "github.com/fluid-cloudnative/fluid/pkg/runtime"
@@ -34,7 +36,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-func Test_convertToTieredstoreInfo(t *testing.T) {
+var _ = Describe("convertToTieredstoreInfo", func() {
 	type args struct {
 		tieredstore v1alpha1.TieredStore
 	}
@@ -122,20 +124,19 @@ func Test_convertToTieredstoreInfo(t *testing.T) {
 		},
 	}
 	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
+		It(tt.name, func() {
 			got, err := convertToTieredstoreInfo(tt.args.tieredstore)
-			if (err != nil) != tt.wantErr {
-				t.Errorf("convertToTieredstoreInfo() error = %v, wantErr %v", err, tt.wantErr)
-				return
-			}
-			if !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("convertToTieredstoreInfo() got = %v, want %v", got, tt.want)
+			if tt.wantErr {
+				Expect(err).To(HaveOccurred())
+			} else {
+				Expect(err).NotTo(HaveOccurred())
+				Expect(got).To(Equal(tt.want))
 			}
 		})
 	}
-}
+})
 
-func TestBuildRuntimeInfo(t *testing.T) {
+var _ = Describe("BuildRuntimeInfo", func() {
 	const runtimetype = "alluxio"
 	type args struct {
 		name        string
@@ -191,30 +192,26 @@ func TestBuildRuntimeInfo(t *testing.T) {
 		},
 	}
 	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
+		It(tt.name, func() {
 			gotRuntime, err := BuildRuntimeInfo(tt.args.name, tt.args.namespace, tt.args.runtimeType, WithTieredStore(tt.args.tieredstore))
-			if (err != nil) != tt.wantErr {
-				t.Errorf("BuildRuntimeInfo() error = %v, wantErr %v", err, tt.wantErr)
-				return
-			}
-
-			tieredstoreInfo, err := convertToTieredstoreInfo(tieredstore)
-			if (err != nil) != tt.wantErr {
-				t.Errorf("convertToTieredstoreInfo() error = %v, wantErr %v", err, tt.wantErr)
+			if tt.wantErr {
+				Expect(err).To(HaveOccurred())
+			} else {
+				Expect(err).NotTo(HaveOccurred())
+
+				tieredstoreInfo, err := convertToTieredstoreInfo(tieredstore)
+				Expect(err).NotTo(HaveOccurred())
+
+				Expect(gotRuntime.GetName()).To(Equal(tt.wantRuntime.GetName()))
+				Expect(gotRuntime.GetNamespace()).To(Equal(tt.wantRuntime.GetNamespace()))
+				Expect(gotRuntime.GetRuntimeType()).To(Equal(tt.wantRuntime.GetRuntimeType()))
+				Expect(gotRuntime.GetTieredStoreInfo()).To(Equal(tieredstoreInfo))
 			}
-
-			if gotRuntime.GetName() != tt.wantRuntime.GetName() ||
-				gotRuntime.GetNamespace() != tt.wantRuntime.GetNamespace() ||
-				gotRuntime.GetRuntimeType() != tt.wantRuntime.GetRuntimeType() ||
-				!reflect.DeepEqual(gotRuntime.GetTieredStoreInfo(), tieredstoreInfo) {
-				t.Errorf("BuildRuntimeInfo() gotRuntime = %v, want %v", gotRuntime, tt.wantRuntime)
-			}
-
 		})
 	}
-}
+})
 
-func TestCleanPolicy(t *testing.T) {
+var _ = Describe("CleanPolicy", func() {
 	s := runtime.NewScheme()
 
 	s.AddKnownTypes(v1alpha1.GroupVersion, &v1alpha1.AlluxioRuntime{})
@@ -793,21 +790,20 @@ func TestCleanPolicy(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
+		It(tt.name, func() {
 			// SetupFuseCleanPolicy will be called in GetRuntimeInfo()
 			got, err := GetRuntimeInfo(tt.args.client, tt.args.name, tt.args.namespace)
-			if (err != nil) != tt.wantErr {
-				t.Errorf("GetRuntimeInfo() error = %v, wantErr %v", err, tt.wantErr)
-				return
-			}
-			if !tt.wantErr && !reflect.DeepEqual(got.GetFuseCleanPolicy(), tt.want.GetFuseCleanPolicy()) {
-				t.Errorf("GetRuntimeInfo() = %#v, want %#v", got, tt.want)
+			if tt.wantErr {
+				Expect(err).To(HaveOccurred())
+			} else {
+				Expect(err).NotTo(HaveOccurred())
+				Expect(got.GetFuseCleanPolicy()).To(Equal(tt.want.GetFuseCleanPolicy()))
 			}
 		})
 	}
-}
+})
 
-func TestGetRuntimeInfo(t *testing.T) {
+var _ = Describe("GetRuntimeInfo", func() {
 	s := runtime.NewScheme()
 
 	alluxioRuntime := v1alpha1.AlluxioRuntime{
@@ -1142,11 +1138,12 @@ func TestGetRuntimeInfo(t *testing.T) {
 		},
 	}
 	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
+		It(tt.name, func() {
 			got, err := GetRuntimeInfo(tt.args.client, tt.args.name, tt.args.namespace)
-			if (err != nil) != tt.wantErr {
-				t.Errorf("GetRuntimeInfo() error = %v, wantErr %v", err, tt.wantErr)
-				return
+			if tt.wantErr {
+				Expect(err).To(HaveOccurred())
+			} else {
+				Expect(err).NotTo(HaveOccurred())
 			}
 			if got != nil {
 				got.SetAPIReader(nil)
@@ -1156,14 +1153,14 @@ func TestGetRuntimeInfo(t *testing.T) {
 				tt.want.SetAPIReader(nil)
 			}
 
-			if !tt.wantErr && !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("GetRuntimeInfo() = %#v\n, want %#v", got, tt.want)
+			if !tt.wantErr {
+				Expect(got).To(Equal(tt.want))
 			}
 		})
 	}
-}
+})
 
-func TestGetRuntimeStatus(t *testing.T) {
+var _ = Describe("GetRuntimeStatus", func() {
 	s := runtime.NewScheme()
 
 	alluxioRuntime := v1alpha1.AlluxioRuntime{
@@ -1373,68 +1370,85 @@ func TestGetRuntimeStatus(t *testing.T) {
 		},
 	}
 	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
+		It(tt.name, func() {
 			_, err := GetRuntimeStatus(tt.args.client, tt.args.runtimeType, tt.args.name, tt.args.namespace)
-			if (err != nil) != tt.wantErr {
-				t.Errorf("GetRuntimeInfo() error = %v, wantErr %v", err, tt.wantErr)
-				return
+			if tt.wantErr {
+				Expect(err).To(HaveOccurred())
+			} else {
+				Expect(err).NotTo(HaveOccurred())
 			}
 		})
 	}
-}
-
-func TestGetSyncRetryDuration(t *testing.T) {
-
-	_, err := getSyncRetryDuration()
-	if err != nil {
-		t.Errorf("Failed to getSyncRetryDuration %v", err)
-	}
-
-	t.Setenv(syncRetryDurationEnv, "s")
-	_, err = getSyncRetryDuration()
-	if err == nil {
-		t.Errorf("Expect to get err, but got nil")
-	}
-
-	t.Setenv(syncRetryDurationEnv, "3s")
-	d, err := getSyncRetryDuration()
-	if err != nil {
-		t.Errorf("Failed to getSyncRetryDuration %v", err)
-	}
-	if d == nil {
-		t.Errorf("Failed to set the duration, expect %v, got %v", time.Duration(3*time.Second), d)
-	}
-}
-
-func TestPermitSync(t *testing.T) {
-
-	id := "test id"
-	ctx := cruntime.ReconcileRequestContext{
-		NamespacedName: types.NamespacedName{
-			Name:      "hbase",
-			Namespace: "fluid",
-		},
-		Log:     fakeutils.NullLogger(),
-		Runtime: &v1alpha1.AlluxioRuntime{},
-	}
-
-	templateEngine := NewTemplateEngine(nil, id, ctx)
-	permit := templateEngine.permitSync(types.NamespacedName{Namespace: ctx.Namespace, Name: ctx.Namespace})
-	if !permit {
-		t.Errorf("expect permit, but got %v", permit)
-	}
+})
+
+var _ = Describe("GetSyncRetryDuration", func() {
+	It("should get default sync retry duration", func() {
+		_, err := getSyncRetryDuration()
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	It("should fail with invalid duration format", func() {
+		oldVal, wasSet := os.LookupEnv(syncRetryDurationEnv)
+		err := os.Setenv(syncRetryDurationEnv, "s")
+		Expect(err).NotTo(HaveOccurred())
+		DeferCleanup(func() {
+			if wasSet {
+				_ = os.Setenv(syncRetryDurationEnv, oldVal)
+			} else {
+				_ = os.Unsetenv(syncRetryDurationEnv)
+			}
+		})
 
-	templateEngine.setTimeOfLastSync()
-	permit = templateEngine.permitSync(types.NamespacedName{Namespace: ctx.Namespace, Name: ctx.Namespace})
-	if permit {
-		t.Errorf("expect not permit, but got %v", permit)
-	}
+		_, err = getSyncRetryDuration()
+		Expect(err).To(HaveOccurred())
+	})
+
+	It("should successfully parse valid duration", func() {
+		oldVal, wasSet := os.LookupEnv(syncRetryDurationEnv)
+		err := os.Setenv(syncRetryDurationEnv, "3s")
+		Expect(err).NotTo(HaveOccurred())
+		DeferCleanup(func() {
+			if wasSet {
+				_ = os.Setenv(syncRetryDurationEnv, oldVal)
+			} else {
+				_ = os.Unsetenv(syncRetryDurationEnv)
+			}
+		})
 
-	templateEngine.setTimeOfLastSync()
-	templateEngine.syncRetryDuration = 1 * time.Microsecond
-	time.Sleep(1 * time.Second)
-	permit = templateEngine.permitSync(types.NamespacedName{Namespace: ctx.Namespace, Name: ctx.Namespace})
-	if !permit {
-		t.Errorf("expect permit, but got %v", permit)
-	}
-}
+		d, err := getSyncRetryDuration()
+		Expect(err).NotTo(HaveOccurred())
+		Expect(d).NotTo(BeNil())
+		Expect(*d).To(Equal(time.Duration(3 * time.Second)))
+	})
+})
+
+var _ = Describe("PermitSync", func() {
+	It("should control sync permission based on time", func() {
+		id := "test id"
+		ctx := cruntime.ReconcileRequestContext{
+			NamespacedName: types.NamespacedName{
+				Name:      "hbase",
+				Namespace: "fluid",
+			},
+			Log:     fakeutils.NullLogger(),
+			Runtime: &v1alpha1.AlluxioRuntime{},
+		}
+
+		templateEngine := NewTemplateEngine(nil, id, ctx)
+		permit := templateEngine.permitSync(ctx.NamespacedName)
+		Expect(permit).To(BeTrue(), "expect permit initially")
+
+		// Set a long syncRetryDuration to ensure the test is not flaky
+		templateEngine.syncRetryDuration = 1 * time.Hour
+		templateEngine.setTimeOfLastSync()
+		permit = templateEngine.permitSync(ctx.NamespacedName)
+		Expect(permit).To(BeFalse(), "expect not permit immediately after sync")
+
+		// Now set a very short duration and verify permit is granted after waiting
+		templateEngine.setTimeOfLastSync()
+		templateEngine.syncRetryDuration = 1 * time.Microsecond
+		time.Sleep(10 * time.Millisecond) // Wait longer than syncRetryDuration
+		permit = templateEngine.permitSync(ctx.NamespacedName)
+		Expect(permit).To(BeTrue(), "expect permit after retry duration elapsed")
+	})
+})