e2e tests: prevent 'Expect(ExitCode())' pattern

Followup to #10932: add a validation check to prevent introduction
of new 'Expect(foo.ExitCode()).To(...)' patterns. If such use is
absolutely necessary -- there is one such instance in the code
already -- require that the assertion include a description.

Also: clean up instances that were introduced since the merging
of #10932.

Also: fix one remaining instance in run_exit_test.go: it had
a FIXME comment mentioning a race condition, but unfortunately
there was no issue or bug ID, hence no way to know if the race
is fixed or not. We will assume it is.

Signed-off-by: Ed Santiago <santiago@redhat.com>
This commit is contained in:
Ed Santiago
2021-07-20 10:42:57 -06:00
parent 389c9b8dca
commit 5952270950
5 changed files with 22 additions and 16 deletions

View File

@ -261,7 +261,7 @@ codespell:
codespell -S bin,vendor,.git,go.sum,changelog.txt,.cirrus.yml,"RELEASE_NOTES.md,*.xz,*.gz,*.tar,*.tgz,bin2img,*ico,*.png,*.1,*.5,copyimg,*.orig,apidoc.go" -L uint,iff,od,seeked,splitted,marge,ERRO,hist,ether -w
.PHONY: validate
validate: gofmt lint .gitvalidation validate.completions man-page-check swagger-check tests-included
validate: gofmt lint .gitvalidation validate.completions man-page-check swagger-check tests-included tests-expect-exit
.PHONY: build-all-new-commits
build-all-new-commits:
@ -605,6 +605,16 @@ test-binaries: test/checkseccomp/checkseccomp test/goecho/goecho install.cataton
tests-included:
contrib/cirrus/pr-should-include-tests
.PHONY: tests-expect-exit
tests-expect-exit:
@if egrep 'Expect.*ExitCode' test/e2e/*.go | egrep -v ', ".*"\)'; then \
echo "^^^ Unhelpful use of Expect(ExitCode())"; \
echo " Please use '.Should(Exit(...))' pattern instead."; \
echo " If that's not possible, please add an annotation (description) to your assertion:"; \
echo " Expect(...).To(..., \"Friendly explanation of this check\")"; \
exit 1; \
fi
###
### Release/Packaging targets
###

View File

@ -84,7 +84,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.ExitCode()).To(Not(Equal(0)))
Expect(session).To(ExitWithError())
})
```

View File

@ -811,7 +811,7 @@ func generateNetworkConfig(p *PodmanTestIntegration) (string, string) {
func (p *PodmanTestIntegration) removeCNINetwork(name string) {
session := p.Podman([]string{"network", "rm", "-f", name})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(BeNumerically("<=", 1))
Expect(session.ExitCode()).To(BeNumerically("<=", 1), "Exit code must be 0 or 1")
}
func (p *PodmanSessionIntegration) jq(jqCommand string) (string, error) {

View File

@ -566,11 +566,11 @@ ENTRYPOINT ["sleep","99999"]
ns := "ns:/proc/self/ns/"
podCreate := podmanTest.Podman([]string{"pod", "create", "--pid", ns, "--name", podName, "--share", "pid"})
podCreate.WaitWithDefaultTimeout()
Expect(podCreate.ExitCode()).To(Equal(0))
Expect(podCreate).Should(Exit(0))
podInspect := podmanTest.Podman([]string{"pod", "inspect", podName})
podInspect.WaitWithDefaultTimeout()
Expect(podInspect.ExitCode()).To(Equal(0))
Expect(podInspect).Should(Exit(0))
podJSON := podInspect.InspectPodToJSON()
Expect(podJSON.InfraConfig.PidNS).To(Equal("path"))
@ -579,11 +579,11 @@ ENTRYPOINT ["sleep","99999"]
podCreate = podmanTest.Podman([]string{"pod", "create", "--pid", ns, "--name", podName, "--share", "pid"})
podCreate.WaitWithDefaultTimeout()
Expect(podCreate.ExitCode()).To(Equal(0))
Expect(podCreate).Should(Exit(0))
podInspect = podmanTest.Podman([]string{"pod", "inspect", podName})
podInspect.WaitWithDefaultTimeout()
Expect(podInspect.ExitCode()).To(Equal(0))
Expect(podInspect).Should(Exit(0))
podJSON = podInspect.InspectPodToJSON()
Expect(podJSON.InfraConfig.PidNS).To(Equal("pod"))
@ -592,11 +592,11 @@ ENTRYPOINT ["sleep","99999"]
podCreate = podmanTest.Podman([]string{"pod", "create", "--pid", ns, "--name", podName, "--share", "pid"})
podCreate.WaitWithDefaultTimeout()
Expect(podCreate.ExitCode()).To(Equal(0))
Expect(podCreate).Should(Exit(0))
podInspect = podmanTest.Podman([]string{"pod", "inspect", podName})
podInspect.WaitWithDefaultTimeout()
Expect(podInspect.ExitCode()).To(Equal(0))
Expect(podInspect).Should(Exit(0))
podJSON = podInspect.InspectPodToJSON()
Expect(podJSON.InfraConfig.PidNS).To(Equal("host"))
@ -605,11 +605,11 @@ ENTRYPOINT ["sleep","99999"]
podCreate = podmanTest.Podman([]string{"pod", "create", "--pid", ns, "--name", podName, "--share", "pid"})
podCreate.WaitWithDefaultTimeout()
Expect(podCreate.ExitCode()).To(Equal(0))
Expect(podCreate).Should(Exit(0))
podInspect = podmanTest.Podman([]string{"pod", "inspect", podName})
podInspect.WaitWithDefaultTimeout()
Expect(podInspect.ExitCode()).To(Equal(0))
Expect(podInspect).Should(Exit(0))
podJSON = podInspect.InspectPodToJSON()
Expect(podJSON.InfraConfig.PidNS).To(Equal("private"))

View File

@ -49,11 +49,7 @@ var _ = Describe("Podman run exit", func() {
It("podman run exit ExecErrorCodeNotFound", func() {
result := podmanTest.Podman([]string{"run", ALPINE, "foobar"})
result.WaitWithDefaultTimeout()
Expect(result.ExitCode()).To(Not(Equal(define.ExecErrorCodeGeneric)))
// TODO This is failing we believe because of a race condition
// Between conmon and podman closing the socket early.
// Test with the following, once the race condition is solved
// Expect(result).Should(Exit(define.ExecErrorCodeNotFound))
Expect(result).Should(Exit(define.ExecErrorCodeNotFound))
})
It("podman run exit 0", func() {