HaveValue matcher fails with nil value even when negated

[JoelSpeed]

I was trying to use the HaveValue matcher to check a *string field is not a particular value. However, of course, as this is a pointer, it could be nil. Logically, I would have said that Not(HaveValue("whatever") should pass if the value is a nil, as it doesn't have a value, but it fails right now.

For a more concrete example, I would expect the first of these to not fail, where I would expect the second to fail, but they both fail with the same error right now.

// I expect this to pass, but it fails
It("should not fail when the value is nil with not", func() {
	var foo *string
	Expect(foo).To(Not(HaveValue(Equal("bar"))))
})

// I expect this to fail
It("should fail when the value is nil", func() {
	var foo *string
	Expect(foo).To(HaveValue(Equal("bar")))
})

Am I misunderstanding?

The workaround I have for now is

It("should not fail when the value is nil with not", func() {
	var foo *string
	Expect(foo).To(SatisfyAny(
		BeNil(),
		Not(HaveValue(Equal("bar"))),
	))
})

[onsi]

/cc @thediveo for his thoughts

I believe the intention is to explicitly not allow comparisons when the pointer is nil which is why a nil value always returns an error.

One way of reading it is "I'm expecting foo to have a value, but for that value to not be bar" - the matcher is failing because foo doesn't actually have a value.

[thediveo]

Yes, this design is on purpose to avoid treating nil in an unexpected manner (with different test writers having different preferences). The above "workaround" isn't so much a workaround or kluge, but instead a concise expression of the expectation/assertion.

[JoelSpeed]

I can see the motivation for the matcher to fail when the value is nil, that makes sense, I'm still struggling to grasp though the motivation that Not(HaveValue()) fails as well. I expected the negation to negate the effect but it doesn't. It seems odd to me that I have to change the assertion when negating it to get an equivalent result. Are there other examples within Gomega that have the same behaviour?

I guess by your design I'm holding it wrong and I need to have the BeNil() thing, but is it really an error to pass a nil into HaveValue? It makes using this matcher in the negative sense much more clunky IMO.

At a minimum I think we should make mention of this in the docs as I expect others will be surprised by the negation behaviour of HaveValue as well

[thediveo]

No you don't hold it wrong. It's a slightly opinionated dresign to err on the safe side, and this meaning in unit tests to not consider nil in this matcher to be an ordinary value, negation or not. Even if this rule sounds odd, but you cannot negate errors, only matches. In cases where I myself find me needing this in many places I put a custom matcher in m of a simple combined set of matchers.

[thediveo]

Does this issue need action and which one? Would improved documentation help and what should we improve? Or would it be in the "works as designed"?

[JoelSpeed]

I would suggest adding a note to the docs of the matcher, something along the lines of

Note: It is considered an error to pass a nil value to this matcher. A nil value will cause the matcher to fail, even when negated.

Wdyt?

[thediveo]

Thank you very much for the issue report!