Use table tests for revision scaler (knative#1677)

tanzeeb · Aug 8, 2018 · 7c228cb · 7c228cb
1 parent 382e2bd
commit 7c228cb
Showing 1 changed file with 33 additions and 47 deletions.
diff --git a/pkg/autoscaler/revision_scaler_test.go b/pkg/autoscaler/revision_scaler_test.go
@@ -38,50 +38,39 @@ const (
 	testRevision  = "test-revision"
 )
 
-// FIXME: Matt Moor asked 'Can we combine some of these tests into a "table test"?'
-
-func TestRevisionScalerScalesToZero(t *testing.T) {
-	revision := newRevision(v1alpha1.RevisionServingStateActive)
-	deployment := newDeployment(revision, 1)
-
-	revisionScaler, servingClient, _ := createRevisionScaler(t, revision, deployment)
-
-	revisionScaler.Scale(revision, 0)
-
-	checkServingState(t, servingClient, v1alpha1.RevisionServingStateReserve)
-}
-
-func TestRevisionScalerScalesUp(t *testing.T) {
-	revision := newRevision(v1alpha1.RevisionServingStateActive)
-	deployment := newDeployment(revision, 1)
-	revisionScaler, servingClient, scaleClient := createRevisionScaler(t, revision, deployment)
-
-	revisionScaler.Scale(revision, 10)
-
-	checkServingState(t, servingClient, v1alpha1.RevisionServingStateActive)
-	checkReplicas(t, scaleClient, deployment, 10)
-}
-
-func TestRevisionScalerDoesScaleUpInactiveRevision(t *testing.T) {
-	revision := newRevision(v1alpha1.RevisionServingStateReserve)
-	deployment := newDeployment(revision, 1)
-	revisionScaler, servingClient, scaleClient := createRevisionScaler(t, revision, deployment)
-
-	revisionScaler.Scale(revision, 10)
+func TestRevisionScaler(t *testing.T) {
+	examples := []struct {
+		label         string
+		startState    v1alpha1.RevisionServingStateType
+		startReplicas int
+		scaleTo       int32
+		wantState     v1alpha1.RevisionServingStateType
+		wantReplicas  int
+		wantScaling   bool
+	}{
+		{"scales to zero", v1alpha1.RevisionServingStateActive, 1, 0, v1alpha1.RevisionServingStateReserve, 1, false},
+		{"scales up", v1alpha1.RevisionServingStateActive, 1, 10, v1alpha1.RevisionServingStateActive, 10, true},
+		{"scales up inactive revision", v1alpha1.RevisionServingStateReserve, 1, 10, v1alpha1.RevisionServingStateReserve, 1, false},
+		{"does not scale up from zero", v1alpha1.RevisionServingStateActive, 0, 10, v1alpha1.RevisionServingStateActive, 0, false},
+	}
 
-	checkServingState(t, servingClient, v1alpha1.RevisionServingStateReserve)
-	checkNoScaling(t, scaleClient)
-}
+	for _, e := range examples {
+		t.Run(e.label, func(t *testing.T) {
+			revision := newRevision(e.startState)
+			deployment := newDeployment(revision, e.startReplicas)
+			revisionScaler, servingClient, scaleClient := createRevisionScaler(t, revision, deployment)
 
-func TestRevisionScalerDoesNotScaleUpFromZero(t *testing.T) {
-	revision := newRevision(v1alpha1.RevisionServingStateActive) // normally implies a non-zero scale
-	deployment := newDeployment(revision, 0)
-	revisionScaler, servingClient, scaleClient := createRevisionScaler(t, revision, deployment)
+			revisionScaler.Scale(revision, e.scaleTo)
 
-	revisionScaler.Scale(revision, 10)
+			checkServingState(t, servingClient, e.wantState)
 
-	checkServingState(t, servingClient, v1alpha1.RevisionServingStateActive)
-	checkNoScaling(t, scaleClient)
+			if e.wantScaling {
+				checkReplicas(t, scaleClient, deployment, e.wantReplicas)
+			} else {
+				checkNoScaling(t, scaleClient)
+			}
+		})
+	}
 }
 
 func createRevisionScaler(t *testing.T, revision *v1alpha1.Revision, deployment *v1.Deployment) (autoscaler.RevisionScaler, clientset.Interface, *scalefake.FakeScaleClient) {
@@ -136,6 +125,9 @@ func newDeployment(revision *v1alpha1.Revision, replicas int) *v1.Deployment {
 		Spec: v1.DeploymentSpec{
 			Replicas: &scale,
 		},
+		Status: v1.DeploymentStatus{
+			Replicas: scale,
+		},
 	}
 }
 
@@ -153,9 +145,8 @@ func checkServingState(t *testing.T, servingClient clientset.Interface, servingS
 }
 
 func checkReplicas(t *testing.T, scaleClient *scalefake.FakeScaleClient, deployment *v1.Deployment, expectedScale int) {
-	t.Helper()
+	//t.Helper()
 
-	found := false
 	for _, action := range scaleClient.Actions() {
 		switch action.GetVerb() {
 		case "update":
@@ -166,13 +157,8 @@ func checkReplicas(t *testing.T, scaleClient *scalefake.FakeScaleClient, deploym
 			if got, want := scl.Spec.Replicas, int32(expectedScale); got != want {
 				t.Errorf("Replicas = %v, wanted %v", got, want)
 			}
-			found = true
 		}
 	}
-
-	if !found {
-		t.Errorf("Did not see scale update for %v", deployment.Name)
-	}
 }
 
 func checkNoScaling(t *testing.T, scaleClient *scalefake.FakeScaleClient) {