From 215db4d46b93d6177b379ec336d49972fcdcd604 Mon Sep 17 00:00:00 2001 From: Ed Bartosh Date: Wed, 2 Jan 2019 15:44:00 +0200 Subject: [PATCH] kubeadm: use T.Run API in app/ Used T.Run API for kubeadm tests in app/ This should improve testing output and make it more visible which test is doing what. --- .../app/componentconfigs/validation_test.go | 37 ++++--- cmd/kubeadm/app/constants/constants_test.go | 98 ++++++++++--------- cmd/kubeadm/app/discovery/discovery_test.go | 28 ++++-- cmd/kubeadm/app/features/features_test.go | 70 +++++++------ cmd/kubeadm/app/preflight/checks_test.go | 85 ++++++++-------- cmd/kubeadm/app/preflight/utils_test.go | 42 ++++---- 6 files changed, 205 insertions(+), 155 deletions(-) diff --git a/cmd/kubeadm/app/componentconfigs/validation_test.go b/cmd/kubeadm/app/componentconfigs/validation_test.go index 29343f8645..308d75ddee 100644 --- a/cmd/kubeadm/app/componentconfigs/validation_test.go +++ b/cmd/kubeadm/app/componentconfigs/validation_test.go @@ -31,11 +31,13 @@ import ( func TestValidateKubeProxyConfiguration(t *testing.T) { var tests = []struct { + name string clusterConfig *kubeadm.ClusterConfiguration msg string expectErr bool }{ { + name: "valid config", clusterConfig: &kubeadm.ClusterConfiguration{ ComponentConfigs: kubeadm.ComponentConfigs{ KubeProxy: &kubeproxyconfig.KubeProxyConfiguration{ @@ -67,6 +69,7 @@ func TestValidateKubeProxyConfiguration(t *testing.T) { expectErr: false, }, { + name: "invalid BindAddress", clusterConfig: &kubeadm.ClusterConfiguration{ ComponentConfigs: kubeadm.ComponentConfigs{ KubeProxy: &kubeproxyconfig.KubeProxyConfiguration{ @@ -100,6 +103,7 @@ func TestValidateKubeProxyConfiguration(t *testing.T) { expectErr: true, }, { + name: "invalid HealthzBindAddress", clusterConfig: &kubeadm.ClusterConfiguration{ ComponentConfigs: kubeadm.ComponentConfigs{ KubeProxy: &kubeproxyconfig.KubeProxyConfiguration{ @@ -133,6 +137,7 @@ func TestValidateKubeProxyConfiguration(t *testing.T) { expectErr: true, }, { + name: "invalid MetricsBindAddress", clusterConfig: &kubeadm.ClusterConfiguration{ ComponentConfigs: kubeadm.ComponentConfigs{ KubeProxy: &kubeproxyconfig.KubeProxyConfiguration{ @@ -166,6 +171,7 @@ func TestValidateKubeProxyConfiguration(t *testing.T) { expectErr: true, }, { + name: "invalid ClusterCIDR", clusterConfig: &kubeadm.ClusterConfiguration{ ComponentConfigs: kubeadm.ComponentConfigs{ KubeProxy: &kubeproxyconfig.KubeProxyConfiguration{ @@ -199,6 +205,7 @@ func TestValidateKubeProxyConfiguration(t *testing.T) { expectErr: true, }, { + name: "invalid UDPIdleTimeout", clusterConfig: &kubeadm.ClusterConfiguration{ ComponentConfigs: kubeadm.ComponentConfigs{ KubeProxy: &kubeproxyconfig.KubeProxyConfiguration{ @@ -232,6 +239,7 @@ func TestValidateKubeProxyConfiguration(t *testing.T) { expectErr: true, }, { + name: "invalid ConfigSyncPeriod", clusterConfig: &kubeadm.ClusterConfiguration{ ComponentConfigs: kubeadm.ComponentConfigs{ KubeProxy: &kubeproxyconfig.KubeProxyConfiguration{ @@ -266,22 +274,26 @@ func TestValidateKubeProxyConfiguration(t *testing.T) { }, } for i, rt := range tests { - err := ValidateKubeProxyConfiguration(rt.clusterConfig, nil).ToAggregate() - if (err != nil) != rt.expectErr { - t.Errorf("%d failed ValidateKubeProxyConfiguration: expected error %t, got error %t", i, rt.expectErr, err != nil) - } - if err != nil && !strings.Contains(err.Error(), rt.msg) { - t.Errorf("%d failed ValidateKubeProxyConfiguration: unexpected error: %v, expected: %s", i, err, rt.msg) - } + t.Run(rt.name, func(t *testing.T) { + err := ValidateKubeProxyConfiguration(rt.clusterConfig, nil).ToAggregate() + if (err != nil) != rt.expectErr { + t.Errorf("%d failed ValidateKubeProxyConfiguration: expected error %t, got error %t", i, rt.expectErr, err != nil) + } + if err != nil && !strings.Contains(err.Error(), rt.msg) { + t.Errorf("%d failed ValidateKubeProxyConfiguration: unexpected error: %v, expected: %s", i, err, rt.msg) + } + }) } } func TestValidateKubeletConfiguration(t *testing.T) { var tests = []struct { + name string clusterConfig *kubeadm.ClusterConfiguration expectErr bool }{ { + name: "valid configuration", clusterConfig: &kubeadm.ClusterConfiguration{ ComponentConfigs: kubeadm.ComponentConfigs{ Kubelet: &kubeletconfig.KubeletConfiguration{ @@ -314,6 +326,7 @@ func TestValidateKubeletConfiguration(t *testing.T) { expectErr: false, }, { + name: "invalid configuration", clusterConfig: &kubeadm.ClusterConfiguration{ ComponentConfigs: kubeadm.ComponentConfigs{ Kubelet: &kubeletconfig.KubeletConfiguration{ @@ -345,9 +358,11 @@ func TestValidateKubeletConfiguration(t *testing.T) { }, } for i, rt := range tests { - err := ValidateKubeletConfiguration(rt.clusterConfig, nil).ToAggregate() - if (err != nil) != rt.expectErr { - t.Errorf("%d failed ValidateKubeletConfiguration: expected error %t, got error %t", i, rt.expectErr, err != nil) - } + t.Run(rt.name, func(t *testing.T) { + err := ValidateKubeletConfiguration(rt.clusterConfig, nil).ToAggregate() + if (err != nil) != rt.expectErr { + t.Errorf("%d failed ValidateKubeletConfiguration: expected error %t, got error %t", i, rt.expectErr, err != nil) + } + }) } } diff --git a/cmd/kubeadm/app/constants/constants_test.go b/cmd/kubeadm/app/constants/constants_test.go index daffc11831..115526bb6d 100644 --- a/cmd/kubeadm/app/constants/constants_test.go +++ b/cmd/kubeadm/app/constants/constants_test.go @@ -100,14 +100,16 @@ func TestGetStaticPodFilepath(t *testing.T) { }, } for _, rt := range tests { - actual := GetStaticPodFilepath(rt.componentName, rt.manifestsDir) - if actual != rt.expected { - t.Errorf( - "failed GetStaticPodFilepath:\n\texpected: %s\n\t actual: %s", - rt.expected, - actual, - ) - } + t.Run(rt.componentName, func(t *testing.T) { + actual := GetStaticPodFilepath(rt.componentName, rt.manifestsDir) + if actual != rt.expected { + t.Errorf( + "failed GetStaticPodFilepath:\n\texpected: %s\n\t actual: %s", + rt.expected, + actual, + ) + } + }) } } @@ -133,14 +135,16 @@ func TestAddSelfHostedPrefix(t *testing.T) { }, } for _, rt := range tests { - actual := AddSelfHostedPrefix(rt.componentName) - if actual != rt.expected { - t.Errorf( - "failed AddSelfHostedPrefix:\n\texpected: %s\n\t actual: %s", - rt.expected, - actual, - ) - } + t.Run(rt.componentName, func(t *testing.T) { + actual := AddSelfHostedPrefix(rt.componentName) + if actual != rt.expected { + t.Errorf( + "failed AddSelfHostedPrefix:\n\texpected: %s\n\t actual: %s", + rt.expected, + actual, + ) + } + }) } } @@ -182,28 +186,30 @@ func TestEtcdSupportedVersion(t *testing.T) { }, } for _, rt := range tests { - actualVersion, actualError := EtcdSupportedVersion(rt.kubernetesVersion) - if actualError != nil { - if rt.expectedError == nil { - t.Errorf("failed EtcdSupportedVersion:\n\texpected no error, but got: %v", actualError) - } else if actualError.Error() != rt.expectedError.Error() { - t.Errorf( - "failed EtcdSupportedVersion:\n\texpected error: %v\n\t actual error: %v", - rt.expectedError, - actualError, - ) + t.Run(rt.kubernetesVersion, func(t *testing.T) { + actualVersion, actualError := EtcdSupportedVersion(rt.kubernetesVersion) + if actualError != nil { + if rt.expectedError == nil { + t.Errorf("failed EtcdSupportedVersion:\n\texpected no error, but got: %v", actualError) + } else if actualError.Error() != rt.expectedError.Error() { + t.Errorf( + "failed EtcdSupportedVersion:\n\texpected error: %v\n\t actual error: %v", + rt.expectedError, + actualError, + ) + } + } else { + if rt.expectedError != nil { + t.Errorf("failed EtcdSupportedVersion:\n\texpected error: %v, but got no error", rt.expectedError) + } else if strings.Compare(actualVersion.String(), rt.expectedVersion.String()) != 0 { + t.Errorf( + "failed EtcdSupportedVersion:\n\texpected version: %s\n\t actual version: %s", + rt.expectedVersion.String(), + actualVersion.String(), + ) + } } - } else { - if rt.expectedError != nil { - t.Errorf("failed EtcdSupportedVersion:\n\texpected error: %v, but got no error", rt.expectedError) - } else if strings.Compare(actualVersion.String(), rt.expectedVersion.String()) != 0 { - t.Errorf( - "failed EtcdSupportedVersion:\n\texpected version: %s\n\t actual version: %s", - rt.expectedVersion.String(), - actualVersion.String(), - ) - } - } + }) } } @@ -222,13 +228,15 @@ func TestGetKubeDNSVersion(t *testing.T) { }, } for _, rt := range tests { - actualDNSVersion := GetDNSVersion(rt.dns) - if actualDNSVersion != rt.expected { - t.Errorf( - "failed GetDNSVersion:\n\texpected: %s\n\t actual: %s", - rt.expected, - actualDNSVersion, - ) - } + t.Run(string(rt.dns), func(t *testing.T) { + actualDNSVersion := GetDNSVersion(rt.dns) + if actualDNSVersion != rt.expected { + t.Errorf( + "failed GetDNSVersion:\n\texpected: %s\n\t actual: %s", + rt.expected, + actualDNSVersion, + ) + } + }) } } diff --git a/cmd/kubeadm/app/discovery/discovery_test.go b/cmd/kubeadm/app/discovery/discovery_test.go index 96fb4f14e6..66ccff9711 100644 --- a/cmd/kubeadm/app/discovery/discovery_test.go +++ b/cmd/kubeadm/app/discovery/discovery_test.go @@ -24,11 +24,17 @@ import ( func TestFor(t *testing.T) { tests := []struct { + name string d kubeadm.JoinConfiguration expect bool }{ - {d: kubeadm.JoinConfiguration{}, expect: false}, { + name: "default Discovery", + d: kubeadm.JoinConfiguration{}, + expect: false, + }, + { + name: "file Discovery with a path", d: kubeadm.JoinConfiguration{ Discovery: kubeadm.Discovery{ File: &kubeadm.FileDiscovery{ @@ -39,6 +45,7 @@ func TestFor(t *testing.T) { expect: false, }, { + name: "file Discovery with an url", d: kubeadm.JoinConfiguration{ Discovery: kubeadm.Discovery{ File: &kubeadm.FileDiscovery{ @@ -49,6 +56,7 @@ func TestFor(t *testing.T) { expect: false, }, { + name: "BootstrapTokenDiscovery", d: kubeadm.JoinConfiguration{ Discovery: kubeadm.Discovery{ BootstrapToken: &kubeadm.BootstrapTokenDiscovery{ @@ -60,13 +68,15 @@ func TestFor(t *testing.T) { }, } for _, rt := range tests { - _, actual := For(&rt.d) - if (actual == nil) != rt.expect { - t.Errorf( - "failed For:\n\texpected: %t\n\t actual: %t", - rt.expect, - (actual == nil), - ) - } + t.Run(rt.name, func(t *testing.T) { + _, actual := For(&rt.d) + if (actual == nil) != rt.expect { + t.Errorf( + "failed For:\n\texpected: %t\n\t actual: %t", + rt.expect, + (actual == nil), + ) + } + }) } } diff --git a/cmd/kubeadm/app/features/features_test.go b/cmd/kubeadm/app/features/features_test.go index 0c01055e09..d40b654a39 100644 --- a/cmd/kubeadm/app/features/features_test.go +++ b/cmd/kubeadm/app/features/features_test.go @@ -108,20 +108,21 @@ func TestNewFeatureGate(t *testing.T) { } for _, test := range tests { + t.Run(test.value, func(t *testing.T) { + r, err := NewFeatureGate(&someFeatures, test.value) - r, err := NewFeatureGate(&someFeatures, test.value) + if !test.expectedError && err != nil { + t.Errorf("NewFeatureGate failed when not expected: %v", err) + return + } else if test.expectedError && err == nil { + t.Error("NewFeatureGate didn't failed when expected") + return + } - if !test.expectedError && err != nil { - t.Errorf("NewFeatureGate failed when not expected: %v", err) - continue - } else if test.expectedError && err == nil { - t.Error("NewFeatureGate didn't failed when expected") - continue - } - - if !reflect.DeepEqual(r, test.expectedFeaturesGate) { - t.Errorf("NewFeatureGate returned a unexpected value") - } + if !reflect.DeepEqual(r, test.expectedFeaturesGate) { + t.Errorf("NewFeatureGate returned a unexpected value") + } + }) } } @@ -132,20 +133,24 @@ func TestValidateVersion(t *testing.T) { } var tests = []struct { + name string requestedVersion string requestedFeatures map[string]bool expectedError bool }{ - { //no min version + { + name: "no min version", requestedFeatures: map[string]bool{"feature1": true}, expectedError: false, }, - { //min version but correct value given + { + name: "min version but correct value given", requestedFeatures: map[string]bool{"feature2": true}, requestedVersion: constants.MinimumControlPlaneVersion.String(), expectedError: false, }, - { //min version and incorrect value given + { + name: "min version and incorrect value given", requestedFeatures: map[string]bool{"feature2": true}, requestedVersion: "v1.11.2", expectedError: true, @@ -153,14 +158,16 @@ func TestValidateVersion(t *testing.T) { } for _, test := range tests { - err := ValidateVersion(someFeatures, test.requestedFeatures, test.requestedVersion) - if !test.expectedError && err != nil { - t.Errorf("ValidateVersion failed when not expected: %v", err) - continue - } else if test.expectedError && err == nil { - t.Error("ValidateVersion didn't failed when expected") - continue - } + t.Run(test.name, func(t *testing.T) { + err := ValidateVersion(someFeatures, test.requestedFeatures, test.requestedVersion) + if !test.expectedError && err != nil { + t.Errorf("ValidateVersion failed when not expected: %v", err) + return + } else if test.expectedError && err == nil { + t.Error("ValidateVersion didn't failed when expected") + return + } + }) } } @@ -185,23 +192,28 @@ func TestCheckDeprecatedFlags(t *testing.T) { } var tests = []struct { + name string features map[string]bool expectedMsg map[string]string }{ - { // feature deprecated + { + name: "deprecated feature", features: map[string]bool{"deprecated": true}, expectedMsg: map[string]string{"deprecated": dummyMessage}, }, - { // valid feature + { + name: "valid feature", features: map[string]bool{"feature1": true}, expectedMsg: map[string]string{}, }, } for _, test := range tests { - msg := CheckDeprecatedFlags(&someFeatures, test.features) - if !reflect.DeepEqual(test.expectedMsg, msg) { - t.Error("CheckDeprecatedFlags didn't returned expected message") - } + t.Run(test.name, func(t *testing.T) { + msg := CheckDeprecatedFlags(&someFeatures, test.features) + if !reflect.DeepEqual(test.expectedMsg, msg) { + t.Error("CheckDeprecatedFlags didn't returned expected message") + } + }) } } diff --git a/cmd/kubeadm/app/preflight/checks_test.go b/cmd/kubeadm/app/preflight/checks_test.go index e936134ae0..3ec7a79059 100644 --- a/cmd/kubeadm/app/preflight/checks_test.go +++ b/cmd/kubeadm/app/preflight/checks_test.go @@ -18,6 +18,7 @@ package preflight import ( "bytes" + "fmt" "io/ioutil" "strings" "testing" @@ -658,33 +659,33 @@ func TestKubeletVersionCheck(t *testing.T) { } for _, tc := range cases { - fcmd := fakeexec.FakeCmd{ - CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ - func() ([]byte, error) { return []byte("Kubernetes " + tc.kubeletVersion), nil }, - }, - } - fexec := &fakeexec.FakeExec{ - CommandScript: []fakeexec.FakeCommandAction{ - func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, - }, - } + t.Run(tc.kubeletVersion, func(t *testing.T) { + fcmd := fakeexec.FakeCmd{ + CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ + func() ([]byte, error) { return []byte("Kubernetes " + tc.kubeletVersion), nil }, + }, + } + fexec := &fakeexec.FakeExec{ + CommandScript: []fakeexec.FakeCommandAction{ + func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, + }, + } - check := KubeletVersionCheck{KubernetesVersion: tc.k8sVersion, exec: fexec} - warnings, errors := check.Check() - - switch { - case warnings != nil && !tc.expectWarnings: - t.Errorf("KubeletVersionCheck: unexpected warnings for kubelet version %q and Kubernetes version %q. Warnings: %v", tc.kubeletVersion, tc.k8sVersion, warnings) - case warnings == nil && tc.expectWarnings: - t.Errorf("KubeletVersionCheck: expected warnings for kubelet version %q and Kubernetes version %q but got nothing", tc.kubeletVersion, tc.k8sVersion) - case errors != nil && !tc.expectErrors: - t.Errorf("KubeletVersionCheck: unexpected errors for kubelet version %q and Kubernetes version %q. errors: %v", tc.kubeletVersion, tc.k8sVersion, errors) - case errors == nil && tc.expectErrors: - t.Errorf("KubeletVersionCheck: expected errors for kubelet version %q and Kubernetes version %q but got nothing", tc.kubeletVersion, tc.k8sVersion) - } + check := KubeletVersionCheck{KubernetesVersion: tc.k8sVersion, exec: fexec} + warnings, errors := check.Check() + switch { + case warnings != nil && !tc.expectWarnings: + t.Errorf("KubeletVersionCheck: unexpected warnings for kubelet version %q and Kubernetes version %q. Warnings: %v", tc.kubeletVersion, tc.k8sVersion, warnings) + case warnings == nil && tc.expectWarnings: + t.Errorf("KubeletVersionCheck: expected warnings for kubelet version %q and Kubernetes version %q but got nothing", tc.kubeletVersion, tc.k8sVersion) + case errors != nil && !tc.expectErrors: + t.Errorf("KubeletVersionCheck: unexpected errors for kubelet version %q and Kubernetes version %q. errors: %v", tc.kubeletVersion, tc.k8sVersion, errors) + case errors == nil && tc.expectErrors: + t.Errorf("KubeletVersionCheck: expected errors for kubelet version %q and Kubernetes version %q but got nothing", tc.kubeletVersion, tc.k8sVersion) + } + }) } - } func TestSetHasItemOrAll(t *testing.T) { @@ -702,15 +703,17 @@ func TestSetHasItemOrAll(t *testing.T) { } for _, rt := range tests { - result := setHasItemOrAll(rt.ignoreSet, rt.testString) - if result != rt.expectedResult { - t.Errorf( - "setHasItemOrAll: expected: %v actual: %v (arguments: %q, %q)", - rt.expectedResult, result, - rt.ignoreSet, - rt.testString, - ) - } + t.Run(rt.testString, func(t *testing.T) { + result := setHasItemOrAll(rt.ignoreSet, rt.testString) + if result != rt.expectedResult { + t.Errorf( + "setHasItemOrAll: expected: %v actual: %v (arguments: %q, %q)", + rt.expectedResult, result, + rt.ignoreSet, + rt.testString, + ) + } + }) } } @@ -790,12 +793,14 @@ func TestNumCPUCheck(t *testing.T) { } for _, rt := range tests { - warnings, errors := NumCPUCheck{NumCPU: rt.numCPU}.Check() - if len(warnings) != rt.numWarnings { - t.Errorf("expected %d warning(s) but got %d: %q", rt.numWarnings, len(warnings), warnings) - } - if len(errors) != rt.numErrors { - t.Errorf("expected %d warning(s) but got %d: %q", rt.numErrors, len(errors), errors) - } + t.Run(fmt.Sprintf("number of CPUs: %d", rt.numCPU), func(t *testing.T) { + warnings, errors := NumCPUCheck{NumCPU: rt.numCPU}.Check() + if len(warnings) != rt.numWarnings { + t.Errorf("expected %d warning(s) but got %d: %q", rt.numWarnings, len(warnings), warnings) + } + if len(errors) != rt.numErrors { + t.Errorf("expected %d warning(s) but got %d: %q", rt.numErrors, len(errors), errors) + } + }) } } diff --git a/cmd/kubeadm/app/preflight/utils_test.go b/cmd/kubeadm/app/preflight/utils_test.go index 608f841c56..b6f46ff8e5 100644 --- a/cmd/kubeadm/app/preflight/utils_test.go +++ b/cmd/kubeadm/app/preflight/utils_test.go @@ -40,26 +40,26 @@ func TestGetKubeletVersion(t *testing.T) { } for _, tc := range cases { - fcmd := fakeexec.FakeCmd{ - CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ - func() ([]byte, error) { return []byte(tc.output), tc.err }, - }, - } - fexec := &fakeexec.FakeExec{ - CommandScript: []fakeexec.FakeCommandAction{ - func(cmd string, args ...string) utilsexec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, - }, - } - ver, err := GetKubeletVersion(fexec) - switch { - case err != nil && tc.valid: - t.Errorf("GetKubeletVersion: unexpected error for %q. Error: %v", tc.output, err) - case err == nil && !tc.valid: - t.Errorf("GetKubeletVersion: error expected for key %q, but result is %q", tc.output, ver) - case ver != nil && ver.String() != tc.expected: - t.Errorf("GetKubeletVersion: unexpected version result for key %q. Expected: %q Actual: %q", tc.output, tc.expected, ver) - } - + t.Run(tc.output, func(t *testing.T) { + fcmd := fakeexec.FakeCmd{ + CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ + func() ([]byte, error) { return []byte(tc.output), tc.err }, + }, + } + fexec := &fakeexec.FakeExec{ + CommandScript: []fakeexec.FakeCommandAction{ + func(cmd string, args ...string) utilsexec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, + }, + } + ver, err := GetKubeletVersion(fexec) + switch { + case err != nil && tc.valid: + t.Errorf("GetKubeletVersion: unexpected error for %q. Error: %v", tc.output, err) + case err == nil && !tc.valid: + t.Errorf("GetKubeletVersion: error expected for key %q, but result is %q", tc.output, ver) + case ver != nil && ver.String() != tc.expected: + t.Errorf("GetKubeletVersion: unexpected version result for key %q. Expected: %q Actual: %q", tc.output, tc.expected, ver) + } + }) } - }