Make runTest easier to understand

Instead of deducing metric type from details of struct describing it
test cases explicitly specify the metric type they use.

pkg/controller/podautoscaler/replica_calculator_test.go

@@ -56,11 +56,21 @@ type resourceInfo struct {
 	expectedValue       int64
 }
 
+type metricType int
+
+const (
+	objectMetric metricType = iota
+	externalMetric
+	externalPerPodMetric
+	podMetric
+)
+
 type metricInfo struct {
 	name         string
 	levels       []int64
 	singleObject *autoscalingv2.CrossVersionObjectReference
 	selector     *metav1.LabelSelector
+	metricType   metricType
 
 	targetUtilization       int64
 	perPodTargetUtilization int64
@@ -317,22 +327,39 @@ func (tc *replicaCalcTestCase) runTest(t *testing.T) {
 		assert.Equal(t, tc.resource.expectedUtilization, outUtilization, "utilization should be as expected")
 		assert.Equal(t, tc.resource.expectedValue, outRawValue, "raw value should be as expected")
 		assert.True(t, tc.timestamp.Equal(outTimestamp), "timestamp should be as expected")
+		return
+	}
 
-	} else {
 	var outReplicas int32
 	var outUtilization int64
 	var outTimestamp time.Time
-		var err error
+	switch tc.metric.metricType {
-		if tc.metric.singleObject != nil {
+	case objectMetric:
-			outReplicas, outUtilization, outTimestamp, err = replicaCalc.GetObjectMetricReplicas(tc.currentReplicas, tc.metric.targetUtilization, tc.metric.name, testNamespace, tc.metric.singleObject, selector)
+		if tc.metric.singleObject == nil {
-		} else if tc.metric.selector != nil {
+			t.Fatal("Metric specified as objectMetric but metric.singleObject is nil.")
-			if tc.metric.targetUtilization > 0 {
-				outReplicas, outUtilization, outTimestamp, err = replicaCalc.GetExternalMetricReplicas(tc.currentReplicas, tc.metric.targetUtilization, tc.metric.name, testNamespace, tc.metric.selector, selector)
-			} else if tc.metric.perPodTargetUtilization > 0 {
-				outReplicas, outUtilization, outTimestamp, err = replicaCalc.GetExternalPerPodMetricReplicas(tc.currentReplicas, tc.metric.perPodTargetUtilization, tc.metric.name, testNamespace, tc.metric.selector)
 		}
-		} else {
+		outReplicas, outUtilization, outTimestamp, err = replicaCalc.GetObjectMetricReplicas(tc.currentReplicas, tc.metric.targetUtilization, tc.metric.name, testNamespace, tc.metric.singleObject, selector)
+	case externalMetric:
+		if tc.metric.selector == nil {
+			t.Fatal("Metric specified as externalMetric but metric.selector is nil.")
+		}
+		if tc.metric.targetUtilization <= 0 {
+			t.Fatalf("Metric specified as externalMetric but metric.targetUtilization is %d which is <=0.", tc.metric.targetUtilization)
+		}
+		outReplicas, outUtilization, outTimestamp, err = replicaCalc.GetExternalMetricReplicas(tc.currentReplicas, tc.metric.targetUtilization, tc.metric.name, testNamespace, tc.metric.selector, selector)
+	case externalPerPodMetric:
+		if tc.metric.selector == nil {
+			t.Fatal("Metric specified as externalPerPodMetric but metric.selector is nil.")
+		}
+		if tc.metric.perPodTargetUtilization <= 0 {
+			t.Fatalf("Metric specified as externalPerPodMetric but metric.perPodTargetUtilization is %d which is <=0.", tc.metric.perPodTargetUtilization)
+		}
+
+		outReplicas, outUtilization, outTimestamp, err = replicaCalc.GetExternalPerPodMetricReplicas(tc.currentReplicas, tc.metric.perPodTargetUtilization, tc.metric.name, testNamespace, tc.metric.selector)
+	case podMetric:
 		outReplicas, outUtilization, outTimestamp, err = replicaCalc.GetMetricReplicas(tc.currentReplicas, tc.metric.targetUtilization, tc.metric.name, testNamespace, selector)
+	default:
+		t.Fatalf("Unknown metric type: %d", tc.metric.metricType)
 	}
 
 	if tc.expectedError != nil {
@@ -345,7 +372,6 @@ func (tc *replicaCalcTestCase) runTest(t *testing.T) {
 	assert.Equal(t, tc.metric.expectedUtilization, outUtilization, "utilization should be as expected")
 	assert.True(t, tc.timestamp.Equal(outTimestamp), "timestamp should be as expected")
 }
-}
 
 func TestReplicaCalcDisjointResourcesMetrics(t *testing.T) {
 	tc := replicaCalcTestCase{
@@ -444,6 +470,7 @@ func TestReplicaCalcScaleUpCM(t *testing.T) {
 			levels:              []int64{20000, 10000, 30000},
 			targetUtilization:   15000,
 			expectedUtilization: 20000,
+			metricType:          podMetric,
 		},
 	}
 	tc.runTest(t)
@@ -459,6 +486,7 @@ func TestReplicaCalcScaleUpCMUnreadyLessScale(t *testing.T) {
 			levels:              []int64{50000, 10000, 30000},
 			targetUtilization:   15000,
 			expectedUtilization: 30000,
+			metricType:          podMetric,
 		},
 	}
 	tc.runTest(t)
@@ -474,6 +502,7 @@ func TestReplicaCalcScaleUpCMUnreadyNoScaleWouldScaleDown(t *testing.T) {
 			levels:              []int64{50000, 15000, 30000},
 			targetUtilization:   15000,
 			expectedUtilization: 15000,
+			metricType:          podMetric,
 		},
 	}
 	tc.runTest(t)
@@ -528,6 +557,7 @@ func TestReplicaCalcScaleUpCMExternal(t *testing.T) {
 			targetUtilization:   4400,
 			expectedUtilization: 8600,
 			selector:            &metav1.LabelSelector{MatchLabels: map[string]string{"label": "value"}},
+			metricType:          podMetric,
 		},
 	}
 	tc.runTest(t)
@@ -544,6 +574,7 @@ func TestReplicaCalcScaleUpCMExternalIgnoresUnreadyPods(t *testing.T) {
 			targetUtilization:   4400,
 			expectedUtilization: 8600,
 			selector:            &metav1.LabelSelector{MatchLabels: map[string]string{"label": "value"}},
+			metricType:          externalMetric,
 		},
 	}
 	tc.runTest(t)
@@ -558,6 +589,7 @@ func TestReplicaCalcScaleUpCMExternalNoLabels(t *testing.T) {
 			levels:              []int64{8600},
 			targetUtilization:   4400,
 			expectedUtilization: 8600,
+			metricType:          podMetric,
 		},
 	}
 	tc.runTest(t)
@@ -573,6 +605,7 @@ func TestReplicaCalcScaleUpPerPodCMExternal(t *testing.T) {
 			perPodTargetUtilization: 2150,
 			expectedUtilization:     2867,
 			selector:                &metav1.LabelSelector{MatchLabels: map[string]string{"label": "value"}},
+			metricType:              externalPerPodMetric,
 		},
 	}
 	tc.runTest(t)
@@ -604,6 +637,7 @@ func TestReplicaCalcScaleDownCM(t *testing.T) {
 			levels:              []int64{12000, 12000, 12000, 12000, 12000},
 			targetUtilization:   20000,
 			expectedUtilization: 12000,
+			metricType:          podMetric,
 		},
 	}
 	tc.runTest(t)
@@ -638,6 +672,7 @@ func TestReplicaCalcScaleDownCMExternal(t *testing.T) {
 			targetUtilization:   14334,
 			expectedUtilization: 8600,
 			selector:            &metav1.LabelSelector{MatchLabels: map[string]string{"label": "value"}},
+			metricType:          externalMetric,
 		},
 	}
 	tc.runTest(t)
@@ -653,6 +688,7 @@ func TestReplicaCalcScaleDownPerPodCMExternal(t *testing.T) {
 			perPodTargetUtilization: 2867,
 			expectedUtilization:     1720,
 			selector:                &metav1.LabelSelector{MatchLabels: map[string]string{"label": "value"}},
+			metricType:              externalPerPodMetric,
 		},
 	}
 	tc.runTest(t)
@@ -721,6 +757,7 @@ func TestReplicaCalcToleranceCM(t *testing.T) {
 			levels:              []int64{20000, 21000, 21000},
 			targetUtilization:   20000,
 			expectedUtilization: 20666,
+			metricType:          podMetric,
 		},
 	}
 	tc.runTest(t)
@@ -755,6 +792,7 @@ func TestReplicaCalcToleranceCMExternal(t *testing.T) {
 			targetUtilization:   8888,
 			expectedUtilization: 8600,
 			selector:            &metav1.LabelSelector{MatchLabels: map[string]string{"label": "value"}},
+			metricType:          externalMetric,
 		},
 	}
 	tc.runTest(t)
@@ -770,6 +808,7 @@ func TestReplicaCalcTolerancePerPodCMExternal(t *testing.T) {
 			perPodTargetUtilization: 2900,
 			expectedUtilization:     2867,
 			selector:                &metav1.LabelSelector{MatchLabels: map[string]string{"label": "value"}},
+			metricType:              externalPerPodMetric,
 		},
 	}
 	tc.runTest(t)