Merge pull request #72398 from bart0sh/PR0048-kubeadm-app-util-system-use-T.Run

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)
+			}
+		})
 	}
 }

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)
+			}
+		})
 	}
 }

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)
+			}
+		})
 	}
 }
 

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)
+			}
+		})
 	}
 }

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)
+			}
+		})
 	}
 }