Merge pull request #26688 from asalkeld/more-unittests-util-env

Automatic merge from submit-queue

Increase the unit test coverage in pkg/util

These are easy to test, so lets increase coverage.

pkg/controller/deployment/util/deployment_util_test.go

@@ -22,6 +22,8 @@ import (
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
+
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/unversioned"
 	"k8s.io/kubernetes/pkg/apis/extensions"
@@ -29,6 +31,7 @@ import (
 	"k8s.io/kubernetes/pkg/client/testing/core"
 	"k8s.io/kubernetes/pkg/client/unversioned/testclient"
 	"k8s.io/kubernetes/pkg/runtime"
+	"k8s.io/kubernetes/pkg/util/intstr"
 )
 
 func addListRSReactor(fakeClient *fake.Clientset, obj runtime.Object) *fake.Clientset {
@@ -594,3 +597,157 @@ func equal(rss1, rss2 []*extensions.ReplicaSet) bool {
 	}
 	return count == len(rss1)
 }
+
+func TestGetReplicaCountForReplicaSets(t *testing.T) {
+	rs1 := generateRS(generateDeployment("foo"))
+	rs1.Spec.Replicas = 1
+	rs1.Status.Replicas = 2
+	rs2 := generateRS(generateDeployment("bar"))
+	rs2.Spec.Replicas = 2
+	rs2.Status.Replicas = 3
+
+	tests := []struct {
+		test           string
+		sets           []*extensions.ReplicaSet
+		expectedCount  int32
+		expectedActual int32
+	}{
+		{
+			"1:2 Replicas",
+			[]*extensions.ReplicaSet{&rs1},
+			1,
+			2,
+		},
+		{
+			"3:5 Replicas",
+			[]*extensions.ReplicaSet{&rs1, &rs2},
+			3,
+			5,
+		},
+	}
+
+	for _, test := range tests {
+		rs := GetReplicaCountForReplicaSets(test.sets)
+		if rs != test.expectedCount {
+			t.Errorf("In test case %s, expectedCount %+v, got %+v", test.test, test.expectedCount, rs)
+		}
+		rs = GetActualReplicaCountForReplicaSets(test.sets)
+		if rs != test.expectedActual {
+			t.Errorf("In test case %s, expectedActual %+v, got %+v", test.test, test.expectedActual, rs)
+		}
+	}
+}
+
+func TestResolveFenceposts(t *testing.T) {
+
+	tests := []struct {
+		maxSurge          string
+		maxUnavailable    string
+		desired           int32
+		expectSurge       int32
+		expectUnavailable int32
+		expectError       string
+	}{
+		{
+			maxSurge:          "0%",
+			maxUnavailable:    "0%",
+			desired:           0,
+			expectSurge:       0,
+			expectUnavailable: 1,
+			expectError:       "",
+		},
+		{
+			maxSurge:          "39%",
+			maxUnavailable:    "39%",
+			desired:           10,
+			expectSurge:       4,
+			expectUnavailable: 3,
+			expectError:       "",
+		},
+		{
+			maxSurge:          "oops",
+			maxUnavailable:    "39%",
+			desired:           10,
+			expectSurge:       0,
+			expectUnavailable: 0,
+			expectError:       "invalid value for IntOrString: invalid value \"oops\": strconv.ParseInt: parsing \"oops\": invalid syntax",
+		},
+		{
+			maxSurge:          "55%",
+			maxUnavailable:    "urg",
+			desired:           10,
+			expectSurge:       0,
+			expectUnavailable: 0,
+			expectError:       "invalid value for IntOrString: invalid value \"urg\": strconv.ParseInt: parsing \"urg\": invalid syntax",
+		},
+	}
+
+	for num, test := range tests {
+		maxSurge := intstr.FromString(test.maxSurge)
+		maxUnavail := intstr.FromString(test.maxUnavailable)
+		surge, unavail, err := ResolveFenceposts(&maxSurge, &maxUnavail, test.desired)
+		if err != nil {
+			if test.expectError == "" {
+				t.Errorf("unexpected error %v", err)
+			} else {
+				assert := assert.New(t)
+				assert.EqualError(err, test.expectError)
+			}
+		}
+		if err == nil && test.expectError != "" {
+			t.Errorf("missing error %v", test.expectError)
+		}
+		if surge != test.expectSurge || unavail != test.expectUnavailable {
+			t.Errorf("#%v got %v:%v, want %v:%v", num, surge, unavail, test.expectSurge, test.expectUnavailable)
+		}
+	}
+}
+
+func TestNewRSNewReplicas(t *testing.T) {
+
+	tests := []struct {
+		test          string
+		strategyType  extensions.DeploymentStrategyType
+		depReplicas   int32
+		newRSReplicas int32
+		maxSurge      int
+		expected      int32
+	}{
+		{
+			"can not scale up - to newRSReplicas",
+			extensions.RollingUpdateDeploymentStrategyType,
+			1, 5, 1, 5,
+		},
+		{
+			"scale up - to depDeplicas",
+			extensions.RollingUpdateDeploymentStrategyType,
+			6, 2, 10, 6,
+		},
+		{
+			"recreate - to depDeplicas",
+			extensions.RecreateDeploymentStrategyType,
+			3, 1, 1, 3,
+		},
+	}
+	newDeployment := generateDeployment("nginx")
+	newRC := generateRS(newDeployment)
+	rs5 := generateRS(newDeployment)
+	rs5.Spec.Replicas = 5
+
+	for _, test := range tests {
+		newDeployment.Spec.Replicas = test.depReplicas
+		newDeployment.Spec.Strategy = extensions.DeploymentStrategy{Type: test.strategyType}
+		newDeployment.Spec.Strategy.RollingUpdate = &extensions.RollingUpdateDeployment{
+			MaxUnavailable: intstr.FromInt(1),
+			MaxSurge:       intstr.FromInt(test.maxSurge),
+		}
+		newRC.Spec.Replicas = test.newRSReplicas
+		rs, err := NewRSNewReplicas(&newDeployment, []*extensions.ReplicaSet{&rs5}, &newRC)
+		if err != nil {
+			t.Errorf("In test case %s, got unexpected error %v", test.test, err)
+		}
+		if rs != test.expected {
+			t.Errorf("In test case %s, expected %+v, got %+v", test.test, test.expected, rs)
+		}
+	}
+}

pkg/util/env/env_test.go

@@ -50,6 +50,12 @@ func TestGetEnvAsIntOrFallback(t *testing.T) {
 	key = "FLOCKER_UNSET_VAR"
 	returnVal, _ = GetEnvAsIntOrFallback(key, expected)
 	assert.Equal(expected, returnVal)
+
+	key = "FLOCKER_SET_VAR"
+	os.Setenv(key, "not-an-int")
+	returnVal, err := GetEnvAsIntOrFallback(key, 1)
+	assert.Equal(expected, returnVal)
+	assert.EqualError(err, "strconv.ParseInt: parsing \"not-an-int\": invalid syntax")
 }
 
 func TestGetEnvAsFloat64OrFallback(t *testing.T) {
@@ -65,4 +71,10 @@ func TestGetEnvAsFloat64OrFallback(t *testing.T) {
 	key = "FLOCKER_UNSET_VAR"
 	returnVal, _ = GetEnvAsFloat64OrFallback(key, 1.0)
 	assert.Equal(expected, returnVal)
+
+	key = "FLOCKER_SET_VAR"
+	os.Setenv(key, "not-a-float")
+	returnVal, err := GetEnvAsFloat64OrFallback(key, 1.0)
+	assert.Equal(expected, returnVal)
+	assert.EqualError(err, "strconv.ParseFloat: parsing \"not-a-float\": invalid syntax")
 }

pkg/util/labels/labels_test.go

@@ -19,6 +19,8 @@ package labels
 import (
 	"reflect"
 	"testing"
+
+	"k8s.io/kubernetes/pkg/api/unversioned"
 )
 
 func TestCloneAndAddLabel(t *testing.T) {
@@ -53,8 +55,157 @@ func TestCloneAndAddLabel(t *testing.T) {
 
 	for _, tc := range cases {
 		got := CloneAndAddLabel(tc.labels, tc.labelKey, tc.labelValue)
+		if !reflect.DeepEqual(got, tc.want) {
+			t.Errorf("[Add] got %v, want %v", got, tc.want)
+		}
+		// now test the inverse.
+		got_rm := CloneAndRemoveLabel(got, tc.labelKey)
+		if !reflect.DeepEqual(got_rm, tc.labels) {
+			t.Errorf("[RM] got %v, want %v", got_rm, tc.labels)
+		}
+	}
+}
+
+func TestAddLabel(t *testing.T) {
+	labels := map[string]string{
+		"foo1": "bar1",
+		"foo2": "bar2",
+		"foo3": "bar3",
+	}
+
+	cases := []struct {
+		labels     map[string]string
+		labelKey   string
+		labelValue string
+		want       map[string]string
+	}{
+		{
+			labels: labels,
+			want:   labels,
+		},
+		{
+			labels:     labels,
+			labelKey:   "foo4",
+			labelValue: "food",
+			want: map[string]string{
+				"foo1": "bar1",
+				"foo2": "bar2",
+				"foo3": "bar3",
+				"foo4": "food",
+			},
+		},
+		{
+			labels:     nil,
+			labelKey:   "foo4",
+			labelValue: "food",
+			want: map[string]string{
+				"foo4": "food",
+			},
+		},
+	}
+
+	for _, tc := range cases {
+		got := AddLabel(tc.labels, tc.labelKey, tc.labelValue)
 		if !reflect.DeepEqual(got, tc.want) {
 			t.Errorf("got %v, want %v", got, tc.want)
 		}
 	}
 }
+
+func TestCloneSelectorAndAddLabel(t *testing.T) {
+	labels := map[string]string{
+		"foo1": "bar1",
+		"foo2": "bar2",
+		"foo3": "bar3",
+	}
+
+	cases := []struct {
+		labels     map[string]string
+		labelKey   string
+		labelValue uint32
+		want       map[string]string
+	}{
+		{
+			labels: labels,
+			want:   labels,
+		},
+		{
+			labels:     labels,
+			labelKey:   "foo4",
+			labelValue: 89,
+			want: map[string]string{
+				"foo1": "bar1",
+				"foo2": "bar2",
+				"foo3": "bar3",
+				"foo4": "89",
+			},
+		},
+		{
+			labels:     nil,
+			labelKey:   "foo4",
+			labelValue: 12,
+			want: map[string]string{
+				"foo4": "12",
+			},
+		},
+	}
+
+	for _, tc := range cases {
+		ls_in := unversioned.LabelSelector{MatchLabels: tc.labels}
+		ls_out := unversioned.LabelSelector{MatchLabels: tc.want}
+
+		got := CloneSelectorAndAddLabel(&ls_in, tc.labelKey, tc.labelValue)
+		if !reflect.DeepEqual(got, &ls_out) {
+			t.Errorf("got %v, want %v", got, tc.want)
+		}
+	}
+}
+
+func TestAddLabelToSelector(t *testing.T) {
+	labels := map[string]string{
+		"foo1": "bar1",
+		"foo2": "bar2",
+		"foo3": "bar3",
+	}
+
+	cases := []struct {
+		labels     map[string]string
+		labelKey   string
+		labelValue string
+		want       map[string]string
+	}{
+		{
+			labels: labels,
+			want:   labels,
+		},
+		{
+			labels:     labels,
+			labelKey:   "foo4",
+			labelValue: "89",
+			want: map[string]string{
+				"foo1": "bar1",
+				"foo2": "bar2",
+				"foo3": "bar3",
+				"foo4": "89",
+			},
+		},
+		{
+			labels:     nil,
+			labelKey:   "foo4",
+			labelValue: "12",
+			want: map[string]string{
+				"foo4": "12",
+			},
+		},
+	}
+
+	for _, tc := range cases {
+		ls_in := unversioned.LabelSelector{MatchLabels: tc.labels}
+		ls_out := unversioned.LabelSelector{MatchLabels: tc.want}
+
+		got := AddLabelToSelector(&ls_in, tc.labelKey, tc.labelValue)
+		if !reflect.DeepEqual(got, &ls_out) {
+			t.Errorf("got %v, want %v", got, tc.want)
+		}
+	}
+}

pkg/util/strings/escape_test.go

@@ -0,0 +1,63 @@
+/*
+Copyright 2016 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package strings
+
+import (
+	"testing"
+)
+
+func TestEscapePluginName(t *testing.T) {
+	testCases := []struct {
+		input  string
+		output string
+	}{
+		{"kubernetes.io/blah", "kubernetes.io~blah"},
+		{"blah/blerg/borg", "blah~blerg~borg"},
+		{"kubernetes.io", "kubernetes.io"},
+	}
+	for i, tc := range testCases {
+		escapee := EscapePluginName(tc.input)
+		if escapee != tc.output {
+			t.Errorf("case[%d]: expected (%q), got (%q)", i, tc.output, escapee)
+		}
+		original := UnescapePluginName(escapee)
+		if original != tc.input {
+			t.Errorf("case[%d]: expected (%q), got (%q)", i, tc.input, original)
+		}
+	}
+}
+
+func TestEscapeQualifiedNameForDisk(t *testing.T) {
+	testCases := []struct {
+		input  string
+		output string
+	}{
+		{"kubernetes.io/blah", "kubernetes.io~blah"},
+		{"blah/blerg/borg", "blah~blerg~borg"},
+		{"kubernetes.io", "kubernetes.io"},
+	}
+	for i, tc := range testCases {
+		escapee := EscapeQualifiedNameForDisk(tc.input)
+		if escapee != tc.output {
+			t.Errorf("case[%d]: expected (%q), got (%q)", i, tc.output, escapee)
+		}
+		original := UnescapeQualifiedNameForDisk(escapee)
+		if original != tc.input {
+			t.Errorf("case[%d]: expected (%q), got (%q)", i, tc.input, original)
+		}
+	}
+}

pkg/util/strings/strings_test.go

@@ -36,6 +36,7 @@ func TestSplitQualifiedName(t *testing.T) {
 		}
 	}
 }
+
 func TestJoinQualifiedName(t *testing.T) {
 	testCases := []struct {
 		input  []string
@@ -52,3 +53,21 @@ func TestJoinQualifiedName(t *testing.T) {
 		}
 	}
 }
+
+func TestShortenString(t *testing.T) {
+	testCases := []struct {
+		input   string
+		out_len int
+		output  string
+	}{
+		{"kubernetes.io", 5, "kuber"},
+		{"blah", 34, "blah"},
+		{"kubernetes.io", 13, "kubernetes.io"},
+	}
+	for i, tc := range testCases {
+		res := ShortenString(tc.input, tc.out_len)
+		if res != tc.output {
+			t.Errorf("case[%d]: expected %q, got %q", i, tc.output, res)
+		}
+	}
+}