Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
267 changes: 267 additions & 0 deletions pkg/ctrl/watch/app_manager_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,267 @@
/*
Copyright 2022 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 watch

import (
"context"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/metrics/server"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
)

var _ = Describe("SetupAppWatcherWithReconciler", func() {
var (
mgr manager.Manager
testScheme *runtime.Scheme
)

BeforeEach(func() {
testScheme = runtime.NewScheme()
Expect(corev1.AddToScheme(testScheme)).To(Succeed())

var err error
mgr, err = ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
Scheme: testScheme,
})
Expect(err).NotTo(HaveOccurred())
Expect(mgr).NotTo(BeNil())
})

Context("when setting up watcher with valid parameters", func() {
It("should successfully create a controller with reconciler", func() {
mockController := &MockController{
name: "test-controller",
}

options := controller.Options{
MaxConcurrentReconciles: 1,
}

err := SetupAppWatcherWithReconciler(mgr, options, mockController)
Expect(err).NotTo(HaveOccurred())
})

It("should set the reconciler in options", func() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This test's description, 'should set the reconciler in options', is not validated by its implementation. The test only checks for successful execution, similar to the test at lines 54-65, making it somewhat redundant. Consider removing this test or renaming it to better reflect its purpose, for example, 'should succeed with non-default options'.

mockController := &MockController{
name: "reconciler-test-controller",
}

options := controller.Options{
MaxConcurrentReconciles: 2,
}

err := SetupAppWatcherWithReconciler(mgr, options, mockController)
Expect(err).NotTo(HaveOccurred())
})
})

Context("when controller name is provided", func() {
It("should use the controller name from the reconciler", func() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This test's description, 'should use the controller name from the reconciler', is not validated by its implementation. The test only checks for successful execution. Consider renaming it to more accurately describe what is being tested, for example, 'should succeed when a controller name is provided'.

expectedName := "my-custom-controller"
mockController := &MockController{
name: expectedName,
}

options := controller.Options{}

err := SetupAppWatcherWithReconciler(mgr, options, mockController)
Expect(err).NotTo(HaveOccurred())
})
})

Context("when managed resource is specified", func() {
It("should watch the managed resource type", func() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This test's description, 'should watch the managed resource type', is not validated by its implementation. The test only checks for successful execution. Consider renaming it to more accurately describe what is being tested, for example, 'should succeed when a managed resource is specified'.

mockController := &MockController{
name: "resource-controller",
managedResource: &corev1.Pod{},
}

options := controller.Options{}

err := SetupAppWatcherWithReconciler(mgr, options, mockController)
Expect(err).NotTo(HaveOccurred())
})
})

Context("when controller creation fails", func() {
It("should return an error if controller cannot be created", func() {
Skip("Requires mocking controller.New which is not easily testable")
})
})

Context("when watch setup fails", func() {
It("should return an error if watch cannot be established", func() {
// This test would require mocking the Watch function
// Similar to above, this requires dependency injection or factory pattern
Skip("Requires mocking controller.Watch which is not easily testable")
})
})
})

// MockController implements the Controller interface for testing
type MockController struct {
name string
managedResource client.Object
}

func (m *MockController) ControllerName() string {
return m.name
}

func (m *MockController) ManagedResource() client.Object {
if m.managedResource != nil {
return m.managedResource
}
return &corev1.Pod{}
}

func (m *MockController) Reconcile(ctx context.Context, req reconcile.Request) (reconcile.Result, error) {
return reconcile.Result{}, nil
}

// Additional test suite for testing with a real manager setup
var _ = Describe("SetupAppWatcherWithReconciler Integration", func() {
var (
mgr manager.Manager
mockController *MockController
controllerName string
)

BeforeEach(func() {
controllerName = "integration-test-controller"
mockController = &MockController{
name: controllerName,
managedResource: &corev1.Pod{},
}

testScheme := runtime.NewScheme()
Expect(corev1.AddToScheme(testScheme)).To(Succeed())

var err error
mgr, err = ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
Scheme: testScheme,
Metrics: server.Options{BindAddress: "0"}, // Disable metrics server
})
Expect(err).NotTo(HaveOccurred())
})

Context("with complete setup", func() {
It("should successfully setup watcher with all components", func() {
options := controller.Options{
MaxConcurrentReconciles: 3,
}

err := SetupAppWatcherWithReconciler(mgr, options, mockController)
Expect(err).NotTo(HaveOccurred())
})

It("should handle multiple controller setups", func() {
firstController := &MockController{
name: "first-controller",
managedResource: &corev1.Pod{},
}

secondController := &MockController{
name: "second-controller",
managedResource: &corev1.Pod{},
}

options := controller.Options{}

err := SetupAppWatcherWithReconciler(mgr, options, firstController)
Expect(err).NotTo(HaveOccurred())

err = SetupAppWatcherWithReconciler(mgr, options, secondController)
Expect(err).NotTo(HaveOccurred())
})
})

Context("with different controller options", func() {
It("should respect MaxConcurrentReconciles setting", func() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This test's description, 'should respect MaxConcurrentReconciles setting', is not validated by its implementation. The test only checks for successful execution. Consider renaming it to more accurately describe what is being tested, for example, 'should succeed with a custom MaxConcurrentReconciles setting'.

options := controller.Options{
MaxConcurrentReconciles: 5,
}

err := SetupAppWatcherWithReconciler(mgr, options, mockController)
Expect(err).NotTo(HaveOccurred())
})

It("should work with default options", func() {
options := controller.Options{}

err := SetupAppWatcherWithReconciler(mgr, options, mockController)
Expect(err).NotTo(HaveOccurred())
})
})
})

// Test suite for Controller interface implementations
var _ = Describe("MockController", func() {
var mockController *MockController

BeforeEach(func() {
mockController = &MockController{
name: "test-mock-controller",
managedResource: &corev1.Pod{},
}
})

Describe("ControllerName", func() {
It("should return the correct controller name", func() {
Expect(mockController.ControllerName()).To(Equal("test-mock-controller"))
})
})

Describe("ManagedResource", func() {
It("should return the configured managed resource", func() {
resource := mockController.ManagedResource()
Expect(resource).NotTo(BeNil())
Expect(resource).To(BeAssignableToTypeOf(&corev1.Pod{}))
})

It("should return default Pod when no resource is set", func() {
controller := &MockController{name: "default"}
resource := controller.ManagedResource()
Expect(resource).To(BeAssignableToTypeOf(&corev1.Pod{}))
})
})

Describe("Reconcile", func() {
It("should successfully reconcile without error", func() {
ctx := context.Background()
req := reconcile.Request{
NamespacedName: types.NamespacedName{
Name: "test-pod",
Namespace: "default",
},
}

result, err := mockController.Reconcile(ctx, req)
Expect(err).NotTo(HaveOccurred())
Expect(result).To(Equal(reconcile.Result{}))
})
})
})
Loading