From 3050a64373dd77f2fa61da78d7774b195e6adb19 Mon Sep 17 00:00:00 2001 From: Ed Santiago Date: Tue, 18 Apr 2023 06:11:56 -0600 Subject: [PATCH 1/2] e2e test cleanup - fix a typo that was resulting in a test being a NOP, and add actual testing to it. - fix two Expects() with incorrectly-ordered actual/expects - remove leading whitespace from an It() test name - To(BeTrue()) is evil. Wherever possible, replace it with useful string or field checks. When not possible, use the annotation field to indicate what failed. I got carried away here, #sorrynotsorry - remove unused system-test code Signed-off-by: Ed Santiago --- test/e2e/commit_test.go | 11 ++--------- test/e2e/container_clone_test.go | 7 +++---- test/e2e/events_test.go | 3 +-- test/e2e/images_test.go | 2 +- test/e2e/inspect_test.go | 2 +- test/e2e/login_logout_test.go | 2 +- test/e2e/logs_test.go | 3 +-- test/e2e/manifest_test.go | 4 ++-- test/e2e/network_create_test.go | 6 +++--- test/e2e/network_test.go | 22 +++++++++++----------- test/e2e/play_kube_test.go | 4 ++-- test/e2e/pod_clone_test.go | 2 +- test/e2e/pod_create_test.go | 11 +++++------ test/e2e/pod_ps_test.go | 2 +- test/e2e/pod_rm_test.go | 3 +-- test/e2e/prune_test.go | 3 ++- test/e2e/ps_test.go | 8 ++++---- test/e2e/pull_test.go | 12 ++++-------- test/e2e/push_test.go | 2 +- test/e2e/quadlet_test.go | 4 ++-- test/e2e/restart_test.go | 4 ++-- test/e2e/run_restart_test.go | 2 +- test/e2e/run_selinux_test.go | 4 +--- test/e2e/run_test.go | 12 +++++------- test/e2e/stats_test.go | 2 +- test/e2e/systemd_test.go | 10 +++++----- test/system/helpers.bash | 10 ---------- 27 files changed, 64 insertions(+), 93 deletions(-) diff --git a/test/e2e/commit_test.go b/test/e2e/commit_test.go index 916dd0c239..bcfeeee2c6 100644 --- a/test/e2e/commit_test.go +++ b/test/e2e/commit_test.go @@ -119,15 +119,8 @@ var _ = Describe("Podman commit", func() { check := podmanTest.Podman([]string{"inspect", "foobar.com/test1-image:latest"}) check.WaitWithDefaultTimeout() - data := check.InspectImageJSON() - foundBlue := false - for _, i := range data[0].Labels { - if i == "blue" { - foundBlue = true - break - } - } - Expect(foundBlue).To(BeTrue()) + inspectResults := check.InspectImageJSON() + Expect(inspectResults[0].Labels).To(HaveKeyWithValue("image", "blue")) }) It("podman commit container with --squash", func() { diff --git a/test/e2e/container_clone_test.go b/test/e2e/container_clone_test.go index 6757db1d6f..1ae2ace615 100644 --- a/test/e2e/container_clone_test.go +++ b/test/e2e/container_clone_test.go @@ -288,10 +288,9 @@ var _ = Describe("Podman container clone", func() { inspect := podmanTest.Podman([]string{"inspect", clone.OutputToString()}) inspect.WaitWithDefaultTimeout() Expect(inspect).To(Exit(0)) - Expect(inspect.InspectContainerToJSON()[0].NetworkSettings.Networks).To(HaveLen(2)) - _, ok := inspect.InspectContainerToJSON()[0].NetworkSettings.Networks["testing123"] - Expect(ok).To(BeTrue()) - + networks := inspect.InspectContainerToJSON()[0].NetworkSettings.Networks + Expect(networks).To(HaveLen(2)) + Expect(networks).To(HaveKey("testing123")) }) It("podman container clone env test", func() { diff --git a/test/e2e/events_test.go b/test/e2e/events_test.go index 2cdeba20a1..b30fcc57e1 100644 --- a/test/e2e/events_test.go +++ b/test/e2e/events_test.go @@ -193,8 +193,7 @@ var _ = Describe("Podman events", func() { Expect(result).Should(Exit(0)) tEnd := time.Now() outDur := tEnd.Sub(untilT) - diff := outDur.Seconds() > 0 - Expect(diff).To(BeTrue()) + Expect(outDur.Seconds()).To(BeNumerically(">", 0), "duration") Expect(result.OutputToString()).To(ContainSubstring(name1)) Expect(result.OutputToString()).To(ContainSubstring(name2)) Expect(result.OutputToString()).To(ContainSubstring(name3)) diff --git a/test/e2e/images_test.go b/test/e2e/images_test.go index 3b5e0692f0..67726411e4 100644 --- a/test/e2e/images_test.go +++ b/test/e2e/images_test.go @@ -316,7 +316,7 @@ WORKDIR /test actual := podmanTest.Podman([]string{"images", "--sort", "created", "-q"}) actual.WaitWithDefaultTimeout() - Expect(expected.Out).Should(Equal(actual.Out)) + Expect(actual.Out).Should(Equal(expected.Out)) }) It("podman images --all flag", func() { diff --git a/test/e2e/inspect_test.go b/test/e2e/inspect_test.go index 6c07a52dd2..e703709b07 100644 --- a/test/e2e/inspect_test.go +++ b/test/e2e/inspect_test.go @@ -509,7 +509,7 @@ var _ = Describe("Podman inspect", func() { Expect(ulimit.Hard).To(BeNumerically("==", -1)) } } - Expect(found).To(BeTrue()) + Expect(found).To(BeTrue(), "found RLIMIT_CORE") }) It("Dropped capabilities are sorted", func() { diff --git a/test/e2e/login_logout_test.go b/test/e2e/login_logout_test.go index a94d171e41..17a9017e5d 100644 --- a/test/e2e/login_logout_test.go +++ b/test/e2e/login_logout_test.go @@ -116,7 +116,7 @@ var _ = Describe("Podman login and logout", func() { Expect(authInfo).To(HaveKey(authsKey)) auths, ok := authInfo[authsKey].(map[string]interface{}) - Expect(ok).To(BeTrue()) + Expect(ok).To(BeTrue(), "authInfo[%s]", authsKey) return auths } diff --git a/test/e2e/logs_test.go b/test/e2e/logs_test.go index 585019dc70..985678f3d9 100644 --- a/test/e2e/logs_test.go +++ b/test/e2e/logs_test.go @@ -4,7 +4,6 @@ import ( "fmt" "os" "os/exec" - "strings" "time" . "github.com/containers/podman/v4/test/utils" @@ -281,7 +280,7 @@ var _ = Describe("Podman logs", func() { output := results.OutputToStringArray() Expect(output).To(HaveLen(6)) - Expect(strings.Contains(output[0], cid1[:12]) || strings.Contains(output[0], cid2[:12])).To(BeTrue()) + Expect(output[0]).To(Or(ContainSubstring(cid1[:12]), ContainSubstring(cid2[:12]))) }) It("podman logs on a created container should result in 0 exit code: "+log, func() { diff --git a/test/e2e/manifest_test.go b/test/e2e/manifest_test.go index 5ef5417012..33f75452b1 100644 --- a/test/e2e/manifest_test.go +++ b/test/e2e/manifest_test.go @@ -154,7 +154,7 @@ var _ = Describe("Podman manifest", func() { Expect(session2.OutputToString()).To(Equal(session.OutputToString())) }) - It(" add --all", func() { + It("add --all", func() { session := podmanTest.Podman([]string{"manifest", "create", "foo"}) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) @@ -375,7 +375,7 @@ var _ = Describe("Podman manifest", func() { break } } - Expect(foundZstdFile).To(BeTrue()) + Expect(foundZstdFile).To(BeTrue(), "found zstd file") }) It("push progress", func() { diff --git a/test/e2e/network_create_test.go b/test/e2e/network_create_test.go index dafc4bfcfd..1e770e7219 100644 --- a/test/e2e/network_create_test.go +++ b/test/e2e/network_create_test.go @@ -80,7 +80,7 @@ var _ = Describe("Podman network create", func() { containerIP, _, err := net.ParseCIDR(try.OutputToString()) Expect(err).ToNot(HaveOccurred()) // Ensure that the IP the container got is within the subnet the user asked for - Expect(subnet.Contains(containerIP)).To(BeTrue()) + Expect(subnet.Contains(containerIP)).To(BeTrue(), "subnet contains containerIP") }) It("podman network create with name and IPv6 subnet", func() { @@ -119,7 +119,7 @@ var _ = Describe("Podman network create", func() { containerIP, _, err := net.ParseCIDR(try.OutputToString()) Expect(err).ToNot(HaveOccurred()) // Ensure that the IP the container got is within the subnet the user asked for - Expect(subnet.Contains(containerIP)).To(BeTrue()) + Expect(subnet.Contains(containerIP)).To(BeTrue(), "subnet contains containerIP") }) It("podman network create with name and IPv6 flag (dual-stack)", func() { @@ -194,7 +194,7 @@ var _ = Describe("Podman network create", func() { containerIP, _, err := net.ParseCIDR(try.OutputToString()) Expect(err).ToNot(HaveOccurred()) // Ensure that the IP the container got is within the subnet the user asked for - Expect(subnet.Contains(containerIP)).To(BeTrue()) + Expect(subnet.Contains(containerIP)).To(BeTrue(), "subnet contains containerIP") // verify the container has an IPv4 address too (the IPv4 subnet is autogenerated) try = podmanTest.Podman([]string{"run", "--rm", "--network", netName, ALPINE, "sh", "-c", "ip addr show eth0 | awk ' /inet / {print $2}'"}) try.WaitWithDefaultTimeout() diff --git a/test/e2e/network_test.go b/test/e2e/network_test.go index 49b72a865c..5f69a7a78e 100644 --- a/test/e2e/network_test.go +++ b/test/e2e/network_test.go @@ -301,8 +301,8 @@ var _ = Describe("Podman network", func() { conData := inspect.InspectContainerToJSON() Expect(conData).To(HaveLen(1)) Expect(conData[0].NetworkSettings.Networks).To(HaveLen(1)) - net, ok := conData[0].NetworkSettings.Networks[netName] - Expect(ok).To(BeTrue()) + Expect(conData[0].NetworkSettings.Networks).To(HaveKey(netName)) + net := conData[0].NetworkSettings.Networks[netName] Expect(net).To(HaveField("NetworkID", netName)) Expect(net).To(HaveField("IPPrefixLen", 24)) Expect(net.IPAddress).To(HavePrefix("10.50.50.")) @@ -337,11 +337,11 @@ var _ = Describe("Podman network", func() { conData := inspect.InspectContainerToJSON() Expect(conData).To(HaveLen(1)) Expect(conData[0].NetworkSettings.Networks).To(HaveLen(2)) - net1, ok := conData[0].NetworkSettings.Networks[netName1] - Expect(ok).To(BeTrue()) + Expect(conData[0].NetworkSettings.Networks).To(HaveKey(netName1)) + Expect(conData[0].NetworkSettings.Networks).To(HaveKey(netName2)) + net1 := conData[0].NetworkSettings.Networks[netName1] Expect(net1).To(HaveField("NetworkID", netName1)) - net2, ok := conData[0].NetworkSettings.Networks[netName2] - Expect(ok).To(BeTrue()) + net2 := conData[0].NetworkSettings.Networks[netName2] Expect(net2).To(HaveField("NetworkID", netName2)) // Necessary to ensure the CNI network is removed cleanly @@ -374,13 +374,13 @@ var _ = Describe("Podman network", func() { conData := inspect.InspectContainerToJSON() Expect(conData).To(HaveLen(1)) Expect(conData[0].NetworkSettings.Networks).To(HaveLen(2)) - net1, ok := conData[0].NetworkSettings.Networks[netName1] - Expect(ok).To(BeTrue()) + Expect(conData[0].NetworkSettings.Networks).To(HaveKey(netName1)) + Expect(conData[0].NetworkSettings.Networks).To(HaveKey(netName2)) + net1 := conData[0].NetworkSettings.Networks[netName1] Expect(net1).To(HaveField("NetworkID", netName1)) Expect(net1).To(HaveField("IPPrefixLen", 25)) Expect(net1.IPAddress).To(HavePrefix("10.50.51.")) - net2, ok := conData[0].NetworkSettings.Networks[netName2] - Expect(ok).To(BeTrue()) + net2 := conData[0].NetworkSettings.Networks[netName2] Expect(net2).To(HaveField("NetworkID", netName2)) Expect(net2).To(HaveField("IPPrefixLen", 26)) Expect(net2.IPAddress).To(HavePrefix("10.50.51.")) @@ -512,7 +512,7 @@ var _ = Describe("Podman network", func() { time.Sleep(interval) interval *= 2 } - Expect(worked).To(BeTrue()) + Expect(worked).To(BeTrue(), "nginx came up") // Nginx is now running so no need to do a loop // Test against the first alias diff --git a/test/e2e/play_kube_test.go b/test/e2e/play_kube_test.go index 4f284f253c..640f2b8788 100644 --- a/test/e2e/play_kube_test.go +++ b/test/e2e/play_kube_test.go @@ -3169,7 +3169,7 @@ spec: // the file should have been created st, err := os.Stat(hostPathLocation) Expect(err).ToNot(HaveOccurred()) - Expect(st.Mode().IsDir()).To(BeTrue()) + Expect(st.Mode().IsDir()).To(BeTrue(), "hostPathLocation is a directory") }) It("podman play kube test with DirectoryOrCreate HostPath type volume and non-existent directory path", func() { @@ -3186,7 +3186,7 @@ spec: // the full path should have been created st, err := os.Stat(hostPathLocation) Expect(err).ToNot(HaveOccurred()) - Expect(st.Mode().IsDir()).To(BeTrue()) + Expect(st.Mode().IsDir()).To(BeTrue(), "hostPathLocation is a directory") }) It("podman play kube test with DirectoryOrCreate HostPath type volume and existent directory path", func() { diff --git a/test/e2e/pod_clone_test.go b/test/e2e/pod_clone_test.go index 543224b560..015cf15fff 100644 --- a/test/e2e/pod_clone_test.go +++ b/test/e2e/pod_clone_test.go @@ -152,7 +152,7 @@ var _ = Describe("Podman pod clone", func() { run.WaitWithDefaultTimeout() Expect(run).Should(Exit(0)) t, strings := run.GrepString("shm on /dev/shm type tmpfs") - Expect(t).To(BeTrue()) + Expect(t).To(BeTrue(), "found /dev/shm") Expect(strings[0]).Should(ContainSubstring("size=10240k")) }) diff --git a/test/e2e/pod_create_test.go b/test/e2e/pod_create_test.go index 3564f8d370..87a83fb4e2 100644 --- a/test/e2e/pod_create_test.go +++ b/test/e2e/pod_create_test.go @@ -102,7 +102,7 @@ var _ = Describe("Podman pod create", func() { webserver := podmanTest.Podman([]string{"run", "--pod", pod, "-dt", NGINX_IMAGE}) webserver.WaitWithDefaultTimeout() Expect(webserver).Should(Exit(0)) - Expect(ncz(port)).To(BeTrue()) + Expect(ncz(port)).To(BeTrue(), "port %d is up", port) }) It("podman create pod with id file with network portbindings", func() { @@ -116,7 +116,7 @@ var _ = Describe("Podman pod create", func() { webserver := podmanTest.Podman([]string{"run", "--pod-id-file", file, "-dt", NGINX_IMAGE}) webserver.WaitWithDefaultTimeout() Expect(webserver).Should(Exit(0)) - Expect(ncz(port)).To(BeTrue()) + Expect(ncz(port)).To(BeTrue(), "port %d is up", port) }) It("podman create pod with no infra but portbindings should fail", func() { @@ -988,8 +988,7 @@ ENTRYPOINT ["sleep","99999"] ctrCreate = podmanTest.Podman([]string{"container", "run", "--pod", podCreate.OutputToString(), ALPINE, "cat", "/proc/self/attr/current"}) ctrCreate.WaitWithDefaultTimeout() Expect(ctrCreate).Should(Exit(0)) - match, _ := ctrCreate.GrepString("spc_t") - Expect(match).Should(BeTrue()) + Expect(ctrCreate.OutputToString()).To(ContainSubstring("spc_t")) }) It("podman pod create --security-opt seccomp", func() { @@ -1138,7 +1137,7 @@ ENTRYPOINT ["sleep","99999"] run.WaitWithDefaultTimeout() Expect(run).Should(Exit(0)) t, strings := run.GrepString("shm on /dev/shm type tmpfs") - Expect(t).To(BeTrue()) + Expect(t).To(BeTrue(), "found /dev/shm") Expect(strings[0]).Should(ContainSubstring("size=10240k")) }) @@ -1192,7 +1191,7 @@ ENTRYPOINT ["sleep","99999"] run.WaitWithDefaultTimeout() Expect(run).Should(Exit(0)) t, strings := run.GrepString("tmpfs on /run/lock") - Expect(t).To(BeTrue()) + Expect(t).To(BeTrue(), "found /run/lock") Expect(strings[0]).Should(ContainSubstring("size=10240k")) }) diff --git a/test/e2e/pod_ps_test.go b/test/e2e/pod_ps_test.go index 7d52617941..2281c5849a 100644 --- a/test/e2e/pod_ps_test.go +++ b/test/e2e/pod_ps_test.go @@ -169,7 +169,7 @@ var _ = Describe("Podman ps", func() { sortedArr := session.OutputToStringArray() - Expect(sort.SliceIsSorted(sortedArr, func(i, j int) bool { return sortedArr[i] < sortedArr[j] })).To(BeTrue()) + Expect(sort.SliceIsSorted(sortedArr, func(i, j int) bool { return sortedArr[i] < sortedArr[j] })).To(BeTrue(), "slice is sorted") }) It("podman pod ps --ctr-names", func() { diff --git a/test/e2e/pod_rm_test.go b/test/e2e/pod_rm_test.go index 18be57a25d..3043b611ad 100644 --- a/test/e2e/pod_rm_test.go +++ b/test/e2e/pod_rm_test.go @@ -144,8 +144,7 @@ var _ = Describe("Podman pod rm", func() { result := podmanTest.Podman([]string{"pod", "rm", "-a"}) result.WaitWithDefaultTimeout() Expect(result).To(ExitWithError()) - foundExpectedError, _ := result.ErrorGrepString("cannot be removed") - Expect(foundExpectedError).To(BeTrue()) + Expect(result.ErrorToString()).To(ContainSubstring("cannot be removed")) numPods = podmanTest.NumberOfPods() ps = podmanTest.Podman([]string{"pod", "ps"}) diff --git a/test/e2e/prune_test.go b/test/e2e/prune_test.go index aa8d009862..ee7fe863f3 100644 --- a/test/e2e/prune_test.go +++ b/test/e2e/prune_test.go @@ -524,9 +524,10 @@ var _ = Describe("Podman prune", func() { }) It("podman system prune --all --external fails", func() { - prune := podmanTest.Podman([]string{"system", "prune", "--all", "--enternal"}) + prune := podmanTest.Podman([]string{"system", "prune", "--all", "--external"}) prune.WaitWithDefaultTimeout() Expect(prune).Should(Exit(125)) + Expect(prune.ErrorToString()).To(ContainSubstring("--external cannot be combined with other options")) }) It("podman system prune --external leaves referenced containers", func() { diff --git a/test/e2e/ps_test.go b/test/e2e/ps_test.go index 5db6fcaffa..e1efdf2a6b 100644 --- a/test/e2e/ps_test.go +++ b/test/e2e/ps_test.go @@ -271,7 +271,7 @@ var _ = Describe("Podman ps", func() { It("podman ps print a human-readable `Status` with json format", func() { _, ec, _ := podmanTest.RunLsContainer("test1") - Expect(ec).To(Equal(0)) + Expect(ec).To(Equal(0), "container exit code") result := podmanTest.Podman([]string{"ps", "-a", "--format", "json"}) result.WaitWithDefaultTimeout() @@ -279,7 +279,7 @@ var _ = Describe("Podman ps", func() { Expect(result.OutputToString()).To(BeValidJSON()) // must contain "Status" match, StatusLine := result.GrepString(`Status`) - Expect(match).To(BeTrue()) + Expect(match).To(BeTrue(), "found 'Status'") // we waited for container to exit, so this must contain `Exited` Expect(StatusLine[0]).To(ContainSubstring("Exited")) }) @@ -460,7 +460,7 @@ var _ = Describe("Podman ps", func() { size1, _ := units.FromHumanSize(matches1[1]) size2, _ := units.FromHumanSize(matches2[1]) return size1 < size2 - })).To(BeTrue()) + })).To(BeTrue(), "slice is sorted") }) @@ -480,7 +480,7 @@ var _ = Describe("Podman ps", func() { Expect(session.OutputToString()).ToNot(ContainSubstring("COMMAND")) sortedArr := session.OutputToStringArray() - Expect(sort.SliceIsSorted(sortedArr, func(i, j int) bool { return sortedArr[i] < sortedArr[j] })).To(BeTrue()) + Expect(sort.SliceIsSorted(sortedArr, func(i, j int) bool { return sortedArr[i] < sortedArr[j] })).To(BeTrue(), "slice is sorted") }) It("podman --pod", func() { diff --git a/test/e2e/pull_test.go b/test/e2e/pull_test.go index ca4197aa20..6e0f2586f0 100644 --- a/test/e2e/pull_test.go +++ b/test/e2e/pull_test.go @@ -44,8 +44,7 @@ var _ = Describe("Podman pull", func() { session.WaitWithDefaultTimeout() Expect(session).Should(Exit(125)) expectedError := "initializing source docker://ibetthisdoesnotexistfr:random" - found, _ := session.ErrorGrepString(expectedError) - Expect(found).To(BeTrue()) + Expect(session.ErrorToString()).To(ContainSubstring(expectedError)) session = podmanTest.Podman([]string{"rmi", "busybox:musl", "alpine", "quay.io/libpod/cirros", "testdigest_v2s2@sha256:755f4d90b3716e2bf57060d249e2cd61c9ac089b1233465c5c2cb2d7ee550fdb"}) session.WaitWithDefaultTimeout() @@ -288,8 +287,7 @@ var _ = Describe("Podman pull", func() { session.WaitWithDefaultTimeout() Expect(session).Should(Exit(125)) expectedError := "Unexpected tar manifest.json: expected 1 item, got 2" - found, _ := session.ErrorGrepString(expectedError) - Expect(found).To(BeTrue()) + Expect(session.ErrorToString()).To(ContainSubstring(expectedError)) // Now pull _one_ image from a multi-image archive via the name // and index syntax. @@ -314,15 +312,13 @@ var _ = Describe("Podman pull", func() { session.WaitWithDefaultTimeout() Expect(session).Should(Exit(125)) expectedError = "Tag \"foo.com/does/not/exist:latest\" not found" - found, _ = session.ErrorGrepString(expectedError) - Expect(found).To(BeTrue()) + Expect(session.ErrorToString()).To(ContainSubstring(expectedError)) session = podmanTest.Podman([]string{"pull", "docker-archive:./testdata/docker-two-images.tar.xz:@2"}) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(125)) expectedError = "Invalid source index @2, only 2 manifest items available" - found, _ = session.ErrorGrepString(expectedError) - Expect(found).To(BeTrue()) + Expect(session.ErrorToString()).To(ContainSubstring(expectedError)) }) It("podman pull from oci-archive", func() { diff --git a/test/e2e/push_test.go b/test/e2e/push_test.go index 5783b8394f..da886e24e7 100644 --- a/test/e2e/push_test.go +++ b/test/e2e/push_test.go @@ -91,7 +91,7 @@ var _ = Describe("Podman push", func() { break } } - Expect(foundZstdFile).To(BeTrue()) + Expect(foundZstdFile).To(BeTrue(), "found zstd file") }) It("podman push to local registry", func() { diff --git a/test/e2e/quadlet_test.go b/test/e2e/quadlet_test.go index d3bd6a4e9e..a5c752d330 100644 --- a/test/e2e/quadlet_test.go +++ b/test/e2e/quadlet_test.go @@ -451,7 +451,7 @@ var _ = Describe("quadlet system generator", func() { current := session.ErrorToStringArray() expected := "No files to parse from [/something]" - Expect(strings.Contains(current[0], expected)).To(BeTrue()) + Expect(current[0]).To(ContainSubstring(expected)) }) It("Should parse a kube file and print it to stdout", func() { @@ -504,7 +504,7 @@ var _ = Describe("quadlet system generator", func() { fmt.Sprintf("ExecStop=%s kube down %s/deployment.yml", podmanPath, quadletDir), } - Expect(expected).To(Equal(current)) + Expect(current).To(Equal(expected)) }) }) diff --git a/test/e2e/restart_test.go b/test/e2e/restart_test.go index 096b70448a..6b52816e8a 100644 --- a/test/e2e/restart_test.go +++ b/test/e2e/restart_test.go @@ -77,7 +77,7 @@ var _ = Describe("Podman restart", func() { It("podman restart running container", func() { _ = podmanTest.RunTopContainer("test1") ok := WaitForContainer(podmanTest) - Expect(ok).To(BeTrue()) + Expect(ok).To(BeTrue(), "test1 container is up") startTime := podmanTest.Podman([]string{"inspect", "--format='{{.State.StartedAt}}'", "test1"}) startTime.WaitWithDefaultTimeout() @@ -92,7 +92,7 @@ var _ = Describe("Podman restart", func() { It("podman container restart running container", func() { _ = podmanTest.RunTopContainer("test1") ok := WaitForContainer(podmanTest) - Expect(ok).To(BeTrue()) + Expect(ok).To(BeTrue(), "test1 container is up") startTime := podmanTest.Podman([]string{"container", "inspect", "--format='{{.State.StartedAt}}'", "test1"}) startTime.WaitWithDefaultTimeout() diff --git a/test/e2e/run_restart_test.go b/test/e2e/run_restart_test.go index c6abedb1cb..3a4a8897fa 100644 --- a/test/e2e/run_restart_test.go +++ b/test/e2e/run_restart_test.go @@ -49,7 +49,7 @@ var _ = Describe("Podman run restart containers", func() { It("Podman start after signal kill", func() { _ = podmanTest.RunTopContainer("test1") ok := WaitForContainer(podmanTest) - Expect(ok).To(BeTrue()) + Expect(ok).To(BeTrue(), "test1 container started") killSession := podmanTest.Podman([]string{"kill", "-s", "9", "test1"}) killSession.WaitWithDefaultTimeout() diff --git a/test/e2e/run_selinux_test.go b/test/e2e/run_selinux_test.go index 33ccc87ecc..66ba22185d 100644 --- a/test/e2e/run_selinux_test.go +++ b/test/e2e/run_selinux_test.go @@ -62,9 +62,7 @@ var _ = Describe("Podman run", func() { session := podmanTest.Podman([]string{"run", ALPINE, "cat", "/proc/self/attr/current"}) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) - match1, _ := session.GrepString("container_t") - match2, _ := session.GrepString("svirt_lxc_net_t") - Expect(match1 || match2).Should(BeTrue()) + Expect(session.OutputToString()).To(Or(ContainSubstring("container_t"), ContainSubstring("svirt_lxc_net_t"))) }) It("podman run selinux type setup test", func() { diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go index d7fed3988a..7db217c3fd 100644 --- a/test/e2e/run_test.go +++ b/test/e2e/run_test.go @@ -1254,11 +1254,9 @@ USER mail`, BB) session := podmanTest.Podman([]string{"run", "--volume", vol1 + ":/myvol1:z", "--volume", vol2 + ":/myvol2:shared,z", fedoraMinimal, "findmnt", "-o", "TARGET,PROPAGATION"}) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) - match, shared := session.GrepString("shared") - Expect(match).Should(BeTrue()) + Expect(session.OutputToString()).To(ContainSubstring("shared")) // make sure it's only shared (and not 'shared,slave') - isSharedOnly := !strings.Contains(shared[0], "shared,") - Expect(isSharedOnly).Should(BeTrue()) + Expect(session.OutputToString()).To(Not(ContainSubstring("shared,"))) }) It("podman run --security-opts proc-opts=", func() { @@ -1417,7 +1415,7 @@ USER mail`, BB) break } } - Expect(found).To(BeTrue()) + Expect(found).To(BeTrue(), "found expected /ran file") err = os.Remove(aliveFile) Expect(err).ToNot(HaveOccurred()) @@ -1433,7 +1431,7 @@ USER mail`, BB) break } } - Expect(found).To(BeTrue()) + Expect(found).To(BeTrue(), "found /ran file after restart") }) It("podman run with restart policy does not restart on manual stop", func() { @@ -2098,7 +2096,7 @@ WORKDIR /madethis`, BB) mount.WaitWithDefaultTimeout() Expect(mount).Should(Exit(0)) t, strings := mount.GrepString("tmpfs on /run/lock") - Expect(t).To(BeTrue()) + Expect(t).To(BeTrue(), "found /run/lock") Expect(strings[0]).Should(ContainSubstring("size=10240k")) }) }) diff --git a/test/e2e/stats_test.go b/test/e2e/stats_test.go index b57850d767..4ea0eff681 100644 --- a/test/e2e/stats_test.go +++ b/test/e2e/stats_test.go @@ -144,7 +144,7 @@ var _ = Describe("Podman stats", func() { } time.Sleep(time.Second) } - Expect(found).To(BeTrue()) + Expect(found).To(BeTrue(), "container has started") stats := podmanTest.Podman([]string{"stats", "--all", "--no-stream", "--format", "json"}) stats.WaitWithDefaultTimeout() Expect(stats).Should(Exit(0)) diff --git a/test/e2e/systemd_test.go b/test/e2e/systemd_test.go index 73078af761..3324f78780 100644 --- a/test/e2e/systemd_test.go +++ b/test/e2e/systemd_test.go @@ -88,7 +88,7 @@ WantedBy=default.target // Give container 10 seconds to start started := podmanTest.WaitContainerReady(ctrName, "Reached target multi-user.target - Multi-User System.", 30, 1) - Expect(started).To(BeTrue()) + Expect(started).To(BeTrue(), "Reached multi-user.target") systemctl := podmanTest.Podman([]string{"exec", ctrName, "systemctl", "status", "--no-pager"}) systemctl.WaitWithDefaultTimeout() @@ -100,7 +100,7 @@ WantedBy=default.target Expect(result).Should(Exit(0)) conData := result.InspectContainerToJSON() Expect(conData).To(HaveLen(1)) - Expect(conData[0].Config.SystemdMode).To(BeTrue()) + Expect(conData[0].Config).To(HaveField("SystemdMode", true)) // stats not supported w/ CGv1 rootless or containerized if isCgroupsV1() && (isRootless() || isContainerized()) { @@ -127,7 +127,7 @@ WantedBy=default.target Expect(result).Should(Exit(0)) conData := result.InspectContainerToJSON() Expect(conData).To(HaveLen(1)) - Expect(conData[0].Config.SystemdMode).To(BeTrue()) + Expect(conData[0].Config).To(HaveField("SystemdMode", true)) }) It("podman systemd in command triggers systemd mode", func() { @@ -152,7 +152,7 @@ CMD /usr/lib/systemd/systemd`, ALPINE) Expect(result).Should(Exit(0)) conData := result.InspectContainerToJSON() Expect(conData).To(HaveLen(1)) - Expect(conData[0].Config.SystemdMode).To(BeTrue()) + Expect(conData[0].Config).To(HaveField("SystemdMode", true)) }) It("podman create container with --uidmap and conmon PidFile accessible", func() { @@ -181,7 +181,7 @@ CMD /usr/lib/systemd/systemd`, ALPINE) Expect(result).Should(Exit(0)) conData := result.InspectContainerToJSON() Expect(conData).To(HaveLen(1)) - Expect(conData[0].Config.SystemdMode).To(BeTrue()) + Expect(conData[0].Config).To(HaveField("SystemdMode", true)) }) It("podman run --systemd container should NOT mount /run noexec", func() { diff --git a/test/system/helpers.bash b/test/system/helpers.bash index 57d3ee314a..486b1cbc3d 100644 --- a/test/system/helpers.bash +++ b/test/system/helpers.bash @@ -527,16 +527,6 @@ function skip_if_journald_unavailable { fi } -function skip_if_root_ubuntu { - if is_ubuntu; then - if ! is_remote; then - if ! is_rootless; then - skip "Cannot run this test on rootful ubuntu, usually due to user errors" - fi - fi - fi -} - function skip_if_aarch64 { if is_aarch64; then skip "${msg:-Cannot run this test on aarch64 systems}" From fbe62f329a4ab9c705f7a8ef4718378009195f18 Mon Sep 17 00:00:00 2001 From: Ed Santiago Date: Tue, 18 Apr 2023 13:04:00 -0600 Subject: [PATCH 2/2] More cleanup: volumes: do not export to stdout This one got complicated, and deserves its own commit. Problem: ginkgo logs have a lot of NUL characters, making them difficult for logformatter to process and for humans to read. Cause: Paul tracked it down to "podman volume export" without "-o" (hence spitting out tar data to stdout). Solution: add "-o tmpfile" to named podman-volume-export. In the process, fix all sorts of other problems with that test. And, since the e2e test no longer tests "volume export" by itself, add a system test that does. It is possible that there are other places that emit NULs. One step at a time. Signed-off-by: Ed Santiago --- test/e2e/volume_create_test.go | 29 +++++++++++++++++++++-------- test/system/160-volumes.bats | 28 ++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/test/e2e/volume_create_test.go b/test/e2e/volume_create_test.go index db3ed3b169..50b56c2b91 100644 --- a/test/e2e/volume_create_test.go +++ b/test/e2e/volume_create_test.go @@ -85,18 +85,30 @@ var _ = Describe("Podman volume create", func() { Skip("Volume export check does not work with a remote client") } - session := podmanTest.Podman([]string{"volume", "create", "myvol"}) + volName := "my_vol_" + RandomString(10) + session := podmanTest.Podman([]string{"volume", "create", volName}) session.WaitWithDefaultTimeout() - volName := session.OutputToString() Expect(session).Should(Exit(0)) + Expect(session.OutputToString()).To(Equal(volName)) - session = podmanTest.Podman([]string{"run", "--volume", volName + ":/data", ALPINE, "sh", "-c", "echo hello >> " + "/data/test"}) + helloString := "hello-" + RandomString(20) + session = podmanTest.Podman([]string{"run", "--volume", volName + ":/data", ALPINE, "sh", "-c", "echo " + helloString + " >> /data/test"}) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) - check := podmanTest.Podman([]string{"volume", "export", volName}) + // export to tar file... + helloTar := filepath.Join(podmanTest.TempDir, "hello.tar") + check := podmanTest.Podman([]string{"volume", "export", "-o", helloTar, volName}) check.WaitWithDefaultTimeout() - Expect(check.OutputToString()).To(ContainSubstring("hello")) + Expect(check).Should(Exit(0)) + + // ...then confirm that tar file has our desired content. + // These flags emit filename to stderr (-v), contents to stdout + tar := SystemExec("tar", []string{"-x", "-v", "--to-stdout", "-f", helloTar}) + tar.WaitWithDefaultTimeout() + Expect(tar).To(Exit(0)) + Expect(tar.ErrorToString()).To(Equal("test")) + Expect(tar.OutputToString()).To(Equal(helloString)) }) It("podman create and import volume", func() { @@ -104,12 +116,13 @@ var _ = Describe("Podman volume create", func() { Skip("Volume export check does not work with a remote client") } - session := podmanTest.Podman([]string{"volume", "create", "my_vol"}) + volName := "my_vol_" + RandomString(10) + session := podmanTest.Podman([]string{"volume", "create", volName}) session.WaitWithDefaultTimeout() - volName := session.OutputToString() Expect(session).Should(Exit(0)) + Expect(session.OutputToString()).To(Equal(volName)) - session = podmanTest.Podman([]string{"run", "--volume", volName + ":/data", ALPINE, "sh", "-c", "echo hello >> " + "/data/test"}) + session = podmanTest.Podman([]string{"run", "--volume", volName + ":/data", ALPINE, "sh", "-c", "echo hello >> /data/test"}) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) diff --git a/test/system/160-volumes.bats b/test/system/160-volumes.bats index 7a6a4871da..369e95447b 100644 --- a/test/system/160-volumes.bats +++ b/test/system/160-volumes.bats @@ -258,6 +258,34 @@ EOF run_podman volume rm my_vol2 } +# stdout with NULs is easier to test here than in ginkgo +@test "podman volume export to stdout" { + skip_if_remote "N/A on podman-remote" + + local volname="myvol_$(random_string 10)" + local mountpoint="/data$(random_string 8)" + + run_podman volume create $volname + assert "$output" == "$volname" "volume create emits the name it was given" + + local content="mycontent-$(random_string 20)-the-end" + run_podman run --rm --volume "$volname:$mountpoint" $IMAGE \ + sh -c "echo $content >$mountpoint/testfile" + assert "$output" = "" + + # We can't use run_podman because bash can't handle NUL characters. + # Can't even store them in variables, so we need immediate redirection + # The "-v" is only for debugging: tar will emit the filename to stderr. + # If this test ever fails, that may give a clue. + echo "$_LOG_PROMPT $PODMAN volume export $volname | tar -x ..." + tar_output="$($PODMAN volume export $volname | tar -x -v --to-stdout)" + echo "$tar_output" + assert "$tar_output" == "$content" "extracted content" + + # Clean up + run_podman volume rm $volname +} + # Podman volume user test @test "podman volume user test" { is_rootless || skip "only meaningful when run rootless"