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

detect and report invalid field names #668

Closed
pohly opened this issue May 31, 2023 · 2 comments
Closed

detect and report invalid field names #668

pohly opened this issue May 31, 2023 · 2 comments

Comments

@pohly
Copy link

pohly commented May 31, 2023

I was experimenting with what happens when I use HaveField with a field name that doesn't exist:

			gomega.Eventually(getNewCalls).WithTimeout(registrationTimeout).Should(gomega.HaveExactElements(
				gomega.And(
					gomega.HaveField("FullMethod", "/pluginregistration.Registration/GetInfo"),
					gomega.HaveField("Err", gomega.BeNil()),
				),
				gomega.And(
					gomega.HaveField("FullMethod", "/pluginregistration.Registration/NotifyRegistrationStatus"),
					gomega.HaveField("Err2", gomega.BeNil()),
				),
			), "kubelet plugin should be registered via two calls, GetInfo and NotifyRegistrationStatus")

getNewCalls returns a slice of

type GRPCCall struct {
	FullMethod string
	Request interface{}
	Response interface{}
	Err error
}

I expected to get some failure message pointing out that there is no Err2 field (Err is correct).

Instead I got:

 [PANICKED] Test Panicked
  In [It] at: vendor/github.com/onsi/gomega/internal/async_assertion.go:370 @ 05/31/23 15:00:16.881

  runtime error: invalid memory address or nil pointer dereference

  Full Stack Trace
    k8s.io/kubernetes/vendor/github.com/onsi/gomega/internal.(*AsyncAssertion).pollMatcher.func1()
    	vendor/github.com/onsi/gomega/internal/async_assertion.go:370 +0x98
    panic({0x4986fa0, 0x86e82a0})
    	/nvme/gopath/src/k8s.io/kubernetes/_output/local/.gimme/versions/go1.20.4.linux.amd64/src/runtime/panic.go:884 +0x213
    k8s.io/kubernetes/vendor/github.com/onsi/gomega/matchers.(*HaveFieldMatcher).FailureMessage(0xc0003e8940, {0xc00052f090?, 0xc001b5aad0?})
    	vendor/github.com/onsi/gomega/matchers/have_field.go:89 +0x7c
    k8s.io/kubernetes/vendor/github.com/onsi/gomega/matchers.(*AndMatcher).FailureMessage(0xc001b5aab0?, {0x4df10e0?, 0xc000afa140?})
    	vendor/github.com/onsi/gomega/matchers/and.go:30 +0x2c
    k8s.io/kubernetes/vendor/github.com/onsi/gomega/matchers.(*HaveExactElementsMatcher).Match(0xc0003e8980, {0x4582de0, 0xc000908558})
    	vendor/github.com/onsi/gomega/matchers/have_exact_elements.go:50 +0x1e2
    k8s.io/kubernetes/vendor/github.com/onsi/gomega/internal.(*AsyncAssertion).pollMatcher(0xc001ad3dd0?, {0x5bf8800?, 0xc0003e8980?}, {0x4582de0?, 0xc000908558?})
    	vendor/github.com/onsi/gomega/internal/async_assertion.go:375 +0x82
    k8s.io/kubernetes/vendor/github.com/onsi/gomega/internal.(*AsyncAssertion).match(0xc0018ce4d0, {0x5bf8800?, 0xc0003e8980}, 0x1, {0xc0016de420, 0x1, 0x1})
    	vendor/github.com/onsi/gomega/internal/async_assertion.go:550 +0xb15
    k8s.io/kubernetes/vendor/github.com/onsi/gomega/internal.(*AsyncAssertion).Should(0xc0018ce4d0, {0x5bf8800, 0xc0003e8980}, {0xc0016de420, 0x1, 0x1})
    	vendor/github.com/onsi/gomega/internal/async_assertion.go:145 +0x8d
    k8s.io/kubernetes/test/e2e_node.glob..func15.1.2({0x0?, 0x0?})
    	test/e2e_node/dra_test.go:74 +0x4a3
@onsi
Copy link
Owner

onsi commented May 31, 2023

ah good catch. looks like an issue in HaveExactElements - it's calling FailureMessage on HaveField when it should instead be reporting the error returned by HaveField.

https://github.com/onsi/gomega/blob/99a29d5128d6fd87bad628e94d1cf92e82dc2b32/matchers/have_exact_elements.go#LL50C1-L50C1

i'll need to fix it...

@onsi onsi closed this as completed in 096f392 Jun 7, 2023
@onsi onsi reopened this Jun 7, 2023
@onsi
Copy link
Owner

onsi commented Jun 7, 2023

I've fixed HaveExactElement and released 1.27.8- this should be working correctly now. Please reopen the issue if you're still having trouble with it.

@onsi onsi closed this as completed Jun 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants