IsNot matcher does not use the wrapped matcher's mismatch description

[GoogleCodeExporter]

I think the IsNot matcher should use the describeMismatch method of the
wrapped matcher when a mismatch occurs. I think this will normally result
in a more useful description than the default version in BaseMatcher.

I have attached a unit test and fix for this. (Also included is a missing
test that the description was being built correctly, which it already is.)

Original issue reported on code.google.com by mhack...@kanayo.com on 3 Mar 2009 at 12:23

Attachments:

• hamcrest-isnot-description-patch.txt

[GoogleCodeExporter]

The trouble is that if the inner matcher matches, it might not write anything 
to the mismatchDescription, leaving 
you with an empty diagnostic.

I've added your description test, but we need to think about this. It might be 
that we need to recast 
mismatchDescription as matchedValueDescription, or something like that. I'll 
file a  new issue for that.

Original comment by smgfree...@gmail.com on 3 Mar 2009 at 9:42

• Changed state: WontFix
• Added labels: 1.2RC5, Priority-High, Type-Enhancement

[GoogleCodeExporter]

Original comment by smgfree...@gmail.com on 21 Mar 2009 at 11:51

• Added labels: 1.2RC4
• Removed labels: 1.2RC5

[GoogleCodeExporter]

I've been thinking about this...

Shouldn't the not's describeFailure() call the composed matcher's describeTo() 
methods?  

They describe what they will match, and we know that if the not failed, then 
the 
composed matcher definitely passed, and matched it's description that it set 
out to 
do.

Yes it can be unweildy when you're not'ing an and - but that's what it's "not" 
matching.

As for:
not(anyOf(matcherA, matcherB, matcherC))

if you care about the exact failure message you can

allOf(not(matcherA), not(matcherB), not(matcherC))

This sort of functionality could be built into the IsNot.not() static factory 
(detect 
AnyOf, and recompose to an AllOf clause

Original comment by stephen....@gmail.com on 26 Mar 2009 at 10:14

[GoogleCodeExporter]

I bet this is why gMock provides an interface DescribeNegationTo in addition to 
the normal DescribeTo. There 
would be a default implementation so that classes don't *have* to implement 
this. Would this make sense for 
hamcrest?

Original comment by jon.m.r...@gmail.com on 27 Mar 2009 at 3:45

[GoogleCodeExporter]

re #3:
assertThat() already does call describeTo() to display the expected result. 
It's in
the second part, reporting what it actually found instead, that the mismatch
description comes into play.

The default action is to call String.valueOf() with the actual value, which is 
fine
for some cases. But when testing some aspect of the object, such as its type, 
the
output may be fairly unhelpful. (For example, the type of the object may not 
appear
at all in the output.)

This is where the mismatch description comes in, telling you exactly what didn't
match, but as Steve points out, it breaks down under negation.

Original comment by mhack...@kanayo.com on 1 Apr 2009 at 4:44

[GoogleCodeExporter]

But this is the exact case!

With a type matcher the description would be something like:
"object with type of MyClass"

This *is* the failure message for the notMatcher.  The fact that the 
composed/notted 
matcher succeeded *is* the failure.  The failure message should be along the 
lines 
of:
"expected to not match 'object with type of MyClass'"

Extra methods like describeNegationTo could be good as an extension later, but 
for 
now, we just need a one line change.

Please fix!

Original comment by stephen....@gmail.com on 1 Apr 2009 at 10:48

[GoogleCodeExporter]

(when I say "please fix", I mean the core problem that the submitter is trying 
to 
resolve, which is almost opposite the described 'solution')

Original comment by stephen....@gmail.com on 1 Apr 2009 at 10:50

[GoogleCodeExporter]

Original comment by smgfree...@gmail.com on 21 May 2009 at 10:45

• Removed labels: 1.2RC4