Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make the check strict to use ExpectNoError() #78740

Merged
merged 1 commit into from
Jul 30, 2019
Merged

Make the check strict to use ExpectNoError() #78740

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion hack/verify-test-code.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ mapfile -t all_e2e_files < <(find test/e2e -name '*.go' | grep -v 'test/e2e/fram
errors_expect_no_error=()
for file in "${all_e2e_files[@]}"
do
if grep "Expect(.*)\.NotTo(.*HaveOccurred()" "${file}" > /dev/null
if grep -E "Expect\(.*\)\.(NotTo|ToNot)\(.*HaveOccurred\(\)" "${file}" > /dev/null
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

verify-test-code.sh adds to detect Expect(err).ToNot(HaveOccurred()).
But, it says about only NotTo as the below.

The above files need to use framework.ExpectNoError(err) instead of
Expect(err).NotTo(HaveOccurred()) or gomega.Expect(err).NotTo(gomega.HaveOccurred())

Why don't you add ToNot to error comment? Is it redundant?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a nice point.
As we see, ToNot is not major and I feel it could be enough to contain NotTo only for easy explanation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I understood that.
/lgtm

then
errors_expect_no_error+=( "${file}" )
fi
Expand Down
5 changes: 3 additions & 2 deletions test/e2e/common/pods.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ var _ = framework.KubeDescribe("Pods", func() {
framework.ExpectNoError(err, "failed to delete pod")

ginkgo.By("verifying the kubelet observed the termination notice")
gomega.Expect(wait.Poll(time.Second*5, time.Second*30, func() (bool, error) {
err = wait.Poll(time.Second*5, time.Second*30, func() (bool, error) {
podList, err := framework.GetKubeletPods(f.ClientSet, pod.Spec.NodeName)
if err != nil {
e2elog.Logf("Unable to retrieve kubelet pods for node %v: %v", pod.Spec.NodeName, err)
Expand All @@ -304,7 +304,8 @@ var _ = framework.KubeDescribe("Pods", func() {
}
e2elog.Logf("no pod exists with the name we were looking for, assuming the termination request was observed and completed")
return true, nil
})).NotTo(gomega.HaveOccurred(), "kubelet never observed the termination notice")
})
framework.ExpectNoError(err, "kubelet never observed the termination notice")

ginkgo.By("verifying pod deletion was observed")
deleted := false
Expand Down
5 changes: 3 additions & 2 deletions test/e2e/windows/density.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,8 +270,9 @@ func deletePodsSync(f *framework.Framework, pods []*v1.Pod) {
err := f.PodClient().Delete(pod.ObjectMeta.Name, metav1.NewDeleteOptions(30))
framework.ExpectNoError(err)

gomega.Expect(e2epod.WaitForPodToDisappear(f.ClientSet, f.Namespace.Name, pod.ObjectMeta.Name, labels.Everything(),
30*time.Second, 10*time.Minute)).NotTo(gomega.HaveOccurred())
err = e2epod.WaitForPodToDisappear(f.ClientSet, f.Namespace.Name, pod.ObjectMeta.Name, labels.Everything(),
30*time.Second, 10*time.Minute)
framework.ExpectNoError(err)
}(pod)
}
wg.Wait()
Expand Down