is_not/not_ will not invert raises() matcher

[hersheleh]

As far as I can tell is_not should invert any matcher. Recently had a situation where I need an to invert a raises and it did not work as expected. In some ways inverting raises is an antipattern but in my case alternatives are uglier. Below is a simplified example of the issue.

Please advise if this is a bug? or is there an alternative way to address this.

Content example.py

from hamcrest import assert_that, calling, raises, not_                            
assert_that(calling(int).with_args('q'), not_(raises(ValueError)))

Output:

Traceback (most recent call last):
  File "/home/test/example1.py", line 2, in <module>
    assert_that(calling(int).with_args('q'), not_(raises(ValueError)))
  File "/home/test/_venv/lib/python3.6/site-packages/hamcrest/core/assert_that.py", line 43, in assert_that
    _assert_match(actual=arg1, matcher=arg2, reason=arg3)
  File "/home/test/_venv/lib/python3.6/site-packages/hamcrest/core/assert_that.py", line 57, in _assert_match
    raise AssertionError(description)
AssertionError: 
Expected: not Expected a callable raising <class 'ValueError'>
     but: was <hamcrest.core.core.raises.DeferredCallable object at 0x7ffbaaba97f0>

Expected: test passing

[brunns]

I'm not sure I understand the issue here - it all seems to be working as expected to me:

assert_that(calling(int).with_args('q'), raises(ValueError))  # Passes, since a ValueError is raised
assert_that(calling(int).with_args(1), not_(raises(ValueError)))  # Passes, since a ValueError is not raised
assert_that(calling(int).with_args('q'), not_(raises(ValueError))) . # Fails, since a ValueError is raised
assert_that(calling(int).with_args(1), raises(ValueError))  # Fails, since a ValueError is not raised

[hersheleh]

So the issues is that assert_that(calling(int).with_args('q'), not_(raises(ValueError))) does fail but does not do so for the RIGHT reasons. Looking at the error message, it fails because an internal DefferedCallable is raised which does not make sense.

Expected: not Expected a callable raising <class 'ValueError'>
     but: was <hamcrest.core.core.raises.DeferredCallable object at 0x7ffbaaba97f0>

where as I am expecting something like

AssertionError: 
Expected: not Expected a callable raising <class 'ValueError'>
     but:  <class 'ValueError'> was raised

In more human terms I am asserting that ValueError should not be raised but is being raised

@offbyone ... any thoughts appreciated.

[offbyone]

I think you're right about what we should see there.

[brunns]

Okey dokey, I'll look into it.

[brunns]

The problem here is that IsNot (and hence not_()) never explains why the matcher it's wrapping matched, and in fact the wrapped matcher has no way of telling it. So, all IsNot can do right now is to print out the item it was given.

Matchers in general use the describe_mismatch() and Description mechanism to describe mismatches, but there's currently no equivalent mechanism for describing successful matches. We could consider building one, but that might be a pretty widespread change.

The standard way to get a test to fail if an exception is raised is of course just a bare call:

int('q')

[offbyone]

If it's not practical, it's not practical. I agree with the original requester about what we might hope to see here, but you're right; describing success is tricky. I haven't been in the code in a while, so I'm not completely sure, but maybe we could leverage the self-describing aspect of it.

I'm definitely not in favor of a radical rearchitecture areound this, though :)

[brunns]

In fact, I think I have a simple and relatively backward compatible solution for this.

Matchers would grow a describe_match() method to parallel the describe_mismatch() method, with the same signature. As with describe_mismatch(), BaseMatcher would provide a simple implementation just showing the item passed to it, so not many existing matchers would need to change.

IsNot could use this to show why the matcher it wraps did in fact match, and Raises could override describe_match() to provide the information we'd like to see. Other matchers we could get to incrementally, and as with describe_mismatch(), many wouldn't need more than the BaseMatcher implementation anyway.

The downside of this is that any custom matchers not inheriting from BaseMatcher might break if wrapped by IsNot. A more complex, completely backward compatible solution would be a mixin, with IsNot doing an isinstance check, but the former approach seems more elegant if we are thinking of a 2.0 release soon. Again BaseMatcher would provide a default implementation, so I doubt too many breakages would occur, but there would be some.

Thoughts? Happy to run with either of these if you give me the go ahead.

[brunns]

I've tried this out on a branch here.

Needs docs before PRing. but you can see what I'm getting at.

[luziferius]

Is this still a valid issue or did you just forget to close this report?

[brunns]

Fixed - should be part of the forthcoming 2.0 release.