From b5e1b8f619e19e76863c8c8cfad33c29bc055383 Mon Sep 17 00:00:00 2001 From: Daniel Canter Date: Sat, 28 May 2022 22:32:29 -0700 Subject: [PATCH] Use t.Run for /pkg/cri tests A majority of the tests in /pkg/cri are testing/validating multiple things per test (generally spec or options validations). This flow lends itself well to using *testing.T's Run method to run each thing as a subtest so `go test` output can actually display which subtest failed/passed. Some of the tests in the packages in pkg/cri already did this, but a bunch simply logged what sub-testcase was currently running without invoking t.Run. Signed-off-by: Daniel Canter --- pkg/cri/io/logger_test.go | 37 +- pkg/cri/server/container_create_linux_test.go | 400 +++++++++--------- pkg/cri/server/container_create_test.go | 68 +-- pkg/cri/server/container_list_test.go | 25 +- pkg/cri/server/container_remove_test.go | 31 +- pkg/cri/server/container_start_test.go | 31 +- .../server/container_stats_list_linux_test.go | 1 - pkg/cri/server/container_status_test.go | 110 ++--- pkg/cri/server/container_stop_test.go | 42 +- .../container_update_resources_linux_test.go | 29 +- pkg/cri/server/helpers_linux_test.go | 7 +- pkg/cri/server/helpers_test.go | 43 +- pkg/cri/server/image_pull_test.go | 42 +- pkg/cri/server/sandbox_list_test.go | 23 +- pkg/cri/server/sandbox_run_linux_test.go | 105 ++--- pkg/cri/server/sandbox_run_test.go | 108 ++--- pkg/cri/server/sandbox_status_test.go | 17 +- pkg/cri/server/sandbox_stop_test.go | 38 +- pkg/cri/store/container/status_test.go | 5 +- pkg/cri/store/image/image_test.go | 43 +- pkg/cri/util/image_test.go | 15 +- 21 files changed, 633 insertions(+), 587 deletions(-) diff --git a/pkg/cri/io/logger_test.go b/pkg/cri/io/logger_test.go index e4eaccbb8..289aa5c3e 100644 --- a/pkg/cri/io/logger_test.go +++ b/pkg/cri/io/logger_test.go @@ -236,23 +236,24 @@ func TestRedirectLogs(t *testing.T) { }, }, } { - t.Logf("TestCase %q", desc) - rc := io.NopCloser(strings.NewReader(test.input)) - buf := bytes.NewBuffer(nil) - wc := cioutil.NewNopWriteCloser(buf) - redirectLogs("test-path", rc, wc, test.stream, test.maxLen) - output := buf.String() - lines := strings.Split(output, "\n") - lines = lines[:len(lines)-1] // Discard empty string after last \n - assert.Len(t, lines, len(test.content)) - for i := range lines { - fields := strings.SplitN(lines[i], string([]byte{delimiter}), 4) - require.Len(t, fields, 4) - _, err := time.Parse(timestampFormat, fields[0]) - assert.NoError(t, err) - assert.EqualValues(t, test.stream, fields[1]) - assert.Equal(t, string(test.tag[i]), fields[2]) - assert.Equal(t, test.content[i], fields[3]) - } + t.Run(desc, func(t *testing.T) { + rc := io.NopCloser(strings.NewReader(test.input)) + buf := bytes.NewBuffer(nil) + wc := cioutil.NewNopWriteCloser(buf) + redirectLogs("test-path", rc, wc, test.stream, test.maxLen) + output := buf.String() + lines := strings.Split(output, "\n") + lines = lines[:len(lines)-1] // Discard empty string after last \n + assert.Len(t, lines, len(test.content)) + for i := range lines { + fields := strings.SplitN(lines[i], string([]byte{delimiter}), 4) + require.Len(t, fields, 4) + _, err := time.Parse(timestampFormat, fields[0]) + assert.NoError(t, err) + assert.EqualValues(t, test.stream, fields[1]) + assert.Equal(t, string(test.tag[i]), fields[2]) + assert.Equal(t, test.content[i], fields[3]) + } + }) } } diff --git a/pkg/cri/server/container_create_linux_test.go b/pkg/cri/server/container_create_linux_test.go index 2d825db17..beb0a099a 100644 --- a/pkg/cri/server/container_create_linux_test.go +++ b/pkg/cri/server/container_create_linux_test.go @@ -238,34 +238,35 @@ func TestContainerCapabilities(t *testing.T) { excludes: util.SubtractStringSlice(allCaps, "CAP_SYS_ADMIN"), }, } { - t.Logf("TestCase %q", desc) - containerConfig, sandboxConfig, imageConfig, specCheck := getCreateContainerTestData() - ociRuntime := config.Runtime{} - c := newTestCRIService() - c.allCaps = allCaps + t.Run(desc, func(t *testing.T) { + containerConfig, sandboxConfig, imageConfig, specCheck := getCreateContainerTestData() + ociRuntime := config.Runtime{} + c := newTestCRIService() + c.allCaps = allCaps - containerConfig.Linux.SecurityContext.Capabilities = test.capability - spec, err := c.containerSpec(testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime) - require.NoError(t, err) + containerConfig.Linux.SecurityContext.Capabilities = test.capability + spec, err := c.containerSpec(testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime) + require.NoError(t, err) - if selinux.GetEnabled() { - assert.NotEqual(t, "", spec.Process.SelinuxLabel) - assert.NotEqual(t, "", spec.Linux.MountLabel) - } + if selinux.GetEnabled() { + assert.NotEqual(t, "", spec.Process.SelinuxLabel) + assert.NotEqual(t, "", spec.Linux.MountLabel) + } - specCheck(t, testID, testSandboxID, testPid, spec) - for _, include := range test.includes { - assert.Contains(t, spec.Process.Capabilities.Bounding, include) - assert.Contains(t, spec.Process.Capabilities.Effective, include) - assert.Contains(t, spec.Process.Capabilities.Permitted, include) - } - for _, exclude := range test.excludes { - assert.NotContains(t, spec.Process.Capabilities.Bounding, exclude) - assert.NotContains(t, spec.Process.Capabilities.Effective, exclude) - assert.NotContains(t, spec.Process.Capabilities.Permitted, exclude) - } - assert.Empty(t, spec.Process.Capabilities.Inheritable) - assert.Empty(t, spec.Process.Capabilities.Ambient) + specCheck(t, testID, testSandboxID, testPid, spec) + for _, include := range test.includes { + assert.Contains(t, spec.Process.Capabilities.Bounding, include) + assert.Contains(t, spec.Process.Capabilities.Effective, include) + assert.Contains(t, spec.Process.Capabilities.Permitted, include) + } + for _, exclude := range test.excludes { + assert.NotContains(t, spec.Process.Capabilities.Bounding, exclude) + assert.NotContains(t, spec.Process.Capabilities.Effective, exclude) + assert.NotContains(t, spec.Process.Capabilities.Permitted, exclude) + } + assert.Empty(t, spec.Process.Capabilities.Inheritable) + assert.Empty(t, spec.Process.Capabilities.Ambient) + }) } } @@ -425,17 +426,18 @@ func TestContainerAndSandboxPrivileged(t *testing.T) { expectError: false, }, } { - t.Logf("TestCase %q", desc) - containerConfig.Linux.SecurityContext.Privileged = test.containerPrivileged - sandboxConfig.Linux.SecurityContext = &runtime.LinuxSandboxSecurityContext{ - Privileged: test.sandboxPrivileged, - } - _, err := c.containerSpec(testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime) - if test.expectError { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } + t.Run(desc, func(t *testing.T) { + containerConfig.Linux.SecurityContext.Privileged = test.containerPrivileged + sandboxConfig.Linux.SecurityContext = &runtime.LinuxSandboxSecurityContext{ + Privileged: test.sandboxPrivileged, + } + _, err := c.containerSpec(testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime) + if test.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) } } @@ -587,20 +589,22 @@ func TestContainerMounts(t *testing.T) { }, }, } { - config := &runtime.ContainerConfig{ - Metadata: &runtime.ContainerMetadata{ - Name: "test-name", - Attempt: 1, - }, - Mounts: test.criMounts, - Linux: &runtime.LinuxContainerConfig{ - SecurityContext: test.securityContext, - }, - } - c := newTestCRIService() - c.os.(*ostesting.FakeOS).StatFn = test.statFn - mounts := c.containerMounts(testSandboxID, config) - assert.Equal(t, test.expectedMounts, mounts, desc) + t.Run(desc, func(t *testing.T) { + config := &runtime.ContainerConfig{ + Metadata: &runtime.ContainerMetadata{ + Name: "test-name", + Attempt: 1, + }, + Mounts: test.criMounts, + Linux: &runtime.LinuxContainerConfig{ + SecurityContext: test.securityContext, + }, + } + c := newTestCRIService() + c.os.(*ostesting.FakeOS).StatFn = test.statFn + mounts := c.containerMounts(testSandboxID, config) + assert.Equal(t, test.expectedMounts, mounts, desc) + }) } } @@ -627,24 +631,24 @@ func TestPrivilegedBindMount(t *testing.T) { expectedCgroupFSRO: false, }, } { - t.Logf("TestCase %q", desc) + t.Run(desc, func(t *testing.T) { + containerConfig.Linux.SecurityContext.Privileged = test.privileged + sandboxConfig.Linux.SecurityContext.Privileged = test.privileged - containerConfig.Linux.SecurityContext.Privileged = test.privileged - sandboxConfig.Linux.SecurityContext.Privileged = test.privileged + spec, err := c.containerSpec(t.Name(), testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime) - spec, err := c.containerSpec(t.Name(), testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime) - - assert.NoError(t, err) - if test.expectedSysFSRO { - checkMount(t, spec.Mounts, "sysfs", "/sys", "sysfs", []string{"ro"}, []string{"rw"}) - } else { - checkMount(t, spec.Mounts, "sysfs", "/sys", "sysfs", []string{"rw"}, []string{"ro"}) - } - if test.expectedCgroupFSRO { - checkMount(t, spec.Mounts, "cgroup", "/sys/fs/cgroup", "cgroup", []string{"ro"}, []string{"rw"}) - } else { - checkMount(t, spec.Mounts, "cgroup", "/sys/fs/cgroup", "cgroup", []string{"rw"}, []string{"ro"}) - } + assert.NoError(t, err) + if test.expectedSysFSRO { + checkMount(t, spec.Mounts, "sysfs", "/sys", "sysfs", []string{"ro"}, []string{"rw"}) + } else { + checkMount(t, spec.Mounts, "sysfs", "/sys", "sysfs", []string{"rw"}, []string{"ro"}) + } + if test.expectedCgroupFSRO { + checkMount(t, spec.Mounts, "cgroup", "/sys/fs/cgroup", "cgroup", []string{"ro"}, []string{"rw"}) + } else { + checkMount(t, spec.Mounts, "cgroup", "/sys/fs/cgroup", "cgroup", []string{"rw"}, []string{"ro"}) + } + }) } } @@ -736,21 +740,22 @@ func TestMountPropagation(t *testing.T) { expectErr: true, }, } { - t.Logf("TestCase %q", desc) - c := newTestCRIService() - c.os.(*ostesting.FakeOS).LookupMountFn = test.fakeLookupMountFn - config, _, _, _ := getCreateContainerTestData() + t.Run(desc, func(t *testing.T) { + c := newTestCRIService() + c.os.(*ostesting.FakeOS).LookupMountFn = test.fakeLookupMountFn + config, _, _, _ := getCreateContainerTestData() - var spec runtimespec.Spec - spec.Linux = &runtimespec.Linux{} + var spec runtimespec.Spec + spec.Linux = &runtimespec.Linux{} - err := opts.WithMounts(c.os, config, []*runtime.Mount{test.criMount}, "")(context.Background(), nil, nil, &spec) - if test.expectErr { - require.Error(t, err) - } else { - require.NoError(t, err) - checkMount(t, spec.Mounts, test.criMount.HostPath, test.criMount.ContainerPath, "bind", test.optionsCheck, nil) - } + err := opts.WithMounts(c.os, config, []*runtime.Mount{test.criMount}, "")(context.Background(), nil, nil, &spec) + if test.expectErr { + require.Error(t, err) + } else { + require.NoError(t, err) + checkMount(t, spec.Mounts, test.criMount.HostPath, test.criMount.ContainerPath, "bind", test.optionsCheck, nil) + } + }) } } @@ -787,11 +792,12 @@ func TestPidNamespace(t *testing.T) { }, }, } { - t.Logf("TestCase %q", desc) - containerConfig.Linux.SecurityContext.NamespaceOptions = &runtime.NamespaceOption{Pid: test.pidNS} - spec, err := c.containerSpec(testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime) - require.NoError(t, err) - assert.Contains(t, spec.Linux.Namespaces, test.expected) + t.Run(desc, func(t *testing.T) { + containerConfig.Linux.SecurityContext.NamespaceOptions = &runtime.NamespaceOption{Pid: test.pidNS} + spec, err := c.containerSpec(testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime) + require.NoError(t, err) + assert.Contains(t, spec.Linux.Namespaces, test.expected) + }) } } @@ -920,7 +926,7 @@ func TestGenerateSeccompSecurityProfileSpecOpts(t *testing.T) { }, }, } { - t.Run(fmt.Sprintf("TestCase %q", desc), func(t *testing.T) { + t.Run(desc, func(t *testing.T) { cri := &criService{} cri.config.UnsetSeccompProfile = test.defaultProfile ssp := test.sp @@ -1073,29 +1079,30 @@ func TestGenerateApparmorSpecOpts(t *testing.T) { }, }, } { - t.Logf("TestCase %q", desc) - asp := test.sp - csp, err := generateApparmorSecurityProfile(test.profile) - if err != nil { - if test.expectErr { - assert.Error(t, err) + t.Run(desc, func(t *testing.T) { + asp := test.sp + csp, err := generateApparmorSecurityProfile(test.profile) + if err != nil { + if test.expectErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } } else { - assert.NoError(t, err) + if asp == nil { + asp = csp + } + specOpts, err := generateApparmorSpecOpts(asp, test.privileged, !test.disable) + assert.Equal(t, + reflect.ValueOf(test.specOpts).Pointer(), + reflect.ValueOf(specOpts).Pointer()) + if test.expectErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } } - } else { - if asp == nil { - asp = csp - } - specOpts, err := generateApparmorSpecOpts(asp, test.privileged, !test.disable) - assert.Equal(t, - reflect.ValueOf(test.specOpts).Pointer(), - reflect.ValueOf(specOpts).Pointer()) - if test.expectErr { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } - } + }) } } @@ -1169,21 +1176,22 @@ func TestMaskedAndReadonlyPaths(t *testing.T) { privileged: true, }, } { - t.Logf("TestCase %q", desc) - c.config.DisableProcMount = test.disableProcMount - containerConfig.Linux.SecurityContext.MaskedPaths = test.masked - containerConfig.Linux.SecurityContext.ReadonlyPaths = test.readonly - containerConfig.Linux.SecurityContext.Privileged = test.privileged - sandboxConfig.Linux.SecurityContext = &runtime.LinuxSandboxSecurityContext{ - Privileged: test.privileged, - } - spec, err := c.containerSpec(testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime) - require.NoError(t, err) - if !test.privileged { // specCheck presumes an unprivileged container - specCheck(t, testID, testSandboxID, testPid, spec) - } - assert.Equal(t, test.expectedMasked, spec.Linux.MaskedPaths) - assert.Equal(t, test.expectedReadonly, spec.Linux.ReadonlyPaths) + t.Run(desc, func(t *testing.T) { + c.config.DisableProcMount = test.disableProcMount + containerConfig.Linux.SecurityContext.MaskedPaths = test.masked + containerConfig.Linux.SecurityContext.ReadonlyPaths = test.readonly + containerConfig.Linux.SecurityContext.Privileged = test.privileged + sandboxConfig.Linux.SecurityContext = &runtime.LinuxSandboxSecurityContext{ + Privileged: test.privileged, + } + spec, err := c.containerSpec(testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime) + require.NoError(t, err) + if !test.privileged { // specCheck presumes an unprivileged container + specCheck(t, testID, testSandboxID, testPid, spec) + } + assert.Equal(t, test.expectedMasked, spec.Linux.MaskedPaths) + assert.Equal(t, test.expectedReadonly, spec.Linux.ReadonlyPaths) + }) } } @@ -1219,15 +1227,16 @@ func TestHostname(t *testing.T) { expectedEnv: "HOSTNAME=real-hostname", }, } { - t.Logf("TestCase %q", desc) - sandboxConfig.Hostname = test.hostname - sandboxConfig.Linux.SecurityContext = &runtime.LinuxSandboxSecurityContext{ - NamespaceOptions: &runtime.NamespaceOption{Network: test.networkNs}, - } - spec, err := c.containerSpec(testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime) - require.NoError(t, err) - specCheck(t, testID, testSandboxID, testPid, spec) - assert.Contains(t, spec.Process.Env, test.expectedEnv) + t.Run(desc, func(t *testing.T) { + sandboxConfig.Hostname = test.hostname + sandboxConfig.Linux.SecurityContext = &runtime.LinuxSandboxSecurityContext{ + NamespaceOptions: &runtime.NamespaceOption{Network: test.networkNs}, + } + spec, err := c.containerSpec(testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime) + require.NoError(t, err) + specCheck(t, testID, testSandboxID, testPid, spec) + assert.Contains(t, spec.Process.Env, test.expectedEnv) + }) } } @@ -1363,24 +1372,24 @@ func TestNonRootUserAndDevices(t *testing.T) { expectedDeviceGID: *testDevice.GID, }, } { - t.Logf("TestCase %q", desc) + t.Run(desc, func(t *testing.T) { + c.config.DeviceOwnershipFromSecurityContext = test.deviceOwnershipFromSecurityContext + containerConfig.Linux.SecurityContext.RunAsUser = test.uid + containerConfig.Linux.SecurityContext.RunAsGroup = test.gid + containerConfig.Devices = []*runtime.Device{ + { + ContainerPath: testDevice.Path, + HostPath: testDevice.Path, + Permissions: "r", + }, + } - c.config.DeviceOwnershipFromSecurityContext = test.deviceOwnershipFromSecurityContext - containerConfig.Linux.SecurityContext.RunAsUser = test.uid - containerConfig.Linux.SecurityContext.RunAsGroup = test.gid - containerConfig.Devices = []*runtime.Device{ - { - ContainerPath: testDevice.Path, - HostPath: testDevice.Path, - Permissions: "r", - }, - } + spec, err := c.containerSpec(t.Name(), testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, config.Runtime{}) + assert.NoError(t, err) - spec, err := c.containerSpec(t.Name(), testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, config.Runtime{}) - assert.NoError(t, err) - - assert.Equal(t, test.expectedDeviceUID, *spec.Linux.Devices[0].UID) - assert.Equal(t, test.expectedDeviceGID, *spec.Linux.Devices[0].GID) + assert.Equal(t, test.expectedDeviceUID, *spec.Linux.Devices[0].UID) + assert.Equal(t, test.expectedDeviceGID, *spec.Linux.Devices[0].GID) + }) } } @@ -1434,38 +1443,37 @@ func TestPrivilegedDevices(t *testing.T) { expectAllDevicesAllowed: true, }, } { - t.Logf("TestCase %q", desc) + t.Run(desc, func(t *testing.T) { + containerConfig.Linux.SecurityContext.Privileged = test.privileged + sandboxConfig.Linux.SecurityContext.Privileged = test.privileged - containerConfig.Linux.SecurityContext.Privileged = test.privileged - sandboxConfig.Linux.SecurityContext.Privileged = test.privileged - - ociRuntime := config.Runtime{ - PrivilegedWithoutHostDevices: test.privilegedWithoutHostDevices, - PrivilegedWithoutHostDevicesAllDevicesAllowed: test.privilegedWithoutHostDevicesAllDevicesAllowed, - } - spec, err := c.containerSpec(t.Name(), testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime) - assert.NoError(t, err) - - hostDevicesRaw, err := oci.HostDevices() - assert.NoError(t, err) - var hostDevices = make([]string, 0) - for _, dev := range hostDevicesRaw { - // https://github.com/containerd/cri/pull/1521#issuecomment-652807951 - if dev.Major != 0 { - hostDevices = append(hostDevices, dev.Path) + ociRuntime := config.Runtime{ + PrivilegedWithoutHostDevices: test.privilegedWithoutHostDevices, + PrivilegedWithoutHostDevicesAllDevicesAllowed: test.privilegedWithoutHostDevicesAllDevicesAllowed, } - } + spec, err := c.containerSpec(t.Name(), testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime) + assert.NoError(t, err) - if test.expectHostDevices { - assert.Len(t, spec.Linux.Devices, len(hostDevices)) - } else { - assert.Empty(t, spec.Linux.Devices) - } + hostDevicesRaw, err := oci.HostDevices() + assert.NoError(t, err) + var hostDevices = make([]string, 0) + for _, dev := range hostDevicesRaw { + // https://github.com/containerd/cri/pull/1521#issuecomment-652807951 + if dev.Major != 0 { + hostDevices = append(hostDevices, dev.Path) + } + } - assert.Len(t, spec.Linux.Resources.Devices, 1) - assert.Equal(t, spec.Linux.Resources.Devices[0].Allow, test.expectAllDevicesAllowed) - assert.Equal(t, spec.Linux.Resources.Devices[0].Access, "rwm") + if test.expectHostDevices { + assert.Len(t, spec.Linux.Devices, len(hostDevices)) + } else { + assert.Empty(t, spec.Linux.Devices) + } + assert.Len(t, spec.Linux.Resources.Devices, 1) + assert.Equal(t, spec.Linux.Resources.Devices[0].Allow, test.expectAllDevicesAllowed) + assert.Equal(t, spec.Linux.Resources.Devices[0].Access, "rwm") + }) } } @@ -1628,34 +1636,34 @@ containerEdits: }, }, } { - t.Logf("TestCase %q", test.description) + t.Run(test.description, func(t *testing.T) { + spec, err := c.containerSpec(testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime) + require.NoError(t, err) - spec, err := c.containerSpec(testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime) - require.NoError(t, err) + specCheck(t, testID, testSandboxID, testPid, spec) - specCheck(t, testID, testSandboxID, testPid, spec) + cdiDir, err := writeFilesToTempDir("containerd-test-CDI-injections-", test.cdiSpecFiles) + if cdiDir != "" { + defer os.RemoveAll(cdiDir) + } + require.NoError(t, err) - cdiDir, err := writeFilesToTempDir("containerd-test-CDI-injections-", test.cdiSpecFiles) - if cdiDir != "" { - defer os.RemoveAll(cdiDir) - } - require.NoError(t, err) + injectFun := oci.WithCDI(test.annotations, []string{cdiDir}) + err = injectFun(nil, nil, nil, spec) + assert.Equal(t, test.expectError, err != nil) - injectFun := oci.WithCDI(test.annotations, []string{cdiDir}) - err = injectFun(nil, nil, nil, spec) - assert.Equal(t, test.expectError, err != nil) - - if err != nil { - if test.expectEnv != nil { - for _, expectedEnv := range test.expectEnv { - assert.Contains(t, spec.Process.Env, expectedEnv) + if err != nil { + if test.expectEnv != nil { + for _, expectedEnv := range test.expectEnv { + assert.Contains(t, spec.Process.Env, expectedEnv) + } + } + if test.expectDevices != nil { + for _, expectedDev := range test.expectDevices { + assert.Contains(t, spec.Linux.Devices, expectedDev) + } } } - if test.expectDevices != nil { - for _, expectedDev := range test.expectDevices { - assert.Contains(t, spec.Linux.Devices, expectedDev) - } - } - } + }) } } diff --git a/pkg/cri/server/container_create_test.go b/pkg/cri/server/container_create_test.go index 3e109360f..5a4275f60 100644 --- a/pkg/cri/server/container_create_test.go +++ b/pkg/cri/server/container_create_test.go @@ -187,21 +187,22 @@ func TestContainerSpecCommand(t *testing.T) { expectErr: true, }, } { + t.Run(desc, func(t *testing.T) { + config, _, imageConfig, _ := getCreateContainerTestData() + config.Command = test.criEntrypoint + config.Args = test.criArgs + imageConfig.Entrypoint = test.imageEntrypoint + imageConfig.Cmd = test.imageArgs - config, _, imageConfig, _ := getCreateContainerTestData() - config.Command = test.criEntrypoint - config.Args = test.criArgs - imageConfig.Entrypoint = test.imageEntrypoint - imageConfig.Cmd = test.imageArgs - - var spec runtimespec.Spec - err := opts.WithProcessArgs(config, imageConfig)(context.Background(), nil, nil, &spec) - if test.expectErr { - assert.Error(t, err) - continue - } - assert.NoError(t, err) - assert.Equal(t, test.expected, spec.Process.Args, desc) + var spec runtimespec.Spec + err := opts.WithProcessArgs(config, imageConfig)(context.Background(), nil, nil, &spec) + if test.expectErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + assert.Equal(t, test.expected, spec.Process.Args, desc) + }) } } @@ -253,26 +254,27 @@ func TestVolumeMounts(t *testing.T) { }, }, } { - t.Logf("TestCase %q", desc) - config := &imagespec.ImageConfig{ - Volumes: test.imageVolumes, - } - c := newTestCRIService() - got := c.volumeMounts(testContainerRootDir, test.criMounts, config) - assert.Len(t, got, len(test.expectedMountDest)) - for _, dest := range test.expectedMountDest { - found := false - for _, m := range got { - if m.ContainerPath == dest { - found = true - assert.Equal(t, - filepath.Dir(m.HostPath), - filepath.Join(testContainerRootDir, "volumes")) - break - } + t.Run(desc, func(t *testing.T) { + config := &imagespec.ImageConfig{ + Volumes: test.imageVolumes, } - assert.True(t, found) - } + c := newTestCRIService() + got := c.volumeMounts(testContainerRootDir, test.criMounts, config) + assert.Len(t, got, len(test.expectedMountDest)) + for _, dest := range test.expectedMountDest { + found := false + for _, m := range got { + if m.ContainerPath == dest { + found = true + assert.Equal(t, + filepath.Dir(m.HostPath), + filepath.Join(testContainerRootDir, "volumes")) + break + } + } + assert.True(t, found) + } + }) } } diff --git a/pkg/cri/server/container_list_test.go b/pkg/cri/server/container_list_test.go index f2415783f..8a27cad97 100644 --- a/pkg/cri/server/container_list_test.go +++ b/pkg/cri/server/container_list_test.go @@ -149,8 +149,10 @@ func TestFilterContainers(t *testing.T) { expect: []*runtime.Container{testContainers[2]}, }, } { - filtered := c.filterCRIContainers(testContainers, test.filter) - assert.Equal(t, test.expect, filtered, desc) + t.Run(desc, func(t *testing.T) { + filtered := c.filterCRIContainers(testContainers, test.filter) + assert.Equal(t, test.expect, filtered, desc) + }) } } @@ -332,14 +334,15 @@ func TestListContainers(t *testing.T) { expect: expectedContainers[:1], }, } { - t.Logf("TestCase: %s", testdesc) - resp, err := c.ListContainers(context.Background(), &runtime.ListContainersRequest{Filter: testdata.filter}) - assert.NoError(t, err) - require.NotNil(t, resp) - containers := resp.GetContainers() - assert.Len(t, containers, len(testdata.expect)) - for _, cntr := range testdata.expect { - assert.Contains(t, containers, cntr) - } + t.Run(testdesc, func(t *testing.T) { + resp, err := c.ListContainers(context.Background(), &runtime.ListContainersRequest{Filter: testdata.filter}) + assert.NoError(t, err) + require.NotNil(t, resp) + containers := resp.GetContainers() + assert.Len(t, containers, len(testdata.expect)) + for _, cntr := range testdata.expect { + assert.Contains(t, containers, cntr) + } + }) } } diff --git a/pkg/cri/server/container_remove_test.go b/pkg/cri/server/container_remove_test.go index 8c38afdd6..e9b181d9b 100644 --- a/pkg/cri/server/container_remove_test.go +++ b/pkg/cri/server/container_remove_test.go @@ -65,21 +65,22 @@ func TestSetContainerRemoving(t *testing.T) { expectErr: false, }, } { - t.Logf("TestCase %q", desc) - container, err := containerstore.NewContainer( - containerstore.Metadata{ID: testID}, - containerstore.WithFakeStatus(test.status), - ) - assert.NoError(t, err) - err = setContainerRemoving(container) - if test.expectErr { - assert.Error(t, err) - assert.Equal(t, test.status, container.Status.Get(), "metadata should not be updated") - } else { + t.Run(desc, func(t *testing.T) { + container, err := containerstore.NewContainer( + containerstore.Metadata{ID: testID}, + containerstore.WithFakeStatus(test.status), + ) assert.NoError(t, err) - assert.True(t, container.Status.Get().Removing, "removing should be set") - assert.NoError(t, resetContainerRemoving(container)) - assert.False(t, container.Status.Get().Removing, "removing should be reset") - } + err = setContainerRemoving(container) + if test.expectErr { + assert.Error(t, err) + assert.Equal(t, test.status, container.Status.Get(), "metadata should not be updated") + } else { + assert.NoError(t, err) + assert.True(t, container.Status.Get().Removing, "removing should be set") + assert.NoError(t, resetContainerRemoving(container)) + assert.False(t, container.Status.Get().Removing, "removing should be reset") + } + }) } } diff --git a/pkg/cri/server/container_start_test.go b/pkg/cri/server/container_start_test.go index 31562748d..4ab50fde0 100644 --- a/pkg/cri/server/container_start_test.go +++ b/pkg/cri/server/container_start_test.go @@ -78,21 +78,22 @@ func TestSetContainerStarting(t *testing.T) { expectErr: true, }, } { - t.Logf("TestCase %q", desc) - container, err := containerstore.NewContainer( - containerstore.Metadata{ID: testID}, - containerstore.WithFakeStatus(test.status), - ) - assert.NoError(t, err) - err = setContainerStarting(container) - if test.expectErr { - assert.Error(t, err) - assert.Equal(t, test.status, container.Status.Get(), "metadata should not be updated") - } else { + t.Run(desc, func(t *testing.T) { + container, err := containerstore.NewContainer( + containerstore.Metadata{ID: testID}, + containerstore.WithFakeStatus(test.status), + ) assert.NoError(t, err) - assert.True(t, container.Status.Get().Starting, "starting should be set") - assert.NoError(t, resetContainerStarting(container)) - assert.False(t, container.Status.Get().Starting, "starting should be reset") - } + err = setContainerStarting(container) + if test.expectErr { + assert.Error(t, err) + assert.Equal(t, test.status, container.Status.Get(), "metadata should not be updated") + } else { + assert.NoError(t, err) + assert.True(t, container.Status.Get().Starting, "starting should be set") + assert.NoError(t, resetContainerStarting(container)) + assert.False(t, container.Status.Get().Starting, "starting should be reset") + } + }) } } diff --git a/pkg/cri/server/container_stats_list_linux_test.go b/pkg/cri/server/container_stats_list_linux_test.go index dba05fe5c..32ceafb93 100644 --- a/pkg/cri/server/container_stats_list_linux_test.go +++ b/pkg/cri/server/container_stats_list_linux_test.go @@ -168,7 +168,6 @@ func TestContainerMetricsCPU(t *testing.T) { expectedFirst *runtime.CpuUsage expectedSecond *runtime.CpuUsage }{ - "v1 metrics": { firstMetrics: &v1.Metrics{ CPU: &v1.CPUStat{ diff --git a/pkg/cri/server/container_status_test.go b/pkg/cri/server/container_status_test.go index 42c96a131..7df5277f1 100644 --- a/pkg/cri/server/container_status_test.go +++ b/pkg/cri/server/container_status_test.go @@ -127,29 +127,32 @@ func TestToCRIContainerStatus(t *testing.T) { expectedReason: errorExitReason, }, } { - metadata, status, _, expected := getContainerStatusTestData() - // Update status with test case. - status.StartedAt = test.startedAt - status.FinishedAt = test.finishedAt - status.ExitCode = test.exitCode - status.Reason = test.reason - status.Message = test.message - container, err := containerstore.NewContainer( - *metadata, - containerstore.WithFakeStatus(*status), - ) - assert.NoError(t, err) - // Set expectation based on test case. - expected.Reason = test.expectedReason - expected.StartedAt = test.startedAt - expected.FinishedAt = test.finishedAt - expected.ExitCode = test.exitCode - expected.Message = test.message - patchExceptedWithState(expected, test.expectedState) - containerStatus := toCRIContainerStatus(container, - expected.Image, - expected.ImageRef) - assert.Equal(t, expected, containerStatus, desc) + t.Run(desc, func(t *testing.T) { + + metadata, status, _, expected := getContainerStatusTestData() + // Update status with test case. + status.StartedAt = test.startedAt + status.FinishedAt = test.finishedAt + status.ExitCode = test.exitCode + status.Reason = test.reason + status.Message = test.message + container, err := containerstore.NewContainer( + *metadata, + containerstore.WithFakeStatus(*status), + ) + assert.NoError(t, err) + // Set expectation based on test case. + expected.Reason = test.expectedReason + expected.StartedAt = test.startedAt + expected.FinishedAt = test.finishedAt + expected.ExitCode = test.exitCode + expected.Message = test.message + patchExceptedWithState(expected, test.expectedState) + containerStatus := toCRIContainerStatus(container, + expected.Image, + expected.ImageRef) + assert.Equal(t, expected, containerStatus, desc) + }) } } @@ -209,37 +212,38 @@ func TestContainerStatus(t *testing.T) { expectErr: true, }, } { - t.Logf("TestCase %q", desc) - c := newTestCRIService() - metadata, status, image, expected := getContainerStatusTestData() - // Update status with test case. - status.StartedAt = test.startedAt - status.FinishedAt = test.finishedAt - status.Reason = test.reason - container, err := containerstore.NewContainer( - *metadata, - containerstore.WithFakeStatus(*status), - ) - assert.NoError(t, err) - if test.exist { - assert.NoError(t, c.containerStore.Add(container)) - } - if test.imageExist { - c.imageStore, err = imagestore.NewFakeStore([]imagestore.Image{*image}) + t.Run(desc, func(t *testing.T) { + c := newTestCRIService() + metadata, status, image, expected := getContainerStatusTestData() + // Update status with test case. + status.StartedAt = test.startedAt + status.FinishedAt = test.finishedAt + status.Reason = test.reason + container, err := containerstore.NewContainer( + *metadata, + containerstore.WithFakeStatus(*status), + ) assert.NoError(t, err) - } - resp, err := c.ContainerStatus(context.Background(), &runtime.ContainerStatusRequest{ContainerId: container.ID}) - if test.expectErr { - assert.Error(t, err) - assert.Nil(t, resp) - continue - } - // Set expectation based on test case. - expected.StartedAt = test.startedAt - expected.FinishedAt = test.finishedAt - expected.Reason = test.reason - patchExceptedWithState(expected, test.expectedState) - assert.Equal(t, expected, resp.GetStatus()) + if test.exist { + assert.NoError(t, c.containerStore.Add(container)) + } + if test.imageExist { + c.imageStore, err = imagestore.NewFakeStore([]imagestore.Image{*image}) + assert.NoError(t, err) + } + resp, err := c.ContainerStatus(context.Background(), &runtime.ContainerStatusRequest{ContainerId: container.ID}) + if test.expectErr { + assert.Error(t, err) + assert.Nil(t, resp) + return + } + // Set expectation based on test case. + expected.StartedAt = test.startedAt + expected.FinishedAt = test.finishedAt + expected.Reason = test.reason + patchExceptedWithState(expected, test.expectedState) + assert.Equal(t, expected, resp.GetStatus()) + }) } } diff --git a/pkg/cri/server/container_stop_test.go b/pkg/cri/server/container_stop_test.go index 83749132c..57ce39956 100644 --- a/pkg/cri/server/container_stop_test.go +++ b/pkg/cri/server/container_stop_test.go @@ -61,25 +61,27 @@ func TestWaitContainerStop(t *testing.T) { expectErr: false, }, } { - c := newTestCRIService() - container, err := containerstore.NewContainer( - containerstore.Metadata{ID: id}, - containerstore.WithFakeStatus(*test.status), - ) - assert.NoError(t, err) - assert.NoError(t, c.containerStore.Add(container)) - ctx := context.Background() - if test.cancel { - cancelledCtx, cancel := context.WithCancel(ctx) - cancel() - ctx = cancelledCtx - } - if test.timeout > 0 { - timeoutCtx, cancel := context.WithTimeout(ctx, test.timeout) - defer cancel() - ctx = timeoutCtx - } - err = c.waitContainerStop(ctx, container) - assert.Equal(t, test.expectErr, err != nil, desc) + t.Run(desc, func(t *testing.T) { + c := newTestCRIService() + container, err := containerstore.NewContainer( + containerstore.Metadata{ID: id}, + containerstore.WithFakeStatus(*test.status), + ) + assert.NoError(t, err) + assert.NoError(t, c.containerStore.Add(container)) + ctx := context.Background() + if test.cancel { + cancelledCtx, cancel := context.WithCancel(ctx) + cancel() + ctx = cancelledCtx + } + if test.timeout > 0 { + timeoutCtx, cancel := context.WithTimeout(ctx, test.timeout) + defer cancel() + ctx = timeoutCtx + } + err = c.waitContainerStop(ctx, container) + assert.Equal(t, test.expectErr, err != nil, desc) + }) } } diff --git a/pkg/cri/server/container_update_resources_linux_test.go b/pkg/cri/server/container_update_resources_linux_test.go index 1f68e3dbc..f2783148d 100644 --- a/pkg/cri/server/container_update_resources_linux_test.go +++ b/pkg/cri/server/container_update_resources_linux_test.go @@ -211,19 +211,20 @@ func TestUpdateOCILinuxResource(t *testing.T) { }, }, } { - t.Logf("TestCase %q", desc) - config := criconfig.Config{ - PluginConfig: criconfig.PluginConfig{ - TolerateMissingHugetlbController: true, - DisableHugetlbController: false, - }, - } - got, err := updateOCIResource(context.Background(), test.spec, test.request, config) - if test.expectErr { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } - assert.Equal(t, test.expected, got) + t.Run(desc, func(t *testing.T) { + config := criconfig.Config{ + PluginConfig: criconfig.PluginConfig{ + TolerateMissingHugetlbController: true, + DisableHugetlbController: false, + }, + } + got, err := updateOCIResource(context.Background(), test.spec, test.request, config) + if test.expectErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + assert.Equal(t, test.expected, got) + }) } } diff --git a/pkg/cri/server/helpers_linux_test.go b/pkg/cri/server/helpers_linux_test.go index ee26df98e..9c2c5e417 100644 --- a/pkg/cri/server/helpers_linux_test.go +++ b/pkg/cri/server/helpers_linux_test.go @@ -54,9 +54,10 @@ func TestGetCgroupsPath(t *testing.T) { expected: "/test-id", }, } { - t.Logf("TestCase %q", desc) - got := getCgroupsPath(test.cgroupsParent, testID) - assert.Equal(t, test.expected, got) + t.Run(desc, func(t *testing.T) { + got := getCgroupsPath(test.cgroupsParent, testID) + assert.Equal(t, test.expected, got) + }) } } diff --git a/pkg/cri/server/helpers_test.go b/pkg/cri/server/helpers_test.go index 0866d9840..a184d470e 100644 --- a/pkg/cri/server/helpers_test.go +++ b/pkg/cri/server/helpers_test.go @@ -75,10 +75,11 @@ func TestGetUserFromImage(t *testing.T) { name: "test", }, } { - t.Logf("TestCase - %q", c) - actualUID, actualName := getUserFromImage(test.user) - assert.Equal(t, test.uid, actualUID) - assert.Equal(t, test.name, actualName) + t.Run(c, func(t *testing.T) { + actualUID, actualName := getUserFromImage(test.user) + assert.Equal(t, test.uid, actualUID) + assert.Equal(t, test.name, actualName) + }) } } @@ -112,12 +113,13 @@ func TestGetRepoDigestAndTag(t *testing.T) { expectedRepoTag: "", }, } { - t.Logf("TestCase %q", desc) - named, err := docker.ParseDockerRef(test.ref) - assert.NoError(t, err) - repoDigest, repoTag := getRepoDigestAndTag(named, digest, test.schema1) - assert.Equal(t, test.expectedRepoDigest, repoDigest) - assert.Equal(t, test.expectedRepoTag, repoTag) + t.Run(desc, func(t *testing.T) { + named, err := docker.ParseDockerRef(test.ref) + assert.NoError(t, err) + repoDigest, repoTag := getRepoDigestAndTag(named, digest, test.schema1) + assert.Equal(t, test.expectedRepoDigest, repoDigest) + assert.Equal(t, test.expectedRepoTag, repoTag) + }) } } @@ -364,17 +366,18 @@ func TestEnvDeduplication(t *testing.T) { }, }, } { - t.Logf("TestCase %q", desc) - var spec runtimespec.Spec - if len(test.existing) > 0 { - spec.Process = &runtimespec.Process{ - Env: test.existing, + t.Run(desc, func(t *testing.T) { + var spec runtimespec.Spec + if len(test.existing) > 0 { + spec.Process = &runtimespec.Process{ + Env: test.existing, + } } - } - for _, kv := range test.kv { - oci.WithEnv([]string{kv[0] + "=" + kv[1]})(context.Background(), nil, nil, &spec) - } - assert.Equal(t, test.expected, spec.Process.Env) + for _, kv := range test.kv { + oci.WithEnv([]string{kv[0] + "=" + kv[1]})(context.Background(), nil, nil, &spec) + } + assert.Equal(t, test.expected, spec.Process.Env) + }) } } diff --git a/pkg/cri/server/image_pull_test.go b/pkg/cri/server/image_pull_test.go index 50209f37f..7b26b7381 100644 --- a/pkg/cri/server/image_pull_test.go +++ b/pkg/cri/server/image_pull_test.go @@ -102,11 +102,12 @@ func TestParseAuth(t *testing.T) { expectedSecret: testPasswd, }, } { - t.Logf("TestCase %q", desc) - u, s, err := ParseAuth(test.auth, test.host) - assert.Equal(t, test.expectErr, err != nil) - assert.Equal(t, test.expectedUser, u) - assert.Equal(t, test.expectedSecret, s) + t.Run(desc, func(t *testing.T) { + u, s, err := ParseAuth(test.auth, test.host) + assert.Equal(t, test.expectErr, err != nil) + assert.Equal(t, test.expectedUser, u) + assert.Equal(t, test.expectedSecret, s) + }) } } @@ -250,12 +251,13 @@ func TestRegistryEndpoints(t *testing.T) { }, }, } { - t.Logf("TestCase %q", desc) - c := newTestCRIService() - c.config.Registry.Mirrors = test.mirrors - got, err := c.registryEndpoints(test.host) - assert.NoError(t, err) - assert.Equal(t, test.expected, got) + t.Run(desc, func(t *testing.T) { + c := newTestCRIService() + c.config.Registry.Mirrors = test.mirrors + got, err := c.registryEndpoints(test.host) + assert.NoError(t, err) + assert.Equal(t, test.expected, got) + }) } } @@ -305,9 +307,10 @@ func TestDefaultScheme(t *testing.T) { expected: "https", }, } { - t.Logf("TestCase %q", desc) - got := defaultScheme(test.host) - assert.Equal(t, test.expected, got) + t.Run(desc, func(t *testing.T) { + got := defaultScheme(test.host) + assert.Equal(t, test.expected, got) + }) } } @@ -325,11 +328,12 @@ func TestEncryptedImagePullOpts(t *testing.T) { expectedOpts: 0, }, } { - t.Logf("TestCase %q", desc) - c := newTestCRIService() - c.config.ImageDecryption.KeyModel = test.keyModel - got := len(c.encryptedImagesPullOpts()) - assert.Equal(t, test.expectedOpts, got) + t.Run(desc, func(t *testing.T) { + c := newTestCRIService() + c.config.ImageDecryption.KeyModel = test.keyModel + got := len(c.encryptedImagesPullOpts()) + assert.Equal(t, test.expectedOpts, got) + }) } } diff --git a/pkg/cri/server/sandbox_list_test.go b/pkg/cri/server/sandbox_list_test.go index 302f887ad..5c46bc7e2 100644 --- a/pkg/cri/server/sandbox_list_test.go +++ b/pkg/cri/server/sandbox_list_test.go @@ -70,13 +70,15 @@ func TestToCRISandbox(t *testing.T) { expectedState: runtime.PodSandboxState_SANDBOX_NOTREADY, }, } { - status := sandboxstore.Status{ - CreatedAt: createdAt, - State: test.state, - } - expect.State = test.expectedState - s := toCRISandbox(meta, status) - assert.Equal(t, expect, s, desc) + t.Run(desc, func(t *testing.T) { + status := sandboxstore.Status{ + CreatedAt: createdAt, + State: test.state, + } + expect.State = test.expectedState + s := toCRISandbox(meta, status) + assert.Equal(t, expect, s, desc) + }) } } @@ -201,8 +203,9 @@ func TestFilterSandboxes(t *testing.T) { expect: []*runtime.PodSandbox{testSandboxes[2]}, }, } { - t.Logf("TestCase: %s", desc) - filtered := c.filterCRISandboxes(testSandboxes, test.filter) - assert.Equal(t, test.expect, filtered, desc) + t.Run(desc, func(t *testing.T) { + filtered := c.filterCRISandboxes(testSandboxes, test.filter) + assert.Equal(t, test.expect, filtered, desc) + }) } } diff --git a/pkg/cri/server/sandbox_run_linux_test.go b/pkg/cri/server/sandbox_run_linux_test.go index 22509b379..181232473 100644 --- a/pkg/cri/server/sandbox_run_linux_test.go +++ b/pkg/cri/server/sandbox_run_linux_test.go @@ -235,26 +235,27 @@ func TestLinuxSandboxContainerSpec(t *testing.T) { }, }, } { - t.Logf("TestCase %q", desc) - c := newTestCRIService() - c.config.EnableUnprivilegedICMP = true - c.config.EnableUnprivilegedPorts = true - config, imageConfig, specCheck := getRunPodSandboxTestData() - if test.configChange != nil { - test.configChange(config) - } - spec, err := c.sandboxContainerSpec(testID, config, imageConfig, nsPath, nil) - if test.expectErr { - assert.Error(t, err) - assert.Nil(t, spec) - continue - } - assert.NoError(t, err) - assert.NotNil(t, spec) - specCheck(t, testID, spec) - if test.specCheck != nil { - test.specCheck(t, spec) - } + t.Run(desc, func(t *testing.T) { + c := newTestCRIService() + c.config.EnableUnprivilegedICMP = true + c.config.EnableUnprivilegedPorts = true + config, imageConfig, specCheck := getRunPodSandboxTestData() + if test.configChange != nil { + test.configChange(config) + } + spec, err := c.sandboxContainerSpec(testID, config, imageConfig, nsPath, nil) + if test.expectErr { + assert.Error(t, err) + assert.Nil(t, spec) + return + } + assert.NoError(t, err) + assert.NotNil(t, spec) + specCheck(t, testID, spec) + if test.specCheck != nil { + test.specCheck(t, spec) + } + }) } } @@ -426,32 +427,33 @@ options timeout:1 }, }, } { - t.Logf("TestCase %q", desc) - c := newTestCRIService() - c.os.(*ostesting.FakeOS).HostnameFn = func() (string, error) { - return realhostname, nil - } - cfg := &runtime.PodSandboxConfig{ - Hostname: test.hostname, - DnsConfig: test.dnsConfig, - Linux: &runtime.LinuxPodSandboxConfig{ - SecurityContext: &runtime.LinuxSandboxSecurityContext{ - NamespaceOptions: &runtime.NamespaceOption{ - Ipc: test.ipcMode, + t.Run(desc, func(t *testing.T) { + c := newTestCRIService() + c.os.(*ostesting.FakeOS).HostnameFn = func() (string, error) { + return realhostname, nil + } + cfg := &runtime.PodSandboxConfig{ + Hostname: test.hostname, + DnsConfig: test.dnsConfig, + Linux: &runtime.LinuxPodSandboxConfig{ + SecurityContext: &runtime.LinuxSandboxSecurityContext{ + NamespaceOptions: &runtime.NamespaceOption{ + Ipc: test.ipcMode, + }, }, }, - }, - } - c.setupSandboxFiles(testID, cfg) - calls := c.os.(*ostesting.FakeOS).GetCalls() - assert.Len(t, calls, len(test.expectedCalls)) - for i, expected := range test.expectedCalls { - if expected.Arguments == nil { - // Ignore arguments. - expected.Arguments = calls[i].Arguments } - assert.Equal(t, expected, calls[i]) - } + c.setupSandboxFiles(testID, cfg) + calls := c.os.(*ostesting.FakeOS).GetCalls() + assert.Len(t, calls, len(test.expectedCalls)) + for i, expected := range test.expectedCalls { + if expected.Arguments == nil { + // Ignore arguments. + expected.Arguments = calls[i].Arguments + } + assert.Equal(t, expected, calls[i]) + } + }) } } @@ -493,14 +495,15 @@ options timeout:1 `, }, } { - t.Logf("TestCase %q", desc) - resolvContent, err := parseDNSOptions(test.servers, test.searches, test.options) - if test.expectErr { - assert.Error(t, err) - continue - } - assert.NoError(t, err) - assert.Equal(t, resolvContent, test.expectedContent) + t.Run(desc, func(t *testing.T) { + resolvContent, err := parseDNSOptions(test.servers, test.searches, test.options) + if test.expectErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + assert.Equal(t, resolvContent, test.expectedContent) + }) } } diff --git a/pkg/cri/server/sandbox_run_test.go b/pkg/cri/server/sandbox_run_test.go index 9c7e7b405..d8159aabb 100644 --- a/pkg/cri/server/sandbox_run_test.go +++ b/pkg/cri/server/sandbox_run_test.go @@ -92,29 +92,30 @@ func TestSandboxContainerSpec(t *testing.T) { }, }, } { - t.Logf("TestCase %q", desc) - c := newTestCRIService() - config, imageConfig, specCheck := getRunPodSandboxTestData() - if test.configChange != nil { - test.configChange(config) - } + t.Run(desc, func(t *testing.T) { + c := newTestCRIService() + config, imageConfig, specCheck := getRunPodSandboxTestData() + if test.configChange != nil { + test.configChange(config) + } - if test.imageConfigChange != nil { - test.imageConfigChange(imageConfig) - } - spec, err := c.sandboxContainerSpec(testID, config, imageConfig, nsPath, - test.podAnnotations) - if test.expectErr { - assert.Error(t, err) - assert.Nil(t, spec) - continue - } - assert.NoError(t, err) - assert.NotNil(t, spec) - specCheck(t, testID, spec) - if test.specCheck != nil { - test.specCheck(t, spec) - } + if test.imageConfigChange != nil { + test.imageConfigChange(imageConfig) + } + spec, err := c.sandboxContainerSpec(testID, config, imageConfig, nsPath, + test.podAnnotations) + if test.expectErr { + assert.Error(t, err) + assert.Nil(t, spec) + return + } + assert.NoError(t, err) + assert.NotNil(t, spec) + specCheck(t, testID, spec) + if test.specCheck != nil { + test.specCheck(t, spec) + } + }) } } @@ -139,25 +140,26 @@ func TestTypeurlMarshalUnmarshalSandboxMeta(t *testing.T) { }, }, } { - t.Logf("TestCase %q", desc) - meta := &sandboxstore.Metadata{ - ID: "1", - Name: "sandbox_1", - NetNSPath: "/home/cloud", - } - meta.Config, _, _ = getRunPodSandboxTestData() - if test.configChange != nil { - test.configChange(meta.Config) - } + t.Run(desc, func(t *testing.T) { + meta := &sandboxstore.Metadata{ + ID: "1", + Name: "sandbox_1", + NetNSPath: "/home/cloud", + } + meta.Config, _, _ = getRunPodSandboxTestData() + if test.configChange != nil { + test.configChange(meta.Config) + } - any, err := typeurl.MarshalAny(meta) - assert.NoError(t, err) - data, err := typeurl.UnmarshalAny(any) - assert.NoError(t, err) - assert.IsType(t, &sandboxstore.Metadata{}, data) - curMeta, ok := data.(*sandboxstore.Metadata) - assert.True(t, ok) - assert.Equal(t, meta, curMeta) + any, err := typeurl.MarshalAny(meta) + assert.NoError(t, err) + data, err := typeurl.UnmarshalAny(any) + assert.NoError(t, err) + assert.IsType(t, &sandboxstore.Metadata{}, data) + curMeta, ok := data.(*sandboxstore.Metadata) + assert.True(t, ok) + assert.Equal(t, meta, curMeta) + }) } } @@ -251,8 +253,9 @@ func TestToCNIPortMappings(t *testing.T) { }, }, } { - t.Logf("TestCase %q", desc) - assert.Equal(t, test.cniPortMappings, toCNIPortMappings(test.criPortMappings)) + t.Run(desc, func(t *testing.T) { + assert.Equal(t, test.cniPortMappings, toCNIPortMappings(test.criPortMappings)) + }) } } @@ -297,16 +300,17 @@ func TestSelectPodIP(t *testing.T) { expectedAdditionalIPs: []string{"2001:db8:85a3::8a2e:370:7334", "2001:db8:85a3::8a2e:370:7335", "192.168.17.45"}, }, } { - t.Logf("TestCase %q", desc) - var ipConfigs []*cni.IPConfig - for _, ip := range test.ips { - ipConfigs = append(ipConfigs, &cni.IPConfig{ - IP: net.ParseIP(ip), - }) - } - ip, additionalIPs := selectPodIPs(context.Background(), ipConfigs, test.pref) - assert.Equal(t, test.expectedIP, ip) - assert.Equal(t, test.expectedAdditionalIPs, additionalIPs) + t.Run(desc, func(t *testing.T) { + var ipConfigs []*cni.IPConfig + for _, ip := range test.ips { + ipConfigs = append(ipConfigs, &cni.IPConfig{ + IP: net.ParseIP(ip), + }) + } + ip, additionalIPs := selectPodIPs(context.Background(), ipConfigs, test.pref) + assert.Equal(t, test.expectedIP, ip) + assert.Equal(t, test.expectedAdditionalIPs, additionalIPs) + }) } } diff --git a/pkg/cri/server/sandbox_status_test.go b/pkg/cri/server/sandbox_status_test.go index 1617d06ca..273c61531 100644 --- a/pkg/cri/server/sandbox_status_test.go +++ b/pkg/cri/server/sandbox_status_test.go @@ -104,13 +104,14 @@ func TestPodSandboxStatus(t *testing.T) { expectedState: runtime.PodSandboxState_SANDBOX_NOTREADY, }, } { - t.Logf("TestCase: %s", desc) - status := sandboxstore.Status{ - CreatedAt: createdAt, - State: test.state, - } - expected.State = test.expectedState - got := toCRISandboxStatus(metadata, status, ip, additionalIPs) - assert.Equal(t, expected, got) + t.Run(desc, func(t *testing.T) { + status := sandboxstore.Status{ + CreatedAt: createdAt, + State: test.state, + } + expected.State = test.expectedState + got := toCRISandboxStatus(metadata, status, ip, additionalIPs) + assert.Equal(t, expected, got) + }) } } diff --git a/pkg/cri/server/sandbox_stop_test.go b/pkg/cri/server/sandbox_stop_test.go index c0a20465c..089fdebb2 100644 --- a/pkg/cri/server/sandbox_stop_test.go +++ b/pkg/cri/server/sandbox_stop_test.go @@ -51,23 +51,25 @@ func TestWaitSandboxStop(t *testing.T) { expectErr: false, }, } { - c := newTestCRIService() - sandbox := sandboxstore.NewSandbox( - sandboxstore.Metadata{ID: id}, - sandboxstore.Status{State: test.state}, - ) - ctx := context.Background() - if test.cancel { - cancelledCtx, cancel := context.WithCancel(ctx) - cancel() - ctx = cancelledCtx - } - if test.timeout > 0 { - timeoutCtx, cancel := context.WithTimeout(ctx, test.timeout) - defer cancel() - ctx = timeoutCtx - } - err := c.waitSandboxStop(ctx, sandbox) - assert.Equal(t, test.expectErr, err != nil, desc) + t.Run(desc, func(t *testing.T) { + c := newTestCRIService() + sandbox := sandboxstore.NewSandbox( + sandboxstore.Metadata{ID: id}, + sandboxstore.Status{State: test.state}, + ) + ctx := context.Background() + if test.cancel { + cancelledCtx, cancel := context.WithCancel(ctx) + cancel() + ctx = cancelledCtx + } + if test.timeout > 0 { + timeoutCtx, cancel := context.WithTimeout(ctx, test.timeout) + defer cancel() + ctx = timeoutCtx + } + err := c.waitSandboxStop(ctx, sandbox) + assert.Equal(t, test.expectErr, err != nil, desc) + }) } } diff --git a/pkg/cri/store/container/status_test.go b/pkg/cri/store/container/status_test.go index b5080a629..d4b2542bf 100644 --- a/pkg/cri/store/container/status_test.go +++ b/pkg/cri/store/container/status_test.go @@ -65,8 +65,9 @@ func TestContainerState(t *testing.T) { state: runtime.ContainerState_CONTAINER_EXITED, }, } { - t.Logf("TestCase %q", c) - assertlib.Equal(t, test.state, test.status.State()) + t.Run(c, func(t *testing.T) { + assertlib.Equal(t, test.state, test.status.State()) + }) } } diff --git a/pkg/cri/store/image/image_test.go b/pkg/cri/store/image/image_test.go index 5fda8a7be..dad5d4e4c 100644 --- a/pkg/cri/store/image/image_test.go +++ b/pkg/cri/store/image/image_test.go @@ -221,28 +221,29 @@ func TestImageStore(t *testing.T) { expected: []Image{}, }, } { - t.Logf("TestCase %q", desc) - s, err := NewFakeStore([]Image{image}) - assert.NoError(err) - assert.NoError(s.update(test.ref, test.image)) - - assert.Len(s.List(), len(test.expected)) - for _, expect := range test.expected { - got, err := s.Get(expect.ID) + t.Run(desc, func(t *testing.T) { + s, err := NewFakeStore([]Image{image}) assert.NoError(err) - equal(got, expect) - for _, ref := range expect.References { - id, err := s.Resolve(ref) - assert.NoError(err) - assert.Equal(expect.ID, id) - } - } + assert.NoError(s.update(test.ref, test.image)) - if test.image == nil { - // Shouldn't be able to index by removed ref. - id, err := s.Resolve(test.ref) - assert.Equal(errdefs.ErrNotFound, err) - assert.Empty(id) - } + assert.Len(s.List(), len(test.expected)) + for _, expect := range test.expected { + got, err := s.Get(expect.ID) + assert.NoError(err) + equal(got, expect) + for _, ref := range expect.References { + id, err := s.Resolve(ref) + assert.NoError(err) + assert.Equal(expect.ID, id) + } + } + + if test.image == nil { + // Shouldn't be able to index by removed ref. + id, err := s.Resolve(test.ref) + assert.Equal(errdefs.ErrNotFound, err) + assert.Empty(id) + } + }) } } diff --git a/pkg/cri/util/image_test.go b/pkg/cri/util/image_test.go index f4d911b1c..34d3bcd06 100644 --- a/pkg/cri/util/image_test.go +++ b/pkg/cri/util/image_test.go @@ -73,12 +73,13 @@ func TestNormalizeImageRef(t *testing.T) { expect: "gcr.io/library/busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582", }, } { - t.Logf("TestCase %q", test.input) - normalized, err := NormalizeImageRef(test.input) - assert.NoError(t, err) - output := normalized.String() - assert.Equal(t, test.expect, output) - _, err = reference.Parse(output) - assert.NoError(t, err, "%q should be containerd supported reference", output) + t.Run(test.input, func(t *testing.T) { + normalized, err := NormalizeImageRef(test.input) + assert.NoError(t, err) + output := normalized.String() + assert.Equal(t, test.expect, output) + _, err = reference.Parse(output) + assert.NoError(t, err, "%q should be containerd supported reference", output) + }) } }