kubernetes: Use framework.ExpectNoError() for e2e tests

I want to take the files below.

test/e2e/apimachinery
test/e2e/network
test/e2e/node
test/e2e/instrumentation
test/e2e/kubectl
test/e2e/scalability
test/e2e/scheduling
test/e2e/servicecatalog
test/e2e/ui
test/e2e/upgrades/apps
test/e2e/upgrades
test/e2e/upgrades/storage
test/e2e/windows

And I use the following cmd to do the refactor, which may be helpful.

ag -l "gomega\.Expect\(err\)\.NotTo\(gomega.HaveOccurred\(\)\)" test/e2e/network/**/*.go test/e2e/node/**/*.go test/e2e/instrumentation/**/*.go ... | xargs sed -i '' -e 's/gomega\.Expect(err)\.NotTo(gomega.HaveOccurred())/framework\.ExpectNoError(err)/g'

I think that we can replace gomega.Expect(err).NotTo(gomega.HaveOccurred()), too. Files in the following directories have it. Could you add these directories to list?

• test/e2e/apimachinery
• test/e2e/apps
• test/e2e/instrumentation
• test/e2e/kubectl
• test/e2e/servicecatalog
• test/e2e/ui
• test/e2e/upgrades/apps
• test/e2e/upgrades
• test/e2e/upgrades/storage
• test/e2e/windows

@k-toyoda-pi

I believe that NotTo is the same behavior as ToNot. So, can we replace Expect(err).ToNot(HaveOccurred()) to ExpectNoError?

This is a really nice point, I also think they are the same. However, it is better to concentrate on ‘NotTo’ case only at this time because that is majority and many people are still adding it. We need to put the CI script for blocking these additions with https://github.com/kubernetes/kubernetes/pull/77612 After that, we will be able to take care of ‘ToNot’ case with another issue.

It seems that some files in e2e/common contain “Expect(err).NotTo(HaveOccurred()”. Is someone working to that? Can I take it?

• test/e2e/common/configmap_volume.go
• test/e2e/common/downwardapi_volume.go
• test/e2e/common/expansion.go
• test/e2e/common/init_container.go
• test/e2e/common/node_lease.go
• test/e2e/common/privileged.go
• test/e2e/common/projected_configmap.go
• test/e2e/common/projected_downwardapi.go
• test/e2e/common/projected_secret.go
• test/e2e/common/secrets_volume.go
• test/e2e/common/volumes.go

Oops, it might cause conflict between #77852 and this …

I’ll work on this after the above PRs get merged.

Following items are target of this issue too. This is a case that is consisted of multiple lines.

test/e2e/windows/density.go:273:				30*time.Second, 10*time.Minute)).NotTo(gomega.HaveOccurred())
test/e2e/scheduling/limit_range.go:151:		})).NotTo(HaveOccurred())
test/e2e/scheduling/limit_range.go:193:		})).NotTo(HaveOccurred(), "kubelet never observed the termination notice")
test/e2e/common/pods.go:307:		})).NotTo(gomega.HaveOccurred(), "kubelet never observed the termination notice")
test/e2e/common/pods.go:808:			})).NotTo(gomega.HaveOccurred())
test/e2e/node/pods.go:162:			})).NotTo(HaveOccurred(), "kubelet never observed the termination notice")
test/e2e/node/kubelet_perf.go:79:	})).NotTo(HaveOccurred())
test/e2e/node/kubelet.go:327:				})).NotTo(HaveOccurred())
test/e2e/node/kubelet.go:333:					time.Second*30)).NotTo(HaveOccurred())
test/e2e/node/kubelet.go:349:					itArg.timeout)).NotTo(HaveOccurred())
test/e2e/apps/daemon_set.go:178:			NotTo(gomega.HaveOccurred(), "error waiting for daemon pod to not be running on nodes")
test/e2e/apps/daemon_set.go:240:			NotTo(gomega.HaveOccurred(), "error waiting for daemon pod to not be running on nodes")
test/e2e/apimachinery/etcd_failure.go:54:		})).NotTo(gomega.HaveOccurred())

These were gotten by grep -n -e "NotTo(.*HaveOccurred" and filtered in manual way. I’ve confirmed that these are target by checking each item.

Yeah, I noticed that and I’ll fix all the remaining case after the PRs above get merged 😃

Following items are target of this issue too. This is a case that is consisted of multiple lines.

test/e2e/windows/density.go:273:				30*time.Second, 10*time.Minute)).NotTo(gomega.HaveOccurred())
test/e2e/scheduling/limit_range.go:151:		})).NotTo(HaveOccurred())
test/e2e/scheduling/limit_range.go:193:		})).NotTo(HaveOccurred(), "kubelet never observed the termination notice")
test/e2e/common/pods.go:307:		})).NotTo(gomega.HaveOccurred(), "kubelet never observed the termination notice")
test/e2e/common/pods.go:808:			})).NotTo(gomega.HaveOccurred())
test/e2e/node/pods.go:162:			})).NotTo(HaveOccurred(), "kubelet never observed the termination notice")
test/e2e/node/kubelet_perf.go:79:	})).NotTo(HaveOccurred())
test/e2e/node/kubelet.go:327:				})).NotTo(HaveOccurred())
test/e2e/node/kubelet.go:333:					time.Second*30)).NotTo(HaveOccurred())
test/e2e/node/kubelet.go:349:					itArg.timeout)).NotTo(HaveOccurred())
test/e2e/apps/daemon_set.go:178:			NotTo(gomega.HaveOccurred(), "error waiting for daemon pod to not be running on nodes")
test/e2e/apps/daemon_set.go:240:			NotTo(gomega.HaveOccurred(), "error waiting for daemon pod to not be running on nodes")
test/e2e/apimachinery/etcd_failure.go:54:		})).NotTo(gomega.HaveOccurred())

These were gotten by grep -n -e "NotTo(.*HaveOccurred" and filtered in manual way. I’ve confirmed that these are target by checking each item.

@draveness

$ ag -l "gomega\.Expect\(err\)\.NotTo\(gomega.HaveOccurred\(\)\)" which you are using cannot cover the following case which contains error message: gomega.Expect(err).NotTo(gomega.HaveOccurred(), "Failed to get pod %q", pod.Name) So still we have a lot of targets at this time 😦

I can work to the following item.

• test/e2e/autoscaling

Thanks, I can work to the following item.

• test/e2e/apps

I can work to following items.

• test/e2e/lifecycle
• test/e2e/lifecycle/bootstrap