Merge pull request #10997 from edsantiago/helpfuler_expects

e2e tests: prevent 'Expect(ExitCode())' pattern
containers · Jul 21, 2021 · 7826343 · 7826343
2 parents 1cb61f9 + 5952270
commit 7826343
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 16 deletions.
diff --git a/Makefile b/Makefile
@@ -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
 ###

diff --git a/test/README.md b/test/README.md
@@ -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())
 	})
 ```
 

diff --git a/test/e2e/common_test.go b/test/e2e/common_test.go
@@ -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) {

diff --git a/test/e2e/pod_create_test.go b/test/e2e/pod_create_test.go
@@ -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"))
 

diff --git a/test/e2e/run_exit_test.go b/test/e2e/run_exit_test.go
@@ -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() {