From e6c3ab013c9523d2af414aa924fd70af8a50ffb8 Mon Sep 17 00:00:00 2001 From: Ed Bartosh Date: Fri, 28 Dec 2018 13:09:07 +0200 Subject: [PATCH] kubeadm: use T.Run API in app/util/system Used T.Run API for kubeadm tests in app/util/system --- .../app/util/system/cgroup_validator_test.go | 15 ++-- .../app/util/system/docker_validator_test.go | 36 ++++++---- .../app/util/system/kernel_validator_test.go | 70 ++++++++++--------- .../app/util/system/os_validator_test.go | 14 ++-- .../app/util/system/package_validator_test.go | 67 +++++++++++------- 5 files changed, 119 insertions(+), 83 deletions(-) diff --git a/cmd/kubeadm/app/util/system/cgroup_validator_test.go b/cmd/kubeadm/app/util/system/cgroup_validator_test.go index c8f8275085..b9de6abd4b 100644 --- a/cmd/kubeadm/app/util/system/cgroup_validator_test.go +++ b/cmd/kubeadm/app/util/system/cgroup_validator_test.go @@ -45,12 +45,13 @@ func TestValidateCgroupSubsystem(t *testing.T) { err: false, }, } { - err := v.validateCgroupSubsystems(cgroupSpec, test.subsystems) - if !test.err { - assert.Nil(t, err, "%q: Expect error not to occur with cgroup", desc) - } else { - assert.NotNil(t, err, "%q: Expect error to occur with docker info", desc) - } - + t.Run(desc, func(t *testing.T) { + err := v.validateCgroupSubsystems(cgroupSpec, test.subsystems) + if !test.err { + assert.Nil(t, err, "%q: Expect error not to occur with cgroup", desc) + } else { + assert.NotNil(t, err, "%q: Expect error to occur with docker info", desc) + } + }) } } diff --git a/cmd/kubeadm/app/util/system/docker_validator_test.go b/cmd/kubeadm/app/util/system/docker_validator_test.go index 19b7e9adf3..2899348664 100644 --- a/cmd/kubeadm/app/util/system/docker_validator_test.go +++ b/cmd/kubeadm/app/util/system/docker_validator_test.go @@ -32,72 +32,84 @@ func TestValidateDockerInfo(t *testing.T) { GraphDriver: []string{"driver_1", "driver_2"}, } for _, test := range []struct { + name string info types.Info err bool warn bool }{ { + name: "unsupported Docker version 1.10.1", info: types.Info{Driver: "driver_1", ServerVersion: "1.10.1"}, err: true, warn: false, }, { + name: "unsupported driver", info: types.Info{Driver: "bad_driver", ServerVersion: "1.11.1"}, err: true, warn: false, }, { + name: "valid Docker version 1.11.1", info: types.Info{Driver: "driver_1", ServerVersion: "1.11.1"}, err: false, warn: false, }, { + name: "valid Docker version 1.12.1", info: types.Info{Driver: "driver_2", ServerVersion: "1.12.1"}, err: false, warn: false, }, { + name: "valid Docker version 1.13.1", info: types.Info{Driver: "driver_2", ServerVersion: "1.13.1"}, err: false, warn: false, }, { + name: "valid Docker version 17.03.0-ce", info: types.Info{Driver: "driver_2", ServerVersion: "17.03.0-ce"}, err: false, warn: false, }, { + name: "valid Docker version 17.06.0-ce", info: types.Info{Driver: "driver_2", ServerVersion: "17.06.0-ce"}, err: false, warn: false, }, { + name: "valid Docker version 17.09.0-ce", info: types.Info{Driver: "driver_2", ServerVersion: "17.09.0-ce"}, err: false, warn: false, }, { + name: "valid Docker version 18.06.0-ce", info: types.Info{Driver: "driver_2", ServerVersion: "18.06.0-ce"}, err: false, warn: false, }, { + name: "Docker version 18.09.0 is not in the list of validated versions", info: types.Info{Driver: "driver_2", ServerVersion: "18.09.0"}, err: false, warn: true, }, } { - warn, err := v.validateDockerInfo(spec, test.info) - if !test.err { - assert.Nil(t, err, "Expect error not to occur with docker info %+v", test.info) - } else { - assert.NotNil(t, err, "Expect error to occur with docker info %+v", test.info) - } - if !test.warn { - assert.Nil(t, warn, "Expect error not to occur with docker info %+v", test.info) - } else { - assert.NotNil(t, warn, "Expect error to occur with docker info %+v", test.info) - } - + t.Run(test.name, func(t *testing.T) { + warn, err := v.validateDockerInfo(spec, test.info) + if !test.err { + assert.Nil(t, err, "Expect error not to occur with docker info %+v", test.info) + } else { + assert.NotNil(t, err, "Expect error to occur with docker info %+v", test.info) + } + if !test.warn { + assert.Nil(t, warn, "Expect error not to occur with docker info %+v", test.info) + } else { + assert.NotNil(t, warn, "Expect error to occur with docker info %+v", test.info) + } + }) } } diff --git a/cmd/kubeadm/app/util/system/kernel_validator_test.go b/cmd/kubeadm/app/util/system/kernel_validator_test.go index 92daad28c6..e1cfedb5cf 100644 --- a/cmd/kubeadm/app/util/system/kernel_validator_test.go +++ b/cmd/kubeadm/app/util/system/kernel_validator_test.go @@ -33,40 +33,45 @@ func TestValidateKernelVersion(t *testing.T) { // not the DefaultSysSpec. The DefaultSysSpec should be tested with node e2e. testRegex := []string{`3\.[1-9][0-9].*`, `4\..*`} for _, test := range []struct { + name string version string err bool }{ - // first version regex matches { + name: "3.19.9-99-test first version regex matches", version: "3.19.9-99-test", err: false, }, - // one of version regexes matches { + name: "4.4.14+ one of version regexes matches", version: "4.4.14+", err: false, }, - // no version regex matches { + name: "2.0.0 no version regex matches", version: "2.0.0", err: true, }, { + name: "5.0.0 no version regex matches", version: "5.0.0", err: true, }, { + name: "3.9.0 no version regex matches", version: "3.9.0", err: true, }, } { - v.kernelRelease = test.version - err := v.validateKernelVersion(KernelSpec{Versions: testRegex}) - if !test.err { - assert.Nil(t, err, "Expect error not to occur with kernel version %q", test.version) - } else { - assert.NotNil(t, err, "Expect error to occur with kenrel version %q", test.version) - } + t.Run(test.name, func(t *testing.T) { + v.kernelRelease = test.version + err := v.validateKernelVersion(KernelSpec{Versions: testRegex}) + if !test.err { + assert.Nil(t, err, "Expect error not to occur with kernel version %q", test.version) + } else { + assert.NotNil(t, err, "Expect error to occur with kenrel version %q", test.version) + } + }) } } @@ -82,13 +87,13 @@ func TestValidateCachedKernelConfig(t *testing.T) { {Name: "FORBIDDEN_2", Aliases: []string{"ALIASE_FORBIDDEN_2"}}, }, } - for c, test := range []struct { + for _, test := range []struct { desc string config map[string]kConfigOption err bool }{ { - desc: "meet all required configurations should not report error.", + desc: "meet all required configurations should not report error", config: map[string]kConfigOption{ "REQUIRED_1": builtIn, "REQUIRED_2": asModule, @@ -96,7 +101,7 @@ func TestValidateCachedKernelConfig(t *testing.T) { err: false, }, { - desc: "one required configuration disabled should report error.", + desc: "one required configuration disabled should report error", config: map[string]kConfigOption{ "REQUIRED_1": leftOut, "REQUIRED_2": builtIn, @@ -104,14 +109,14 @@ func TestValidateCachedKernelConfig(t *testing.T) { err: true, }, { - desc: "one required configuration missing should report error.", + desc: "one required configuration missing should report error", config: map[string]kConfigOption{ "REQUIRED_1": builtIn, }, err: true, }, { - desc: "alias of required configuration should not report error.", + desc: "alias of required configuration should not report error", config: map[string]kConfigOption{ "REQUIRED_1": builtIn, "ALIASE_REQUIRED_2": asModule, @@ -119,7 +124,7 @@ func TestValidateCachedKernelConfig(t *testing.T) { err: false, }, { - desc: "optional configuration set or not should not report error.", + desc: "optional configuration set or not should not report error", config: map[string]kConfigOption{ "REQUIRED_1": builtIn, "REQUIRED_2": asModule, @@ -128,7 +133,7 @@ func TestValidateCachedKernelConfig(t *testing.T) { err: false, }, { - desc: "forbidden configuration disabled should not report error.", + desc: "forbidden configuration disabled should not report error", config: map[string]kConfigOption{ "REQUIRED_1": builtIn, "REQUIRED_2": asModule, @@ -137,7 +142,7 @@ func TestValidateCachedKernelConfig(t *testing.T) { err: false, }, { - desc: "forbidden configuration built-in should report error.", + desc: "forbidden configuration built-in should report error", config: map[string]kConfigOption{ "REQUIRED_1": builtIn, "REQUIRED_2": asModule, @@ -146,7 +151,7 @@ func TestValidateCachedKernelConfig(t *testing.T) { err: true, }, { - desc: "forbidden configuration built as module should report error.", + desc: "forbidden configuration built as module should report error", config: map[string]kConfigOption{ "REQUIRED_1": builtIn, "REQUIRED_2": asModule, @@ -155,7 +160,7 @@ func TestValidateCachedKernelConfig(t *testing.T) { err: true, }, { - desc: "alias of forbidden configuration should report error.", + desc: "alias of forbidden configuration should report error", config: map[string]kConfigOption{ "REQUIRED_1": builtIn, "REQUIRED_2": asModule, @@ -164,18 +169,19 @@ func TestValidateCachedKernelConfig(t *testing.T) { err: true, }, } { - t.Logf("TestCase #%d %s", c, test.desc) - // Add kernel config prefix. - for k, v := range test.config { - delete(test.config, k) - test.config[kConfigPrefix+k] = v - } - err := v.validateCachedKernelConfig(test.config, testKernelSpec) - if !test.err { - assert.Nil(t, err, "Expect error not to occur with kernel config %q", test.config) - } else { - assert.NotNil(t, err, "Expect error to occur with kenrel config %q", test.config) - } + t.Run(test.desc, func(t *testing.T) { + // Add kernel config prefix. + for k, v := range test.config { + delete(test.config, k) + test.config[kConfigPrefix+k] = v + } + err := v.validateCachedKernelConfig(test.config, testKernelSpec) + if !test.err { + assert.Nil(t, err, "Expect error not to occur with kernel config %q", test.config) + } else { + assert.NotNil(t, err, "Expect error to occur with kenrel config %q", test.config) + } + }) } } diff --git a/cmd/kubeadm/app/util/system/os_validator_test.go b/cmd/kubeadm/app/util/system/os_validator_test.go index f4338f74a7..6c32502592 100644 --- a/cmd/kubeadm/app/util/system/os_validator_test.go +++ b/cmd/kubeadm/app/util/system/os_validator_test.go @@ -44,11 +44,13 @@ func TestValidateOS(t *testing.T) { err: true, }, } { - err := v.validateOS(test.os, specOS) - if !test.err { - assert.Nil(t, err, "Expect error not to occur with os %q", test.os) - } else { - assert.NotNil(t, err, "Expect error to occur with os %q", test.os) - } + t.Run(test.os, func(t *testing.T) { + err := v.validateOS(test.os, specOS) + if !test.err { + assert.Nil(t, err, "Expect error not to occur with os %q", test.os) + } else { + assert.NotNil(t, err, "Expect error to occur with os %q", test.os) + } + }) } } diff --git a/cmd/kubeadm/app/util/system/package_validator_test.go b/cmd/kubeadm/app/util/system/package_validator_test.go index 3387a454a6..c88956a632 100644 --- a/cmd/kubeadm/app/util/system/package_validator_test.go +++ b/cmd/kubeadm/app/util/system/package_validator_test.go @@ -17,6 +17,7 @@ limitations under the License. package system import ( + "fmt" "reflect" "testing" @@ -49,10 +50,12 @@ func TestExtractUpstreamVersion(t *testing.T) { expected: "1.0.6", }, } { - got := extractUpstreamVersion(test.input) - if test.expected != got { - t.Errorf("extractUpstreamVersion(%q) = %q, want %q", test.input, got, test.expected) - } + t.Run(fmt.Sprintf("input:%s,expected:%s", test.input, test.expected), func(t *testing.T) { + got := extractUpstreamVersion(test.input) + if test.expected != got { + t.Errorf("extractUpstreamVersion(%q) = %q, want %q", test.input, got, test.expected) + } + }) } } @@ -106,10 +109,12 @@ func TestToSemVer(t *testing.T) { expected: "8.0.95", }, } { - got := toSemVer(test.input) - if test.expected != got { - t.Errorf("toSemVer(%q) = %q, want %q", test.input, got, test.expected) - } + t.Run(fmt.Sprintf("input:%s,expected:%s", test.input, test.expected), func(t *testing.T) { + got := toSemVer(test.input) + if test.expected != got { + t.Errorf("toSemVer(%q) = %q, want %q", test.input, got, test.expected) + } + }) } } @@ -143,10 +148,12 @@ func TestToSemVerRange(t *testing.T) { expected: ">1.x || >3.1.0 !4.2.x", }, } { - got := toSemVerRange(test.input) - if test.expected != got { - t.Errorf("toSemVerRange(%q) = %q, want %q", test.input, got, test.expected) - } + t.Run(fmt.Sprintf("input:%s,expected:%s", test.input, test.expected), func(t *testing.T) { + got := toSemVerRange(test.input) + if test.expected != got { + t.Errorf("toSemVerRange(%q) = %q, want %q", test.input, got, test.expected) + } + }) } } @@ -209,32 +216,37 @@ func TestValidatePackageVersion(t *testing.T) { }, }, } { - _, err := v.validate(test.specs, manager) - if test.err == nil && err != nil { - t.Errorf("%s: v.validate(): err = %s", test.desc, err) - } - if test.err != nil { - if err == nil { - t.Errorf("%s: v.validate() is expected to fail.", test.desc) - } else if test.err.Error() != err.Error() { - t.Errorf("%s: v.validate(): err = %q, want = %q", test.desc, err, test.err) + t.Run(test.desc, func(t *testing.T) { + _, err := v.validate(test.specs, manager) + if test.err == nil && err != nil { + t.Errorf("%s: v.validate(): err = %s", test.desc, err) } - } + if test.err != nil { + if err == nil { + t.Errorf("%s: v.validate() is expected to fail.", test.desc) + } else if test.err.Error() != err.Error() { + t.Errorf("%s: v.validate(): err = %q, want = %q", test.desc, err, test.err) + } + } + }) } } func TestApplyPackageOverride(t *testing.T) { for _, test := range []struct { + name string overrides []PackageSpecOverride osDistro string specs []PackageSpec expected []PackageSpec }{ { + name: "foo>=1.0", specs: []PackageSpec{{Name: "foo", VersionRange: ">=1.0"}}, expected: []PackageSpec{{Name: "foo", VersionRange: ">=1.0"}}, }, { + name: "ubuntu:foo>=1.0/bar>=2.0", osDistro: "ubuntu", overrides: []PackageSpecOverride{ { @@ -247,6 +259,7 @@ func TestApplyPackageOverride(t *testing.T) { expected: []PackageSpec{{Name: "bar", VersionRange: ">=2.0"}}, }, { + name: "ubuntu:foo>=1.0/debian:foo", osDistro: "ubuntu", overrides: []PackageSpecOverride{ { @@ -258,9 +271,11 @@ func TestApplyPackageOverride(t *testing.T) { expected: []PackageSpec{{Name: "foo", VersionRange: ">=1.0"}}, }, } { - got := applyPackageSpecOverride(test.specs, test.overrides, test.osDistro) - if !reflect.DeepEqual(test.expected, got) { - t.Errorf("applyPackageSpecOverride(%+v, %+v, %s) = %+v, want = %+v", test.specs, test.overrides, test.osDistro, got, test.expected) - } + t.Run(test.name, func(t *testing.T) { + got := applyPackageSpecOverride(test.specs, test.overrides, test.osDistro) + if !reflect.DeepEqual(test.expected, got) { + t.Errorf("applyPackageSpecOverride(%+v, %+v, %s) = %+v, want = %+v", test.specs, test.overrides, test.osDistro, got, test.expected) + } + }) } }