From a89aeb328f3ebdbc087649d5f1e960d960e88d90 Mon Sep 17 00:00:00 2001 From: Harsh Date: Wed, 1 Apr 2026 17:25:58 +0530 Subject: [PATCH 1/5] test(webhook/mutating): migrate tests to Ginkgo/Gomega Signed-off-by: Harsh --- .../handler/mutating/mutating_handler_test.go | 16 +++------------- .../handler/mutating/mutating_suite_test.go | 18 +++++++++++++++++- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/pkg/webhook/handler/mutating/mutating_handler_test.go b/pkg/webhook/handler/mutating/mutating_handler_test.go index 4a46b021e74..85a85ddcd5b 100644 --- a/pkg/webhook/handler/mutating/mutating_handler_test.go +++ b/pkg/webhook/handler/mutating/mutating_handler_test.go @@ -23,7 +23,6 @@ import ( "github.com/agiledragon/gomonkey/v2" datav1alpha1 "github.com/fluid-cloudnative/fluid/api/v1alpha1" "github.com/fluid-cloudnative/fluid/pkg/common" - "github.com/fluid-cloudnative/fluid/pkg/utils" "github.com/fluid-cloudnative/fluid/pkg/utils/fake" "github.com/fluid-cloudnative/fluid/pkg/webhook/plugins" . "github.com/onsi/ginkgo/v2" @@ -1465,7 +1464,6 @@ var _ = Describe("Handle - Global Injection Disabled", func() { var ( decoder *admission.Decoder s *runtime.Scheme - patch *gomonkey.Patches ) BeforeEach(func() { @@ -1474,19 +1472,11 @@ var _ = Describe("Handle - Global Injection Disabled", func() { Expect(corev1.AddToScheme(s)).To(Succeed()) }) - AfterEach(func() { - if patch != nil { - patch.Reset() - } - }) - It("should skip mutation when global injection is disabled", func() { - patch = gomonkey.ApplyFunc(utils.GetBoolValueFromEnv, func(key string, defaultValue bool) bool { - if key == common.EnvDisableInjection { - return true - } - return defaultValue + DeferCleanup(func() { + os.Unsetenv(common.EnvDisableInjection) }) + Expect(os.Setenv(common.EnvDisableInjection, "true")).To(Succeed()) req := admission.Request{ AdmissionRequest: admissionv1.AdmissionRequest{ diff --git a/pkg/webhook/handler/mutating/mutating_suite_test.go b/pkg/webhook/handler/mutating/mutating_suite_test.go index 0d91c0b5fe8..69d58f244c7 100644 --- a/pkg/webhook/handler/mutating/mutating_suite_test.go +++ b/pkg/webhook/handler/mutating/mutating_suite_test.go @@ -1,4 +1,20 @@ -package mutating_test +/* +Copyright 2026 The Fluid Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package mutating import ( "testing" From cc0fa65d566e531df3276ac3d8e1ccd39bec9022 Mon Sep 17 00:00:00 2001 From: Harsh Date: Wed, 1 Apr 2026 19:48:42 +0530 Subject: [PATCH 2/5] fix(webhook/mutating): use GinkgoT().Setenv for env-var restoration Address Copilot and Gemini review findings: replace manual DeferCleanup(os.Unsetenv) with GinkgoT().Setenv() which automatically saves and restores the prior environment variable value after the spec. Signed-off-by: Harsh --- pkg/webhook/handler/mutating/mutating_handler_test.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/webhook/handler/mutating/mutating_handler_test.go b/pkg/webhook/handler/mutating/mutating_handler_test.go index 85a85ddcd5b..44247333ab2 100644 --- a/pkg/webhook/handler/mutating/mutating_handler_test.go +++ b/pkg/webhook/handler/mutating/mutating_handler_test.go @@ -1473,10 +1473,7 @@ var _ = Describe("Handle - Global Injection Disabled", func() { }) It("should skip mutation when global injection is disabled", func() { - DeferCleanup(func() { - os.Unsetenv(common.EnvDisableInjection) - }) - Expect(os.Setenv(common.EnvDisableInjection, "true")).To(Succeed()) + GinkgoT().Setenv(common.EnvDisableInjection, "true") req := admission.Request{ AdmissionRequest: admissionv1.AdmissionRequest{ From e2fbe6b19c547b3f8e95243165ae9511ae9178c4 Mon Sep 17 00:00:00 2001 From: Harsh Date: Wed, 1 Apr 2026 20:03:30 +0530 Subject: [PATCH 3/5] test(webhook/mutating): add webhook_test.go for HandlerMap coverage Signed-off-by: Harsh --- pkg/webhook/handler/mutating/webhook_test.go | 36 ++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 pkg/webhook/handler/mutating/webhook_test.go diff --git a/pkg/webhook/handler/mutating/webhook_test.go b/pkg/webhook/handler/mutating/webhook_test.go new file mode 100644 index 00000000000..3f7633fa841 --- /dev/null +++ b/pkg/webhook/handler/mutating/webhook_test.go @@ -0,0 +1,36 @@ +/* +Copyright 2026 The Fluid Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package mutating + +import ( + "github.com/fluid-cloudnative/fluid/pkg/common" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("HandlerMap", func() { + It("should register exactly one handler under WebhookSchedulePodPath", func() { + Expect(HandlerMap).To(HaveLen(1)) + Expect(HandlerMap).To(HaveKey(common.WebhookSchedulePodPath)) + }) + + It("should map WebhookSchedulePodPath to a *FluidMutatingHandler", func() { + handler, ok := HandlerMap[common.WebhookSchedulePodPath] + Expect(ok).To(BeTrue()) + Expect(handler).To(BeAssignableToTypeOf(&FluidMutatingHandler{})) + }) +}) From 6ab2e8e1693304ac8ce1e532e8a098475298f534 Mon Sep 17 00:00:00 2001 From: Harsh Date: Wed, 1 Apr 2026 23:15:04 +0530 Subject: [PATCH 4/5] fix(webhook/mutating): remove brittle HaveLen assertion from HandlerMap test Signed-off-by: Harsh --- pkg/webhook/handler/mutating/webhook_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/webhook/handler/mutating/webhook_test.go b/pkg/webhook/handler/mutating/webhook_test.go index 3f7633fa841..8b57f62920d 100644 --- a/pkg/webhook/handler/mutating/webhook_test.go +++ b/pkg/webhook/handler/mutating/webhook_test.go @@ -23,8 +23,7 @@ import ( ) var _ = Describe("HandlerMap", func() { - It("should register exactly one handler under WebhookSchedulePodPath", func() { - Expect(HandlerMap).To(HaveLen(1)) + It("should register a handler under WebhookSchedulePodPath", func() { Expect(HandlerMap).To(HaveKey(common.WebhookSchedulePodPath)) }) From 21431ee35b40c6ae62aa4815b8c12e8b4d86377f Mon Sep 17 00:00:00 2001 From: Harsh Date: Thu, 2 Apr 2026 00:59:52 +0530 Subject: [PATCH 5/5] fix(webhook/mutating): assert handler is non-nil before type check Signed-off-by: Harsh --- pkg/webhook/handler/mutating/webhook_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/webhook/handler/mutating/webhook_test.go b/pkg/webhook/handler/mutating/webhook_test.go index 8b57f62920d..69917035bfb 100644 --- a/pkg/webhook/handler/mutating/webhook_test.go +++ b/pkg/webhook/handler/mutating/webhook_test.go @@ -30,6 +30,7 @@ var _ = Describe("HandlerMap", func() { It("should map WebhookSchedulePodPath to a *FluidMutatingHandler", func() { handler, ok := HandlerMap[common.WebhookSchedulePodPath] Expect(ok).To(BeTrue()) + Expect(handler).NotTo(BeNil()) Expect(handler).To(BeAssignableToTypeOf(&FluidMutatingHandler{})) }) })