test: remove redundant comments and clean up test code

internal/pkg/controller/controller.go

@@ -42,16 +42,13 @@ type Controller struct {
 }
 
 // controllerInitialized flag determines whether controlled is being initialized
-var secretControllerInitialized bool = false
-var configmapControllerInitialized bool = false
+var secretControllerInitialized = false
+var configmapControllerInitialized = false
 var selectedNamespacesCache []string
 
 // NewController for initializing a Controller
-func NewController(
-	client kubernetes.Interface, resource string, namespace string, ignoredNamespaces []string, namespaceLabelSelector string, resourceLabelSelector string, collectors metrics.Collectors) (
-	*Controller, error,
-) {
-
+func NewController(client kubernetes.Interface, resource string, namespace string, ignoredNamespaces []string, namespaceLabelSelector string, resourceLabelSelector string, collectors metrics.Collectors) (*Controller,
+	error) {
 	if options.SyncAfterRestart {
 		secretControllerInitialized = true
 		configmapControllerInitialized = true

internal/pkg/handler/create_test.go

@@ -210,7 +210,6 @@ func TestResourceCreatedHandler_GetConfig(t *testing.T) {
 }
 
 func TestResourceCreatedHandler_GetConfig_Annotations(t *testing.T) {
-	// Test that annotations are properly captured in config
 	cm := &v1.ConfigMap{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "annotated-cm",
@@ -236,7 +235,6 @@ func TestResourceCreatedHandler_GetConfig_Annotations(t *testing.T) {
 }
 
 func TestResourceCreatedHandler_GetConfig_Labels(t *testing.T) {
-	// Test that labels are properly captured in config
 	secret := &v1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "labeled-secret",
@@ -270,7 +268,7 @@ func TestResourceCreatedHandler_Handle(t *testing.T) {
 		{
 			name:        "Nil resource",
 			resource:    nil,
-			expectError: false, // logs error but returns nil
+			expectError: false,
 		},
 		{
 			name: "Valid ConfigMap - no workloads to update",
@@ -315,7 +313,6 @@ func TestResourceCreatedHandler_Handle(t *testing.T) {
 }
 
 func TestResourceCreatedHandler_SHAConsistency(t *testing.T) {
-	// Test that same data produces same SHA
 	data := map[string]string{"key": "value"}
 
 	cm1 := &v1.ConfigMap{
@@ -333,12 +330,10 @@ func TestResourceCreatedHandler_SHAConsistency(t *testing.T) {
 	config1, _ := handler1.GetConfig()
 	config2, _ := handler2.GetConfig()
 
-	// Same data should produce same SHA
 	assert.Equal(t, config1.SHAValue, config2.SHAValue)
 }
 
 func TestResourceCreatedHandler_SHADifference(t *testing.T) {
-	// Test that different data produces different SHA
 	cm1 := &v1.ConfigMap{
 		ObjectMeta: metav1.ObjectMeta{Name: "cm", Namespace: "default"},
 		Data:       map[string]string{"key": "value1"},
@@ -354,6 +349,5 @@ func TestResourceCreatedHandler_SHADifference(t *testing.T) {
 	config1, _ := handler1.GetConfig()
 	config2, _ := handler2.GetConfig()
 
-	// Different data should produce different SHA
 	assert.NotEqual(t, config1.SHAValue, config2.SHAValue)
 }

internal/pkg/handler/delete_test.go

@@ -129,7 +129,7 @@ func TestRemoveContainerEnvVars(t *testing.T) {
 							},
 						},
 					},
-					Env: []v1.EnvVar{}, // No env vars
+					Env: []v1.EnvVar{},
 				},
 			},
 			volumes: []v1.Volume{},
@@ -201,7 +201,6 @@ func TestRemoveContainerEnvVars(t *testing.T) {
 			assert.Equal(t, tt.expected, result.Result)
 
 			if tt.envVarRemoved {
-				// Verify env var was removed from container
 				containers := deployment.Spec.Template.Spec.Containers
 				for _, c := range containers {
 					for _, env := range c.Env {
@@ -215,7 +214,6 @@ func TestRemoveContainerEnvVars(t *testing.T) {
 }
 
 func TestInvokeDeleteStrategy(t *testing.T) {
-	// Save original strategy and restore after test
 	originalStrategy := options.ReloadStrategy
 	defer func() {
 		options.ReloadStrategy = originalStrategy
@@ -297,7 +295,6 @@ func TestInvokeDeleteStrategy(t *testing.T) {
 
 			result := invokeDeleteStrategy(funcs, deployment, tt.config, true)
 
-			// Should return a valid result
 			assert.NotNil(t, result)
 		})
 	}
@@ -345,12 +342,11 @@ func TestRemovePodAnnotations(t *testing.T) {
 				VolumesFunc:        mockVolumesFunc,
 				PodAnnotationsFunc: mockPodAnnotationsFunc,
 				PatchTemplatesFunc: mockPatchTemplatesFunc,
-				SupportsPatch:      false, // No patch for annotations removal test
+				SupportsPatch:      false,
 			}
 
 			result := removePodAnnotations(funcs, deployment, tt.config, true)
 
-			// Should return Updated since it sets the SHA to empty data hash
 			assert.Equal(t, constants.Updated, result.Result)
 		})
 	}

internal/pkg/handler/handlers_test.go

@@ -55,7 +55,7 @@ func TestResourceCreatedHandler_GetConfig_ConfigMap(t *testing.T) {
 	assert.Equal(t, "default", config.Namespace)
 	assert.Equal(t, constants.ConfigmapEnvVarPostfix, config.Type)
 	assert.NotEmpty(t, config.SHAValue)
-	assert.Empty(t, oldSHA) // oldSHA is always empty for create handler
+	assert.Empty(t, oldSHA)
 }
 
 func TestResourceCreatedHandler_GetConfig_Secret(t *testing.T) {
@@ -75,7 +75,6 @@ func TestResourceCreatedHandler_GetConfig_Secret(t *testing.T) {
 }
 
 func TestResourceCreatedHandler_GetConfig_InvalidResource(t *testing.T) {
-	// Test with an invalid resource type
 	handler := ResourceCreatedHandler{
 		Resource:   "invalid",
 		Collectors: createTestCollectors(),
@@ -83,7 +82,6 @@ func TestResourceCreatedHandler_GetConfig_InvalidResource(t *testing.T) {
 
 	config, _ := handler.GetConfig()
 
-	// Config should be empty/zero for invalid resources
 	assert.Empty(t, config.ResourceName)
 }
 
@@ -95,7 +93,6 @@ func TestResourceCreatedHandler_Handle_NilResource(t *testing.T) {
 
 	err := handler.Handle()
 
-	// Should not return error even with nil resource (just logs error)
 	assert.NoError(t, err)
 }
 
@@ -178,7 +175,6 @@ func TestResourceUpdatedHandler_GetConfig_ConfigMap(t *testing.T) {
 	assert.Equal(t, constants.ConfigmapEnvVarPostfix, config.Type)
 	assert.NotEmpty(t, config.SHAValue)
 	assert.NotEmpty(t, oldSHA)
-	// SHAs should be different since data changed
 	assert.NotEqual(t, config.SHAValue, oldSHA)
 }
 
@@ -195,7 +191,6 @@ func TestResourceUpdatedHandler_GetConfig_ConfigMap_SameData(t *testing.T) {
 	config, oldSHA := handler.GetConfig()
 
 	assert.Equal(t, "test-cm", config.ResourceName)
-	// SHAs should be the same since data didn't change
 	assert.Equal(t, config.SHAValue, oldSHA)
 }
 
@@ -232,7 +227,6 @@ func TestResourceUpdatedHandler_GetConfig_Secret_SameData(t *testing.T) {
 	config, oldSHA := handler.GetConfig()
 
 	assert.Equal(t, "test-secret", config.ResourceName)
-	// SHAs should be the same since data didn't change
 	assert.Equal(t, config.SHAValue, oldSHA)
 }
 
@@ -270,16 +264,14 @@ func TestResourceUpdatedHandler_Handle_NilOldResource(t *testing.T) {
 
 	err := handler.Handle()
 
-	// Should not return error (just logs error)
 	assert.NoError(t, err)
 }
 
 func TestResourceUpdatedHandler_Handle_NoChange(t *testing.T) {
-	// When SHA values are the same, Handle should return nil without doing anything
 	cm := createTestConfigMap(map[string]string{"key": "same-value"})
 	handler := ResourceUpdatedHandler{
 		Resource:    cm,
-		OldResource: cm, // Same resource = same SHA
+		OldResource: cm,
 		Collectors:  createTestCollectors(),
 	}
 

internal/pkg/handler/update_test.go

@@ -108,7 +108,7 @@ func TestResourceUpdatedHandler_GetConfig(t *testing.T) {
 			expectedNS:        "default",
 			expectedType:      constants.ConfigmapEnvVarPostfix,
 			expectSHANotEmpty: true,
-			expectSHAChanged:  false, // Only data affects SHA, not labels
+			expectSHAChanged:  false,
 		},
 		{
 			name: "ConfigMap only annotations changed - SHA unchanged",
@@ -132,7 +132,7 @@ func TestResourceUpdatedHandler_GetConfig(t *testing.T) {
 			expectedNS:        "default",
 			expectedType:      constants.ConfigmapEnvVarPostfix,
 			expectSHANotEmpty: true,
-			expectSHAChanged:  false, // Only data affects SHA, not annotations
+			expectSHAChanged:  false,
 		},
 		{
 			name: "Secret data changed",
@@ -257,7 +257,7 @@ func TestResourceUpdatedHandler_Handle(t *testing.T) {
 			name:        "Both resources nil",
 			oldResource: nil,
 			newResource: nil,
-			expectError: false, // logs error but returns nil
+			expectError: false,
 		},
 		{
 			name:        "Old resource nil",
@@ -299,7 +299,7 @@ func TestResourceUpdatedHandler_Handle(t *testing.T) {
 				ObjectMeta: metav1.ObjectMeta{Name: "cm", Namespace: "default"},
 				Data:       map[string]string{"key": "new"},
 			},
-			expectError: false, // No error, but no workloads to update in test
+			expectError: false,
 		},
 		{
 			name: "Secret unchanged - no action",
@@ -347,7 +347,6 @@ func TestResourceUpdatedHandler_Handle(t *testing.T) {
 }
 
 func TestResourceUpdatedHandler_GetConfig_Annotations(t *testing.T) {
-	// Test that annotations from the new resource are captured
 	oldCM := &v1.ConfigMap{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "cm",
@@ -378,15 +377,12 @@ func TestResourceUpdatedHandler_GetConfig_Annotations(t *testing.T) {
 
 	config, _ := handler.GetConfig()
 
-	// Should have new annotations
 	assert.Equal(t, "new-value", config.ResourceAnnotations["new-annotation"])
-	// Should not have old annotations
 	_, hasOld := config.ResourceAnnotations["old-annotation"]
 	assert.False(t, hasOld)
 }
 
 func TestResourceUpdatedHandler_GetConfig_Labels(t *testing.T) {
-	// Test that labels from the new resource are captured
 	oldSecret := &v1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "secret",
@@ -413,12 +409,10 @@ func TestResourceUpdatedHandler_GetConfig_Labels(t *testing.T) {
 
 	config, _ := handler.GetConfig()
 
-	// Should have new labels
 	assert.Equal(t, "v2", config.Labels["version"])
 }
 
 func TestResourceUpdatedHandler_EmptyToNonEmpty(t *testing.T) {
-	// Test transition from empty data to non-empty data
 	oldCM := &v1.ConfigMap{
 		ObjectMeta: metav1.ObjectMeta{Name: "cm", Namespace: "default"},
 		Data:       map[string]string{},
@@ -440,7 +434,6 @@ func TestResourceUpdatedHandler_EmptyToNonEmpty(t *testing.T) {
 }
 
 func TestResourceUpdatedHandler_NonEmptyToEmpty(t *testing.T) {
-	// Test transition from non-empty data to empty data
 	oldCM := &v1.ConfigMap{
 		ObjectMeta: metav1.ObjectMeta{Name: "cm", Namespace: "default"},
 		Data:       map[string]string{"key": "value"},
@@ -462,7 +455,6 @@ func TestResourceUpdatedHandler_NonEmptyToEmpty(t *testing.T) {
 }
 
 func TestResourceUpdatedHandler_BinaryDataChange(t *testing.T) {
-	// Test ConfigMap binary data change
 	oldCM := &v1.ConfigMap{
 		ObjectMeta: metav1.ObjectMeta{Name: "cm", Namespace: "default"},
 		BinaryData: map[string][]byte{"binary": []byte("old-binary")},
@@ -484,7 +476,6 @@ func TestResourceUpdatedHandler_BinaryDataChange(t *testing.T) {
 }
 
 func TestResourceUpdatedHandler_MixedDataAndBinaryData(t *testing.T) {
-	// Test ConfigMap with both Data and BinaryData
 	oldCM := &v1.ConfigMap{
 		ObjectMeta: metav1.ObjectMeta{Name: "cm", Namespace: "default"},
 		Data:       map[string]string{"text": "value"},
@@ -508,7 +499,6 @@ func TestResourceUpdatedHandler_MixedDataAndBinaryData(t *testing.T) {
 }
 
 func TestResourceUpdatedHandler_DifferentNamespaces(t *testing.T) {
-	// Edge case: what if namespaces are different (shouldn't happen in practice)
 	oldCM := &v1.ConfigMap{
 		ObjectMeta: metav1.ObjectMeta{Name: "cm", Namespace: "ns1"},
 		Data:       map[string]string{"key": "value"},
@@ -526,6 +516,5 @@ func TestResourceUpdatedHandler_DifferentNamespaces(t *testing.T) {
 
 	config, _ := handler.GetConfig()
 
-	// Should use new resource's namespace
 	assert.Equal(t, "ns2", config.Namespace)
 }

internal/pkg/handler/upgrade_test.go

@@ -203,7 +203,7 @@ func TestGetVolumeMountName(t *testing.T) {
 					},
 				},
 			},
-			mountType:  constants.ConfigmapEnvVarPostfix, // Looking for configmap but volume is secret
+			mountType:  constants.ConfigmapEnvVarPostfix,
 			volumeName: "my-secret",
 			expected:   "",
 		},
@@ -509,7 +509,7 @@ func TestUpdateEnvVar(t *testing.T) {
 		envVar    string
 		shaData   string
 		expected  constants.Result
-		newValue  string // expected value after update
+		newValue  string
 	}{
 		{
 			name: "Update existing env var with different value",
@@ -567,7 +567,6 @@ func TestUpdateEnvVar(t *testing.T) {
 			assert.Equal(t, tt.expected, result)
 
 			if tt.expected == constants.Updated || tt.expected == constants.NotUpdated {
-				// Verify the value in the container
 				for _, env := range tt.container.Env {
 					if env.Name == tt.envVar {
 						assert.Equal(t, tt.newValue, env.Value)
@@ -658,7 +657,6 @@ func TestCreateReloadedAnnotations(t *testing.T) {
 		},
 	}
 
-	// Use a simple func that doesn't require patch templates
 	funcs := callbacks.RollingUpgradeFuncs{
 		SupportsPatch: false,
 	}
@@ -672,7 +670,6 @@ func TestCreateReloadedAnnotations(t *testing.T) {
 			} else {
 				assert.NoError(t, err)
 				assert.NotNil(t, annotations)
-				// Verify annotation key exists
 				_, exists := annotations[getReloaderAnnotationKey()]
 				assert.True(t, exists)
 			}
@@ -782,7 +779,7 @@ func TestGetContainerUsingResource(t *testing.T) {
 			},
 			autoReload:   false,
 			expectNil:    false,
-			expectedName: "main-app", // Returns first container when init container has the mount
+			expectedName: "main-app",
 		},
 		{
 			name: "EnvFrom ConfigMap in regular container",
@@ -846,7 +843,7 @@ func TestGetContainerUsingResource(t *testing.T) {
 				ResourceName: "external-configmap",
 				Type:         constants.ConfigmapEnvVarPostfix,
 			},
-			autoReload:   false, // Explicit annotation should use first container fallback
+			autoReload:   false,
 			expectNil:    false,
 			expectedName: "first-container",
 		},
@@ -861,7 +858,7 @@ func TestGetContainerUsingResource(t *testing.T) {
 				ResourceName: "unmounted-configmap",
 				Type:         constants.ConfigmapEnvVarPostfix,
 			},
-			autoReload: true, // Auto mode should NOT use first container fallback
+			autoReload: true,
 			expectNil:  true,
 		},
 		{
@@ -902,7 +899,7 @@ func TestGetContainerUsingResource(t *testing.T) {
 				Type:         constants.ConfigmapEnvVarPostfix,
 			},
 			autoReload: false,
-			expectNil:  true, // No regular containers to return
+			expectNil:  true,
 		},
 		{
 			name: "CSI SecretProviderClass volume",
@@ -1038,7 +1035,9 @@ func TestRetryOnConflict(t *testing.T) {
 				matched bool
 				err     error
 			}{
-				{matched: false, err: apierrors.NewConflict(schema.GroupResource{Group: "", Resource: "deployments"}, "test", errors.New("conflict"))},
+				{matched: false,
+					err: apierrors.NewConflict(schema.GroupResource{Group: "", Resource: "deployments"}, "test",
+						errors.New("conflict"))},
 				{matched: true, err: nil},
 			},
 			expectMatched: true,
@@ -1086,7 +1085,6 @@ func TestRetryOnConflict(t *testing.T) {
 			callCount := 0
 			fn := func(fetchResource bool) (bool, error) {
 				if callCount >= len(tt.fnResults) {
-					// Should not happen in tests, but return success to prevent infinite loop
 					return true, nil
 				}
 				result := tt.fnResults[callCount]
@@ -1107,7 +1105,6 @@ func TestRetryOnConflict(t *testing.T) {
 }
 
 func TestGetVolumeMountNameCSI(t *testing.T) {
-	// Test CSI SecretProviderClass volume specifically
 	tests := []struct {
 		name       string
 		volumes    []v1.Volume
@@ -1303,11 +1300,9 @@ func TestSecretProviderClassAnnotationReloaded(t *testing.T) {
 }
 
 func TestInvokeReloadStrategy(t *testing.T) {
-	// Save original value and restore after test
 	originalStrategy := options.ReloadStrategy
 	defer func() { options.ReloadStrategy = originalStrategy }()
 
-	// Create a minimal deployment for testing
 	deployment := createTestDeployment(
 		[]v1.Container{
 			{
@@ -1365,14 +1360,13 @@ func TestInvokeReloadStrategy(t *testing.T) {
 			name:           "Env vars strategy with container found",
 			reloadStrategy: constants.EnvVarsReloadStrategy,
 			autoReload:     false,
-			expectResult:   constants.Updated, // Creates env var when not found
+			expectResult:   constants.Updated,
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			options.ReloadStrategy = tt.reloadStrategy
-			// Reset annotations for each test
 			deployment.Spec.Template.Annotations = map[string]string{}
 
 			result := invokeReloadStrategy(funcs, deployment, config, tt.autoReload)