config: test: Merge test functions together

Due to recent changes, the two test functions can now be safely merged together.

The second function verified the overall synchronization of the configuration,
which is now also being exercised in the first function.

The tests cases were migrated over to the first function or were already present.

Author: DavidHurta

pkg/cvo/configuration/configuration_test.go

@@ -17,26 +17,41 @@ import (
 	operatorexternalversions "github.com/openshift/client-go/operator/informers/externalversions"
 )
 
-func TestClusterVersionOperatorConfiguration_sync(t *testing.T) {
+func TestClusterVersionOperatorConfiguration_APIServerSync(t *testing.T) {
 	tests := []struct {
 		name                   string
-		config                 operatorv1alpha1.ClusterVersionOperator
-		expectedConfig         operatorv1alpha1.ClusterVersionOperator
+		config                 *operatorv1alpha1.ClusterVersionOperator
+		expectedConfig         *operatorv1alpha1.ClusterVersionOperator
 		internalConfig         configuration
 		expectedInternalConfig configuration
 		handlerFunctionCalled  bool
 	}{
+		{
+			name:           "the configuration resource does not exist in the cluster -> default configuration",
+			config:         nil,
+			expectedConfig: nil,
+			internalConfig: configuration{
+				lastObservedGeneration: 3,
+				desiredLogLevel:        "Trace",
+			},
+			expectedInternalConfig: configuration{
+				lastObservedGeneration: 3,       // TODO: Default to 0
+				desiredLogLevel:        "Trace", // TODO: Default to Normal
+			},
+			// TODO: Apply the log level when the defaulting is implemented
+			handlerFunctionCalled: false,
+		},
 		{
 			name: "first sync run correctly updates the status",
-			config: operatorv1alpha1.ClusterVersionOperator{
+			config: &operatorv1alpha1.ClusterVersionOperator{
 				ObjectMeta: metav1.ObjectMeta{
 					Generation: 1,
 				},
 				Spec: operatorv1alpha1.ClusterVersionOperatorSpec{
 					OperatorLogLevel: operatorv1.Normal,
 				},
 			},
-			expectedConfig: operatorv1alpha1.ClusterVersionOperator{
+			expectedConfig: &operatorv1alpha1.ClusterVersionOperator{
 				ObjectMeta: metav1.ObjectMeta{
 					Generation: 1,
 				},
@@ -59,7 +74,7 @@ func TestClusterVersionOperatorConfiguration_sync(t *testing.T) {
 		},
 		{
 			name: "sync updates observed generation correctly",
-			config: operatorv1alpha1.ClusterVersionOperator{
+			config: &operatorv1alpha1.ClusterVersionOperator{
 				ObjectMeta: metav1.ObjectMeta{
 					Generation: 3,
 				},
@@ -70,7 +85,7 @@ func TestClusterVersionOperatorConfiguration_sync(t *testing.T) {
 					ObservedGeneration: 2,
 				},
 			},
-			expectedConfig: operatorv1alpha1.ClusterVersionOperator{
+			expectedConfig: &operatorv1alpha1.ClusterVersionOperator{
 				ObjectMeta: metav1.ObjectMeta{
 					Generation: 3,
 				},
@@ -93,7 +108,7 @@ func TestClusterVersionOperatorConfiguration_sync(t *testing.T) {
 		},
 		{
 			name: "sync updates desired log level correctly",
-			config: operatorv1alpha1.ClusterVersionOperator{
+			config: &operatorv1alpha1.ClusterVersionOperator{
 				ObjectMeta: metav1.ObjectMeta{
 					Generation: 4,
 				},
@@ -104,7 +119,7 @@ func TestClusterVersionOperatorConfiguration_sync(t *testing.T) {
 					ObservedGeneration: 3,
 				},
 			},
-			expectedConfig: operatorv1alpha1.ClusterVersionOperator{
+			expectedConfig: &operatorv1alpha1.ClusterVersionOperator{
 				ObjectMeta: metav1.ObjectMeta{
 					Generation: 4,
 				},
@@ -127,7 +142,7 @@ func TestClusterVersionOperatorConfiguration_sync(t *testing.T) {
 		},
 		{
 			name: "number of not observed generations does not impact sync",
-			config: operatorv1alpha1.ClusterVersionOperator{
+			config: &operatorv1alpha1.ClusterVersionOperator{
 				ObjectMeta: metav1.ObjectMeta{
 					Generation: 40,
 				},
@@ -138,7 +153,7 @@ func TestClusterVersionOperatorConfiguration_sync(t *testing.T) {
 					ObservedGeneration: 3,
 				},
 			},
-			expectedConfig: operatorv1alpha1.ClusterVersionOperator{
+			expectedConfig: &operatorv1alpha1.ClusterVersionOperator{
 				ObjectMeta: metav1.ObjectMeta{
 					Generation: 40,
 				},
@@ -163,12 +178,13 @@ func TestClusterVersionOperatorConfiguration_sync(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			// Initialize testing logic
-			tt.config.Name = ClusterVersionOperatorConfigurationName
-			tt.expectedConfig.Name = ClusterVersionOperatorConfigurationName
-
-			client := operatorclientsetfake.NewClientset(&tt.config)
+			client := operatorclientsetfake.NewClientset()
+			if tt.config != nil {
+				tt.config.Name = ClusterVersionOperatorConfigurationName
+				tt.expectedConfig.Name = ClusterVersionOperatorConfigurationName
+				client = operatorclientsetfake.NewClientset(tt.config)
+			}
 			factory := operatorexternalversions.NewSharedInformerFactoryWithOptions(client, time.Minute)
-
 			configController := NewClusterVersionOperatorConfiguration(client, factory)
 
 			called := false
@@ -198,10 +214,15 @@ func TestClusterVersionOperatorConfiguration_sync(t *testing.T) {
 			}
 
 			config, err := client.OperatorV1alpha1().ClusterVersionOperators().Get(ctx, ClusterVersionOperatorConfigurationName, metav1.GetOptions{})
-			if err != nil {
+			if err != nil && !apierrors.IsNotFound(err) {
 				t.Errorf("unexpected error %v", err)
 			}
-			if diff := cmp.Diff(tt.expectedConfig, *config, cmpopts.IgnoreFields(metav1.ObjectMeta{}, "ManagedFields")); diff != "" {
+
+			// Set nil to differentiate between nonexisting configurations
+			if apierrors.IsNotFound(err) {
+				config = nil
+			}
+			if diff := cmp.Diff(tt.expectedConfig, config, cmpopts.IgnoreFields(metav1.ObjectMeta{}, "ManagedFields")); diff != "" {
 				t.Errorf("unexpected config (-want, +got) = %v", diff)
 			}
 
@@ -214,88 +235,3 @@ func TestClusterVersionOperatorConfiguration_sync(t *testing.T) {
 		})
 	}
 }
-
-func TestClusterVersionOperatorConfiguration_Sync(t *testing.T) {
-	tests := []struct {
-		name           string
-		config         *operatorv1alpha1.ClusterVersionOperator
-		expectedConfig *operatorv1alpha1.ClusterVersionOperator
-	}{
-		{
-			name:           "the configuration resource does not exist in the cluster -> ignore",
-			config:         nil,
-			expectedConfig: nil,
-		},
-		{
-			name: "Sync updates the ClusterVersionOperator resource",
-			config: &operatorv1alpha1.ClusterVersionOperator{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:       "cluster",
-					Generation: 4,
-				},
-				Spec: operatorv1alpha1.ClusterVersionOperatorSpec{
-					OperatorLogLevel: operatorv1.Trace,
-				},
-				Status: operatorv1alpha1.ClusterVersionOperatorStatus{
-					ObservedGeneration: 3,
-				},
-			},
-			expectedConfig: &operatorv1alpha1.ClusterVersionOperator{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:       "cluster",
-					Generation: 4,
-				},
-				Spec: operatorv1alpha1.ClusterVersionOperatorSpec{
-					OperatorLogLevel: operatorv1.Trace,
-				},
-				Status: operatorv1alpha1.ClusterVersionOperatorStatus{
-					ObservedGeneration: 4,
-				},
-			},
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			// Initialize testing logic
-			var client *operatorclientsetfake.Clientset
-			if tt.config != nil {
-				client = operatorclientsetfake.NewClientset(tt.config)
-			} else {
-				client = operatorclientsetfake.NewClientset()
-			}
-
-			factory := operatorexternalversions.NewSharedInformerFactoryWithOptions(client, time.Minute)
-			cvoConfiguration := NewClusterVersionOperatorConfiguration(client, factory)
-			defer cvoConfiguration.queue.ShutDown()
-
-			ctx, cancelFunc := context.WithDeadline(context.Background(), time.Now().Add(time.Minute))
-			defer cancelFunc()
-
-			err := cvoConfiguration.Start(ctx)
-			if err != nil {
-				t.Errorf("unexpected error %v", err)
-			}
-
-			// Run tested functionality
-			err = cvoConfiguration.Sync(ctx, "ClusterVersionOperator/cluster")
-			if err != nil {
-				t.Errorf("unexpected error %v", err)
-			}
-
-			// Verify results
-			config, err := client.OperatorV1alpha1().ClusterVersionOperators().Get(ctx, "cluster", metav1.GetOptions{})
-			if err != nil && !apierrors.IsNotFound(err) {
-				t.Errorf("unexpected error %v", err)
-			}
-
-			switch {
-			case apierrors.IsNotFound(err) && tt.expectedConfig != nil:
-				t.Errorf("expected config to be '%v', got NotFound", *tt.expectedConfig)
-			case err == nil:
-				if diff := cmp.Diff(*tt.expectedConfig, *config, cmpopts.IgnoreFields(metav1.ObjectMeta{}, "ManagedFields")); diff != "" {
-					t.Errorf("unexpected config (-want, +got) = %v", diff)
-				}
-			}
-		})
-	}
-}