Skip to content

Commit

Permalink
e2e: redefine ExitWithError() to require exit code
Browse files Browse the repository at this point in the history
...and an optional error-message string, to be checked
against stderr.

This is a starting point and baby-steps progress toward #18188.
There are 249 ExitWithError() checks in test/e2e. It will take
weeks to fix them all. This commit enables new functionality:

    Expect(ExitWithError(125, "expected substring"))

...while also allowing the current empty-args form. Once
all 249 empty-args uses are modernized, the matcher code
will be cleaned up.

I expect it will take several months of light effort to get
all e2e tests transitioned to the new form. I am choosing to
do so in pieces, for (relative) ease of review. This PR:

  1) makes the initial changes described above; and
  2) updates a small subset of e2e _test.go files such that:
     a) ExitWithError() is given an exit code and error string; and
     b) Exit(Nonzero) is changed to ExitWithError(Nonzero, "string")
        (when possible)

Signed-off-by: Ed Santiago <[email protected]>
  • Loading branch information
edsantiago committed Apr 10, 2024
1 parent 2debcf6 commit 2d91598
Show file tree
Hide file tree
Showing 17 changed files with 183 additions and 186 deletions.
2 changes: 1 addition & 1 deletion test/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ file itself. Consider the following actual test:
It("podman inspect bogus pod", func() {
session := podmanTest.Podman([]string{"pod", "inspect", "foobar"})
session.WaitWithDefaultTimeout()
Expect(session).To(ExitWithError())
Expect(session).To(ExitWithError(125, "no such pod foobar"))
})
```

Expand Down
9 changes: 4 additions & 5 deletions test/e2e/attach_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,14 @@ import (
. "github.com/containers/podman/v5/test/utils"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
. "github.com/onsi/gomega/gexec"
)

var _ = Describe("Podman attach", func() {

It("podman attach to bogus container", func() {
session := podmanTest.Podman([]string{"attach", "foobar"})
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(125))
Expect(session).Should(ExitWithError(125, `no container with name or ID "foobar" found: no such container`))
})

It("podman attach to non-running container", func() {
Expand All @@ -25,7 +24,7 @@ var _ = Describe("Podman attach", func() {

results := podmanTest.Podman([]string{"attach", "test1"})
results.WaitWithDefaultTimeout()
Expect(results).Should(Exit(125))
Expect(results).Should(ExitWithError(125, "you can only attach to running containers"))
})

It("podman container attach to non-running container", func() {
Expand All @@ -36,7 +35,7 @@ var _ = Describe("Podman attach", func() {

results := podmanTest.Podman([]string{"container", "attach", "test1"})
results.WaitWithDefaultTimeout()
Expect(results).Should(Exit(125))
Expect(results).Should(ExitWithError(125, "you can only attach to running containers"))
})

It("podman attach to multiple containers", func() {
Expand All @@ -50,7 +49,7 @@ var _ = Describe("Podman attach", func() {

results := podmanTest.Podman([]string{"attach", "test1", "test2"})
results.WaitWithDefaultTimeout()
Expect(results).Should(Exit(125))
Expect(results).Should(ExitWithError(125, " attach` accepts at most one argument"))
})

It("podman attach to a running container", func() {
Expand Down
3 changes: 1 addition & 2 deletions test/e2e/checkpoint_image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ var _ = Describe("Podman checkpoint", func() {
checkpointImage := "foobar-checkpoint"
session := podmanTest.Podman([]string{"container", "checkpoint", "--create-image", checkpointImage, "foobar"})
session.WaitWithDefaultTimeout()
Expect(session).To(ExitWithError())
Expect(session.ErrorToString()).To(ContainSubstring("no container with name or ID \"foobar\" found"))
Expect(session).To(ExitWithError(125, `no container with name or ID "foobar" found: no such container`))
})

It("podman checkpoint --create-image with running container", func() {
Expand Down
45 changes: 17 additions & 28 deletions test/e2e/checkpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,13 @@ var _ = Describe("Podman checkpoint", func() {
It("podman checkpoint bogus container", func() {
session := podmanTest.Podman([]string{"container", "checkpoint", "foobar"})
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(125))
Expect(session.ErrorToString()).To(ContainSubstring("no such container"))
Expect(session).Should(ExitWithError(125, "no such container"))
})

It("podman restore bogus container", func() {
session := podmanTest.Podman([]string{"container", "restore", "foobar"})
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(125))
Expect(session.ErrorToString()).To(ContainSubstring("no such container or image"))
Expect(session).Should(ExitWithError(125, "no such container or image"))
})

It("podman checkpoint a running container by id", func() {
Expand Down Expand Up @@ -228,7 +226,7 @@ var _ = Describe("Podman checkpoint", func() {
result = podmanTest.Podman([]string{"pause", cid})
result.WaitWithDefaultTimeout()

Expect(result).Should(Exit(125))
Expect(result).Should(ExitWithError(125, `"exited" is not running, can't pause: container state improper`))
Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0))
Expect(podmanTest.GetContainerStatus()).To(ContainSubstring("Exited"))

Expand All @@ -239,7 +237,7 @@ var _ = Describe("Podman checkpoint", func() {

result = podmanTest.Podman([]string{"rm", cid})
result.WaitWithDefaultTimeout()
Expect(result).Should(Exit(2))
Expect(result).Should(ExitWithError(2, " as it is running - running or paused containers cannot be removed without force: container state improper"))
Expect(podmanTest.NumberOfContainersRunning()).To(Equal(1))

result = podmanTest.Podman([]string{"rm", "-t", "1", "-f", cid})
Expand Down Expand Up @@ -354,7 +352,9 @@ var _ = Describe("Podman checkpoint", func() {
result := podmanTest.Podman([]string{"container", "checkpoint", cid})
result.WaitWithDefaultTimeout()

Expect(result).Should(Exit(125))
// FIXME: criu emits an error message, but podman never sees it:
// "CRIU checkpointing failed -52. Please check CRIU logfile /...."
Expect(result).Should(ExitWithError(125, "failed: exit status 1"))
Expect(podmanTest.NumberOfContainersRunning()).To(Equal(1))
Expect(podmanTest.GetContainerStatus()).To(ContainSubstring("Up"))

Expand All @@ -370,7 +370,8 @@ var _ = Describe("Podman checkpoint", func() {
result = podmanTest.Podman([]string{"container", "restore", cid})
result.WaitWithDefaultTimeout()

Expect(result).Should(Exit(125))
// FIXME: CRIU failure message not seen by podman (same as above)
Expect(result).Should(ExitWithError(125))
Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0))
Expect(podmanTest.GetContainerStatus()).To(ContainSubstring("Exited"))

Expand Down Expand Up @@ -618,8 +619,7 @@ var _ = Describe("Podman checkpoint", func() {
result = podmanTest.Podman([]string{"container", "checkpoint", cid, "-e", fileName, "-c", "non-existing"})
result.WaitWithDefaultTimeout()

Expect(result).Should(Exit(125))
Expect(result.ErrorToString()).To(ContainSubstring("not supported"))
Expect(result).Should(ExitWithError(125, `selected compression algorithm ("non-existing") not supported. Please select one from`))
Expect(podmanTest.NumberOfContainersRunning()).To(Equal(1))
Expect(podmanTest.NumberOfContainers()).To(Equal(1))

Expand Down Expand Up @@ -731,8 +731,7 @@ var _ = Describe("Podman checkpoint", func() {
// Verify the changes to the container's root file-system
result = podmanTest.Podman([]string{"exec", cid, "cat", "/test.output"})
result.WaitWithDefaultTimeout()
Expect(result).Should(Exit(1))
Expect(result.ErrorToString()).To(ContainSubstring("cat: can't open '/test.output': No such file or directory"))
Expect(result).Should(ExitWithError(1, "cat: can't open '/test.output': No such file or directory"))

// Remove exported checkpoint
os.Remove(fileName)
Expand Down Expand Up @@ -773,8 +772,7 @@ var _ = Describe("Podman checkpoint", func() {
// Verify the changes to the container's root file-system
result = podmanTest.Podman([]string{"exec", cid, "cat", "/test.output"})
result.WaitWithDefaultTimeout()
Expect(result).Should(Exit(1))
Expect(result.ErrorToString()).To(ContainSubstring("cat: can't open '/test.output': No such file or directory"))
Expect(result).Should(ExitWithError(1, "cat: can't open '/test.output': No such file or directory"))

// Remove exported checkpoint
os.Remove(fileName)
Expand Down Expand Up @@ -833,8 +831,7 @@ var _ = Describe("Podman checkpoint", func() {
// Checkpoint the container - this should fail as it was started with --rm
result := podmanTest.Podman([]string{"container", "checkpoint", cid})
result.WaitWithDefaultTimeout()
Expect(result).To(ExitWithError())
Expect(result.ErrorToString()).To(ContainSubstring("cannot checkpoint containers that have been started with '--rm'"))
Expect(result).To(ExitWithError(125, "cannot checkpoint containers that have been started with '--rm'"))

// Checkpointing with --export should still work
fileName := filepath.Join(podmanTest.TempDir, "/checkpoint-"+cid+".tar.gz")
Expand Down Expand Up @@ -922,10 +919,7 @@ var _ = Describe("Podman checkpoint", func() {
// Restore container should fail because named volume still exists
result = podmanTest.Podman([]string{"container", "restore", "-i", checkpointFileName})
result.WaitWithDefaultTimeout()
Expect(result).To(ExitWithError())
Expect(result.ErrorToString()).To(ContainSubstring(
"volume with name my-test-vol already exists. Use --ignore-volumes to not restore content of volumes",
))
Expect(result).To(ExitWithError(125, "volume with name my-test-vol already exists. Use --ignore-volumes to not restore content of volumes"))

// Remove named volume
session = podmanTest.Podman([]string{"volume", "rm", "my-test-vol"})
Expand Down Expand Up @@ -1205,8 +1199,7 @@ var _ = Describe("Podman checkpoint", func() {
fileName,
})
result.WaitWithDefaultTimeout()
Expect(result).To(Exit(125))
Expect(result.ErrorToString()).To(ContainSubstring("does not share the"))
Expect(result).To(ExitWithError(125, "does not share the "))

// Remove the pod and create a new pod
result = podmanTest.Podman([]string{
Expand Down Expand Up @@ -1458,8 +1451,7 @@ var _ = Describe("Podman checkpoint", func() {
// Checkpoint is expected to fail without --file-locks
result := podmanTest.Podman([]string{"container", "checkpoint", "test_name"})
result.WaitWithDefaultTimeout()
Expect(result).Should(Exit(125))
Expect(result.ErrorToString()).To(ContainSubstring("failed: exit status 1"))
Expect(result).Should(ExitWithError(125, "failed: exit status 1"))
Expect(podmanTest.NumberOfContainersRunning()).To(Equal(1))

// Checkpoint is expected to succeed with --file-locks
Expand Down Expand Up @@ -1703,10 +1695,7 @@ var _ = Describe("Podman checkpoint", func() {
})
result.WaitWithDefaultTimeout()

Expect(result).Should(Exit(125))
Expect(result.ErrorToString()).To(
ContainSubstring("and cannot be restored with runtime"),
)
Expect(result).Should(ExitWithError(125, "and cannot be restored with runtime"))

result = podmanTest.Podman([]string{
"--runtime",
Expand Down
8 changes: 3 additions & 5 deletions test/e2e/commit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ var _ = Describe("Podman commit", func() {

session := podmanTest.Podman([]string{"commit", "test1", "--change", "BOGUS=foo", "foobar.com/test1-image:latest"})
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(125))
Expect(session.ErrorToString()).To(HaveSuffix(`applying changes: processing change "BOGUS foo": did not understand change instruction "BOGUS foo"`))
Expect(session).Should(ExitWithError(125, `applying changes: processing change "BOGUS foo": did not understand change instruction "BOGUS foo"`))

session = podmanTest.Podman([]string{"commit", "test1", "foobar.com/test1-image:latest"})
session.WaitWithDefaultTimeout()
Expand All @@ -47,8 +46,7 @@ var _ = Describe("Podman commit", func() {
// commit second time with --quiet, should not write to stderr
session = podmanTest.Podman([]string{"commit", "--quiet", "bogus", "foobar.com/test1-image:latest"})
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(125))
Expect(session.ErrorToString()).To(Equal("Error: no container with name or ID \"bogus\" found: no such container"))
Expect(session).Should(ExitWithError(125, `no container with name or ID "bogus" found: no such container`))
})

It("podman commit single letter container", func() {
Expand Down Expand Up @@ -346,7 +344,7 @@ var _ = Describe("Podman commit", func() {

session = podmanTest.Podman([]string{"run", "foobar.com/test1-image:latest", "cat", "/run/secrets/mysecret"})
session.WaitWithDefaultTimeout()
Expect(session).To(ExitWithError())
Expect(session).To(ExitWithError(1, "can't open '/run/secrets/mysecret': No such file or directory"))

})

Expand Down
18 changes: 6 additions & 12 deletions test/e2e/containers_conf_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -441,8 +441,7 @@ var _ = Describe("Verify podman containers.conf usage", func() {
It("--add-host and no-hosts=true fails", func() {
session := podmanTest.Podman([]string{"run", "-dt", "--add-host", "test1:127.0.0.1", ALPINE, "top"})
session.WaitWithDefaultTimeout()
Expect(session).To(ExitWithError())
Expect(session.ErrorToString()).To(ContainSubstring("--no-hosts and --add-host cannot be set together"))
Expect(session).To(ExitWithError(125, "--no-hosts and --add-host cannot be set together"))

session = podmanTest.Podman([]string{"run", "-dt", "--add-host", "test1:127.0.0.1", "--no-hosts=false", ALPINE, "top"})
session.WaitWithDefaultTimeout()
Expand Down Expand Up @@ -533,8 +532,7 @@ var _ = Describe("Verify podman containers.conf usage", func() {
if !IsRemote() {
session = podmanTest.Podman([]string{"info", "--format", "{{.Store.ImageCopyTmpDir}}"})
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(125))
Expect(session.ErrorToString()).To(ContainSubstring("invalid image_copy_tmp_dir value \"storage1\" (relative paths are not accepted)"))
Expect(session).Should(ExitWithError(125, `invalid image_copy_tmp_dir value "storage1" (relative paths are not accepted)`))

os.Setenv("TMPDIR", "/hoge")
session = podmanTest.Podman([]string{"info", "--format", "{{.Store.ImageCopyTmpDir}}"})
Expand Down Expand Up @@ -573,18 +571,15 @@ var _ = Describe("Verify podman containers.conf usage", func() {

result := podmanTest.Podman([]string{"pod", "create", "--infra-image", infra2})
result.WaitWithDefaultTimeout()
Expect(result).Should(Exit(125))
Expect(result.ErrorToString()).To(ContainSubstring(error2String))
Expect(result).Should(ExitWithError(125, error2String))

result = podmanTest.Podman([]string{"pod", "create"})
result.WaitWithDefaultTimeout()
Expect(result).Should(Exit(125))
Expect(result.ErrorToString()).To(ContainSubstring(errorString))
Expect(result).Should(ExitWithError(125, errorString))

result = podmanTest.Podman([]string{"create", "--pod", "new:pod1", ALPINE})
result.WaitWithDefaultTimeout()
Expect(result).Should(Exit(125))
Expect(result.ErrorToString()).To(ContainSubstring(errorString))
Expect(result).Should(ExitWithError(125, errorString))
})

It("set .engine.remote=true", func() {
Expand Down Expand Up @@ -679,8 +674,7 @@ var _ = Describe("Verify podman containers.conf usage", func() {
podman.WaitWithDefaultTimeout()

if mode == "invalid" {
Expect(podman).Should(Exit(125))
Expect(podman.ErrorToString()).Should(ContainSubstring("invalid default_rootless_network_cmd option \"invalid\""))
Expect(podman).Should(ExitWithError(125, `invalid default_rootless_network_cmd option "invalid"`))
continue
}
Expect(podman).Should(ExitCleanly())
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/cp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ var _ = Describe("Podman cp", func() {
// Cannot copy to a nonexistent path (note the trailing "/").
session = podmanTest.Podman([]string{"cp", srcFile.Name(), name + ":foo/"})
session.WaitWithDefaultTimeout()
Expect(session).To(ExitWithError())
Expect(session).To(ExitWithError(125, `"foo/" could not be found on container`))

// The file will now be created (and written to).
session = podmanTest.Podman([]string{"cp", srcFile.Name(), name + ":foo"})
Expand Down
15 changes: 5 additions & 10 deletions test/e2e/create_staticip_test.go
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
package integration

import (
"fmt"
"time"

. "github.com/containers/podman/v5/test/utils"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
. "github.com/onsi/gomega/gexec"
)

var _ = Describe("Podman create with --ip flag", func() {

It("Podman create --ip with garbage address", func() {
result := podmanTest.Podman([]string{"create", "--name", "test", "--ip", "114232346", ALPINE, "ls"})
result.WaitWithDefaultTimeout()
Expect(result).To(ExitWithError())
Expect(result).To(ExitWithError(125, `"114232346" is not an ip address`))
})

It("Podman create --ip with non-allocatable IP", func() {
Expand All @@ -25,7 +25,7 @@ var _ = Describe("Podman create with --ip flag", func() {

result = podmanTest.Podman([]string{"start", "test"})
result.WaitWithDefaultTimeout()
Expect(result).To(ExitWithError())
Expect(result).To(ExitWithError(125, "requested static ip 203.0.113.124 not in any subnet on network podman"))
})

It("Podman create with specified static IP has correct IP", func() {
Expand All @@ -34,7 +34,7 @@ var _ = Describe("Podman create with --ip flag", func() {
result.WaitWithDefaultTimeout()
// Rootless static ip assignment without network should error
if isRootless() {
Expect(result).Should(Exit(125))
Expect(result).Should(ExitWithError(125, "invalid config provided: networks and static ip/mac address can only be used with Bridge mode networking"))
} else {
Expect(result).Should(ExitCleanly())

Expand Down Expand Up @@ -74,11 +74,6 @@ var _ = Describe("Podman create with --ip flag", func() {
// test1 container is running with the given IP.
result = podmanTest.Podman([]string{"start", "-a", "test2"})
result.WaitWithDefaultTimeout()
Expect(result).To(ExitWithError())
if podmanTest.NetworkBackend == CNI {
Expect(result.ErrorToString()).To(ContainSubstring("requested IP address %s is not available", ip))
} else if podmanTest.NetworkBackend == Netavark {
Expect(result.ErrorToString()).To(ContainSubstring("requested ip address %s is already allocated", ip))
}
Expect(result).To(ExitWithError(125, fmt.Sprintf("requested ip address %s is already allocated", ip)))
})
})
Loading

0 comments on commit 2d91598

Please sign in to comment.