From 9ba5f5869d13eecdb6c340a3e7f299b5df49614d Mon Sep 17 00:00:00 2001 From: Angus Salkeld Date: Wed, 1 Jun 2016 11:14:00 +1000 Subject: [PATCH 1/4] Increase coverage in pkg/util/env to 100% --- pkg/util/env/env_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/pkg/util/env/env_test.go b/pkg/util/env/env_test.go index eb55a9f21db..a2d53cc998d 100644 --- a/pkg/util/env/env_test.go +++ b/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") } From 2076071bbfb2cb5aab448cb87c24147c0db931e7 Mon Sep 17 00:00:00 2001 From: Angus Salkeld Date: Wed, 1 Jun 2016 13:48:26 +1000 Subject: [PATCH 2/4] Increase coverage in pkg/util/strings to 100% --- pkg/util/strings/escape_test.go | 63 ++++++++++++++++++++++++++++++++ pkg/util/strings/strings_test.go | 19 ++++++++++ 2 files changed, 82 insertions(+) create mode 100644 pkg/util/strings/escape_test.go diff --git a/pkg/util/strings/escape_test.go b/pkg/util/strings/escape_test.go new file mode 100644 index 00000000000..4c92f638a46 --- /dev/null +++ b/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) + } + } +} diff --git a/pkg/util/strings/strings_test.go b/pkg/util/strings/strings_test.go index 0e87fee7544..13c72599565 100644 --- a/pkg/util/strings/strings_test.go +++ b/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) + } + } +} From 448081b8bd8c68c0a3189bb7e9f2025633391e13 Mon Sep 17 00:00:00 2001 From: Angus Salkeld Date: Wed, 1 Jun 2016 14:18:33 +1000 Subject: [PATCH 3/4] Increase coverage in pkg/util/labels --- pkg/util/labels/labels_test.go | 151 +++++++++++++++++++++++++++++++++ 1 file changed, 151 insertions(+) diff --git a/pkg/util/labels/labels_test.go b/pkg/util/labels/labels_test.go index 34f385ae95f..de727dedca2 100644 --- a/pkg/util/labels/labels_test.go +++ b/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) + } + } +} From 6a0577cfcc33684da8582e3ae9b498016a7439b4 Mon Sep 17 00:00:00 2001 From: Angus Salkeld Date: Thu, 2 Jun 2016 17:31:15 +1000 Subject: [PATCH 4/4] Increase coverage in pkg/util/deployment --- .../deployment/util/deployment_util_test.go | 157 ++++++++++++++++++ 1 file changed, 157 insertions(+) diff --git a/pkg/controller/deployment/util/deployment_util_test.go b/pkg/controller/deployment/util/deployment_util_test.go index 262631da8a3..48ead0b43c8 100644 --- a/pkg/controller/deployment/util/deployment_util_test.go +++ b/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) + } + } +}