From 63e0507fd92f447370a5e572536be54f947878f2 Mon Sep 17 00:00:00 2001
From: Kenichi Omichi <ken-oomichi@wx.jp.nec.com>
Date: Wed, 29 May 2019 16:39:38 +0000
Subject: [PATCH] Check e2e test code to use ExpectError()

We can use framework.ExpectError() for checking the expected error
happens. However Expect(err).To(HaveOccurred()) can be used instead
and that makes the e2e test code unreadable.
This adds the check to use framework.ExpectError() for readable code.
---
 hack/verify-test-code.sh            | 26 +++++++++++++++++++++++++-
 test/e2e/node/mount_propagation.go  |  2 +-
 test/e2e/storage/csi_mock_volume.go |  4 ++--
 test/e2e/storage/utils/utils.go     |  2 +-
 test/e2e/storage/volume_expand.go   |  2 +-
 5 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/hack/verify-test-code.sh b/hack/verify-test-code.sh
index 8894e16d2ca..cf776393892 100755
--- a/hack/verify-test-code.sh
+++ b/hack/verify-test-code.sh
@@ -20,7 +20,8 @@ set -o pipefail
 KUBE_ROOT=$(dirname "${BASH_SOURCE[0]}")/..
 cd "${KUBE_ROOT}"
 
-mapfile -t all_e2e_files < <(find test/e2e -name '*.go')
+# NOTE: This checks e2e test code without the e2e framework which contains Expect().To(HaveOccurred())
+mapfile -t all_e2e_files < <(find test/e2e -name '*.go' | grep -v 'test/e2e/framework/')
 errors_expect_no_error=()
 for file in "${all_e2e_files[@]}"
 do
@@ -30,6 +31,15 @@ do
     fi
 done
 
+errors_expect_error=()
+for file in "${all_e2e_files[@]}"
+do
+    if grep "Expect(.*)\.To(.*HaveOccurred()" "${file}" > /dev/null
+    then
+        errors_expect_error+=( "${file}" )
+    fi
+done
+
 if [ ${#errors_expect_no_error[@]} -ne 0 ]; then
   {
     echo "Errors:"
@@ -44,4 +54,18 @@ if [ ${#errors_expect_no_error[@]} -ne 0 ]; then
   exit 1
 fi
 
+if [ ${#errors_expect_error[@]} -ne 0 ]; then
+  {
+    echo "Errors:"
+    for err in "${errors_expect_error[@]}"; do
+      echo "$err"
+    done
+    echo
+    echo 'The above files need to use framework.ExpectError(err) instead of '
+    echo 'Expect(err).To(HaveOccurred()) or gomega.Expect(err).To(gomega.HaveOccurred())'
+    echo
+  } >&2
+  exit 1
+fi
+
 echo 'Congratulations!  All e2e test source files are valid.'
diff --git a/test/e2e/node/mount_propagation.go b/test/e2e/node/mount_propagation.go
index 48427871d73..5dcc1fe77d8 100644
--- a/test/e2e/node/mount_propagation.go
+++ b/test/e2e/node/mount_propagation.go
@@ -175,7 +175,7 @@ var _ = SIGDescribe("Mount propagation", func() {
 					gomega.Expect(stdout).To(gomega.Equal(mountName), msg)
 				} else {
 					// We *expect* cat to return error here
-					gomega.Expect(err).To(gomega.HaveOccurred(), msg)
+					framework.ExpectError(err, msg)
 				}
 			}
 		}
diff --git a/test/e2e/storage/csi_mock_volume.go b/test/e2e/storage/csi_mock_volume.go
index 0c0fea51a3e..47229a796ee 100644
--- a/test/e2e/storage/csi_mock_volume.go
+++ b/test/e2e/storage/csi_mock_volume.go
@@ -279,7 +279,7 @@ var _ = utils.SIGDescribe("CSI mock volume", func() {
 					}
 				}
 				if test.disableAttach {
-					gomega.Expect(err).To(gomega.HaveOccurred(), "Unexpected VolumeAttachment found")
+					framework.ExpectError(err, "Unexpected VolumeAttachment found")
 				}
 			})
 
@@ -449,7 +449,7 @@ var _ = utils.SIGDescribe("CSI mock volume", func() {
 				}
 				if test.expectFailure {
 					err = waitForResizingCondition(pvc, m.cs, csiResizingConditionWait)
-					gomega.Expect(err).To(gomega.HaveOccurred(), "unexpected resizing condition on PVC")
+					framework.ExpectError(err, "unexpected resizing condition on PVC")
 					return
 				}
 
diff --git a/test/e2e/storage/utils/utils.go b/test/e2e/storage/utils/utils.go
index 6d40cf976f1..7332224e0f4 100644
--- a/test/e2e/storage/utils/utils.go
+++ b/test/e2e/storage/utils/utils.go
@@ -91,7 +91,7 @@ func VerifyExecInPodFail(pod *v1.Pod, bashExec string, exitCode int) {
 				bashExec, exitCode, err)
 		}
 	}
-	gomega.Expect(err).To(gomega.HaveOccurred(), "%q should fail with exit code %d, but exit without error", bashExec, exitCode)
+	framework.ExpectError(err, "%q should fail with exit code %d, but exit without error", bashExec, exitCode)
 }
 
 // KubeletCommand performs `start`, `restart`, or `stop` on the kubelet running on the node of the target pod and waits
diff --git a/test/e2e/storage/volume_expand.go b/test/e2e/storage/volume_expand.go
index 6042ce2c60d..ba5eeca2e4f 100644
--- a/test/e2e/storage/volume_expand.go
+++ b/test/e2e/storage/volume_expand.go
@@ -101,7 +101,7 @@ var _ = utils.SIGDescribe("Volume expand", func() {
 		ginkgo.By("Expanding non-expandable pvc")
 		newSize := resource.MustParse("6Gi")
 		pvc, err = expandPVCSize(pvc, newSize, c)
-		gomega.Expect(err).To(gomega.HaveOccurred(), "While updating non-expandable PVC")
+		framework.ExpectError(err, "While updating non-expandable PVC")
 	})
 
 	ginkgo.It("Verify if editing PVC allows resize", func() {