test: refactoring for unittests

Author: nexus49

.testcoverage.yml

@@ -3,4 +3,5 @@ exclude:
     - ^controller/testSupport	# exclude test support files
     - mocks	# exclude generated mock files
     - ^test/openfga
+    - logger/testlogger
 

config/config.go

@@ -212,6 +212,17 @@ func BindConfigToFlags(v *viper.Viper, cmd *cobra.Command, config any) error {
 	return nil
 }
 
+// GenerateFlagSet is exported for testing.
+func GenerateFlagSet(config any) (*pflag.FlagSet, error) {
+	return generateFlagSet(config)
+}
+
+// UnmarshalIntoStruct is exported for testing.
+func UnmarshalIntoStruct(v *viper.Viper, cfg any) func() {
+	return unmarshalIntoStruct(v, cfg)
+}
+
+// unmarshalIntoStruct returns a function that unmarshals viper config into cfg and panics on error.
 func unmarshalIntoStruct(v *viper.Viper, cfg any) func() {
 	return func() {
 		if err := v.Unmarshal(cfg); err != nil {

config/config_test.go

@@ -168,3 +168,34 @@ func TestNewDefaultConfig(t *testing.T) {
 	err = v.Unmarshal(&config.CommonServiceConfig{})
 	assert.NoError(t, err)
 }
+
+func TestGenerateFlagSetUnsupportedType(t *testing.T) {
+	type test struct {
+		UnsupportedField []string `mapstructure:"unsupported-field"`
+	}
+	testStruct := test{}
+	err := config.BindConfigToFlags(viper.New(), &cobra.Command{}, &testStruct)
+	assert.Error(t, err)
+}
+
+func TestGenerateFlagSetInvalidStruct(t *testing.T) {
+	notStruct := 123
+	_, err := config.GenerateFlagSet(notStruct)
+	assert.Error(t, err)
+}
+
+func TestUnmarshalIntoStructPanic(t *testing.T) {
+	v := viper.New()
+	cfg := struct {
+		InvalidField int `mapstructure:"invalid-field"`
+	}{}
+	// Set a string value to cause unmarshal error for int field
+	v.Set("invalid-field", "not-an-int")
+	defer func() {
+		if r := recover(); r == nil {
+			t.Errorf("expected panic but did not panic")
+		}
+	}()
+	unmarshal := config.UnmarshalIntoStruct(v, &cfg)
+	unmarshal()
+}

controller/lifecycle/lifecycle_test.go

@@ -1327,3 +1327,107 @@ func TestUpdateStatus(t *testing.T) {
 		assert.Equal(t, "status field not found in current object", err.Error())
 	})
 }
+
+func TestAddFinalizersIfNeeded(t *testing.T) {
+	instance := &pmtesting.TestApiObject{ObjectMeta: metav1.ObjectMeta{Name: "instance1"}}
+	fakeClient := pmtesting.CreateFakeClient(t, instance)
+	sub := pmtesting.FinalizerSubroutine{Client: fakeClient}
+	// Should add finalizer
+	err := AddFinalizersIfNeeded(context.Background(), fakeClient, instance, []subroutine.Subroutine{sub}, false)
+	assert.NoError(t, err)
+	assert.Contains(t, instance.Finalizers, pmtesting.SubroutineFinalizer)
+
+	// Should not add if readonly
+	instance2 := &pmtesting.TestApiObject{}
+	err = AddFinalizersIfNeeded(context.Background(), fakeClient, instance2, []subroutine.Subroutine{sub}, true)
+	assert.NoError(t, err)
+	assert.NotContains(t, instance2.Finalizers, pmtesting.SubroutineFinalizer)
+
+	// Should not add if deletion timestamp is set
+	now := metav1.Now()
+	instance3 := &pmtesting.TestApiObject{ObjectMeta: metav1.ObjectMeta{DeletionTimestamp: &now}}
+	err = AddFinalizersIfNeeded(context.Background(), fakeClient, instance3, []subroutine.Subroutine{sub}, false)
+	assert.NoError(t, err)
+}
+
+func TestAddFinalizerIfNeeded(t *testing.T) {
+	instance := &pmtesting.TestApiObject{}
+	sub := pmtesting.FinalizerSubroutine{}
+	// Should add and return true
+	added := AddFinalizerIfNeeded(instance, sub)
+	assert.True(t, added)
+	// Should not add again
+	added = AddFinalizerIfNeeded(instance, sub)
+	assert.False(t, added)
+}
+
+func TestRemoveFinalizerIfNeeded(t *testing.T) {
+	instance := &pmtesting.TestApiObject{ObjectMeta: metav1.ObjectMeta{Name: "instance1"}}
+	sub := pmtesting.FinalizerSubroutine{}
+	AddFinalizerIfNeeded(instance, sub)
+	fakeClient := pmtesting.CreateFakeClient(t, instance)
+	// Should remove finalizer if not readonly and RequeueAfter == 0
+	res := ctrl.Result{}
+	err := removeFinalizerIfNeeded(context.Background(), instance, sub, res, false, fakeClient)
+	assert.Nil(t, err)
+	assert.NotContains(t, instance.Finalizers, pmtesting.SubroutineFinalizer)
+
+	// Should not remove if readonly
+	AddFinalizerIfNeeded(instance, sub)
+	err = removeFinalizerIfNeeded(context.Background(), instance, sub, res, true, fakeClient)
+	assert.Nil(t, err)
+	assert.Contains(t, instance.Finalizers, pmtesting.SubroutineFinalizer)
+
+	// Should not remove if RequeueAfter > 0
+	res = ctrl.Result{RequeueAfter: 1}
+	err = removeFinalizerIfNeeded(context.Background(), instance, sub, res, false, fakeClient)
+	assert.Nil(t, err)
+}
+
+func TestContainsFinalizer(t *testing.T) {
+	instance := &pmtesting.TestApiObject{}
+	sub := pmtesting.FinalizerSubroutine{}
+	assert.False(t, containsFinalizer(instance, sub.Finalizers()))
+	AddFinalizerIfNeeded(instance, sub)
+	assert.True(t, containsFinalizer(instance, sub.Finalizers()))
+}
+
+func TestMarkResourceAsFinal(t *testing.T) {
+	instance := &pmtesting.ImplementingSpreadReconciles{}
+	logcfg := logger.DefaultConfig()
+	logcfg.NoJSON = true
+	log, _ := logger.New(logcfg)
+	conds := []metav1.Condition{}
+	mgr := &pmtesting.TestLifecycleManager{Logger: log}
+	MarkResourceAsFinal(instance, log, conds, metav1.ConditionTrue, mgr)
+	assert.Equal(t, instance.Status.ObservedGeneration, instance.Generation)
+}
+
+func TestHandleClientError(t *testing.T) {
+	log := testlogger.New().Logger
+	result, err := HandleClientError("msg", log, fmt.Errorf("err"), true, sentry.Tags{})
+	assert.Error(t, err)
+	assert.Equal(t, ctrl.Result{}, result)
+}
+
+func TestHandleOperatorError(t *testing.T) {
+	log := testlogger.New().Logger
+	opErr := operrors.NewOperatorError(fmt.Errorf("err"), false, false)
+	result, err := HandleOperatorError(context.Background(), opErr, "msg", true, log)
+	assert.Nil(t, err)
+	assert.Equal(t, ctrl.Result{}, result)
+
+	ctx := sentry.ContextWithSentryTags(context.Background(), sentry.Tags{"test": "tag"})
+	opErr = operrors.NewOperatorError(fmt.Errorf("err"), true, true)
+	result, err = HandleOperatorError(ctx, opErr, "msg", true, log)
+	assert.Error(t, err)
+	assert.Equal(t, ctrl.Result{}, result)
+}
+
+func TestValidateInterfaces(t *testing.T) {
+	log := testlogger.New().Logger
+	instance := &pmtesting.ImplementingSpreadReconciles{}
+	mgr := &pmtesting.TestLifecycleManager{Logger: log}
+	err := ValidateInterfaces(instance, log, mgr)
+	assert.NoError(t, err)
+}

traces/otel.go

@@ -30,19 +30,27 @@ type Config struct {
 	CollectorEndpoint string `mapstructure:"tracing-config-collector-endpoint" description:"Set the tracing collector endpoint used to send traces to the collector"`
 }
 
+// --- Wrappers for patching (default to real functions) ---
+var (
+	resourceNewFunc      = resource.New
+	grpcNewClientFunc    = grpc.NewClient
+	otlptracegrpcNewFunc = otlptracegrpc.New // type: func(ctx context.Context, opts ...otlptracegrpc.Option) (*otlptrace.Exporter, error)
+	stdouttraceNewFunc   = stdouttrace.New   // type: func(opts ...stdouttrace.Option) (*stdouttrace.Exporter, error)
+)
+
 // InitProvider creates an OpenTelemetry provider for the concrete service.
 // If the collector in the destination endpoint isn't reachable, then the init function will return an error.
 func InitProvider(ctx context.Context, config Config) (func(ctx context.Context) error, error) {
 	connCtx, cancel := context.WithTimeout(ctx, time.Second)
 	defer cancel()
 
-	client, err := grpc.NewClient(config.CollectorEndpoint,
+	client, err := grpcNewClientFunc(config.CollectorEndpoint,
 		grpc.WithTransportCredentials(insecure.NewCredentials()))
 	if err != nil {
 		return nil, fmt.Errorf("failed to create gRPC connection to collector: %w", err)
 	}
 
-	traceExporter, err := otlptracegrpc.New(connCtx, otlptracegrpc.WithGRPCConn(client))
+	traceExporter, err := otlptracegrpcNewFunc(connCtx, otlptracegrpc.WithGRPCConn(client))
 	if err != nil {
 		return nil, fmt.Errorf("failed to create trace exporter: %w", err)
 	}
@@ -58,7 +66,7 @@ func InitLocalProvider(ctx context.Context, config Config, exportToConsole bool)
 		fileTarget = os.Stdout
 	}
 
-	traceExporter, err := stdouttrace.New(
+	traceExporter, err := stdouttraceNewFunc(
 		stdouttrace.WithWriter(fileTarget),
 		stdouttrace.WithPrettyPrint(),
 	)
@@ -70,7 +78,7 @@ func InitLocalProvider(ctx context.Context, config Config, exportToConsole bool)
 }
 
 func (c Config) initProvider(ctx context.Context, exporter sdkTrace.SpanExporter) (func(ctx context.Context) error, error) {
-	res, err := resource.New(ctx,
+	res, err := resourceNewFunc(ctx,
 		resource.WithAttributes(
 			semconv.ServiceName(c.ServiceName),
 			semconv.ServiceVersion(c.ServiceVersion),