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.

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

GoMega Assertions in Eventually are Not Being Raised Without Calling Should(Succeed()) #2495

Closed
lnhanks opened this issue Aug 14, 2023 · 1 comment

Comments

@lnhanks
Copy link
Contributor

lnhanks commented Aug 14, 2023

What happened:
When writing a new test suite, using erroneous values in existing Eventually() Assertions would still have the test suite pass. It didn't seem like the expect statements are being caught correctly. Upon looking at GoMega documentation, .Should((Succeed)) needs to be added at the end of Eventually to actually get these assertions to be caught.

What you expected to happen:
I expected the test suite to fail once I put in erroneous values, but it passed. I haven't looked into this globally, but I've seen it in multiple spots in the integration test suite ipamd.

How to reproduce it (as minimally and precisely as possible):

I am running ENI/IP Leak Test and changing the oldIP and oldENI to erroneous values ie. 100000 and the test still passes. When I follow the documentation and add .Should(Succeed()) the test will fail.

Code Change to impossible values on eni_ip_leak_test.go line 53:

Eventually(func(g Gomega) {
    ip, eni := getCountOfIPandENIOnPrimaryInstance()
    g.Expect(ip).To(Equal(1000000))
    g.Expect(eni).To(Equal(100000))
}).WithTimeout(6 * time.Minute).WithPolling(10 * time.Second)

Comment out line 29

Command:

ginkgo -v -r --focus="ENI/IP Leak Test" ./test/integration/ipamd --timeout 30m --fail-on-pending -- --cluster-kubeconfig="/home/lnhanks/.kube/config" --cluster-name="lindsayvpccnimetrics" --aws-region="us-west-2" --aws-vpc-id="vpc-01d1f63bf6a0720dd" --ng-name-label-key="[kubernetes.io/os](http://kubernetes.io/os)" --ng-name-label-val="linux"

Test will pass

Add .Should(Succeed()) after the Eventually and run it again, test will fail

Change values back to original values, add .Should(Succeed()), test will pass again.

Anything else we need to know?:
Documentation: https://onsi.github.io/gomega/
Search "Category 3: Making assertions in the function passed into Eventually"

Environment:

  • Kubernetes version v1.27.1
  • CNI Version
  • OS (e.g: cat /etc/os-release): Amazon Linux 2
  • Kernel (e.g. uname -a): Linux dev-dsk-lnhanks-2a-271b2fcf.us-west-2.amazon.com 5.10.184-153.749.amzn2int.x86_64 Initial commit of amazon-vpc-cni-k8s #1 SMP Wed Jul 12 19:43:54 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants